From 9e19bd298b4028e67e537245b0e105574f6fce73 Mon Sep 17 00:00:00 2001 From: Parham Saremi Date: Wed, 20 Jul 2022 16:17:31 +0430 Subject: [PATCH 1/2] Update .gitignore add .vscode to .gitignore --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index f7a990a0c..85d024508 100644 --- a/.gitignore +++ b/.gitignore @@ -190,4 +190,5 @@ tests/FSharpLint.FunctionalTest.TestedProject/FSharpLintMSBuildTaskTest/ DesignTimeBuild/ .vs/ -.ionide/ \ No newline at end of file +.ionide/ +.vscode/ From dcf2cc4a54aa000aefaa6ef0a46a2a8e5f6b725b Mon Sep 17 00:00:00 2001 From: Parham Saremi Date: Wed, 20 Jul 2022 16:18:18 +0430 Subject: [PATCH 2/2] Add new rule IndexerAccessorStyleConsistency Checks for OCaml and CSharp styles. Fixes https://github.com/fsprojects/FSharpLint/issues/532 --- docs/content/how-tos/rule-configuration.md | 1 + docs/content/how-tos/rules/FL0077.md | 32 ++++++++++ .../Application/Configuration.fs | 7 ++- src/FSharpLint.Core/FSharpLint.Core.fsproj | 1 + .../IndexerAccessorStyleConsistency.fs | 53 +++++++++++++++++ src/FSharpLint.Core/Rules/Identifiers.fs | 1 + src/FSharpLint.Core/Text.resx | 3 + src/FSharpLint.Core/fsharplint.json | 6 ++ .../FSharpLint.Core.Tests.fsproj | 1 + .../IndexerAccessorStyleConsistency.fs | 58 +++++++++++++++++++ 10 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 docs/content/how-tos/rules/FL0077.md create mode 100644 src/FSharpLint.Core/Rules/Conventions/IndexerAccessorStyleConsistency.fs create mode 100644 tests/FSharpLint.Core.Tests/Rules/Conventions/IndexerAccessorStyleConsistency.fs diff --git a/docs/content/how-tos/rule-configuration.md b/docs/content/how-tos/rule-configuration.md index f53668bf9..e7b92346b 100644 --- a/docs/content/how-tos/rule-configuration.md +++ b/docs/content/how-tos/rule-configuration.md @@ -117,3 +117,4 @@ The following rules can be specified for linting. - [FavourConsistentThis (FL0074)](rules/FL0074.html) - [AvoidTooShortNames (FL0075)](rules/FL0075.html) - [FavourStaticEmptyFields (FL0076)](rules/FL0076.html) +- [IndexerAccessorStyleConsistency (FL0077)](rules/FL0077.html) diff --git a/docs/content/how-tos/rules/FL0077.md b/docs/content/how-tos/rules/FL0077.md new file mode 100644 index 000000000..070a50b3a --- /dev/null +++ b/docs/content/how-tos/rules/FL0077.md @@ -0,0 +1,32 @@ +--- +title: FL0077 +category: how-to +hide_menu: true +--- + +# IndexerAccessorStyleConsistency (FL0077) + +*Introduced in `0.21.3`* + +## Cause + +Use of OCaml style indexer accessors instead of CSharp or viceversa. + +## Rationale + +F# 6.0 introduces a new style for indexer accessor, similar to the one in C#. But it's convenient for our codebase to be consistent in which accessor style to use. + +## How To Fix + +If the config style is OCaml use `someArray.[1]` and if the default style is CSharp use `someArray[1]`. + +## Rule Settings + + { + "favourStaticEmptyFields": { + "enabled": false, + "config": { + "style": "OCaml" + } + } + } diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index 7762fe4f7..14c34e4a5 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -307,6 +307,7 @@ with type ConventionsConfig = { recursiveAsyncFunction:EnabledConfig option avoidTooShortNames:EnabledConfig option + indexerAccessorStyleConsistency: RuleConfig option redundantNewKeyword:EnabledConfig option favourStaticEmptyFields:EnabledConfig option nestedStatements:RuleConfig option @@ -324,7 +325,8 @@ with member this.Flatten() = [| this.recursiveAsyncFunction |> Option.bind (constructRuleIfEnabled RecursiveAsyncFunction.rule) |> Option.toArray - this.avoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule) |> Option.toArray + this.avoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule) |> Option.toArray + this.indexerAccessorStyleConsistency |> Option.bind (constructRuleWithConfig IndexerAccessorStyleConsistency.rule) |> Option.toArray this.redundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule) |> Option.toArray this.favourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule) |> Option.toArray this.favourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule) |> Option.toArray @@ -394,6 +396,7 @@ type Configuration = PatternMatchExpressionIndentation:EnabledConfig option RecursiveAsyncFunction:EnabledConfig option AvoidTooShortNames:EnabledConfig option + IndexerAccessorStyleConsistency:RuleConfig option RedundantNewKeyword:EnabledConfig option FavourReRaise:EnabledConfig option FavourStaticEmptyFields:EnabledConfig option @@ -478,6 +481,7 @@ with PatternMatchExpressionIndentation = None RecursiveAsyncFunction = None AvoidTooShortNames = None + IndexerAccessorStyleConsistency = None RedundantNewKeyword = None FavourReRaise = None FavourStaticEmptyFields = None @@ -625,6 +629,7 @@ let flattenConfig (config:Configuration) = config.PatternMatchExpressionIndentation |> Option.bind (constructRuleIfEnabled PatternMatchExpressionIndentation.rule) config.RecursiveAsyncFunction |> Option.bind (constructRuleIfEnabled RecursiveAsyncFunction.rule) config.AvoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule) + config.IndexerAccessorStyleConsistency |> Option.bind (constructRuleWithConfig IndexerAccessorStyleConsistency.rule) config.RedundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule) config.FavourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule) config.FavourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule) diff --git a/src/FSharpLint.Core/FSharpLint.Core.fsproj b/src/FSharpLint.Core/FSharpLint.Core.fsproj index aa55c8d30..2ad6b7e7f 100644 --- a/src/FSharpLint.Core/FSharpLint.Core.fsproj +++ b/src/FSharpLint.Core/FSharpLint.Core.fsproj @@ -104,6 +104,7 @@ + diff --git a/src/FSharpLint.Core/Rules/Conventions/IndexerAccessorStyleConsistency.fs b/src/FSharpLint.Core/Rules/Conventions/IndexerAccessorStyleConsistency.fs new file mode 100644 index 000000000..ec95e4eb1 --- /dev/null +++ b/src/FSharpLint.Core/Rules/Conventions/IndexerAccessorStyleConsistency.fs @@ -0,0 +1,53 @@ +module FSharpLint.Rules.IndexerAccessorStyleConsistency + +open FSharp.Compiler.Syntax +open FSharpLint.Framework.Ast +open FSharpLint.Framework.AstInfo +open FSharpLint.Framework.Rules +open FSharpLint.Framework.Suggestion +open FSharpLint.Framework +open System + +[] +type Config = { + Style: string +} + +let generateOutput (range: FSharp.Compiler.Text.Range) = + { Range = range + Message = Resources.GetString "RulesIndexerAccessorStyleConsistency" + SuggestedFix = None + TypeChecks = List.Empty } |> Array.singleton + +let runner (config: Config) (args: AstNodeRuleParams) = + let styleType = config.Style + if String.Equals (styleType, "ocaml", StringComparison.InvariantCultureIgnoreCase) then + match args.AstNode with + | AstNode.Binding binding -> + match binding with + | SynBinding (_, _, _, _, _, _, _, SynPat.Named _, _ + , SynExpr.App (ExprAtomicFlag.Atomic, _, SynExpr.Ident _, SynExpr.ArrayOrListOfSeqExpr (_, expr, range), _), _, _) -> + generateOutput range + | _ -> + Array.empty + | _ -> + Array.empty + elif String.Equals (styleType, "csharp", StringComparison.InvariantCultureIgnoreCase) then + match args.AstNode with + | AstNode.Binding binding -> + match binding with + | SynBinding (_, _, _, _, _, _, _, SynPat.Named _, _ + , SynExpr.DotIndexedGet (_, _, _, range), _, _) -> + generateOutput range + | _ -> + Array.empty + | _ -> + Array.empty + else + failwithf "Unknown style type %s" styleType + +let rule config = + { Name = "IndexerAccessorStyleConsistency" + Identifier = Identifiers.IndexerAccessorStyleConsistency + RuleConfig = { AstNodeRuleConfig.Runner = runner config; Cleanup = ignore } } + |> AstNodeRule diff --git a/src/FSharpLint.Core/Rules/Identifiers.fs b/src/FSharpLint.Core/Rules/Identifiers.fs index 95fd12259..d9a59fec5 100644 --- a/src/FSharpLint.Core/Rules/Identifiers.fs +++ b/src/FSharpLint.Core/Rules/Identifiers.fs @@ -81,3 +81,4 @@ let FavourReRaise = identifier 73 let FavourConsistentThis = identifier 74 let AvoidTooShortNames = identifier 75 let FavourStaticEmptyFields = identifier 76 +let IndexerAccessorStyleConsistency = identifier 77 diff --git a/src/FSharpLint.Core/Text.resx b/src/FSharpLint.Core/Text.resx index 1c90e1adc..609a574de 100644 --- a/src/FSharpLint.Core/Text.resx +++ b/src/FSharpLint.Core/Text.resx @@ -177,6 +177,9 @@ Consider using a longer name, as it is currently too short. + + Consider switching the indexer accessor from OCaml style to CSharp style or viceversa. + Consider changing `{0}` to be prefixed with `{1}`. diff --git a/src/FSharpLint.Core/fsharplint.json b/src/FSharpLint.Core/fsharplint.json index 06cf0a47e..0568e4cce 100644 --- a/src/FSharpLint.Core/fsharplint.json +++ b/src/FSharpLint.Core/fsharplint.json @@ -274,6 +274,12 @@ } }, "avoidTooShortNames": { "enabled": false }, + "indexerAccessorStyleConsistency": { + "enabled": false, + "config": { + "style": "OCaml" + } + }, "indentation": { "enabled": false }, diff --git a/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj b/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj index abc0ec7ca..41156c5ab 100644 --- a/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj +++ b/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj @@ -39,6 +39,7 @@ + diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/IndexerAccessorStyleConsistency.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/IndexerAccessorStyleConsistency.fs new file mode 100644 index 000000000..a4c14adb9 --- /dev/null +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/IndexerAccessorStyleConsistency.fs @@ -0,0 +1,58 @@ +module FSharpLint.Core.Tests.Rules.Conventions.IndexerAccessorStyleConsistency + +open NUnit.Framework +open FSharpLint.Framework.Rules +open FSharpLint.Rules.IndexerAccessorStyleConsistency +open FSharpLint.Rules + +[] +type TestConventionsIndexerAccessorStyleConsistencyCSharp() = + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(IndexerAccessorStyleConsistency.rule { Style = "CSharp" }) + + [] + member this.IndexerAccessorStyleConsistencyOCamlStyleWhenUsingCSharp() = + this.Parse """ +module Program + +let someArray = [| "foo" ; "bar" |] +let bar = someArray.[1] +System.Console.WriteLine bar""" + + Assert.IsTrue this.ErrorsExist + + [] + member this.IndexerAccessorStyleConsistencyCSharpStyleWhenUsingCSharp() = + this.Parse """ +module Program + +let someArray = [| "foo" ; "bar" |] +let bar = someArray[1] +System.Console.WriteLine bar""" + + Assert.IsTrue this.NoErrorsExist + +[] +type TestConventionsIndexerAccessorStyleConsistencyOCaml() = + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(IndexerAccessorStyleConsistency.rule { Style = "OCaml" }) + + [] + member this.IndexerAccessorStyleConsistencyCSharpStyleWhenUsingOCaml() = + this.Parse """ +module Program + +let someArray = [| "foo" ; "bar" |] +let bar = someArray[1] +System.Console.WriteLine bar""" + + Assert.IsTrue this.ErrorsExist + + [] + member this.IndexerAccessorStyleConsistencyOCamlStyleWhenUsingOCaml() = + this.Parse """ +module Program + +let someArray = [| "foo" ; "bar" |] +let bar = someArray.[1] +System.Console.WriteLine bar""" + + Assert.IsTrue this.NoErrorsExist