Merge pull request #20 from dawedawe/add_emptystringanalyzer

Add EmptyStringAnalyzer

CHANGELOG.md

@@ -1,13 +1,14 @@
 # Changelog
 
-## [Unreleased]
+## 0.2.0 - 2023-11-09
 
 ### Fixed
 * Fix analyzers urls. [#19](https://github.com/ionide/ionide-analyzers/pull/19)
 * Fix analyzers codes. [#22](https://github.com/ionide/ionide-analyzers/pull/22)
 
 ### Added
 * Support for referencing a local analyzers SDK. [#18](https://github.com/ionide/ionide-analyzers/pull/18)
+* EmptyStringAnalyzer. [#20](https://github.com/ionide/ionide-analyzers/pull/20)
 
 ## 0.1.1 - 2023-11-07
 

docs/suggestion/005.md

@@ -0,0 +1,27 @@
+---
+title: EmptyString
+category: suggestion
+categoryindex: 2
+index: 4
+---
+# EmptyStringAnalyzer
+
+## Problem
+
+Testing symbolically for an empty string is not the most efficient way.  
+Using the `String.Length` property or the `String.IsNullOrEmpty` method is the preferred way.  
+This is a port of the Roslyn analyzer [ca1820](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1820)
+
+```fsharp
+let s = "foo"
+let b = s = "" // Triggers analyzer 
+```
+
+## Fix
+
+Use the `Length` property if you know the reference is not null or the `String.IsNullOrEmpty` method otherwise.
+
+```fsharp
+let s = "foo"
+let b = s.Length = 0
+```

src/Ionide.Analyzers/Ionide.Analyzers.fsproj

@@ -16,6 +16,7 @@
         <Compile Include="Suggestion\CopyAndUpdateRecordChangesAllFieldsAnalyzer.fs"/>
         <Compile Include="Suggestion\IgnoreFunctionAnalyzer.fs"/>
         <Compile Include="Suggestion\UnnamedDiscriminatedUnionFieldAnalyzer.fs"/>
+        <Compile Include="Suggestion\EmptyStringAnalyzer.fs"/>
         <Compile Include="Style\SquareBracketArrayAnalyzer.fs"/>
     </ItemGroup>
 

src/Ionide.Analyzers/Suggestion/EmptyStringAnalyzer.fs

@@ -0,0 +1,65 @@
+module Ionide.Analyzers.Suggestion.EmptyStringAnalyzer
+
+open FSharp.Analyzers.SDK
+open FSharp.Analyzers.SDK.TASTCollecting
+open FSharp.Compiler.Symbols
+open FSharp.Compiler.Text
+
+let (|EmptyStringConst|_|) (e: FSharpExpr) =
+    if e.Type.ErasedType.BasicQualifiedName = "System.String" then
+        match e with
+        | FSharpExprPatterns.Const(o, _type) when not (isNull o) && (string o).Length = 0 -> Some()
+        | _ -> None
+    else
+        None
+
+let invalidStringFunctionUseAnalyzer (typedTree: FSharpImplementationFileContents) =
+    let ranges = ResizeArray<range>()
+
+    let walker =
+        { new TypedTreeCollectorBase() with
+            override _.WalkCall _ (mfv: FSharpMemberOrFunctionOrValue) _ _ (args: FSharpExpr list) (m: range) =
+                match (mfv.Assembly.SimpleName, mfv.FullName, args) with
+                | "FSharp.Core", "Microsoft.FSharp.Core.Operators.(=)", [ _; EmptyStringConst ]
+                | "FSharp.Core", "Microsoft.FSharp.Core.Operators.(=)", [ EmptyStringConst; _ ] -> ranges.Add m
+                | _ -> ()
+
+        }
+
+    walkTast walker typedTree
+
+    ranges
+    |> Seq.map (fun r ->
+        {
+            Type = "EmptyString analyzer"
+            Message =
+                "Test for empty strings should use the String.Length property or the String.IsNullOrEmpty method."
+            Code = "IONIDE-005"
+            Severity = Warning
+            Range = r
+            Fixes = []
+        }
+    )
+    |> Seq.toList
+
+[<EditorAnalyzer("EmptyStringAnalyzer",
+                 "Verifies testing for an empty string is done efficiently.",
+                 "https://ionide.io/ionide-analyzers/suggestion/005.html")>]
+let emptyStringEditorAnalyzer (ctx: EditorContext) =
+    async {
+        return
+            ctx.TypedTree
+            |> Option.map invalidStringFunctionUseAnalyzer
+            |> Option.defaultValue []
+    }
+
+[<CliAnalyzer("EmptyStringAnalyzer",
+              "Verifies testing for an empty string is done efficiently.",
+              "https://ionide.io/ionide-analyzers/suggestion/005.html")>]
+let emptyStringCliAnalyzer (ctx: CliContext) =
+    async {
+        return
+            ctx.TypedTree
+            |> Option.map invalidStringFunctionUseAnalyzer
+            |> Option.defaultValue []
+    }

tests/Ionide.Analyzers.Tests/Ionide.Analyzers.Tests.fsproj

@@ -12,6 +12,7 @@
     <Compile Include="Suggestion\CopyAndUpdateRecordChangesAllFieldsAnalyzerTests.fs" />
     <Compile Include="Suggestion\IgnoreFunctionAnalyzerTests.fs" />
     <Compile Include="Suggestion\UnnamedDiscriminatedUnionFieldAnalyzerTests.fs" />
+    <Compile Include="Suggestion\EmptyStringAnalyzerTests.fs" />
     <Compile Include="Style\SquareBracketArrayAnalyzerTests.fs" />
     <Compile Include="Program.fs" />
   </ItemGroup>

tests/Ionide.Analyzers.Tests/Suggestion/EmptyStringAnalyzerTests.fs

@@ -0,0 +1,140 @@
+module Ionide.Analyzers.Tests.Suggestion.EmptyStringAnalyzerTests
+
+open NUnit.Framework
+open FSharp.Compiler.CodeAnalysis
+open FSharp.Analyzers.SDK.Testing
+open Ionide.Analyzers.Suggestion.EmptyStringAnalyzer
+
+let mutable projectOptions: FSharpProjectOptions = FSharpProjectOptions.zero
+
+[<SetUp>]
+let Setup () =
+    task {
+        let! opts = mkOptionsFromProject "net7.0" []
+
+        projectOptions <- opts
+    }
+
+[<Test>]
+let ``Operator based test for zero-length`` () =
+    async {
+        let source =
+            """
+module M
+
+let s = "foo"
+let x = s = ""
+    """
+
+        let ctx = getContext projectOptions source
+        let! msgs = emptyStringCliAnalyzer ctx
+        Assert.IsNotEmpty msgs
+
+        Assert.IsTrue(
+            Assert.messageContains
+                "Test for empty strings should use the String.Length property or the String.IsNullOrEmpty method."
+                msgs[0]
+        )
+    }
+
+[<Test>]
+let ``Operator based test for zero-length reversed`` () =
+    async {
+        let source =
+            """
+module M
+
+let s = "foo"
+let x = "" = s
+    """
+
+        let ctx = getContext projectOptions source
+        let! msgs = emptyStringCliAnalyzer ctx
+        Assert.IsNotEmpty msgs
+
+        Assert.IsTrue(
+            Assert.messageContains
+                "Test for empty strings should use the String.Length property or the String.IsNullOrEmpty method."
+                msgs[0]
+        )
+    }
+
+[<Test>]
+let ``Operator based equality test`` () =
+    async {
+        let source =
+            """
+module M
+
+let s = "foo"
+let x = s = "bar"
+    """
+
+        let ctx = getContext projectOptions source
+        let! msgs = emptyStringCliAnalyzer ctx
+        Assert.IsEmpty msgs
+    }
+
+[<Test>]
+let ``Operator based equality test reversed`` () =
+    async {
+        let source =
+            """
+module M
+
+let s = "foo"
+let x = "bar" = s
+    """
+
+        let ctx = getContext projectOptions source
+        let! msgs = emptyStringCliAnalyzer ctx
+        Assert.IsEmpty msgs
+    }
+
+[<Test>]
+let ``Operator based null test`` () =
+    async {
+        let source =
+            """
+module M
+
+let s = "foo"
+let x = s = null
+    """
+
+        let ctx = getContext projectOptions source
+        let! msgs = emptyStringCliAnalyzer ctx
+        Assert.IsEmpty msgs
+    }
+
+[<Test>]
+let ``Operator based null test reversed`` () =
+    async {
+        let source =
+            """
+module M
+
+let s = "foo"
+let x = null = s
+    """
+
+        let ctx = getContext projectOptions source
+        let! msgs = emptyStringCliAnalyzer ctx
+        Assert.IsEmpty msgs
+    }
+
+[<Test>]
+let ``Property based length test`` () =
+    async {
+        let source =
+            """
+module M
+
+let s = "foo"
+let x = s.Length = 0
+    """
+
+        let ctx = getContext projectOptions source
+        let! msgs = emptyStringCliAnalyzer ctx
+        Assert.IsEmpty msgs
+    }