Merge PR #794 from knocte/issue532-squashed-fixed

IndexerAccessorStyleConsistency: new rule.

Author: knocte

docs/content/how-tos/rule-configuration.md

@@ -128,3 +128,4 @@ The following rules can be specified for linting.
 - [EnsureTailCallDiagnosticsInRecursiveFunctions (FL0085)](rules/FL0085.html)
 - [FavourAsKeyword (FL0086)](rules/FL0086.html)
 - [InterpolatedStringWithNoSubstitution (FL0087)](rules/FL0087.html)
+- [IndexerAccessorStyleConsistency (FL0088)](rules/FL0088.html)

docs/content/how-tos/rules/FL0088.md

@@ -0,0 +1,36 @@
+---
+title: FL0088
+category: how-to
+hide_menu: true
+---
+
+# IndexerAccessorStyleConsistency (FL0088)
+
+*Introduced in `0.26.10`*
+
+## Cause
+
+Use of different styles of indexer accessors in the same codebase without consistency.
+
+## Rationale
+
+F# 6.0 introduced a new style for indexer accessor, similar to the syntax of C#. But if your team is not
+convinced about this new style, you might want to make sure to stick with the previous OCaml style. Otherwise,
+you might want to convert all the accessors in your codebase to the new style. Consistency aides readability.
+
+## How To Fix
+
+If the config style is OCaml use `someArray.[0]` and if the default style is CSharp use `someArray[0]`.
+
+## Rule Settings
+
+    {
+        "indexerAccessorStyleConsistency": {
+            "enabled": false,
+            "config": {
+                "style": "OCaml"
+            }
+        }
+    }
+
+* *style* - chosen style for this convention: "OCaml" or "CSharp".

src/FSharpLint.Core/Application/Configuration.fs

@@ -6,6 +6,7 @@ open System.IO
 open System.Reflection
 open System.Text.Json
 open System.Text.Json.Serialization
+open Microsoft.FSharp.Reflection
 open FSharpLint.Framework
 open FSharpLint.Framework.Rules
 open FSharpLint.Framework.HintParser
@@ -17,6 +18,40 @@ let SettingsFileName = "fsharplint.json"
 exception ConfigurationException of string
 
 module internal FSharpJsonConverter =
+    
+    type private SimpleDUConverter<'TDiscriminatedUnion>() =
+        inherit JsonConverter<'TDiscriminatedUnion>()
+
+        let allCases = FSharpType.GetUnionCases typeof<'TDiscriminatedUnion> 
+
+        override this.Read (reader: byref<Text.Json.Utf8JsonReader>, typeToConvert: Type, options: Text.Json.JsonSerializerOptions): 'TDiscriminatedUnion = 
+            let value = reader.GetString()
+            match allCases |> Array.tryFind (fun case -> case.Name = value) with
+            | Some case -> FSharpValue.MakeUnion(case, Array.empty) :?> 'TDiscriminatedUnion
+            | _ -> failwithf "Unexpected value: %A. Expected one of: %A" value allCases
+
+        override this.Write (writer: Text.Json.Utf8JsonWriter, value: 'TDiscriminatedUnion, options: Text.Json.JsonSerializerOptions): unit = 
+            writer.WriteStringValue(value.ToString())
+
+    /// JSON converter  for discriminated unions that only have cases with no fields.
+    /// Maps DUs to strings, like JsonStringEnumConverter.
+    type SimpleDiscriminatedUnionJsonConverter() =
+        inherit JsonConverterFactory()
+
+        override this.CanConvert (typeToConvert: Type): bool = 
+            FSharpType.IsUnion typeToConvert 
+            && (FSharpType.GetUnionCases typeToConvert |> Array.forall (fun typ -> typ.GetFields().Length = 0))
+
+        override this.CreateConverter (typeToConvert: Type, options: JsonSerializerOptions): JsonConverter =
+            let converterType = typedefof<SimpleDUConverter<_>>
+            Activator.CreateInstance(
+                converterType.MakeGenericType(Array.singleton typeToConvert),
+                BindingFlags.Instance ||| BindingFlags.Public,
+                binder=null,
+                args=Array.empty,
+                culture=null
+            )
+            :?> JsonConverter
 
     let jsonOptions =
         let options =
@@ -25,6 +60,7 @@ module internal FSharpJsonConverter =
                 DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull
             )
         options.Converters.Add(JsonStringEnumConverter())
+        options.Converters.Add(SimpleDiscriminatedUnionJsonConverter())
         options
 
 module IgnoreFiles =
