From 0beb43def20a00e62598388841fcd0e4b5de3584 Mon Sep 17 00:00:00 2001 From: webwarrior Date: Thu, 21 Dec 2023 14:56:45 +0100 Subject: [PATCH 1/3] Core.Tests: add tests for UsedUnderscorePrefixedElements quickfix Added tests for suggested quick fix for UsedUnderscorePrefixedElements rule. --- .../UsedUnderscorePrefixedElements.fs | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) 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) From 5403c27a5a1722c72ca077d78c9ed68dc96a3298 Mon Sep 17 00:00:00 2001 From: webwarrior Date: Mon, 25 Dec 2023 15:16:31 +0100 Subject: [PATCH 2/3] UsedUnderscorePrefixedElements: implement quick fix Implement quick fix for UsedUnderscorePrefixedElements. Quick fix is only applied if no other identifiers with suggested name (without underscore) exist. There are cases when such identifiers exist, but not in the same scope, and stil no quick fix is applied. Co-authored-by: Mehrshad --- .../Smells/UsedUnderscorePrefixedElements.fs | 49 ++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-) 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 From 345c6b601764fd15ce8cab4d78e944dd00d16e5b Mon Sep 17 00:00:00 2001 From: webwarrior-ws Date: Tue, 30 Dec 2025 12:27:02 +0100 Subject: [PATCH 3/3] Tests.Core: apply all suggested fixes Make ApplyQuickFix method apply all fixes and not only first one. It's necessary when testing rules that can create more than one suggested fix. Co-authored-by: Mehrshad Co-authored-by: Andrii Chebukin --- .../Rules/TestRuleBase.fs | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) 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