DRAFT: Add quick fix to rule82 (multi-fix)

src/FSharpLint.Core/Rules/Smells/UsedUnderscorePrefixedElements.fs

@@ -2,6 +2,9 @@ module FSharpLint.Rules.UsedUnderscorePrefixedElements
 
 open System
 
+open FSharp.Compiler.Symbols
+open FSharp.Compiler.CodeAnalysis
+
 open FSharpLint.Framework
 open FSharpLint.Framework.Suggestion
 open FSharp.Compiler.Syntax
@@ -13,29 +16,51 @@ open FSharpLint.Framework.Rules
 let runner (args: AstNodeRuleParams) =
     // hack to only run rule once
     if args.NodeIndex = 0 then
-        let processSymbolUse (usage: FSharpSymbolUse) =
+        let processSymbolUse (allUsages: seq<FSharpSymbolUse>) (usage: FSharpSymbolUse) = seq {
             match usage.Symbol with
-            | :? FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue as symbol -> 
+            | :? FSharpMemberOrFunctionOrValue as symbol -> 
                 let conditions =
                     not usage.IsFromDefinition
                     && symbol.FullName.StartsWith "_"
                     && symbol.FullName <> "_"
                     && not symbol.IsCompilerGenerated
                 if conditions then
-                    Some {
-                        Range = usage.Range
-                        Message = String.Format(Resources.GetString ("RulesUsedUnderscorePrefixedElements"))
-                        SuggestedFix = None
-                        TypeChecks = List.Empty
-                    }
+                    let nameWithoutUnderscore = symbol.DisplayName.[1..]
+                    let clashesWithOtherDefinitions =
+                        allUsages
+                        |> Seq.exists 
+                            (fun each -> each.Symbol.DisplayName = nameWithoutUnderscore && each.IsFromDefinition)
+
+                    if clashesWithOtherDefinitions then
+                        yield  {
+                            Range = usage.Range
+                            Message = String.Format(Resources.GetString ("RulesUsedUnderscorePrefixedElements"))
+                            SuggestedFix = None
+                            TypeChecks = List.Empty
+                        }
+                    else
+                        for range in [usage.Range; symbol.DeclarationLocation] do
+                            let warningDetrails = 
+                                lazy(
+                                    let fromText = symbol.DisplayName
+                                    Some { FromRange = range; FromText = fromText; ToText = nameWithoutUnderscore })
+                            yield {
+                                Range = range
+                                Message = String.Format(Resources.GetString ("RulesUsedUnderscorePrefixedElements"))
+                                SuggestedFix = Some warningDetrails
+                                TypeChecks = List.Empty
+                            }
                 else
-                    None
-            | _ -> None
+                    ()
+            | _ -> ()
+        }
 
         match args.CheckInfo with
         | Some checkResults -> 
-            checkResults.GetAllUsesOfAllSymbolsInFile() 
-            |> Seq.choose processSymbolUse
+            let allUsages = checkResults.GetAllUsesOfAllSymbolsInFile() 
+            allUsages
+            |> Seq.collect (processSymbolUse allUsages)
+            |> Seq.distinctBy (fun wargningDetails -> wargningDetails.Range)
             |> Seq.toArray
         | None -> Array.empty
     else

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

@@ -124,3 +124,64 @@ type CustomerName(firstName) =
 """
 
         Assert.IsFalse this.ErrorsExist
+
+    [<Test>]
+    member this.``Used variable with underscore prefix should be renamed by removing underscore``() =
+        let source = """
+module MyModule =
+    let MyFunc () =
+        let _random = System.Random()
+        printfn "%A" _random
+        ()
+
+    let Func2 () = 
+        ()"""
+
+        let expected = """
+module MyModule =
+    let MyFunc () =
+        let random = System.Random()
+        printfn "%A" random
+        ()
+
+    let Func2 () = 
+        ()"""
+
+        this.Parse source
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(expected, result)
+
+    [<Test>]
+    member this.``Used variable with underscore prefix should not be renamed if such renaming clashes with existing variable``() =
+        let source = """
+module MyModule =
+    let MyFunc () =
+        let random = 0
+        let _random = System.Random()
+        printfn "%A" _random
+        () """
+
+        this.Parse source
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(source, result)
+
+    [<Test>]
+    member this.``Used variable with underscore prefix should not be renamed if such renaming clashes with existing variable in outer scope``() =
+        let source = """
+module MyModule =
+    let random = 0
+
+    let MyFunc () =
+        let _random = System.Random()
+        printfn "%A" _random
+        () """
+
+        this.Parse source
+
+        let result = this.ApplyQuickFix source
+
+        Assert.AreEqual(source, result)

tests/FSharpLint.Core.Tests/Rules/TestRuleBase.fs

@@ -75,24 +75,29 @@ type TestRuleBase () =
         Assert.IsFalse(this.ErrorsExist, "Expected no errors, but was: " + this.ErrorMsg)
 
     member this.ApplyQuickFix (source:string) =
-        let firstSuggestedFix =
+        let suggestedFixes =
             suggestions
             |> Seq.choose (fun linterSuggestion -> linterSuggestion.Details.SuggestedFix)
-            |> Seq.tryHead
-
-        match Option.bind (fun (suggestedFix: Lazy<option<SuggestedFix>>) -> suggestedFix.Value) firstSuggestedFix with
-        | Some(fix) ->
-            let startIndex = ExpressionUtilities.findPos fix.FromRange.Start source
-            let endIndex = ExpressionUtilities.findPos fix.FromRange.End source
-
-            match (startIndex, endIndex) with
-            | (Some(startIndex), Some(endIndex)) ->
-                (StringBuilder source)
-                    .Remove(startIndex, endIndex - startIndex)
-                    .Insert(startIndex, fix.ToText)
-                    .ToString()
-            | _ -> source
-        | None -> source
+
+        let applyFix (source: string) (lazyFix: Lazy<Option<SuggestedFix>>) =
+            match lazyFix.Value with
+            | Some fix ->
+                let startIndex = ExpressionUtilities.findPos fix.FromRange.Start source
+                let endIndex = ExpressionUtilities.findPos fix.FromRange.End source
+
+                match (startIndex, endIndex) with
+                | (Some startIndex, Some endIndex) ->
+                    (StringBuilder source)
+                        .Remove(startIndex, endIndex - startIndex)
+                        .Insert(startIndex, fix.ToText)
+                        .ToString()
+                | _ -> source
+            | None -> source
+
+        Seq.fold 
+            applyFix
+            source
+            suggestedFixes
 
     [<SetUp>]
     member _.SetUp() = suggestions.Clear()