Merge PR #649 from Mersho/ImplementQuickFixes

Add quickfix suggestions for several rules.

Author: knocte

src/FSharpLint.Core/Rules/Conventions/Binding/FavourIgnoreOverLetWild.fs

@@ -7,25 +7,39 @@ open FSharp.Compiler.Syntax
 open FSharpLint.Framework.Ast
 open FSharpLint.Framework.Rules
 
-let private checkForBindingToAWildcard pattern range =
+let private checkForBindingToAWildcard pattern range fileContent (expr: SynExpr) letBindingRange =
     let rec findWildAndIgnoreParens = function
         | SynPat.Paren(pattern, _) -> findWildAndIgnoreParens pattern
         | SynPat.Wild(_) -> true
         | _ -> false
 
-    if findWildAndIgnoreParens pattern then
-        { Range = range
-          Message = Resources.GetString("RulesFavourIgnoreOverLetWildError")
-          SuggestedFix = None
-          TypeChecks = [] } |> Array.singleton
-    else
-        Array.empty
+    match ExpressionUtilities.tryFindTextOfRange expr.Range fileContent with
+    | Some exprText -> 
+        if findWildAndIgnoreParens pattern then
+            { Range = range
+              Message = Resources.GetString("RulesFavourIgnoreOverLetWildError")
+              SuggestedFix = Some (lazy (Some({ FromRange = letBindingRange
+                                                FromText = fileContent
+                                                ToText = sprintf "(%s) |> ignore" exprText })))
+              TypeChecks = [] } |> Array.singleton
+        else
+            Array.empty
+    | None -> Array.empty
 
 let private runner (args:AstNodeRuleParams) =
     match args.AstNode with
-    | AstNode.Binding(SynBinding(_, _, _, _, _, _, _, pattern, _, _, range, _, _))
-            when Helper.Binding.isLetBinding args.NodeIndex args.SyntaxArray ->
-        checkForBindingToAWildcard pattern range
+    | AstNode.Binding(SynBinding(_, _, _, _, _, _, _, pattern, _, expr, range, _, _)) ->
+        let bindingRange  = 
+            match args.GetParents(args.NodeIndex) with
+            | AstNode.ModuleDeclaration(SynModuleDecl.Let(_, _, range)) :: _
+            | AstNode.Expression(SynExpr.LetOrUse(_, false, _, _, range, _)) :: _ -> 
+                Some(range)
+            | _ -> None
+
+        match bindingRange with
+        | Some letBindingRange -> 
+            checkForBindingToAWildcard pattern range args.FileContent expr letBindingRange
+        | None -> Array.empty
     | _ -> Array.empty
 
 /// Checks if any code uses 'let _ = ...' and suggests to use the ignore function.

src/FSharpLint.Core/Rules/Conventions/Binding/UselessBinding.fs

@@ -9,7 +9,7 @@ open FSharp.Compiler.CodeAnalysis
 open FSharpLint.Framework.Ast
 open FSharpLint.Framework.Rules
 
-let private checkForUselessBinding (checkInfo:FSharpCheckFileResults option) pattern expr range =
+let private checkForUselessBinding (checkInfo:FSharpCheckFileResults option) pattern expr range maybeSuggestedFix =
     match checkInfo with
     | Some checkInfo ->
         let rec findBindingIdentifier = function
@@ -42,17 +42,23 @@ let private checkForUselessBinding (checkInfo:FSharpCheckFileResults option) pat
         |> Option.map (fun ident ->
             { Range = range
               Message = Resources.GetString("RulesUselessBindingError")
-              SuggestedFix = None
+              SuggestedFix = Some (lazy(maybeSuggestedFix))
               TypeChecks = [ checkNotMutable ident ] })
         |> Option.toArray
     | _ -> Array.empty
 
 let private runner (args:AstNodeRuleParams) =
+    let maybeSuggestedFix = 
+        match args.GetParents(args.NodeIndex) with
+        | AstNode.ModuleDeclaration(SynModuleDecl.Let(_, _, range)) :: _ ->
+            Some({ FromRange = range; FromText = "let"; ToText = "" })
+        | AstNode.Expression(SynExpr.LetOrUse(_, false, _, _, range, _)) :: _ -> 
+            Some({ FromRange = range; FromText = "use"; ToText = "" })
+        | _ -> None
     match args.AstNode with
     | AstNode.Binding(SynBinding(_, _, _, isMutable, _, _, _, pattern, _, expr, range, _, _))
