Add rule to encourage non-mutable property initialization

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

@@ -123,3 +123,4 @@ The following rules can be specified for linting.
 - [UnnestedFunctionNames (FL0080)](rules/FL0080.html)
 - [NestedFunctionNames (FL0081)](rules/FL0081.html)
 - [UsedUnderscorePrefixedElements (FL0082)](rules/FL0082.html)
+- [FavourNonMutablePropertyInitialization (FL0083)](rules/FL0083.html)

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

@@ -0,0 +1,29 @@
+---
+title: FL0083
+category: how-to
+hide_menu: true
+---
+
+# FavourNonMutablePropertyInitialization (FL0077)
+
+*Introduced in `0.23.2`*
+
+## Cause
+
+Use of mutation to initialize properties with values.
+
+## Rationale
+
+There's no need to use mutation to initialize properties with values because F# has specific syntax for this kind of intitialization.
+Both approaches will compile to the same IL but non-mutable code doesn't look unsafe because it prevents the use of the `<-` operator.
+
+## How To Fix
+
+Replace all occurrences of mutable property initialization with non mutable property initialization.
+Example: `Cookie(Domain = "example.com")` instead of `let c = Cookie(); c.Domain <- "example.com"`
+
+## Rule Settings
+
+    {
+        "FavourNonMutablePropertyInitialization": { "enabled": false }
+    }

src/FSharpLint.Core/Application/Configuration.fs

@@ -48,10 +48,7 @@ module FSharpJsonConverter =
         |]
 
     let serializerSettings =
-        let settings = JsonSerializerSettings()
-        settings.NullValueHandling <- NullValueHandling.Ignore
-        settings.Converters <- converters
-        settings
+        JsonSerializerSettings(NullValueHandling = NullValueHandling.Ignore, Converters = converters)
 
 module IgnoreFiles =
 
@@ -310,6 +307,7 @@ type ConventionsConfig =
       redundantNewKeyword:EnabledConfig option
       favourStaticEmptyFields:EnabledConfig option
       asyncExceptionWithoutReturn:EnabledConfig option
+      favourNonMutablePropertyInitialization:EnabledConfig option
       nestedStatements:RuleConfig<NestedStatements.Config> option
       cyclomaticComplexity:RuleConfig<CyclomaticComplexity.Config> option
       reimplementsFunction:EnabledConfig option
@@ -330,6 +328,7 @@ with
             this.recursiveAsyncFunction |> Option.bind (constructRuleIfEnabled RecursiveAsyncFunction.rule) |> Option.toArray
             this.avoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule) |> Option.toArray           
             this.redundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule) |> Option.toArray
+            this.favourNonMutablePropertyInitialization |> Option.bind (constructRuleIfEnabled FavourNonMutablePropertyInitialization.rule) |> Option.toArray
             this.favourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule) |> Option.toArray
             this.favourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule) |> Option.toArray
             this.asyncExceptionWithoutReturn |> Option.bind (constructRuleIfEnabled AsyncExceptionWithoutReturn.rule) |> Option.toArray
@@ -403,6 +402,7 @@ type Configuration =
       RecursiveAsyncFunction:EnabledConfig option
       AvoidTooShortNames:EnabledConfig option
       RedundantNewKeyword:EnabledConfig option
+      FavourNonMutablePropertyInitialization:EnabledConfig option
       FavourReRaise:EnabledConfig option
       FavourStaticEmptyFields:EnabledConfig option
       AsyncExceptionWithoutReturn:EnabledConfig option
@@ -491,6 +491,7 @@ with
         RecursiveAsyncFunction = None
         AvoidTooShortNames = None
         RedundantNewKeyword = None
+        FavourNonMutablePropertyInitialization = None
         FavourReRaise = None
         FavourStaticEmptyFields = None
         AsyncExceptionWithoutReturn = None
@@ -642,6 +643,7 @@ let flattenConfig (config:Configuration) =
             config.RecursiveAsyncFunction |> Option.bind (constructRuleIfEnabled RecursiveAsyncFunction.rule)
             config.AvoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule)
             config.RedundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule)
