From dad257102b8b381afd863154935c1433e8239a7f Mon Sep 17 00:00:00 2001 From: Richard Webb Date: Fri, 27 Jun 2025 21:23:36 +0100 Subject: [PATCH 1/2] Extend the string analyzers to analyze uses of String.LastIndexOf in the same way it does for String.IndexOf This is laregely copy/pasted from the IndexOf analyzer, with just a few changes to the function names. --- docs/analyzers/StringAnalyzer.md | 1 + src/FSharp.Analyzers/StringAnalyzer.fs | 33 +++++++++++++++++++ src/FSharp.Analyzers/StringAnalyzer.fsi | 15 +++++++++ .../StringAnalyzerTests.fs | 5 +-- .../Constant String - Single Argument.fs | 3 ++ ...stant String - Single Argument.fs.expected | 1 + .../Constant String - String Int Int.fs | 3 ++ ...nstant String - String Int Int.fs.expected | 1 + .../Constant String - String Int.fs | 3 ++ .../Constant String - String Int.fs.expected | 1 + .../Function call - Single Argument.fs | 4 +++ ...unction call - Single Argument.fs.expected | 1 + .../Prefixed Value - Single Argument.fs | 6 ++++ ...efixed Value - Single Argument.fs.expected | 1 + .../lastindexof/Value - Single Argument.fs | 4 +++ .../Value - Single Argument.fs.expected | 1 + .../lastindexof/negative/Char Int32 Int32.fs | 4 +++ .../string/lastindexof/negative/Char Int32.fs | 4 +++ .../data/string/lastindexof/negative/Char.fs | 4 +++ .../String Int32 Int32 StringComparison.fs | 4 +++ .../negative/String Int32 StringComparison.fs | 4 +++ .../negative/String StringComparison.fs | 4 +++ 22 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - Single Argument.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - Single Argument.fs.expected create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int Int.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int Int.fs.expected create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int.fs.expected create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Function call - Single Argument.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Function call - Single Argument.fs.expected create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Prefixed Value - Single Argument.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Prefixed Value - Single Argument.fs.expected create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Value - Single Argument.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/Value - Single Argument.fs.expected create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char Int32 Int32.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char Int32.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String Int32 Int32 StringComparison.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String Int32 StringComparison.fs create mode 100644 tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String StringComparison.fs diff --git a/docs/analyzers/StringAnalyzer.md b/docs/analyzers/StringAnalyzer.md index 338e5f2..4d85fca 100644 --- a/docs/analyzers/StringAnalyzer.md +++ b/docs/analyzers/StringAnalyzer.md @@ -12,6 +12,7 @@ There are multiple analyzers for various string comparison functions: - String.EndsWith Analyzer - String.StartsWith Analyzer - String.IndexOf Analyzer +- String.LastIndexOf Analyzer ## Problem diff --git a/src/FSharp.Analyzers/StringAnalyzer.fs b/src/FSharp.Analyzers/StringAnalyzer.fs index b25bb6b..c181036 100644 --- a/src/FSharp.Analyzers/StringAnalyzer.fs +++ b/src/FSharp.Analyzers/StringAnalyzer.fs @@ -14,6 +14,9 @@ let StringStartsWithCode = "GRA-STRING-002" [] let StringIndexOfCode = "GRA-STRING-003" +[] +let StringLastIndexOfCode : string = "GRA-STRING-004" + [] let HelpUri = "https://g-research.github.io/fsharp-analyzers/analyzers/StringAnalyzer.html" @@ -159,3 +162,33 @@ let indexOfCliAnalyzer (ctx : CliContext) : Async = [] let indexOfEditorAnalyzer (ctx : EditorContext) : Async = async { return ctx.TypedTree |> Option.map indexOfAnalyze |> Option.defaultValue [] } + +let lastIndexOfAnalyze (typedTree : FSharpImplementationFileContents) = + invalidStringFunctionUseAnalyzer + "LastIndexOf" + StringLastIndexOfCode + "The usage of String.LastIndexOf with a single string argument is discouraged. Signal your intention explicitly by calling an overload." + Severity.Warning + typedTree + (fun args -> + match args with + | [ StringExpr ] + | [ StringExpr ; IntExpr ] + | [ StringExpr ; IntExpr ; IntExpr ] -> true + | _ -> false + ) + +[] +let StringLastIndexOfName = "String.LastIndexOf Analyzer" + +[] +let StringLastIndexOfShortDescription = + "Verifies the correct usage of System.String.LastIndexOf" + +[] +let lastIndexOfCliAnalyzer (ctx : CliContext) : Async = + async { return ctx.TypedTree |> Option.map lastIndexOfAnalyze |> Option.defaultValue [] } + +[] +let lastIndexOfEditorAnalyzer (ctx : EditorContext) : Async = + async { return ctx.TypedTree |> Option.map lastIndexOfAnalyze |> Option.defaultValue [] } diff --git a/src/FSharp.Analyzers/StringAnalyzer.fsi b/src/FSharp.Analyzers/StringAnalyzer.fsi index 864398a..9320564 100644 --- a/src/FSharp.Analyzers/StringAnalyzer.fsi +++ b/src/FSharp.Analyzers/StringAnalyzer.fsi @@ -49,3 +49,18 @@ val indexOfCliAnalyzer : ctx : CliContext -> Async [] val indexOfEditorAnalyzer : ctx : EditorContext -> Async + +[] +val StringLastIndexOfCode : string = "GRA-STRING-004" + +[] +val StringLastIndexOfName : string = "String.LastIndexOf Analyzer" + +[] +val StringLastIndexOfShortDescription : string = "Verifies the correct usage of System.String.LastIndexOf" + +[] +val lastIndexOfCliAnalyzer : ctx : CliContext -> Async + +[] +val lastIndexOfEditorAnalyzer : ctx : EditorContext -> Async diff --git a/tests/FSharp.Analyzers.Tests/StringAnalyzerTests.fs b/tests/FSharp.Analyzers.Tests/StringAnalyzerTests.fs index 0f59ce8..d07a681 100644 --- a/tests/FSharp.Analyzers.Tests/StringAnalyzerTests.fs +++ b/tests/FSharp.Analyzers.Tests/StringAnalyzerTests.fs @@ -21,7 +21,7 @@ type TestCases() = interface IEnumerable with member _.GetEnumerator () : IEnumerator = - [| "endswith" ; "indexof" ; "startswith" |] + [| "endswith" ; "indexof" ; "startswith" ; "lastindexof" |] |> Seq.collect (fun subFolder -> let folder = Path.Combine (dataFolder, "string", subFolder) Directory.EnumerateFiles (folder, "*.fs") @@ -36,6 +36,7 @@ let findStringAnalyzerFor (fileName : string) = | "endswith" -> StringAnalyzer.endsWithCliAnalyzer | "startswith" -> StringAnalyzer.startsWithCliAnalyzer | "indexof" -> StringAnalyzer.indexOfCliAnalyzer + | "lastindexof" -> StringAnalyzer.lastIndexOfCliAnalyzer | unknown -> failwithf $"Unknown subfolder \"%s{unknown}\", please configure analyzer" [)>] @@ -55,7 +56,7 @@ type NegativeTestCases() = interface IEnumerable with member _.GetEnumerator () : IEnumerator = - [| "endswith" ; "indexof" ; "startswith" |] + [| "endswith" ; "indexof" ; "startswith" ; "lastindexof" |] |> Seq.collect (fun subFolder -> let folder = Path.Combine (dataFolder, "string", subFolder, "negative") Directory.EnumerateFiles (folder, "*.fs") diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - Single Argument.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - Single Argument.fs new file mode 100644 index 0000000..cd9b4d6 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - Single Argument.fs @@ -0,0 +1,3 @@ +module M + +"foo".LastIndexOf("p") diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - Single Argument.fs.expected b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - Single Argument.fs.expected new file mode 100644 index 0000000..7802b67 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - Single Argument.fs.expected @@ -0,0 +1 @@ +GRA-STRING-004 | Warning | (3,0 - 3,22) | The usage of String.LastIndexOf with a single string argument is discouraged. Signal your intention explicitly by calling an overload. | [] diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int Int.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int Int.fs new file mode 100644 index 0000000..c2f415f --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int Int.fs @@ -0,0 +1,3 @@ +module M + +"foo".LastIndexOf("p", 0 ,1) diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int Int.fs.expected b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int Int.fs.expected new file mode 100644 index 0000000..b142379 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int Int.fs.expected @@ -0,0 +1 @@ +GRA-STRING-004 | Warning | (3,0 - 3,28) | The usage of String.LastIndexOf with a single string argument is discouraged. Signal your intention explicitly by calling an overload. | [] diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int.fs new file mode 100644 index 0000000..30688de --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int.fs @@ -0,0 +1,3 @@ +module M + +"foo".LastIndexOf("p", 0) diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int.fs.expected b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int.fs.expected new file mode 100644 index 0000000..0669b11 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Constant String - String Int.fs.expected @@ -0,0 +1 @@ +GRA-STRING-004 | Warning | (3,0 - 3,25) | The usage of String.LastIndexOf with a single string argument is discouraged. Signal your intention explicitly by calling an overload. | [] diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Function call - Single Argument.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Function call - Single Argument.fs new file mode 100644 index 0000000..cc3ef00 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Function call - Single Argument.fs @@ -0,0 +1,4 @@ +module M + +let f () = "f" +"g".LastIndexOf(f()) diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Function call - Single Argument.fs.expected b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Function call - Single Argument.fs.expected new file mode 100644 index 0000000..5b1a39c --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Function call - Single Argument.fs.expected @@ -0,0 +1 @@ +GRA-STRING-004 | Warning | (4,0 - 4,20) | The usage of String.LastIndexOf with a single string argument is discouraged. Signal your intention explicitly by calling an overload. | [] diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Prefixed Value - Single Argument.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Prefixed Value - Single Argument.fs new file mode 100644 index 0000000..0d019e1 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Prefixed Value - Single Argument.fs @@ -0,0 +1,6 @@ +module M + +module A = + let b = "b" + +A.b.LastIndexOf("b") diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Prefixed Value - Single Argument.fs.expected b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Prefixed Value - Single Argument.fs.expected new file mode 100644 index 0000000..d323225 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Prefixed Value - Single Argument.fs.expected @@ -0,0 +1 @@ +GRA-STRING-004 | Warning | (6,0 - 6,20) | The usage of String.LastIndexOf with a single string argument is discouraged. Signal your intention explicitly by calling an overload. | [] diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Value - Single Argument.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Value - Single Argument.fs new file mode 100644 index 0000000..822fc5f --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Value - Single Argument.fs @@ -0,0 +1,4 @@ +module M + +let a = "a" +a.LastIndexOf("a") diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Value - Single Argument.fs.expected b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Value - Single Argument.fs.expected new file mode 100644 index 0000000..24a12ff --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/Value - Single Argument.fs.expected @@ -0,0 +1 @@ +GRA-STRING-004 | Warning | (4,0 - 4,18) | The usage of String.LastIndexOf with a single string argument is discouraged. Signal your intention explicitly by calling an overload. | [] diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char Int32 Int32.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char Int32 Int32.fs new file mode 100644 index 0000000..45c9ded --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char Int32 Int32.fs @@ -0,0 +1,4 @@ +module M + +open System +"foo".LastIndexOf('f',0,1) diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char Int32.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char Int32.fs new file mode 100644 index 0000000..09e2ef4 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char Int32.fs @@ -0,0 +1,4 @@ +module M + +open System +"foo".LastIndexOf('f',0) diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char.fs new file mode 100644 index 0000000..96cb075 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/Char.fs @@ -0,0 +1,4 @@ +module M + +open System +"foo".LastIndexOf('f') diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String Int32 Int32 StringComparison.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String Int32 Int32 StringComparison.fs new file mode 100644 index 0000000..0fe46d6 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String Int32 Int32 StringComparison.fs @@ -0,0 +1,4 @@ +module M + +open System +"foo".LastIndexOf("f",0,1,StringComparison.InvariantCulture) diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String Int32 StringComparison.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String Int32 StringComparison.fs new file mode 100644 index 0000000..f741828 --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String Int32 StringComparison.fs @@ -0,0 +1,4 @@ +module M + +open System +"foo".LastIndexOf("f",0,StringComparison.InvariantCulture) diff --git a/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String StringComparison.fs b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String StringComparison.fs new file mode 100644 index 0000000..27e31cd --- /dev/null +++ b/tests/FSharp.Analyzers.Tests/data/string/lastindexof/negative/String StringComparison.fs @@ -0,0 +1,4 @@ +module M + +open System +"foo".LastIndexOf("f",StringComparison.InvariantCulture) From e9d5f16e3f7172f592e402e4f2d3f071009f1408 Mon Sep 17 00:00:00 2001 From: Richard Webb Date: Mon, 30 Jun 2025 13:03:51 +0100 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc1dea5..fc9d423 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 0.16.0 - 2025-06-30 + +### Added +* StringAnalyzer now supports String.LastIndexOf. [#91](https://github.com/G-Research/fsharp-analyzers/pull/91) + ## 0.15.0 - 2025-05-18 ### Changed