Skip to content

Commit 45c2580

Browse files
committed
DisallowShadowing: fix selfCheck violations in rule code
1 parent e52e002 commit 45c2580

File tree

1 file changed

+71
-53
lines changed

1 file changed

+71
-53
lines changed

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

Lines changed: 71 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,69 @@ open FSharpLint.Framework.Suggestion
88
open FSharpLint.Framework.Ast
99
open FSharpLint.Framework.Rules
1010

11-
let private extractIdentifiersFromSimplePats (simplePats: SynSimplePats) : List<Ident> =
12-
let rec extractIdentifier (pattern: SynSimplePat) =
13-
match pattern with
14-
| SynSimplePat.Id(ident, _, _, _, _, _) ->
15-
ident
16-
| SynSimplePat.Attrib(pat, _, _)
17-
| SynSimplePat.Typed(pat, _, _) ->
18-
extractIdentifier pat
11+
[<TailCall>]
12+
let rec private extractIdentifier (pattern: SynSimplePat) =
13+
match pattern with
14+
| SynSimplePat.Id(ident, _, _, _, _, _) ->
15+
ident
16+
| SynSimplePat.Attrib(pat, _, _)
17+
| SynSimplePat.Typed(pat, _, _) ->
18+
extractIdentifier pat
1919

20+
let private extractIdentifiersFromSimplePats (simplePats: SynSimplePats) : List<Ident> =
2021
match simplePats with
2122
| SynSimplePats.SimplePats(patterns, _, _) ->
2223
patterns |> List.map extractIdentifier
2324

25+
let private rangeIncludesDefinitions (definitions: array<FSharpSymbolUse>) range =
26+
definitions
27+
|> Array.exists (fun usage -> ExpressionUtilities.rangeContainsOtherRange range usage.Range)
28+
29+
[<TailCall>]
30+
let rec private processExpressions
31+
(processBinding: SynBinding -> bool)
32+
(processArgs: SynSimplePats -> bool)
33+
(expressions: list<SynExpr>) =
34+
match expressions with
35+
| SynExpr.LetOrUse(_, _, bindings, _, _, _) :: rest ->
36+
bindings |> List.exists processBinding
37+
|| processExpressions processBinding processArgs rest
38+
| SynExpr.Sequential(_, _, expr1, expr2, _, _) :: rest ->
39+
processExpressions processBinding processArgs (expr1 :: expr2 :: rest)
40+
| SynExpr.Lambda(_, _, arguments, body, _, _, _) :: rest ->
41+
processArgs arguments
42+
|| processExpressions processBinding processArgs (body :: rest)
43+
| _ -> false
44+
45+
[<TailCall>]
46+
let rec private processPatterns (definitionsAndPatterns: list<array<FSharpSymbolUse> * SynPat>) =
47+
match definitionsAndPatterns with
48+
| (definitions, SynPat.Named(SynIdent(ident, _), _, _, _)) :: rest ->
49+
rangeIncludesDefinitions definitions ident.idRange
50+
|| processPatterns rest
51+
| (definitions, SynPat.Ands(pats, _)) :: rest ->
52+
processPatterns ((pats |> List.map (fun pat -> (definitions, pat))) @ rest)
53+
| (definitions, SynPat.Or(lhs, rhs, _, _)) :: rest ->
54+
let definitionsExcludingLhs =
55+
definitions
56+
|> Array.filter (fun usage -> not <| ExpressionUtilities.rangeContainsOtherRange lhs.Range usage.Range)
57+
processPatterns ([ (definitionsExcludingLhs, lhs); (definitionsExcludingLhs, rhs) ] @ rest)
58+
| (definitions, SynPat.ArrayOrList(_, pats, _)) :: rest ->
59+
processPatterns ((pats |> List.map (fun pat -> (definitions, pat))) @ rest)
60+
| (definitions, SynPat.As(_lhs, rhs, _)) :: rest ->
61+
processPatterns ((definitions, rhs) :: rest)
62+
| (definitions, SynPat.OptionalVal(ident, _)) :: rest ->
63+
rangeIncludesDefinitions definitions ident.idRange
64+
|| processPatterns rest
65+
| (definitions, SynPat.Paren(pat, _)) :: rest ->
66+
processPatterns ((definitions, pat) :: rest)
67+
| (definitions, SynPat.Record(fieldPats, _)) :: rest ->
68+
processPatterns ((fieldPats |> List.map (fun (_, _, pat) -> (definitions, pat))) @ rest)
69+
| (definitions, SynPat.Tuple(_, pats, _, _)) :: rest ->
70+
processPatterns ((pats |> List.map (fun pat -> (definitions, pat))) @ rest)
71+
| (definitions, SynPat.Typed(pat, _, _)) :: rest ->
72+
processPatterns ((definitions, pat) :: rest)
73+
| _ -> false
2474

2575
let private checkIdentifier (args: AstNodeRuleParams) (identifier: Ident) : array<WarningDetails> =
2676
let name = identifier.idText
@@ -39,67 +89,35 @@ let private checkIdentifier (args: AstNodeRuleParams) (identifier: Ident) : arra
3989
< (identifier.idRange.StartLine, identifier.idRange.StartColumn) )
4090
|> Seq.toArray
4191

42-
let rangeIncludedsDefinitions (definitions: array<FSharpSymbolUse>) range =
43-
definitions
44-
|> Array.exists (fun usage -> ExpressionUtilities.rangeContainsOtherRange range usage.Range)
45-
46-
let rangeIncludedsDefinitionsBeforeCurrent =
47-
rangeIncludedsDefinitions definitionsBeforeCurrent
92+
let rangeIncludesDefinitionsBeforeCurrent =
93+
rangeIncludesDefinitions definitionsBeforeCurrent
4894