+            config.FavourNonMutablePropertyInitialization |> Option.bind (constructRuleIfEnabled FavourNonMutablePropertyInitialization.rule)
             config.FavourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule)
             config.FavourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule)
             config.AsyncExceptionWithoutReturn |> Option.bind (constructRuleIfEnabled AsyncExceptionWithoutReturn.rule)

src/FSharpLint.Core/Application/Lint.fs

@@ -261,17 +261,18 @@ module Lint =
         ReachedEnd(fileInfo.File, fileWarnings |> Seq.toList) |> lintInfo.ReportLinterProgress
 
     let private runProcess (workingDir:string) (exePath:string) (args:string) =
-        let psi = System.Diagnostics.ProcessStartInfo()
-        psi.FileName <- exePath
-        psi.WorkingDirectory <- workingDir
-        psi.RedirectStandardOutput <- true
-        psi.RedirectStandardError <- true
-        psi.Arguments <- args
-        psi.CreateNoWindow <- true
-        psi.UseShellExecute <- false
-
-        use p = new System.Diagnostics.Process()
-        p.StartInfo <- psi
+        let psi = 
+            System.Diagnostics.ProcessStartInfo(
+                FileName = exePath,
+                WorkingDirectory = workingDir,
+                RedirectStandardOutput = true,
+                RedirectStandardError = true,
+                Arguments = args,
+                CreateNoWindow = true,
+                UseShellExecute = false
+            )
+
+        use p = new System.Diagnostics.Process(StartInfo = psi)
         let sbOut = System.Text.StringBuilder()
         p.OutputDataReceived.Add(fun ea -> sbOut.AppendLine(ea.Data) |> ignore)
         let sbErr = System.Text.StringBuilder()

src/FSharpLint.Core/FSharpLint.Core.fsproj

@@ -54,6 +54,7 @@
     <Compile Include="Rules\Conventions\AvoidSinglePipeOperator.fs" />
     <Compile Include="Rules\Conventions\SuggestUseAutoProperty.fs" />
     <Compile Include="Rules\Conventions\UsedUnderscorePrefixedElements.fs" />
+    <Compile Include="Rules\Conventions\FavourNonMutablePropertyInitialization.fs" />
     <Compile Include="Rules\Conventions\RaiseWithTooManyArguments\RaiseWithTooManyArgumentsHelper.fs" />
     <Compile Include="Rules\Conventions\RaiseWithTooManyArguments\FailwithWithSingleArgument.fs" />
     <Compile Include="Rules\Conventions\RaiseWithTooManyArguments\RaiseWithSingleArgument.fs" />

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