@@ -300,6 +336,7 @@ with
 type ConventionsConfig =
     { recursiveAsyncFunction:EnabledConfig option
       avoidTooShortNames:EnabledConfig option
+      indexerAccessorStyleConsistency: RuleConfig<IndexerAccessorStyleConsistency.Config> option
       redundantNewKeyword:EnabledConfig option
       favourStaticEmptyFields:EnabledConfig option
       asyncExceptionWithoutReturn:EnabledConfig option
@@ -346,6 +383,7 @@ with
                 this.binding |> Option.map (fun config -> config.Flatten()) |> Option.toArray |> Array.concat
                 this.suggestUseAutoProperty |> Option.bind (constructRuleIfEnabled SuggestUseAutoProperty.rule) |> Option.toArray
                 this.ensureTailCallDiagnosticsInRecursiveFunctions |> Option.bind (constructRuleIfEnabled EnsureTailCallDiagnosticsInRecursiveFunctions.rule) |> Option.toArray
+                this.indexerAccessorStyleConsistency |> Option.bind (constructRuleWithConfig IndexerAccessorStyleConsistency.rule) |> Option.toArray
             |]
 
 type TypographyConfig =
@@ -403,6 +441,7 @@ type Configuration =
       PatternMatchExpressionIndentation:EnabledConfig option
       RecursiveAsyncFunction:EnabledConfig option
       AvoidTooShortNames:EnabledConfig option
+      IndexerAccessorStyleConsistency:RuleConfig<IndexerAccessorStyleConsistency.Config> option
       RedundantNewKeyword:EnabledConfig option
       FavourNonMutablePropertyInitialization:EnabledConfig option
       FavourReRaise:EnabledConfig option
@@ -498,6 +537,7 @@ with
         PatternMatchExpressionIndentation = None
         RecursiveAsyncFunction = None
         AvoidTooShortNames = None
+        IndexerAccessorStyleConsistency = None
         RedundantNewKeyword = None
         FavourNonMutablePropertyInitialization = None
         FavourReRaise = None
@@ -693,6 +733,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.FavourNonMutablePropertyInitialization |> Option.bind (constructRuleIfEnabled FavourNonMutablePropertyInitialization.rule)
                 config.FavourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule)

src/FSharpLint.Core/FSharpLint.Core.fsproj

@@ -124,6 +124,7 @@
     <Compile Include="Rules\Conventions\SourceLength\MaxLinesInFile.fs" />
     <Compile Include="Rules\Formatting\Typography\TrailingNewLineInFile.fs" />
     <Compile Include="Rules\Formatting\Typography\NoTabCharacters.fs" />
+    <Compile Include="Rules\Conventions\IndexerAccessorStyleConsistency.fs" />
     <Compile Include="Rules\Hints\HintsHelper.fs" />
     <Compile Include="Rules\Hints\HintMatcher.fs" />
     <!-- Application  -->

src/FSharpLint.Core/Rules/Conventions/IndexerAccessorStyleConsistency.fs

@@ -0,0 +1,50 @@
+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 IndexerAccessorStyle =
+    | OCaml
+    | CSharp
+
+[<RequireQualifiedAccess>]
+type Config = {
+    Style: IndexerAccessorStyle
+}
+
+let generateOutput (range: FSharp.Compiler.Text.Range) (style: IndexerAccessorStyle) =
+    Array.singleton
+        {
+            Range = range
+            Message = String.Format(Resources.GetString "RulesIndexerAccessorStyleConsistency", style.ToString())
+            SuggestedFix = None
+            TypeChecks = List.Empty
+        }
+
+let runner (config: Config) (args: AstNodeRuleParams) =
+    match config.Style with
+    | IndexerAccessorStyle.OCaml ->
+        match args.AstNode with
+        | AstNode.Expression (SynExpr.App (ExprAtomicFlag.Atomic, _, SynExpr.Ident _, SynExpr.ArrayOrListComputed (_, _expr, range), _)) ->
+            generateOutput range IndexerAccessorStyle.OCaml
+        | _ -> 
+            Array.empty
+    | IndexerAccessorStyle.CSharp ->
+        match args.AstNode with
+        | AstNode.Expression (SynExpr.DotIndexedGet (_, _, _, range)) ->
+            generateOutput range IndexerAccessorStyle.CSharp
+        | _ -> 
+            Array.empty
+
+let rule config =
+    AstNodeRule
+        {
+            Name = "IndexerAccessorStyleConsistency"
+            Identifier = Identifiers.IndexerAccessorStyleConsistency
+            RuleConfig = { AstNodeRuleConfig.Runner = runner config; Cleanup = ignore }
+        }

src/FSharpLint.Core/Rules/Identifiers.fs

