Merge PR #798 from webwarrior-ws/fix-792

Fix FavourStaticEmptyFields false negatives.

Author: knocte

src/FSharpLint.Core/Rules/Conventions/FavourStaticEmptyFields.fs

@@ -38,30 +38,56 @@ let private generateError (fileContents: string) (range:FSharp.Compiler.Text.Ran
           SuggestedFix = Some suggestedFix
           TypeChecks = List.Empty }
 
-let private runner (args: AstNodeRuleParams) =
-    match args.AstNode with
-    | AstNode.Expression(SynExpr.Const (SynConst.String ("", _, range), _)) -> 
-        generateError args.FileContent range EmptyStringLiteral
-    | AstNode.Expression(SynExpr.ArrayOrList (isArray, [], range)) ->
+[<TailCall>]
+let rec private processExpressions (errorsSoFar: array<WarningDetails>) (args: AstNodeRuleParams) (expressions: list<SynExpr>) =
+    match expressions with
+    | SynExpr.Const(SynConst.String ("", _, range), _) :: tail -> 
+        let errors =
+            Array.append
+                errorsSoFar
+                (generateError args.FileContent range EmptyStringLiteral)
+        processExpressions errors args tail
+    | SynExpr.DotIndexedGet(_, indexArgs, _ , _) :: tail ->
+        processExpressions errorsSoFar args (indexArgs :: tail)
+    | SynExpr.DotIndexedSet(_, indexArgs, _ , _, _, _) :: tail ->
+        processExpressions errorsSoFar args (indexArgs :: tail)
+    | SynExpr.Paren(expr, _, _, _) :: tail ->
+        processExpressions errorsSoFar args (expr :: tail)
+    | SynExpr.Tuple(_, expressions, _, _) :: tail ->
+        processExpressions errorsSoFar args (List.append expressions tail)
+    | SynExpr.ArrayOrList(isArray, [], range) :: tail ->
         let emptyLiteralType =
             if isArray then EmptyArrayLiteral else EmptyListLiteral
-        generateError args.FileContent range emptyLiteralType
-    | AstNode.Expression(SynExpr.Record(_, _, synExprRecordField, _)) ->
+        let errors =
+            Array.append
+                errorsSoFar
+                (generateError args.FileContent range emptyLiteralType)
+        processExpressions errors args tail
+    | SynExpr.ArrayOrListComputed(_, expr, _) :: tail ->
+        processExpressions errorsSoFar args (expr :: tail)
+    | SynExpr.App(_, _, funcExpr, argExpr, _) :: tail ->
+        processExpressions errorsSoFar args (List.append [ funcExpr; argExpr ] tail)
+    | SynExpr.AnonRecd(_, _, recordFields, _, _) :: tail ->
+        let fieldExpressions = 
+            recordFields
+            |> List.map (fun (_, _, expr) ->  expr)
+        processExpressions errorsSoFar args (List.append fieldExpressions tail)
+    | SynExpr.Record(_, _, synExprRecordFields, _) :: tail ->
         let mapping =
             function
-            | SynExprRecordField(_, _, expr, _) ->
-                match expr with
-                | Some(SynExpr.ArrayOrList(isArray, [], range)) ->
-                    let emptyLiteralType = if isArray then EmptyArrayLiteral else EmptyListLiteral
-                    generateError args.FileContent range emptyLiteralType
-                | Some(SynExpr.Const (SynConst.String ("", _, range), _)) ->
-                    generateError args.FileContent range EmptyStringLiteral
-                | Some(SynExpr.App(_, _, _, SynExpr.Const (SynConst.String ("", _, range), _), _)) ->
-                    generateError args.FileContent range EmptyStringLiteral
-                | _ -> Array.empty
-        synExprRecordField
-        |> List.map mapping
-        |> Array.concat
+            | SynExprRecordField(_, _, expr, _) -> expr
+        let fieldExpressions = 
+            synExprRecordFields
+            |> List.choose mapping
+        processExpressions errorsSoFar args (List.append fieldExpressions tail)
+    | _ :: tail ->
+        processExpressions errorsSoFar args tail
+    | [] -> errorsSoFar
+
+let private runner (args: AstNodeRuleParams) =
+    match args.AstNode with
+    | AstNode.Expression(expr) -> 
+        processExpressions Array.empty args (List.singleton expr)
     | _ -> Array.empty
 
 