@@ -0,0 +1,92 @@
+module FSharpLint.Rules.FavourNonMutablePropertyInitialization
+
+open FSharpLint.Framework
+open FSharpLint.Framework.Suggestion
+open FSharp.Compiler.Syntax
+open FSharpLint.Framework.Ast
+open FSharpLint.Framework.Rules
+open System
+
+let private getWarningDetails (ident: Ident) =
+    let formatError errorName =
+        String.Format(Resources.GetString errorName, ident.idText)
+
+    "RulesFavourNonMutablePropertyInitializationError"
+    |> formatError
+    |> Array.singleton
+    |> Array.map (fun message ->
+        { Range = ident.idRange
+          Message = message
+          SuggestedFix = None
+          TypeChecks = List.Empty })
+
+let private extraInstanceMethod (app:SynExpr) (instanceMethodCalls: List<string>) =
+    match app with
+    | SynExpr.App(_, _, expression, _, _) ->
+        match expression with
+        | SynExpr.LongIdent(_, LongIdentWithDots(identifiers, _), _, _) ->
+            match List.tryLast identifiers with
+            | Some _ ->
+                identifiers.[0].idText::instanceMethodCalls
+            | _ -> instanceMethodCalls
+        | _ -> instanceMethodCalls
+    | _ -> instanceMethodCalls
+
+let rec private extraFromBindings (bindings: List<SynBinding>) (classInstances: List<string>) =
+    match bindings with
+    | SynBinding(_, _, _, _, _, _, _, SynPat.Named(_, ident, _, _, _), _, _expression, _, _)::rest ->
+        extraFromBindings rest (ident.idText::classInstances)
+    | _ -> classInstances
+
+let rec private processLetBinding (instanceNames: Set<string>) (body: SynExpr) : array<WarningDetails> =
+    match body with
+    | SynExpr.LongIdentSet(LongIdentWithDots(identifiers, _), _, _) ->
+        match identifiers with
+        | [instanceIdent; propertyIdent] when Set.contains instanceIdent.idText instanceNames ->
+            getWarningDetails propertyIdent
+        | _ -> Array.empty
+    | SynExpr.Sequential(_, _, expr1, expr2, _) ->
+        let instanceNames =
+            Set.difference
+                instanceNames
+                (extraInstanceMethod expr1 List.empty |> Set.ofList)
+        Array.append
+            (processLetBinding instanceNames expr1)
+            (processLetBinding instanceNames expr2)
+    | _ -> [||]
+
+and processExpression (expression: SynExpr) : array<WarningDetails> =
+    match expression with
+    | SynExpr.LetOrUse(_, _, bindings, body, _) ->
+        let instanceNames = extraFromBindings bindings [] |> Set.ofList
+        processLetBinding instanceNames body
+    | SynExpr.Sequential(_, _, expr1, expr2, _) ->
+        Array.append
+            (processExpression expr1)
+            (processExpression expr2)
+    | _ -> Array.empty
+
+let runner args =
+    match args.AstNode with
+    | Binding(SynBinding(_, _, _, _, _, _, _, _, _, SynExpr.LetOrUse(_, _, bindings, body, _), _, _)) ->
+        let instanceNames = extraFromBindings bindings [] |> Set.ofList
+        processLetBinding instanceNames body
+    | Match(SynMatchClause(_, _, expr, _, _)) ->
+        processExpression expr
+    | Lambda(lambda, _) ->
+        processExpression lambda.Body
+    | Expression(SynExpr.TryWith(tryExpr, _, _, _, _, _, _)) ->
+        processExpression tryExpr
+    | Expression(SynExpr.TryFinally(tryExpr, finallyExpr, _, _, _)) ->
+        Array.append
+            (processExpression tryExpr)
+            (processExpression finallyExpr)
+    | Expression(SynExpr.CompExpr(_, _, expr, _)) ->
+        processExpression expr
+    | _ -> Array.empty
+
+let rule =
+    { Name = "FavourNonMutablePropertyInitialization"
+      Identifier = Identifiers.FavourNonMutablePropertyInitialization
+      RuleConfig = { AstNodeRuleConfig.Runner = runner; Cleanup = ignore } }
+    |> AstNodeRule

src/FSharpLint.Core/Rules/Identifiers.fs

@@ -86,4 +86,5 @@ let AsyncExceptionWithoutReturn = identifier 78
 let SuggestUseAutoProperty = identifier 79
 let UnnestedFunctionNames = identifier 80
 let NestedFunctionNames = identifier 81
-let UsedUnderscorePrefixedElements = identifier 82
+let UsedUnderscorePrefixedElements = identifier 82
+let FavourNonMutablePropertyInitialization = identifier 83

src/FSharpLint.Core/Text.resx

@@ -357,4 +357,7 @@
   <data name="RulesSuggestUseAutoProperty" xml:space="preserve">
     <value>Consider using auto-properties via the 'val' keyword.</value>
   </data>
+  <data name="RulesFavourNonMutablePropertyInitializationError" xml:space="preserve">
+    <value>Consider using non-mutable property initialization.</value>
+  </data>
 </root>

src/FSharpLint.Core/fsharplint.json

@@ -281,6 +281,7 @@
     "uselessBinding": { "enabled": true },
     "tupleOfWildcards": { "enabled": true },
     "favourTypedIgnore": { "enabled": false },
+    "favourNonMutablePropertyInitialization": { "enabled": true },
     "favourReRaise": { "enabled": true },
     "favourStaticEmptyFields": { "enabled": false },
     "favourConsistentThis": {

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

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