From 873d1452a0151f5541ff5470d7558cd0d6edd036 Mon Sep 17 00:00:00 2001 From: Nathanael Ribeiro Date: Sun, 9 Jun 2024 18:08:21 -0300 Subject: [PATCH] Add FavourAsKeyword rule (#709) * FavourAsKeyword: add rule and config * FavourAsKeyword: add docs * FavourAsKeyword: add tests * FavourAsKeyword: add rule to fsharplint.json * FavourAsKeyword: refactor tests * FavourAsKeyword: refactor error message text --- docs/content/how-tos/rule-configuration.md | 1 + docs/content/how-tos/rules/FL0086.md | 29 ++++++++++ .../Application/Configuration.fs | 7 ++- src/FSharpLint.Core/FSharpLint.Core.fsproj | 1 + .../Conventions/Binding/FavourAsKeyword.fs | 58 +++++++++++++++++++ src/FSharpLint.Core/Rules/Identifiers.fs | 1 + src/FSharpLint.Core/Text.resx | 5 +- src/FSharpLint.Core/fsharplint.json | 1 + .../FSharpLint.Core.Tests.fsproj | 1 + .../Rules/Binding/FavourAsKeyword.fs | 53 +++++++++++++++++ 10 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 docs/content/how-tos/rules/FL0086.md create mode 100644 src/FSharpLint.Core/Rules/Conventions/Binding/FavourAsKeyword.fs create mode 100644 tests/FSharpLint.Core.Tests/Rules/Binding/FavourAsKeyword.fs diff --git a/docs/content/how-tos/rule-configuration.md b/docs/content/how-tos/rule-configuration.md index 55022f3b5..ba7855f70 100644 --- a/docs/content/how-tos/rule-configuration.md +++ b/docs/content/how-tos/rule-configuration.md @@ -126,3 +126,4 @@ The following rules can be specified for linting. - [UnneededRecKeyword (FL0083)](rules/FL0083.html) - [FavourNonMutablePropertyInitialization (FL0084)](rules/FL0084.html) - [EnsureTailCallDiagnosticsInRecursiveFunctions (FL0085)](rules/FL0085.html) +- [FavourAsKeyword (FL0086)](rules/FL0086.html) diff --git a/docs/content/how-tos/rules/FL0086.md b/docs/content/how-tos/rules/FL0086.md new file mode 100644 index 000000000..30af8dc5b --- /dev/null +++ b/docs/content/how-tos/rules/FL0086.md @@ -0,0 +1,29 @@ +--- +title: FL0086 +category: how-to +hide_menu: true +--- + +# FavourAsKeyword (FL0086) + +*Introduced in `0.24.3`* + +## Cause + +A named pattern is used just to be compared in the guard against a constant expression e.g. `match something with | bar when bar = "baz" -> ()` + +## Rationale + +The named pattern can be changed to an as pattern that uses a constant pattern, improving the pattern matching exhaustiveness check + +## How To Fix + +Remove the guard and replace the named pattern with the as pattern using a constant pattern, e.g. change `match something with | bar when bar = "baz" -> ()` to `match something with | "baz" as bar -> ()` + +## Rule Settings + + { + "favourAsKeyword": { + "enabled": true + } + } diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index d5b25c0ee..1ba4ba8a2 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -297,6 +297,7 @@ type BindingConfig = wildcardNamedWithAsPattern:EnabledConfig option uselessBinding:EnabledConfig option tupleOfWildcards:EnabledConfig option + favourAsKeyword:EnabledConfig option favourTypedIgnore:EnabledConfig option } with member this.Flatten() = @@ -306,6 +307,7 @@ with this.wildcardNamedWithAsPattern |> Option.bind (constructRuleIfEnabled WildcardNamedWithAsPattern.rule) |> Option.toArray this.uselessBinding |> Option.bind (constructRuleIfEnabled UselessBinding.rule) |> Option.toArray this.tupleOfWildcards |> Option.bind (constructRuleIfEnabled TupleOfWildcards.rule) |> Option.toArray + this.favourAsKeyword |> Option.bind (constructRuleIfEnabled FavourAsKeyword.rule) |> Option.toArray |] |> Array.concat type ConventionsConfig = @@ -479,7 +481,8 @@ type Configuration = NoTabCharacters:EnabledConfig option NoPartialFunctions:RuleConfig option SuggestUseAutoProperty:EnabledConfig option - EnsureTailCallDiagnosticsInRecursiveFunctions:EnabledConfig option } + EnsureTailCallDiagnosticsInRecursiveFunctions:EnabledConfig option + FavourAsKeyword:EnabledConfig option } with static member Zero = { Global = None @@ -571,6 +574,7 @@ with NoPartialFunctions = None SuggestUseAutoProperty = None EnsureTailCallDiagnosticsInRecursiveFunctions = None + FavourAsKeyword = None } // fsharplint:enable RecordFieldNames @@ -725,6 +729,7 @@ let flattenConfig (config:Configuration) = config.NoTabCharacters |> Option.bind (constructRuleIfEnabled NoTabCharacters.rule) config.NoPartialFunctions |> Option.bind (constructRuleWithConfig NoPartialFunctions.rule) config.EnsureTailCallDiagnosticsInRecursiveFunctions |> Option.bind (constructRuleIfEnabled EnsureTailCallDiagnosticsInRecursiveFunctions.rule) + config.FavourAsKeyword |> Option.bind (constructRuleIfEnabled FavourAsKeyword.rule) |] |> Array.choose id if config.NonPublicValuesNames.IsSome && diff --git a/src/FSharpLint.Core/FSharpLint.Core.fsproj b/src/FSharpLint.Core/FSharpLint.Core.fsproj index 5c16d52fc..3f9649f2c 100644 --- a/src/FSharpLint.Core/FSharpLint.Core.fsproj +++ b/src/FSharpLint.Core/FSharpLint.Core.fsproj @@ -112,6 +112,7 @@ + diff --git a/src/FSharpLint.Core/Rules/Conventions/Binding/FavourAsKeyword.fs b/src/FSharpLint.Core/Rules/Conventions/Binding/FavourAsKeyword.fs new file mode 100644 index 000000000..dc7333289 --- /dev/null +++ b/src/FSharpLint.Core/Rules/Conventions/Binding/FavourAsKeyword.fs @@ -0,0 +1,58 @@ +module FSharpLint.Rules.FavourAsKeyword + +open System +open FSharpLint.Framework +open FSharpLint.Framework.Suggestion +open FSharp.Compiler.Syntax +open FSharp.Compiler.Text +open FSharpLint.Framework.Ast +open FSharpLint.Framework.Rules + +let private checkForNamedPatternEqualsConstant (args:AstNodeRuleParams) pattern whenExpr (range:Range) = + let patternIdent = + match pattern with + | SynPat.Named(SynIdent.SynIdent(ident, _), _, _, _) -> Some(ident.idText) + | _ -> None + + + match whenExpr with + | SynExpr.App(_, _, funcExpr, SynExpr.Const(_, constRange), _) -> + match funcExpr with + | SynExpr.App(_, _, ExpressionUtilities.Identifier([opIdent], _), SynExpr.Ident(ident), _) + when opIdent.idText = "op_Equality" && Option.contains ident.idText patternIdent -> + + let fromRange = Range.mkRange "" range.Start constRange.End + + let suggestedFix = + ExpressionUtilities.tryFindTextOfRange fromRange args.FileContent + |> Option.bind (fun text -> + + ExpressionUtilities.tryFindTextOfRange constRange args.FileContent + |> Option.bind (fun constText -> + + lazy (Some { FromText = text; FromRange = fromRange; ToText = constText + " as " + ident.idText}) + |> Some + ) + + ) + + { Range = fromRange + Message = Resources.GetString("RulesFavourAsKeyword") + SuggestedFix = suggestedFix + TypeChecks = [] } |> Array.singleton + + | _ -> Array.empty + | _ -> Array.empty + +let private runner (args:AstNodeRuleParams) = + match args.AstNode with + | AstNode.Match(SynMatchClause.SynMatchClause(pat, Some(whenExpr), _, range, _, _)) -> + checkForNamedPatternEqualsConstant args pat whenExpr range + + | _ -> Array.empty + +let rule = + { Name = "FavourAsKeyword" + Identifier = Identifiers.FavourAsKeyword + RuleConfig = { AstNodeRuleConfig.Runner = runner; Cleanup = ignore } } + |> AstNodeRule diff --git a/src/FSharpLint.Core/Rules/Identifiers.fs b/src/FSharpLint.Core/Rules/Identifiers.fs index 833ff2d13..c44b38b70 100644 --- a/src/FSharpLint.Core/Rules/Identifiers.fs +++ b/src/FSharpLint.Core/Rules/Identifiers.fs @@ -90,3 +90,4 @@ let UsedUnderscorePrefixedElements = identifier 82 let UnneededRecKeyword = identifier 83 let FavourNonMutablePropertyInitialization = identifier 84 let EnsureTailCallDiagnosticsInRecursiveFunctions = identifier 85 +let FavourAsKeyword = identifier 86 diff --git a/src/FSharpLint.Core/Text.resx b/src/FSharpLint.Core/Text.resx index a1d25417e..17681b968 100644 --- a/src/FSharpLint.Core/Text.resx +++ b/src/FSharpLint.Core/Text.resx @@ -368,5 +368,8 @@ The '{0}' function has a "rec" keyword, but no [<TailCall>] attribute. Consider adding [<TailCall>] attribute to the function and <WarningsAsErrors>FS3569</WarningsAsErrors> property to project file (but only on .NET 8 and higher). - + + + Prefer using the 'as' pattern to match a constant and bind it to a variable. + diff --git a/src/FSharpLint.Core/fsharplint.json b/src/FSharpLint.Core/fsharplint.json index 0fdf64934..8c5ec8307 100644 --- a/src/FSharpLint.Core/fsharplint.json +++ b/src/FSharpLint.Core/fsharplint.json @@ -332,6 +332,7 @@ } }, "ensureTailCallDiagnosticsInRecursiveFunctions": { "enabled": false }, + "favourAsKeyword": { "enabled": true }, "hints": { "add": [ "not (a = b) ===> a <> b", diff --git a/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj b/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj index 724951461..ddd4e4bd4 100644 --- a/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj +++ b/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj @@ -80,6 +80,7 @@ + diff --git a/tests/FSharpLint.Core.Tests/Rules/Binding/FavourAsKeyword.fs b/tests/FSharpLint.Core.Tests/Rules/Binding/FavourAsKeyword.fs new file mode 100644 index 000000000..94c9154c2 --- /dev/null +++ b/tests/FSharpLint.Core.Tests/Rules/Binding/FavourAsKeyword.fs @@ -0,0 +1,53 @@ +module FSharpLint.Core.Tests.Rules.Binding.FavourAsKeyword + +open NUnit.Framework +open FSharpLint.Rules + +[] +type TestBindingFavourAsKeyword() = + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(FavourAsKeyword.rule) + + [] + member this.FavourAsKeywordShouldQuickFix() = + let source = """ +module Program + +match "" with +| bar when bar = "baz" -> () +""" + + this.Parse(source) + + let expected = """ +module Program + +match "" with +| "baz" as bar -> () +""" + + Assert.AreEqual(expected, this.ApplyQuickFix source) + + + [] + member this.FavourAsKeywordShouldProduceError() = + this.Parse """ +module Program + +match "" with +| bar when bar = "baz" -> () +""" + + this.AssertErrorWithMessageExists("Prefer using the 'as' pattern to match a constant and bind it to a variable.") + + + [] + member this.FavourAsKeywordShouldNotProduceError() = + this.Parse """ +module Program + +match "" with +| "baz" as bar -> () +""" + + Assert.IsTrue(this.NoErrorsExist) +