4995
let processBinding (binding: SynBinding) =
5096
if ExpressionUtilities.rangeContainsOtherRange binding.RangeOfBindingWithRhs identifier.idRange then
5197
// inside binding's scope
52-
rangeIncludedsDefinitionsBeforeCurrent binding.RangeOfHeadPattern
98+
rangeIncludesDefinitionsBeforeCurrent binding.RangeOfHeadPattern
5399
else
54100
// outside binding's scope
55101
match binding with
56102
| SynBinding(_, _, _, _, _, _, _, headPat, _, _, _, _, _) ->
57103
match headPat with
58-
| SynPat.Named(SynIdent(ident, _), _, _, _) -> rangeIncludedsDefinitionsBeforeCurrent ident.idRange
104+
| SynPat.Named(SynIdent(ident, _), _, _, _) -> rangeIncludesDefinitionsBeforeCurrent ident.idRange
59105
| SynPat.LongIdent(SynLongIdent([ ident ], _, _), _, _, _, _, _) ->
60-
rangeIncludedsDefinitionsBeforeCurrent ident.idRange
106+
rangeIncludesDefinitionsBeforeCurrent ident.idRange
61107
| _ -> false
62108

63-
let processArgs (args: SynSimplePats) =
64-
args
109+
let processArgs (arguments: SynSimplePats) =
110+
arguments
65111
|> extractIdentifiersFromSimplePats
66-
|> List.exists (fun ident -> rangeIncludedsDefinitionsBeforeCurrent ident.idRange)
67-
68-
let rec processExpression (expression: SynExpr) =
69-
match expression with
70-
| SynExpr.LetOrUse(_, _, bindings, _, _, _) ->
71-
bindings |> List.exists processBinding
72-
| SynExpr.Sequential(_, _, expr1, expr2, _, _) ->
73-
processExpression expr1 || processExpression expr2
74-
| SynExpr.Lambda(_, _, args, body, _, _, _) ->
75-
processExpression body || processArgs args
76-
| _ -> false
77-
78-
let rec processPattern (definitions: array<FSharpSymbolUse>) (pattern: SynPat) =
79-
match pattern with
80-
| SynPat.Named(SynIdent(ident, _), _, _, _) -> rangeIncludedsDefinitions definitions ident.idRange
81-
| SynPat.Ands(pats, _) -> pats |> List.exists (processPattern definitions)
82-
| SynPat.Or(lhs, rhs, _, _) ->
83-
let definitionsExcludingLhs =
84-
definitions
85-
|> Array.filter (fun usage -> not <| ExpressionUtilities.rangeContainsOtherRange lhs.Range usage.Range)
86-
processPattern definitionsExcludingLhs lhs || processPattern definitionsExcludingLhs rhs
87-
| SynPat.ArrayOrList(_, pats, _) -> pats |> List.exists (processPattern definitions)
88-
| SynPat.As(_lhs, rhs, _) -> processPattern definitions rhs
89-
| SynPat.OptionalVal(ident, _) -> rangeIncludedsDefinitions definitions ident.idRange
90-
| SynPat.Paren(pat, _) -> processPattern definitions pat
91-
| SynPat.Record(fieldPats, _) -> fieldPats |> List.exists (fun (_, _, pat) -> processPattern definitions pat)
92-
| SynPat.Tuple(_, pats, _, _) -> pats |> List.exists (processPattern definitions)
93-
| SynPat.Typed(pat, _, _) -> processPattern definitions pat
94-
| _ -> false
112+
|> List.exists (fun ident -> rangeIncludesDefinitionsBeforeCurrent ident.idRange)
95113

96114
let processModuleDeclaration (moduleDecl: SynModuleDecl) =
97115
match moduleDecl with
98-
| SynModuleDecl.Let(_, bindings, _) ->
99-
bindings
100-
|> List.exists processBinding
116+
| SynModuleDecl.Let(_, bindings, _) -> bindings |> List.exists processBinding
101117
| _ -> false
102118

119+
let processExpression (expr: SynExpr) = processExpressions processBinding processArgs (List.singleton expr)
120+
103121
let processAstNode (node: AstNode) =
104122
match node with
105123
| ModuleOrNamespace(SynModuleOrNamespace(_, _, _, declarations, _, _, _, _, _)) ->
@@ -110,11 +128,11 @@ let private checkIdentifier (args: AstNodeRuleParams) (identifier: Ident) : arra
110128
processExpression lambda.Body
111129
|| (lambda.Arguments |> List.exists processArgs)
112130
| Match(SynMatchClause(pattern, _, _, _, _, _)) ->
113-
processPattern definitionsBeforeCurrent pattern
131+
processPatterns (List.singleton (definitionsBeforeCurrent, pattern))
114132
| MemberDefinition(SynMemberDefn.Member(memberDefn, _)) ->
115133
processBinding memberDefn
116134
| TypeDefinition(SynTypeDefn(_, _, _, Some(SynMemberDefn.ImplicitCtor(_, _, ctorArgs, _, _, _, _)), _, _)) ->
117-
processPattern definitionsBeforeCurrent ctorArgs
135+
processPatterns (List.singleton (definitionsBeforeCurrent, ctorArgs))
118136
| _ -> false
119137

120138
let parents = args.GetParents args.NodeIndex

0 commit comments

Comments
 (0)