-            when Helper.Binding.isLetBinding args.NodeIndex args.SyntaxArray
-                 && not isMutable ->
-        checkForUselessBinding args.CheckInfo pattern expr range
+            when maybeSuggestedFix.IsSome && not isMutable ->
+                checkForUselessBinding args.CheckInfo pattern expr range maybeSuggestedFix
     | _ ->
         Array.empty
 

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

@@ -24,10 +24,11 @@ let runner (config: Config) args =
             if identifiers.Length = 2 then
                 match identifiers  with
                 | head::_ when isNotConsistent head.idText symbol ->
+                    let suggestedFix = lazy(Some({ FromRange = head.idRange; FromText = head.idText; ToText = symbol }))
                     let error =
                         { Range = range
                           Message = String.Format(Resources.GetString "RulesFavourConsistentThis", config.Symbol)
-                          SuggestedFix = None
+                          SuggestedFix = Some suggestedFix
                           TypeChecks = List.Empty }
                         |> Array.singleton
                     error

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

@@ -8,23 +8,24 @@ open FSharpLint.Framework.Ast
 open FSharpLint.Framework.Rules
 
 let private runner (args: AstNodeRuleParams) =
-    let generateError range =
+    let generateError suggestedFix range =
         { Range = range
           Message = Resources.GetString "RulesFavourReRaise"
-          SuggestedFix = None
+          SuggestedFix = Some suggestedFix
           TypeChecks = List.empty }
         |> Array.singleton
 
     let rec checkExpr (expr) maybeIdent =
         match expr with
         | SynExpr.App (_, _, SynExpr.Ident raiseId, expression, range) when raiseId.idText = "raise" ->
+            let suggestedFix = lazy(Some({ FromRange = range; FromText = raiseId.idText; ToText = "reraise()" }))
             match expression with
             | SynExpr.Ident ident ->
                 match maybeIdent with
                 | Some id when id = ident.idText ->
-                    generateError range
+                    generateError suggestedFix range
                 | _ -> Array.empty
-            | SynExpr.LongIdent (_, SynLongIdent (id, _, _), _, range) -> generateError range
+            | SynExpr.LongIdent (_, SynLongIdent (id, _, _), _, range) -> generateError suggestedFix range
             | _ -> Array.empty
         | SynExpr.TryWith (expressions, clauseList, _range, _, _, _) as expr ->
             clauseList

src/FSharpLint.Core/Rules/Typography/NoTabCharacters.fs

@@ -28,7 +28,15 @@ let checkNoTabCharacters literalStrings (args:LineRuleParams) =
         if isInLiteralString literalStrings range |> not then
             { Range = range
               Message = Resources.GetString("RulesTypographyTabCharacterError")
-              SuggestedFix = None
+              SuggestedFix =
+                Some(
+                    lazy
+                        (Some(
+                            { FromRange = range
+                              FromText = "\t"
+                              ToText = String.replicate args.GlobalConfig.numIndentationSpaces " " }
+                        ))
+                )
               TypeChecks = [] } |> Array.singleton
         else
             Array.empty

tests/FSharpLint.Core.Tests/Rules/Binding/FavourIgnoreOverLetWild.fs

@@ -50,3 +50,40 @@ let a = List.iter (fun x -> ()) []
 
         Assert.IsFalse(this.ErrorsExist)
 
+    [<Test>]
+    member this.LetWildcardUnitValueSuggestedFix() =
+        let source = """
+module Program
+
+let _ = ()"""
+        let expected = """
+module Program
+
+(()) |> ignore"""
+        this.Parse source
+
+        Assert.IsTrue(this.ErrorExistsAt(4, 4))
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(expected, result)
+
+    [<Test>]
+    member this.LetWildCardInParanUnitValueSuggestedFix() =
+        let source = """
+module Program
+
+let ((((_)))) = List.iter (fun x -> ()) []"""
+
+        let expected = """
+module Program
+
+(List.iter (fun x -> ()) []) |> ignore"""
+
+        this.Parse source
+
+        Assert.IsTrue(this.ErrorExistsAt(4, 4))
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(expected, result)

