diff --git a/src/FSharpLint.Core/Rules/Smells/UsedUnderscorePrefixedElements.fs b/src/FSharpLint.Core/Rules/Smells/UsedUnderscorePrefixedElements.fs index 513db8f9a..9a4e2d7a6 100644 --- a/src/FSharpLint.Core/Rules/Smells/UsedUnderscorePrefixedElements.fs +++ b/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) (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 diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/UsedUnderscorePrefixedElements.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/UsedUnderscorePrefixedElements.fs index ced27330f..fc5293ca8 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/UsedUnderscorePrefixedElements.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/UsedUnderscorePrefixedElements.fs @@ -124,3 +124,64 @@ type CustomerName(firstName) = """ Assert.IsFalse this.ErrorsExist + + [] + 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) + + [] + 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) + + [] + 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) diff --git a/tests/FSharpLint.Core.Tests/Rules/TestRuleBase.fs b/tests/FSharpLint.Core.Tests/Rules/TestRuleBase.fs index 2d03f1f77..7801e3f4b 100644 --- a/tests/FSharpLint.Core.Tests/Rules/TestRuleBase.fs +++ b/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>) -> 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>) = + 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 [] member _.SetUp() = suggestions.Clear() \ No newline at end of file