SuggestUseAutoProperty: check for lists/arrays

Make rule pass tests introduced in previous commit by checking if poperty returns list or array of immutable values.
webwarrior-ws · Dec 6, 2023 · 323b879 · 323b879
1 parent 049db7d
commit 323b879
Showing 1 changed file with 45 additions and 33 deletions.
diff --git a/src/FSharpLint.Core/Rules/Conventions/SuggestUseAutoProperty.fs b/src/FSharpLint.Core/Rules/Conventions/SuggestUseAutoProperty.fs
@@ -7,6 +7,43 @@ open FSharp.Compiler.Syntax
 open FSharpLint.Framework.Ast
 open FSharpLint.Framework.Rules
 
+let rec private isImmutableValueExpression (args: AstNodeRuleParams) (expression: SynExpr) =
+    match expression with
+    | SynExpr.Const (constant, range) -> true
+    | SynExpr.Ident ident ->
+        let isMutableVariable =
+            match args.GetParents args.NodeIndex with
+            | TypeDefinition (SynTypeDefn (_, SynTypeDefnRepr.ObjectModel (_, members, _), _, _, _)) :: _ ->
+                members
+                |> List.exists (fun (memberDef: SynMemberDefn) ->
+                    match memberDef with
+                    | SynMemberDefn.LetBindings (bindings, _, _, _) ->
+                        bindings
+                        |> List.exists (fun (SynBinding (_, _, _, isMutable, _, _, _, headPat, _, _, _, _)) ->
+                            match headPat with
+                            | SynPat.Named (_, bindingIdent, _, _, _) when isMutable ->
+                                bindingIdent.idText = ident.idText
+                            | _ -> false)
+                    | _ -> false)
+            | _ -> false
+
+        not isMutableVariable
+    | SynExpr.ArrayOrList (_, elements, _) ->
+        elements
+        |> List.forall (isImmutableValueExpression args)
+    | SynExpr.ArrayOrListOfSeqExpr (_, SynExpr.CompExpr(true, _, innerExpr, _), _) ->
+        isImmutableValueExpression args innerExpr
+        || isImmutableSequentialExpression args innerExpr
+    | _ -> false
+
+and isImmutableSequentialExpression args expression =
+    match expression with
+    | SynExpr.Sequential (_, _, expr1, expr2, _) ->
+        isImmutableValueExpression args expr1
+        && (isImmutableSequentialExpression args expr2
+            || isImmutableValueExpression args expr2)
+    | _ -> false
+
 let private runner (args: AstNodeRuleParams) =
     match args.AstNode with
     | MemberDefinition
@@ -15,56 +52,31 @@ let private runner (args: AstNodeRuleParams) =
                 (
                     SynBinding
                         (
-                            accessibility,
-                            kind,
+                            _accessibility,
+                            _kind,
                             _,
                             false,
                             _attributes,
                             _xmlDoc,
-                            valData,
+                            _valData,
                             SynPat.LongIdent (_, _, _, argPats, _, _),
-                            returnInfo,
+                            _returnInfo,
                             expr,
-                            bindingRange,
+                            _bindingRange,
                             _
                         ),
                     memberRange
                 )
         ) ->
-        match (expr, argPats) with
-        | (_, SynArgPats.Pats pats) when pats.Length > 0 -> // non-property member
+        match expr, argPats with
+        | _, SynArgPats.Pats pats when pats.Length > 0 -> // non-property member
             Array.empty
-        | (SynExpr.Const (constant, range), _) ->
+        | expression, _ when isImmutableValueExpression args expression ->
             { Range = memberRange
               Message = Resources.GetString "RulesSuggestUseAutoProperty"
               SuggestedFix = None
               TypeChecks = List.Empty }
             |> Array.singleton
-        | (SynExpr.Ident ident, _) ->
-            let isMutableVariable =
-                match args.GetParents args.NodeIndex with
-                | TypeDefinition (SynTypeDefn (_, SynTypeDefnRepr.ObjectModel (_, members, _), _, _, _)) :: _ ->
-                    members
-                    |> List.exists (fun (memberDef: SynMemberDefn) ->
-                        match memberDef with
-                        | SynMemberDefn.LetBindings (bindings, _, _, _) ->
-                            bindings
-                            |> List.exists (fun (SynBinding (_, _, _, isMutable, _, _, _, headPat, _, _, _, _)) ->
-                                match headPat with
-                                | SynPat.Named (_, bindingIdent, _, _, _) when isMutable ->
-                                    bindingIdent.idText = ident.idText
-                                | _ -> false)
-                        | _ -> false)
-                | _ -> false
-
-            if isMutableVariable then
-                Array.empty
-            else
-                { Range = memberRange
-                  Message = Resources.GetString "RulesSuggestUseAutoProperty"
-                  SuggestedFix = None
-                  TypeChecks = List.Empty }
-                |> Array.singleton
         | _ -> Array.empty
     | _ -> Array.empty