@@ -92,3 +92,4 @@ let FavourNonMutablePropertyInitialization = identifier 84
 let EnsureTailCallDiagnosticsInRecursiveFunctions = identifier 85
 let FavourAsKeyword = identifier 86
 let InterpolatedStringWithNoSubstitution = identifier 87
+let IndexerAccessorStyleConsistency = identifier 88

src/FSharpLint.Core/Text.resx

@@ -384,4 +384,7 @@
   <data name="InterpolatedStringWithNoSubstitution" xml:space="preserve">
     <value>Do not use interpolated string syntax (with $ prefix) or formatting functions (sprintf, failwithf) when not really performing any interpolation.</value>
   </data>
+  <data name="RulesIndexerAccessorStyleConsistency" xml:space="preserve">
+    <value>Consider switching the indexer accessor to {0} style.</value>
+  </data>
 </root>

src/FSharpLint.Core/fsharplint.json

@@ -334,6 +334,12 @@
     "ensureTailCallDiagnosticsInRecursiveFunctions": { "enabled": true },
     "favourAsKeyword": { "enabled": true },
     "interpolatedStringWithNoSubstitution": { "enabled": true },
+    "indexerAccessorStyleConsistency": {
+        "enabled": false,
+        "config": {
+            "style": "OCaml"
+        }
+    },
     "hints": {
         "add": [
             "not (a =  b) ===> a <> b",

tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj

@@ -47,6 +47,7 @@
     <Compile Include="Rules\Conventions\UsedUnderscorePrefixedElements.fs" />
     <Compile Include="Rules\Conventions\FavourNonMutablePropertyInitialization.fs" />
     <Compile Include="Rules\Conventions\EnsureTailCallDiagnosticsInRecursiveFunctions.fs" />
+    <Compile Include="Rules\Conventions\IndexerAccessorStyleConsistency.fs" />
     <Compile Include="Rules\Conventions\Naming\NamingHelpers.fs" />
     <Compile Include="Rules\Conventions\Naming\InterfaceNames.fs" />
     <Compile Include="Rules\Conventions\Naming\ExceptionNames.fs" />

tests/FSharpLint.Core.Tests/Rules/Conventions/IndexerAccessorStyleConsistency.fs

@@ -0,0 +1,70 @@
+module FSharpLint.Core.Tests.Rules.Conventions.IndexerAccessorStyleConsistency
+
+open NUnit.Framework
+open FSharpLint.Rules
+open FSharpLint.Core.Tests
+open FSharpLint.Rules.IndexerAccessorStyleConsistency
+
+[<TestFixture>]
+type TestConventionsIndexerAccessorStyleConsistencyCSharp() =
+    inherit TestAstNodeRuleBase.TestAstNodeRuleBase(IndexerAccessorStyleConsistency.rule { Style = IndexerAccessorStyle.CSharp })
+    
+    [<Test>]
+    member this.IndexerAccessorStyleConsistencyOCamlStyleWhenUsingCSharp() =
+        this.Parse """
+module Program
+
+let someArray = [| "foo" ; "bar" |]
+let bar = someArray.[1]
+System.Console.WriteLine bar"""
+
+        Assert.IsTrue this.ErrorsExist
+    
+    [<Test>]
+    member this.IndexerAccessorStyleConsistencyCSharpStyleWhenUsingCSharp() =
+        this.Parse """
+module Program
+
+let someArray = [| "foo" ; "bar" |]
+let bar = someArray[1]
+System.Console.WriteLine bar"""
+
+        Assert.IsTrue this.NoErrorsExist
+
+[<TestFixture>]
+type TestConventionsIndexerAccessorStyleConsistencyOCaml() =
+    inherit TestAstNodeRuleBase.TestAstNodeRuleBase(IndexerAccessorStyleConsistency.rule { Style = IndexerAccessorStyle.OCaml })
+
+    [<Test>]
+    member this.IndexerAccessorStyleConsistencyCSharpStyleWhenUsingOCaml() =
+        this.Parse """
+module Program
+
+let someArray = [| "foo" ; "bar" |]
+let bar = someArray[1]
+System.Console.WriteLine bar"""
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.IndexerAccessorStyleConsistencyCSharpStyleWhenUsingOCaml2() =
+        this.Parse """
+module Program
+
+let withoutPrefix = [ "foo" ]
+let firstChar = withoutPrefix[0] |> string
+let rest = withoutPrefix.Substring 1
+prefix + firstChar + rest"""
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.IndexerAccessorStyleConsistencyOCamlStyleWhenUsingOCaml() =
+        this.Parse """
+module Program
+
+let someArray = [| "foo" ; "bar" |]
+let bar = someArray.[1]
+System.Console.WriteLine bar"""
+
+        Assert.IsTrue this.NoErrorsExist