From c3b0b9af4e49eb4ce77ceeae6c0d22b572b8b687 Mon Sep 17 00:00:00 2001 From: webwarrior Date: Mon, 28 Jul 2025 13:18:34 +0200 Subject: [PATCH 1/2] Core(Framework,Rules): SuggestedFix refactoring Removed FromText property of SuggestedFix as it duplicates FromRange functionality and is not actually used anywhere. --- src/FSharpLint.Core/Framework/Suggestion.fs | 3 - .../Conventions/AvoidSinglePipeOperator.fs | 2 +- .../Conventions/Binding/FavourAsKeyword.fs | 2 +- .../Binding/FavourIgnoreOverLetWild.fs | 1 - .../Conventions/Binding/FavourTypedIgnore.fs | 3 +- .../Conventions/Binding/TupleOfWildcards.fs | 2 +- .../Conventions/Binding/UselessBinding.fs | 4 +- .../Binding/WildcardNamedWithAsPattern.fs | 2 +- .../Rules/Conventions/FailwithBadUsage.fs | 3 +- .../Rules/Conventions/FavourConsistentThis.fs | 2 +- .../Rules/Conventions/FavourReRaise.fs | 2 +- .../Conventions/FavourStaticEmptyFields.fs | 2 +- .../CanBeReplacedWithComposition.fs | 2 +- .../ReimplementsFunction.fs | 2 +- .../Rules/Conventions/Naming/NamingHelper.fs | 16 ++-- .../Rules/Conventions/NoPartialFunctions.fs | 74 ++++--------------- .../Conventions/RecursiveAsyncFunction.fs | 1 - .../Rules/Conventions/RedundantNewKeyword.fs | 2 +- .../Conventions/SuggestUseAutoProperty.fs | 3 +- .../Rules/Conventions/TypePrefixing.fs | 6 +- .../TupleFormatting/TupleCommaSpacing.fs | 1 - .../TupleFormatting/TupleParentheses.fs | 1 - .../Rules/Formatting/TypedItemSpacing.fs | 1 - .../Formatting/Typography/NoTabCharacters.fs | 1 - .../Rules/Hints/HintMatcher.fs | 2 +- 25 files changed, 42 insertions(+), 98 deletions(-) diff --git a/src/FSharpLint.Core/Framework/Suggestion.fs b/src/FSharpLint.Core/Framework/Suggestion.fs index afc263a78..47f6cc0b0 100644 --- a/src/FSharpLint.Core/Framework/Suggestion.fs +++ b/src/FSharpLint.Core/Framework/Suggestion.fs @@ -6,9 +6,6 @@ open FSharp.Compiler.Text /// Information for consuming applications to provide an automated fix for a lint suggestion. [] type SuggestedFix = { - /// Text to be replaced. - FromText:string - /// Location of the text to be replaced. FromRange:Range diff --git a/src/FSharpLint.Core/Rules/Conventions/AvoidSinglePipeOperator.fs b/src/FSharpLint.Core/Rules/Conventions/AvoidSinglePipeOperator.fs index 5eb9e0331..4998dc41b 100644 --- a/src/FSharpLint.Core/Rules/Conventions/AvoidSinglePipeOperator.fs +++ b/src/FSharpLint.Core/Rules/Conventions/AvoidSinglePipeOperator.fs @@ -52,7 +52,7 @@ let runner (args: AstNodeRuleParams) = match (maybeFuncText, maybeArgText) with | Some(funcText), Some(argText) -> let replacementText = sprintf "%s %s" funcText argText - Some { FromText=args.FileContent; FromRange=range; ToText=replacementText } + Some { FromRange=range; ToText=replacementText } | _ -> None) errors ident.idRange (Some suggestedFix) else diff --git a/src/FSharpLint.Core/Rules/Conventions/Binding/FavourAsKeyword.fs b/src/FSharpLint.Core/Rules/Conventions/Binding/FavourAsKeyword.fs index 81e78db32..2dbd4446d 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Binding/FavourAsKeyword.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Binding/FavourAsKeyword.fs @@ -29,7 +29,7 @@ let private checkForNamedPatternEqualsConstant (args:AstNodeRuleParams) pattern ExpressionUtilities.tryFindTextOfRange constRange args.FileContent |> Option.bind (fun constText -> - Some (lazy (Some { FromText = text; FromRange = fromRange; ToText = $"{constText} as {ident.idText}"})) + Some (lazy (Some { FromRange = fromRange; ToText = $"{constText} as {ident.idText}"})) ) ) diff --git a/src/FSharpLint.Core/Rules/Conventions/Binding/FavourIgnoreOverLetWild.fs b/src/FSharpLint.Core/Rules/Conventions/Binding/FavourIgnoreOverLetWild.fs index 4a40f86fa..6fc1fd1c3 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Binding/FavourIgnoreOverLetWild.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Binding/FavourIgnoreOverLetWild.fs @@ -20,7 +20,6 @@ let private checkForBindingToAWildcard pattern range fileContent (expr: SynExpr) { Range = range Message = Resources.GetString("RulesFavourIgnoreOverLetWildError") SuggestedFix = Some (lazy (Some({ FromRange = letBindingRange - FromText = fileContent ToText = sprintf "(%s) |> ignore" exprText }))) TypeChecks = List.Empty } else diff --git a/src/FSharpLint.Core/Rules/Conventions/Binding/FavourTypedIgnore.fs b/src/FSharpLint.Core/Rules/Conventions/Binding/FavourTypedIgnore.fs index ce5bec3cf..f60e2d7b1 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Binding/FavourTypedIgnore.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Binding/FavourTypedIgnore.fs @@ -15,8 +15,7 @@ let private runner (args: AstNodeRuleParams) = (ExpressionUtilities.tryFindTextOfRange range text |> Option.map (fun fromText -> - { FromText = fromText - FromRange = range + { FromRange = range ToText = identifier })) { diff --git a/src/FSharpLint.Core/Rules/Conventions/Binding/TupleOfWildcards.fs b/src/FSharpLint.Core/Rules/Conventions/Binding/TupleOfWildcards.fs index 87821f70f..70200faa3 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Binding/TupleOfWildcards.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Binding/TupleOfWildcards.fs @@ -28,7 +28,7 @@ let private checkTupleOfWildcards fileContents pattern identifier identifierRang let refactorTo = constructorString 1 let error = String.Format(errorFormat, refactorFrom, refactorTo) let suggestedFix = lazy( - Some { SuggestedFix.FromRange = identifierRange; FromText = fileContents; ToText = refactorTo }) + Some { SuggestedFix.FromRange = identifierRange; ToText = refactorTo }) Array.singleton { Range = range diff --git a/src/FSharpLint.Core/Rules/Conventions/Binding/UselessBinding.fs b/src/FSharpLint.Core/Rules/Conventions/Binding/UselessBinding.fs index 380457eb2..ee6191223 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Binding/UselessBinding.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Binding/UselessBinding.fs @@ -51,9 +51,9 @@ let private runner (args:AstNodeRuleParams) = let maybeSuggestedFix = match args.GetParents(args.NodeIndex) with | AstNode.ModuleDeclaration(SynModuleDecl.Let(_, _, range)) :: _ -> - Some({ FromRange = range; FromText = "let"; ToText = String.Empty }) + Some({ FromRange = range; ToText = String.Empty }) | AstNode.Expression(SynExpr.LetOrUse(_, false, _, _, range, _)) :: _ -> - Some({ FromRange = range; FromText = "use"; ToText = String.Empty }) + Some({ FromRange = range; ToText = String.Empty }) | _ -> None match args.AstNode with | AstNode.Binding(SynBinding(_, _, _, isMutable, _, _, _, pattern, _, expr, range, _, _)) diff --git a/src/FSharpLint.Core/Rules/Conventions/Binding/WildcardNamedWithAsPattern.fs b/src/FSharpLint.Core/Rules/Conventions/Binding/WildcardNamedWithAsPattern.fs index db0a4b854..784836db7 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Binding/WildcardNamedWithAsPattern.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Binding/WildcardNamedWithAsPattern.fs @@ -13,7 +13,7 @@ let private checkForWildcardNamedWithAsPattern fileContents pattern = when wildcardRange <> range -> let suggestedFix = lazy( - Some { FromRange = range; FromText = fileContents; ToText = identifier.idText }) + Some { FromRange = range; ToText = identifier.idText }) Array.singleton { Range = range Message = Resources.GetString("RulesWildcardNamedWithAsPattern") diff --git a/src/FSharpLint.Core/Rules/Conventions/FailwithBadUsage.fs b/src/FSharpLint.Core/Rules/Conventions/FailwithBadUsage.fs index 1ac1b8063..9d3c6cf8a 100644 --- a/src/FSharpLint.Core/Rules/Conventions/FailwithBadUsage.fs +++ b/src/FSharpLint.Core/Rules/Conventions/FailwithBadUsage.fs @@ -37,8 +37,7 @@ let private runner (args: AstNodeRuleParams) = Some( lazy (Some - { FromText = $"%s{failwithKeyword} %s{failwithErrorMessage}" - FromRange = range + { FromRange = range ToText = $"raise <| Exception(\"%s{failwithErrorMessage}\", %s{param})" }) ) | _ -> None diff --git a/src/FSharpLint.Core/Rules/Conventions/FavourConsistentThis.fs b/src/FSharpLint.Core/Rules/Conventions/FavourConsistentThis.fs index 4a4d800a1..e3dc346f5 100644 --- a/src/FSharpLint.Core/Rules/Conventions/FavourConsistentThis.fs +++ b/src/FSharpLint.Core/Rules/Conventions/FavourConsistentThis.fs @@ -24,7 +24,7 @@ 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 suggestedFix = lazy(Some({ FromRange = head.idRange; ToText = symbol })) let error = Array.singleton { Range = range diff --git a/src/FSharpLint.Core/Rules/Conventions/FavourReRaise.fs b/src/FSharpLint.Core/Rules/Conventions/FavourReRaise.fs index b1dfc2d2d..9d241e404 100644 --- a/src/FSharpLint.Core/Rules/Conventions/FavourReRaise.fs +++ b/src/FSharpLint.Core/Rules/Conventions/FavourReRaise.fs @@ -18,7 +18,7 @@ let private runner (args: AstNodeRuleParams) = 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()" })) + let suggestedFix = lazy(Some({ FromRange = range; ToText = "reraise()" })) match expression with | SynExpr.Ident ident -> match maybeIdent with diff --git a/src/FSharpLint.Core/Rules/Conventions/FavourStaticEmptyFields.fs b/src/FSharpLint.Core/Rules/Conventions/FavourStaticEmptyFields.fs index 48339f811..292e5c7a9 100644 --- a/src/FSharpLint.Core/Rules/Conventions/FavourStaticEmptyFields.fs +++ b/src/FSharpLint.Core/Rules/Conventions/FavourStaticEmptyFields.fs @@ -31,7 +31,7 @@ let private generateError (fileContents: string) (range:FSharp.Compiler.Text.Ran | EmptyStringLiteral -> "String.Empty" | EmptyListLiteral -> "List.Empty" | EmptyArrayLiteral -> "Array.empty" - Some({ FromRange = range; FromText = fileContents; ToText = replacementText })) + Some({ FromRange = range; ToText = replacementText })) Array.singleton { Range = range Message = getStaticEmptyErrorMessage range emptyLiteralType diff --git a/src/FSharpLint.Core/Rules/Conventions/FunctionReimplementation/CanBeReplacedWithComposition.fs b/src/FSharpLint.Core/Rules/Conventions/FunctionReimplementation/CanBeReplacedWithComposition.fs index fbfbb60bc..76e985cc6 100644 --- a/src/FSharpLint.Core/Rules/Conventions/FunctionReimplementation/CanBeReplacedWithComposition.fs +++ b/src/FSharpLint.Core/Rules/Conventions/FunctionReimplementation/CanBeReplacedWithComposition.fs @@ -64,7 +64,7 @@ let private validateLambdaCannotBeReplacedWithComposition fileContents _ lambda | Some funcStrings -> let suggestedFix = lazy( - Some { FromRange = range; FromText = fileContents; ToText = String.Join(" >> ", funcStrings) }) + Some { FromRange = range; ToText = String.Join(" >> ", funcStrings) }) Array.singleton { Range = range Message = Resources.GetString("RulesCanBeReplacedWithComposition") diff --git a/src/FSharpLint.Core/Rules/Conventions/FunctionReimplementation/ReimplementsFunction.fs b/src/FSharpLint.Core/Rules/Conventions/FunctionReimplementation/ReimplementsFunction.fs index 60bb0c259..210729d86 100644 --- a/src/FSharpLint.Core/Rules/Conventions/FunctionReimplementation/ReimplementsFunction.fs +++ b/src/FSharpLint.Core/Rules/Conventions/FunctionReimplementation/ReimplementsFunction.fs @@ -33,7 +33,7 @@ let private validateLambdaIsNotPointless (text:string) lambda range = let suggestedFix = lazy( ExpressionUtilities.tryFindTextOfRange range text - |> Option.map (fun fromText -> { FromText = fromText; FromRange = range; ToText = identifier })) + |> Option.map (fun fromText -> { FromRange = range; ToText = identifier })) { Range = range diff --git a/src/FSharpLint.Core/Rules/Conventions/Naming/NamingHelper.fs b/src/FSharpLint.Core/Rules/Conventions/Naming/NamingHelper.fs index 8e8fd6f4e..19f9fb19e 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Naming/NamingHelper.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Naming/NamingHelper.fs @@ -15,24 +15,24 @@ open FSharp.Compiler.Symbols module QuickFixes = let removeAllUnderscores (ident:Ident) = lazy( let toText = ident.idText.Replace("_", String.Empty) - Some { FromText = ident.idText; FromRange = ident.idRange; ToText = toText }) + Some { FromRange = ident.idRange; ToText = toText }) let removeNonPrefixingUnderscores (ident:Ident) = lazy( let prefixingUnderscores = ident.idText |> Seq.takeWhile (fun char -> char = '_') |> String.Concat let toText = prefixingUnderscores + ident.idText.Replace("_", String.Empty) - Some { FromText = ident.idText; FromRange = ident.idRange; ToText = toText }) + Some { FromRange = ident.idRange; ToText = toText }) let removePrefixingAndSuffixingUnderscores (ident:Ident) = lazy( let toText = ident.idText.Trim '_' - Some { FromText = ident.idText; FromRange = ident.idRange; ToText = toText }) + Some { FromRange = ident.idRange; ToText = toText }) let addPrefix prefix (ident:Ident) = lazy( - Some { FromText = ident.idText; FromRange = ident.idRange; ToText = prefix + ident.idText }) + Some { FromRange = ident.idRange; ToText = prefix + ident.idText }) let addSuffix suffix (ident:Ident) = lazy( - Some { FromText = ident.idText; FromRange = ident.idRange; ToText = ident.idText + suffix }) + Some { FromRange = ident.idRange; ToText = ident.idText + suffix }) let private mapFirstChar map (str:string) = let prefix = @@ -46,11 +46,11 @@ module QuickFixes = let toPascalCase (ident:Ident) = lazy( let pascalCaseIdent = mapFirstChar Char.ToUpper ident.idText - Some { FromText = ident.idText; FromRange = ident.idRange; ToText = pascalCaseIdent }) + Some { FromRange = ident.idRange; ToText = pascalCaseIdent }) let toCamelCase (ident:Ident) = lazy( let camelCaseIdent = mapFirstChar Char.ToLower ident.idText - Some { FromText = ident.idText; FromRange = ident.idRange; ToText = camelCaseIdent }) + Some { FromRange = ident.idRange; ToText = camelCaseIdent }) let splitByCaseChange (name: string) : seq = let partitionPoints = @@ -74,7 +74,7 @@ module QuickFixes = let parts = splitByCaseChange ident.idText |> Seq.map caseMapping String.Join('_', parts) | _ -> caseMapping ident.idText - Some { FromText = ident.idText; FromRange = ident.idRange; ToText = newIdent } + Some { FromRange = ident.idRange; ToText = newIdent } ) let toAllLowercase = convertAllToCase (fun part -> part.ToLower()) diff --git a/src/FSharpLint.Core/Rules/Conventions/NoPartialFunctions.fs b/src/FSharpLint.Core/Rules/Conventions/NoPartialFunctions.fs index f09cdc138..b2241ba3d 100644 --- a/src/FSharpLint.Core/Rules/Conventions/NoPartialFunctions.fs +++ b/src/FSharpLint.Core/Rules/Conventions/NoPartialFunctions.fs @@ -110,7 +110,7 @@ let private checkIfPartialIdentifier (config:Config) (identifier:string) (range: { Range = range Message = String.Format(Resources.GetString "RulesConventionsNoPartialFunctionsReplacementError", replacementFunction, identifier) - SuggestedFix = Some (lazy ( Some { FromText = identifier; FromRange = range; ToText = replacementFunction })) + SuggestedFix = Some (lazy ( Some { FromRange = range; ToText = replacementFunction })) TypeChecks = List.Empty }) @@ -295,41 +295,16 @@ let private isNonStaticInstanceMemberCall (checkFile:FSharpCheckFileResults) nam if typeMatches then match replacementStrategy with - | PatternMatch -> - Some - { - Range = range - Message = - String.Format( - Resources.GetString - "RulesConventionsNoPartialFunctionsPatternMatchError", - fullyQualifiedInstanceMember - ) - SuggestedFix = None - TypeChecks = (fun () -> typeMatches) |> List.singleton - } - | Function replacementFunctionName -> - Some - { - Range = range - Message = - String.Format( - Resources.GetString "RulesConventionsNoPartialFunctionsReplacementError", - replacementFunctionName, - fullyQualifiedInstanceMember - ) - SuggestedFix = - Some( - lazy - (Some - { - FromText = (String.concat "." names) - FromRange = range - ToText = replacementFunctionName - }) - ) - TypeChecks = (fun () -> typeMatches) |> List.singleton - } + | PatternMatch -> + Some { Range = range + Message = String.Format(Resources.GetString "RulesConventionsNoPartialFunctionsPatternMatchError", fullyQualifiedInstanceMember) + SuggestedFix = None + TypeChecks = (fun () -> typeMatches) |> List.singleton } + | Function replacementFunctionName -> + Some { Range = range + Message = String.Format(Resources.GetString "RulesConventionsNoPartialFunctionsReplacementError", replacementFunctionName, fullyQualifiedInstanceMember) + SuggestedFix = Some (lazy ( Some { FromRange = range; ToText = replacementFunctionName })) + TypeChecks = (fun () -> typeMatches) |> List.singleton } else None | _ -> None @@ -373,29 +348,10 @@ let private checkMemberCallOnExpression TypeChecks = (fun () -> true) |> List.singleton } | Function replacementFunctionName -> - Some - { - Range = originalRange - Message = - String.Format( - Resources.GetString "RulesConventionsNoPartialFunctionsReplacementError", - replacementFunctionName, - fullyQualifiedInstanceMember - ) - SuggestedFix = - Some( - lazy - (Some - { - FromText = - (ExpressionUtilities.tryFindTextOfRange originalRange flieContent) - .Value - FromRange = originalRange - ToText = replacementFunctionName - }) - ) - TypeChecks = (fun () -> true) |> List.singleton - } + Some { Range = originalRange + Message = String.Format(Resources.GetString "RulesConventionsNoPartialFunctionsReplacementError", replacementFunctionName, fullyQualifiedInstanceMember) + SuggestedFix = Some (lazy ( Some { FromRange = originalRange; ToText = replacementFunctionName })) + TypeChecks = (fun () -> true) |> List.singleton } else None diff --git a/src/FSharpLint.Core/Rules/Conventions/RecursiveAsyncFunction.fs b/src/FSharpLint.Core/Rules/Conventions/RecursiveAsyncFunction.fs index 834ba4ac0..a49af3a50 100644 --- a/src/FSharpLint.Core/Rules/Conventions/RecursiveAsyncFunction.fs +++ b/src/FSharpLint.Core/Rules/Conventions/RecursiveAsyncFunction.fs @@ -38,7 +38,6 @@ let checkRecursiveAsyncFunction (args:AstNodeRuleParams) (range:Range) (doBangEx (ExpressionUtilities.tryFindTextOfRange doTokenRange args.FileContent |> Option.map (fun fromText -> { - FromText = fromText FromRange = doTokenRange ToText = "return!" })) diff --git a/src/FSharpLint.Core/Rules/Conventions/RedundantNewKeyword.fs b/src/FSharpLint.Core/Rules/Conventions/RedundantNewKeyword.fs index 90d1a3064..c6d1881b5 100644 --- a/src/FSharpLint.Core/Rules/Conventions/RedundantNewKeyword.fs +++ b/src/FSharpLint.Core/Rules/Conventions/RedundantNewKeyword.fs @@ -40,7 +40,7 @@ let private generateFix (text:string) range = lazy( |> Option.map (fun fromText -> let withoutLeadingWhitespace = fromText.TrimStart() let newKeywordRemoved = withoutLeadingWhitespace.Substring(3).TrimStart() - { FromText = fromText; FromRange = range; ToText = newKeywordRemoved })) + { FromRange = range; ToText = newKeywordRemoved })) let runner args = diff --git a/src/FSharpLint.Core/Rules/Conventions/SuggestUseAutoProperty.fs b/src/FSharpLint.Core/Rules/Conventions/SuggestUseAutoProperty.fs index f98b6ce5c..22ee464fa 100644 --- a/src/FSharpLint.Core/Rules/Conventions/SuggestUseAutoProperty.fs +++ b/src/FSharpLint.Core/Rules/Conventions/SuggestUseAutoProperty.fs @@ -114,8 +114,7 @@ let private runner (args: AstNodeRuleParams) = (match memberIdentifier.LongIdent with | [ _; memberName ] -> Some - { FromText = args.FileContent - FromRange = memberIdentifier.Range + { FromRange = memberIdentifier.Range ToText = $"val {memberName.idText}" } | _ -> None) diff --git a/src/FSharpLint.Core/Rules/Conventions/TypePrefixing.fs b/src/FSharpLint.Core/Rules/Conventions/TypePrefixing.fs index 01b51ad1a..6f4dfdc84 100644 --- a/src/FSharpLint.Core/Rules/Conventions/TypePrefixing.fs +++ b/src/FSharpLint.Core/Rules/Conventions/TypePrefixing.fs @@ -33,7 +33,7 @@ let checkTypePrefixing (typePrefixingConfig: CheckTypePrefixingConfig) = let prefixSuggestion typeName = let suggestedFix = lazy( (ExpressionUtilities.tryFindTextOfRange typePrefixingConfig.Range typePrefixingConfig.Args.FileContent, typePrefixingConfig.TypeArgs) - ||> Option.map2 (fun fromText typeArgs -> { FromText = fromText; FromRange = typePrefixingConfig.Range; ToText = $"{typeName}<{typeArgs}>" })) + ||> Option.map2 (fun fromText typeArgs -> { FromRange = typePrefixingConfig.Range; ToText = $"{typeName}<{typeArgs}>" })) Some { Range = typePrefixingConfig.Range @@ -55,7 +55,7 @@ let checkTypePrefixing (typePrefixingConfig: CheckTypePrefixingConfig) = then let suggestedFix = lazy( (ExpressionUtilities.tryFindTextOfRange typePrefixingConfig.Range typePrefixingConfig.Args.FileContent, typePrefixingConfig.TypeArgs) - ||> Option.map2 (fun fromText typeArgs -> { FromText = fromText; FromRange = typePrefixingConfig.Range; ToText = $"{typeArgs} {typeName}" })) + ||> Option.map2 (fun fromText typeArgs -> { FromRange = typePrefixingConfig.Range; ToText = $"{typeArgs} {typeName}" })) Some { Range = typePrefixingConfig.Range @@ -73,7 +73,7 @@ let checkTypePrefixing (typePrefixingConfig: CheckTypePrefixingConfig) = // Prefer special postfix (e.g. int []). let suggestedFix = lazy( (ExpressionUtilities.tryFindTextOfRange typePrefixingConfig.Range typePrefixingConfig.Args.FileContent, typePrefixingConfig.TypeArgs) - ||> Option.map2 (fun fromText typeArgs -> { FromText = fromText; FromRange = typePrefixingConfig.Range; ToText = $"{typeArgs} []" })) + ||> Option.map2 (fun fromText typeArgs -> { FromRange = typePrefixingConfig.Range; ToText = $"{typeArgs} []" })) Some { Range = typePrefixingConfig.Range diff --git a/src/FSharpLint.Core/Rules/Formatting/TupleFormatting/TupleCommaSpacing.fs b/src/FSharpLint.Core/Rules/Formatting/TupleFormatting/TupleCommaSpacing.fs index beda74140..ccef6fe57 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TupleFormatting/TupleCommaSpacing.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TupleFormatting/TupleCommaSpacing.fs @@ -19,7 +19,6 @@ let checkTupleCommaSpacing (args:AstNodeRuleParams) (tupleExprs:SynExpr list) tu (Some { FromRange = commaRange - FromText = commaText ToText = ", " }) let suggestedFix = diff --git a/src/FSharpLint.Core/Rules/Formatting/TupleFormatting/TupleParentheses.fs b/src/FSharpLint.Core/Rules/Formatting/TupleFormatting/TupleParentheses.fs index d388940b5..bace8da17 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TupleFormatting/TupleParentheses.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TupleFormatting/TupleParentheses.fs @@ -15,7 +15,6 @@ let checkTupleHasParentheses (args:AstNodeRuleParams) _ range parentNode = (Some { FromRange = range - FromText = text ToText = $"({text})" }) diff --git a/src/FSharpLint.Core/Rules/Formatting/TypedItemSpacing.fs b/src/FSharpLint.Core/Rules/Formatting/TypedItemSpacing.fs index 683e436a9..937983158 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TypedItemSpacing.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TypedItemSpacing.fs @@ -56,7 +56,6 @@ let private checkRange (config:Config) (args:AstNodeRuleParams) (range:Range) = let spacesAfterString = " " |> String.replicate expectedSpacesAfter let suggestedFix = lazy( Some { FromRange = range; - FromText = text; ToText = $"{trimmedOtherText}{spacesBeforeString}:{spacesAfterString}{trimmedTypeText}" } ) let errorFormatString = Resources.GetString("RulesFormattingTypedItemSpacingError") diff --git a/src/FSharpLint.Core/Rules/Formatting/Typography/NoTabCharacters.fs b/src/FSharpLint.Core/Rules/Formatting/Typography/NoTabCharacters.fs index 4fa34d427..114e5d2a9 100644 --- a/src/FSharpLint.Core/Rules/Formatting/Typography/NoTabCharacters.fs +++ b/src/FSharpLint.Core/Rules/Formatting/Typography/NoTabCharacters.fs @@ -34,7 +34,6 @@ let checkNoTabCharacters literalStrings (args:LineRuleParams) = lazy (Some( { FromRange = range - FromText = "\t" ToText = String.replicate args.GlobalConfig.numIndentationSpaces " " } )) ) diff --git a/src/FSharpLint.Core/Rules/Hints/HintMatcher.fs b/src/FSharpLint.Core/Rules/Hints/HintMatcher.fs index 12c5c24d6..c5a9e4dc1 100644 --- a/src/FSharpLint.Core/Rules/Hints/HintMatcher.fs +++ b/src/FSharpLint.Core/Rules/Hints/HintMatcher.fs @@ -696,7 +696,7 @@ let private hintError (config: HintErrorConfig) = let suggestedFix = lazy( ExpressionUtilities.tryFindTextOfRange config.Range config.Args.FileContent - |> Option.map (fun fromText -> { FromText = fromText; FromRange = config.Range; ToText = toText })) + |> Option.map (fun fromText -> { FromRange = config.Range; ToText = toText })) { Range = config.Range; Message = error; SuggestedFix = Some suggestedFix; TypeChecks = config.TypeChecks } | Suggestion.Message(message) -> From 73eeff5be794f2e6bab0c59ded55692c5c1b55ec Mon Sep 17 00:00:00 2001 From: ijanus Date: Thu, 30 Jun 2022 13:28:44 +0100 Subject: [PATCH 2/2] Add fix command-line to apply quickfixes and unit test Co-authored-by: Parham Saremi Co-authored-by: webwarrior-ws --- src/FSharpLint.Console/Program.fs | 222 ++++++++++++++---- .../Application/Configuration.fs | 27 ++- src/FSharpLint.Core/Application/Lint.fs | 4 +- src/FSharpLint.Core/Application/Lint.fsi | 2 + src/FSharpLint.Core/Text.resx | 3 + tests/FSharpLint.Console.Tests/TestApp.fs | 62 +++++ 6 files changed, 265 insertions(+), 55 deletions(-) diff --git a/src/FSharpLint.Console/Program.fs b/src/FSharpLint.Console/Program.fs index d62691ed3..b03011294 100644 --- a/src/FSharpLint.Console/Program.fs +++ b/src/FSharpLint.Console/Program.fs @@ -4,6 +4,8 @@ open Argu open System open System.IO open System.Reflection +open System.Linq +open System.Text open FSharpLint.Framework open FSharpLint.Application @@ -23,12 +25,17 @@ type internal FileType = type ExitCode = | Failure = -1 | Success = 0 + | NoSuchRuleName = 1 + | NoSuggestedFix = 2 + +let fileTypeHelp = "Input type the linter will run against. If this is not set, the file type will be inferred from the file extension." // Allowing underscores in union case names for proper Argu command line option formatting. // fsharplint:disable UnionCasesNames type private ToolArgs = | [] Format of OutputFormat | [] Lint of ParseResults + | [] Fix of ParseResults | Version with interface IArgParserTemplate with @@ -36,6 +43,7 @@ with match this with | Format _ -> "Output format of the linter." | Lint _ -> "Runs FSharpLint against a file or a collection of files." + | Fix _ -> "Apply quickfixes for specified rule name or names (comma separated)." | Version -> "Prints current version." // TODO: investigate erroneous warning on this type definition @@ -50,10 +58,32 @@ with member this.Usage = match this with | Target _ -> "Input to lint." - | File_Type _ -> "Input type the linter will run against. If this is not set, the file type will be inferred from the file extension." + | File_Type _ -> fileTypeHelp | Lint_Config _ -> "Path to the config for the lint." + +// TODO: investigate erroneous warning on this type definition +// fsharplint:disable UnionDefinitionIndentation +and private FixArgs = + | [] Fix_Target of ruleName:string * target:string + | Fix_File_Type of FileType +// fsharplint:enable UnionDefinitionIndentation +with + interface IArgParserTemplate with + member this.Usage = + match this with + | Fix_Target _ -> "Rule name to be applied with suggestedFix and input to lint." + | Fix_File_Type _ -> fileTypeHelp // fsharplint:enable UnionCasesNames +type private LintingArgs = + { + FileType: FileType + LintParams: OptionalLintParameters + Target: string + ToolsPath: Ionide.ProjInfo.Types.ToolsPath + RuleNameToApplySuggestedFixFrom: string option + } + /// Expands a wildcard pattern to a list of matching files. /// Supports recursive search using ** (e.g., "**/*.fs" or "src/**/*.fs") let internal expandWildcard (pattern:string) = @@ -117,69 +147,167 @@ let internal inferFileType (target:string) = else FileType.Source -let private lint - (lintArgs: ParseResults) - (output: Output.IOutput) - (toolsPath:Ionide.ProjInfo.Types.ToolsPath) - : ExitCode = - let mutable exitCode = ExitCode.Success +let private outputWarnings (output: Output.IOutput) (warnings: List) = + String.Format(Resources.GetString "ConsoleFinished", List.length warnings) + |> output.WriteInfo + +let private handleLintResult (output: Output.IOutput) (lintResult: LintResult) = + match lintResult with + | LintResult.Success warnings -> + outputWarnings output warnings + if List.isEmpty warnings |> not then + ExitCode.Failure + else + ExitCode.Success + | LintResult.Failure failure -> + output.WriteError failure.Description + ExitCode.Failure + +let private getParams (output: Output.IOutput) config = + let paramConfig = + match config with + | Some configPath -> FromFile configPath + | None -> Default - let handleError (str:string) = - output.WriteError str - exitCode <- ExitCode.Failure + { CancellationToken = None + ReceivedWarning = Some output.WriteWarning + Configuration = paramConfig + ReportLinterProgress = parserProgress output |> Some } - let handleLintResult = function - | LintResult.Success(warnings) -> - String.Format(Resources.GetString("ConsoleFinished"), List.length warnings) - |> output.WriteInfo - if not (List.isEmpty warnings) then - exitCode <- ExitCode.Failure - | LintResult.Failure(failure) -> - handleError failure.Description +let private handleFixResult (output: Output.IOutput) (target: string) (ruleName: string) (lintResult: LintResult) : ExitCode = + match lintResult with + | LintResult.Success warnings -> + String.Format(Resources.GetString "ConsoleApplyingSuggestedFixFile", target) |> output.WriteInfo + let increment = 1 + let noFixIncrement = 0 - let lintConfig = lintArgs.TryGetResult Lint_Config + let countFixes (element: Suggestion.LintWarning) = + let sourceCode = File.ReadAllText element.FilePath + if String.Equals(ruleName, element.RuleName, StringComparison.InvariantCultureIgnoreCase) then + match element.Details.SuggestedFix with + | Some lazySuggestedFix -> + match lazySuggestedFix.Force() with + | Some suggestedFix -> + let updatedSourceCode = + let builder = StringBuilder(sourceCode.Length + suggestedFix.ToText.Length) + let firstPart = + sourceCode.AsSpan( + 0, + (ExpressionUtilities.findPos suggestedFix.FromRange.Start sourceCode) + |> Option.defaultWith + (fun () -> failwith "Could not find index from position (suggestedFix.FromRange.Start)") + ) + let secondPart = + sourceCode.AsSpan + (ExpressionUtilities.findPos suggestedFix.FromRange.End sourceCode + |> Option.defaultWith + (fun () -> failwith "Could not find index from position (suggestedFix.FromRange.End)")) + builder + .Append(firstPart) + .Append(suggestedFix.ToText) + .Append(secondPart) + .ToString() + File.WriteAllText( + element.FilePath, + updatedSourceCode, + Encoding.UTF8) + | _ -> () + increment + | None -> noFixIncrement + else + noFixIncrement - let configParam = - match lintConfig with - | Some configPath -> FromFile configPath - | None -> Default + let countSuggestedFix = + warnings |> List.sumBy countFixes + outputWarnings output warnings - let lintParams = - { - CancellationToken = None - ReceivedWarning = Some output.WriteWarning - Configuration = configParam - ReportLinterProgress = Some (parserProgress output) - } + if countSuggestedFix > 0 then + ExitCode.Success + else + ExitCode.NoSuggestedFix - let target = lintArgs.GetResult Target - let fileType = lintArgs.TryGetResult File_Type |> Option.defaultValue (inferFileType target) + | LintResult.Failure failure -> + output.WriteError failure.Description + ExitCode.Failure +let private performLinting (output: Output.IOutput) (args: LintingArgs) = try let lintResult = - match fileType with - | FileType.File -> Lint.asyncLintFile lintParams target |> Async.RunSynchronously - | FileType.Source -> Lint.asyncLintSource lintParams target |> Async.RunSynchronously - | FileType.Solution -> Lint.asyncLintSolution lintParams target toolsPath |> Async.RunSynchronously + match args.FileType with + | FileType.File -> Lint.asyncLintFile args.LintParams args.Target |> Async.RunSynchronously + | FileType.Source -> Lint.asyncLintSource args.LintParams args.Target |> Async.RunSynchronously + | FileType.Solution -> Lint.asyncLintSolution args.LintParams args.Target args.ToolsPath |> Async.RunSynchronously | FileType.Wildcard -> output.WriteInfo "Wildcard detected, but not recommended. Using a project (slnx/sln/fsproj) can detect more issues." - let files = expandWildcard target + let files = expandWildcard args.Target if List.isEmpty files then - output.WriteInfo $"No files matching pattern '%s{target}' were found." + output.WriteInfo $"No files matching pattern '%s{args.Target}' were found." LintResult.Success List.empty else - output.WriteInfo $"Found %d{List.length files} file(s) matching pattern '%s{target}'." - Lint.asyncLintFiles lintParams files |> Async.RunSynchronously + output.WriteInfo $"Found %d{List.length files} file(s) matching pattern '%s{args.Target}'." + Lint.asyncLintFiles args.LintParams files |> Async.RunSynchronously | FileType.Project - | _ -> Lint.asyncLintProject lintParams target toolsPath |> Async.RunSynchronously - handleLintResult lintResult + | _ -> Lint.asyncLintProject args.LintParams args.Target args.ToolsPath |> Async.RunSynchronously + + match args.RuleNameToApplySuggestedFixFrom with + | Some ruleName -> handleFixResult output args.Target ruleName lintResult + | None -> handleLintResult output lintResult with | exn -> - let target = if fileType = FileType.Source then "source" else target - handleError - $"Lint failed while analysing %s{target}.{Environment.NewLine}Failed with: %s{exn.Message}{Environment.NewLine}Stack trace: {exn.StackTrace}" + let target = if args.FileType = FileType.Source then "source" else args.Target + output.WriteError $"Lint failed while analysing %s{target}.{Environment.NewLine}Failed with: %s{exn.Message}{Environment.NewLine}Stack trace: {exn.StackTrace}" + ExitCode.Failure - exitCode +let private lint + (lintArgs: ParseResults) + (output: Output.IOutput) + (toolsPath:Ionide.ProjInfo.Types.ToolsPath) + : ExitCode = + let lintConfig = lintArgs.TryGetResult Lint_Config + + let lintParams = getParams output lintConfig + let target = lintArgs.GetResult Target + let fileType = lintArgs.TryGetResult File_Type |> Option.defaultValue (inferFileType target) + + performLinting + output + { FileType = fileType + LintParams = lintParams + Target = target + ToolsPath = toolsPath + RuleNameToApplySuggestedFixFrom = None } + +let private applySuggestedFix (fixArgs: ParseResults) (output: Output.IOutput) toolsPath = + let fixParams = getParams output None + let ruleName, target = fixArgs.GetResult Fix_Target + let fileType = fixArgs.TryGetResult Fix_File_Type |> Option.defaultValue (inferFileType target) + + let allRules = + match getConfig fixParams.Configuration with + | Ok config -> Some (Configuration.flattenConfig config false) + | _ -> None + + let allRuleNames = + match allRules with + | Some rules -> (fun (loadedRules:Configuration.LoadedRules) -> ([| + loadedRules.LineRules.IndentationRule |> Option.map (fun rule -> rule.Name) |> Option.toArray + loadedRules.LineRules.NoTabCharactersRule |> Option.map (fun rule -> rule.Name) |> Option.toArray + loadedRules.LineRules.GenericLineRules |> Array.map (fun rule -> rule.Name) + loadedRules.AstNodeRules |> Array.map (fun rule -> rule.Name) + |] |> Array.concat |> Set.ofArray)) rules + | _ -> Set.empty + + if allRuleNames.Any(fun aRuleName -> String.Equals(aRuleName, ruleName, StringComparison.InvariantCultureIgnoreCase)) then + performLinting + output + { FileType = fileType + LintParams = fixParams + Target = target + ToolsPath = toolsPath + RuleNameToApplySuggestedFixFrom = Some ruleName } + else + output.WriteError <| sprintf "Rule '%s' does not exist." ruleName + ExitCode.NoSuchRuleName let private start (arguments:ParseResults) (toolsPath:Ionide.ProjInfo.Types.ToolsPath) = let output = @@ -203,6 +331,8 @@ let private start (arguments:ParseResults) (toolsPath:Ionide.ProjInfo. match arguments.GetSubCommand() with | Lint lintArgs -> lint lintArgs output toolsPath + | Fix fixArgs -> + applySuggestedFix fixArgs output toolsPath | _ -> ExitCode.Failure diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index f02086d1e..530120db9 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -150,13 +150,20 @@ type RuleConfig<'Config type EnabledConfig = RuleConfig -let constructRuleIfEnabled rule ruleConfig = if ruleConfig.Enabled then Some rule else None +let constructRuleIfEnabledBase (onlyEnabled: bool) rule ruleConfig = + if not onlyEnabled || ruleConfig.Enabled then Some rule else None -let constructRuleWithConfig rule ruleConfig = - if ruleConfig.Enabled then - Option.map rule ruleConfig.Config - else - None +let constructRuleIfEnabled rule ruleConfig = + constructRuleIfEnabledBase true rule ruleConfig + +let constructRuleWithConfigBase (onlyEnabled: bool) (rule: 'TRuleConfig -> 'TRule) (ruleConfig: RuleConfig<'TRuleConfig>): Option<'TRule> = + if not onlyEnabled || ruleConfig.Enabled then + ruleConfig.Config |> Option.map rule + else + None + +let constructRuleWithConfig (rule: 'TRuleConfig -> 'TRule) (ruleConfig: RuleConfig<'TRuleConfig>): Option<'TRule> = + constructRuleWithConfigBase true rule ruleConfig let constructTypePrefixingRuleWithConfig rule (ruleConfig: RuleConfig) = if ruleConfig.Enabled then @@ -755,7 +762,7 @@ let findDeprecation config deprecatedAllRules allRules = } // fsharplint:disable MaxLinesInFunction -let flattenConfig (config:Configuration) = +let flattenConfig (config:Configuration) (onlyEnabled:bool) = let deprecatedAllRules = Array.concat [| @@ -768,6 +775,12 @@ let flattenConfig (config:Configuration) = config.Hints |> Option.map (fun config -> HintMatcher.rule { HintMatcher.Config.HintTrie = parseHints (getOrEmptyList config.add) }) |> Option.toArray |] + let constructRuleIfEnabled rule ruleConfig = + constructRuleIfEnabledBase onlyEnabled rule ruleConfig + + let constructRuleWithConfig (rule: 'TRuleConfig -> 'TRule) (ruleConfig: RuleConfig<'TRuleConfig>): Option<'TRule> = + constructRuleWithConfigBase onlyEnabled rule ruleConfig + let allRules = Array.choose id diff --git a/src/FSharpLint.Core/Application/Lint.fs b/src/FSharpLint.Core/Application/Lint.fs index eca7b1fd3..120bd7333 100644 --- a/src/FSharpLint.Core/Application/Lint.fs +++ b/src/FSharpLint.Core/Application/Lint.fs @@ -239,7 +239,7 @@ module Lint = | Some(value) -> not value.IsCancellationRequested | None -> true - let enabledRules = Configuration.flattenConfig lintInfo.Configuration + let enabledRules = Configuration.flattenConfig lintInfo.Configuration true let lines = String.toLines fileInfo.Text |> Array.map (fun (line, _, _) -> line) let allRuleNames = @@ -422,7 +422,7 @@ module Lint = } /// Gets a FSharpLint Configuration based on the provided ConfigurationParam. - let private getConfig (configParam:ConfigurationParam) = + let getConfig (configParam:ConfigurationParam) = match configParam with | Configuration config -> Ok config | FromFile filePath -> diff --git a/src/FSharpLint.Core/Application/Lint.fsi b/src/FSharpLint.Core/Application/Lint.fsi index 618d48317..9c22e4255 100644 --- a/src/FSharpLint.Core/Application/Lint.fsi +++ b/src/FSharpLint.Core/Application/Lint.fsi @@ -188,3 +188,5 @@ module Lint = /// Lints an F# file that has already been parsed using /// `FSharp.Compiler.Services` in the calling application. val lintParsedFile : optionalParams:OptionalLintParameters -> parsedFileInfo:ParsedFileInformation -> filePath:string -> LintResult + + val getConfig : ConfigurationParam -> Result diff --git a/src/FSharpLint.Core/Text.resx b/src/FSharpLint.Core/Text.resx index a6f38a3f4..9d891c0f5 100644 --- a/src/FSharpLint.Core/Text.resx +++ b/src/FSharpLint.Core/Text.resx @@ -138,6 +138,9 @@ ========== Linting {0} ========== + + ========== Applying fixes to {0} ========== + MSBuild could not load the project file {0} because: {1} diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 432d233d3..d3d7c7d04 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -97,6 +97,68 @@ type TestConsoleApplication() = Assert.AreEqual(int ExitCode.Success, returnCode) Assert.AreEqual(Set.empty, errors) + [] + member __.``Lint source with fix option``() = + let sourceCode = """ +module Fass = + let foo = new System.Collections.Generic.Dictionary() |> ignore + let goo = new Guid() |> ignore + let ntoo = new Int32() |> ignore +module Fall = + let uoo = new Uid() |> ignore + let version = new System.Version() + let xoo = new Uint32() |> ignore + """ + + let expected = """ +module Fass = + let foo = System.Collections.Generic.Dictionary() |> ignore + let goo = Guid() |> ignore + let ntoo = Int32() |> ignore +module Fall = + let uoo = Uid() |> ignore + let version = System.Version() + let xoo = Uint32() |> ignore + """ + let ruleName = "RedundantNewKeyword" + use input = new TemporaryFile(sourceCode, "fs") + let (exitCode, errors) = main [| "fix"; ruleName; input.FileName |] + + Assert.AreEqual(int ExitCode.Success, exitCode) + Assert.AreEqual(set ["Usage of `new` keyword here is redundant."], errors) + Assert.AreEqual(expected, File.ReadAllText input.FileName) + + [] + member __.``Lint source with fix option with wrong rulename``() = + let sourceCode = """ +printfn "Hello" + """ + + let ruleName = "ssrffss" + use input = new TemporaryFile(sourceCode, "fs") + let (exitCode, errors) = main [| "fix"; ruleName; input.FileName |] + + Assert.AreEqual(int ExitCode.NoSuchRuleName, exitCode) + + [] + member __.``Lint source with fix option no need for fix``() = + let sourceCode = """ +module Fass = + let foo = System.Collections.Generic.Dictionary() |> ignore + let goo = Guid() |> ignore + let ntoo = Int32() |> ignore +module Fall = + let uoo = Uid() |> ignore + let version = System.Version() + let xoo = Uint32() |> ignore + """ + let ruleName = "RedundantNewKeyword" + use input = new TemporaryFile(sourceCode, "fs") + let (exitCode, errors) = main [| "fix"; ruleName; input.FileName |] + + Assert.AreEqual(int ExitCode.NoSuggestedFix, exitCode) + Assert.AreEqual(sourceCode, File.ReadAllText input.FileName) + [] member _.``Regression test: typePrefixing rule with old config format should still work``() = let fileContent = """