src/FSharpLint.Core/Rules/Conventions/SourceLength/MaxLinesInFile.fs

@@ -15,7 +15,7 @@ let private checkNumberOfLinesInFile numberOfLines line maxLines =
         Array.singleton
             {
                 Range =
-                    Range.mkRange "" (Position.mkPos (maxLines + 1) 0) (Position.mkPos numberOfLines (String.length line))
+                    Range.mkRange String.Empty (Position.mkPos (maxLines + 1) 0) (Position.mkPos numberOfLines (String.length line))
                 Message = String.Format(errorFormatString, (maxLines + 1))
                 SuggestedFix = None
                 TypeChecks = List.Empty

src/FSharpLint.Core/Rules/Formatting/TupleFormatting/TupleIndentation.fs

@@ -16,7 +16,7 @@ let checkTupleIndentation _ (tupleExprs:SynExpr list) _ _ =
         if expr.Range.StartColumn <> nextExpr.Range.StartColumn then
             Some
                 {
-                    Range = Range.mkRange "" expr.Range.Start nextExpr.Range.End
+                    Range = Range.mkRange String.Empty expr.Range.Start nextExpr.Range.End
                     Message = Resources.GetString("RulesFormattingTupleIndentationError")
                     SuggestedFix = None
                     TypeChecks = List.Empty

src/FSharpLint.Core/Rules/Formatting/Typography/TrailingNewLineInFile.fs

@@ -11,7 +11,7 @@ let checkTrailingNewLineInFile (args:LineRuleParams) =
         let pos = Position.mkPos args.LineNumber 0
         Array.singleton
             {
-                Range = Range.mkRange "" pos pos
+                Range = Range.mkRange String.Empty pos pos
                 Message = Resources.GetString("RulesTypographyTrailingLineError")
                 SuggestedFix = None
                 TypeChecks = List.Empty

src/FSharpLint.Core/Rules/Hints/HintMatcher.fs

@@ -770,7 +770,7 @@ let private confirmFuzzyMatch (args:AstNodeRuleParams) (hint:HintParserTypes.Hin
         |> suggestions.Add
     | AstNode.Expression(expr), HintExpr(hintExpr) ->
         let arguments =
-            { MatchExpression.LambdaArguments = Map.ofList []
+            { MatchExpression.LambdaArguments = Map.empty
               MatchExpression.MatchedVariables = Dictionary<_, _>()
               MatchExpression.Expression = args.AstNode
               MatchExpression.Hint = hintExpr

tests/FSharpLint.Core.Tests/Rules/Conventions/FavourStaticEmptyFields.fs

@@ -118,6 +118,36 @@ type Person =
         Assert.IsTrue this.ErrorsExist
         Assert.IsTrue (this.ErrorMsg.Contains "String.Empty")
 
+    [<Test>]
+    member this.FavourStaticEmptyFieldsShouldProduceError13() =
+        this.Parse """
+let foo =
+    {
+        Range = Range.mkRange "" pos pos
+    }"""
+
+        Assert.IsTrue this.ErrorsExist
+        Assert.IsTrue (this.ErrorMsg.Contains "String.Empty")
+
+    [<Test>]
+    member this.FavourStaticEmptyFieldsShouldProduceError14() =
+        this.Parse """
+let foo =
+    {
+        Bar = ("a", "")
+    }"""
+
+        Assert.IsTrue this.ErrorsExist
+        Assert.IsTrue (this.ErrorMsg.Contains "String.Empty")
+
+    [<Test>]
+    member this.FavourStaticEmptyFieldsShouldProduceError15() =
+        this.Parse """
+foo.[""] <- 0"""
+
+        Assert.IsTrue this.ErrorsExist
+        Assert.IsTrue (this.ErrorMsg.Contains "String.Empty")
+
     [<Test>]
     member this.FavourStaticEmptyFieldsShouldNotProduceError1() =
         this.Parse "let bar = String.Empty"