tests/FSharpLint.Core.Tests/Rules/Binding/UselessBinding.fs

@@ -66,3 +66,46 @@ type Cat() =
 """
 
         Assert.IsFalse(this.ErrorsExist)
+
+    [<Test>]
+    member this.UselessBindingSuggestedFix() =
+        let source = """
+module Program
+
+let a = 10
+let a = a"""
+        let expected = """
+module Program
+
+let a = 10
+"""
+
+        this.Parse source
+
+        Assert.IsTrue(this.ErrorExistsAt(5, 4))
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(expected, result)
+
+    [<Test>]
+    member this.UselessBindingWithParensSuggestedFix() =
+        let source = """
+module Program
+
+let a = 10
+let ((a)) = ((a))"""
+
+        let expected = """
+module Program
+
+let a = 10
+"""
+
+        this.Parse source
+
+        Assert.IsTrue(this.ErrorExistsAt(5, 4))
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(expected, result)

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

@@ -111,3 +111,77 @@ type TorMessageDigest(isSha256: bool) =
 
         Assert.IsTrue this.NoErrorsExist
 
+    [<Test>]
+    member this.InConsistentSelfShouldProduceErrorSuggestedFix() =
+        let source = """
+type Foo =
+    { Bar : Baz }
+    member self.FooBar = 
+        ()
+    member this.FooBarBaz x =
+        failwith "foobarbaz" """
+
+        let expected = """
+type Foo =
+    { Bar : Baz }
+    member this.FooBar = 
+        ()
+    member this.FooBarBaz x =
+        failwith "foobarbaz" """
+
+        this.Parse source
+
+        Assert.IsTrue this.ErrorsExist
+        
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(expected, result)
+
+    [<Test>]
+    member this.InConsistentSelfShouldProduceErrorSuggestedFix2() =
+        let source = """
+type Foo =
+    { Bar : Baz }
+    member this.FooBar = 
+        ()
+    member self.FooBarBaz x =
+        failwith "foobarbaz" """
+
+        let expected = """
+type Foo =
+    { Bar : Baz }
+    member this.FooBar = 
+        ()
+    member this.FooBarBaz x =
+        failwith "foobarbaz" """
+
+        this.Parse source
+
+        Assert.IsTrue this.ErrorsExist
+        Assert.IsTrue(this.ErrorExistsAt(6, 11))
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(expected, result)
+
+    [<Test>]
+    member this.InConsistentSelfShouldProduceErrorSuggestedFix3() =
+        let source = """
+type CustomerName(firstName, middleInitial, lastName) =
+    member this.FirstName = firstName
+    member self.MiddleInitial = middleInitial
+    member this.LastName = lastName """
+        let expected = """
+type CustomerName(firstName, middleInitial, lastName) =
+    member this.FirstName = firstName
+    member this.MiddleInitial = middleInitial
+    member this.LastName = lastName """
+
+        this.Parse source
+
+        Assert.IsTrue this.ErrorsExist
+        Assert.IsTrue(this.ErrorExistsAt(4, 11))
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(expected, result)

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

@@ -96,3 +96,30 @@ with
 """
 
         this.AssertNoWarnings()
+
+    [<Test>]
+    member this.``using raise ex must generate error suggested fix``() =
+        let source = """
+try
+    foo ()
+with
+| ex ->
+    if someCondition then
+        raise ex """
+        
+        let expected = """
+try
+    foo ()
+with
+| ex ->
+    if someCondition then
+        reraise() """
+        
+        this.Parse source
+
+        Assert.IsTrue(this.ErrorsExist)
+        Assert.IsTrue(this.ErrorExistsAt(7, 8))
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(expected, result)

tests/FSharpLint.Core.Tests/Rules/Typography/NoTabCharacters.fs

@@ -27,3 +27,16 @@ type TestTypographyTabCharacterInFile() =
 
         Assert.IsFalse(this.ErrorExistsAt(2, 23))
         Assert.IsFalse(this.ErrorExistsAt(4, 13))
+
+    [<Test>]
+    member this.TabCharacterInFileSuggestedFix() =
+        let source = "\t"
+        let expected = String.replicate 4 " "
+        this.Parse source
+
+        Assert.IsTrue(this.ErrorExistsAt(1, 0))
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(expected, result)
+