Skip to content

Commit 58b062f

Browse files
committed
PreferStringInterpolationWithSprintf: get rid of mutable state
That made tests flaky.
1 parent eb67825 commit 58b062f

File tree

1 file changed

+24
-53
lines changed

1 file changed

+24
-53
lines changed

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

Lines changed: 24 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -7,30 +7,6 @@ open FSharp.Compiler.Syntax
77
open FSharpLint.Framework.Ast
88
open FSharpLint.Framework.Rules
99

10-
let mutable moduleIdentifiers = Set.empty
11-
let mutable letIdentifiers = Set.empty
12-
13-
[<TailCall>]
14-
let rec isVisible (id: Ident) asts =
15-
let isNamedBinding binding =
16-
match binding with
17-
| SynBinding(_, _, _, _, _, _, _, SynPat.Named(SynIdent.SynIdent(ident, _), _, _, _), _, _, _, _, _) ->
18-
id.idText = ident.idText
19-
| _ -> false
20-
21-
match asts with
22-
| AstNode.Expression (SynExpr.LetOrUse (_, _, [binding], _, _, _)) :: rest ->
23-
isNamedBinding binding || isVisible id rest
24-
| _ :: rest -> isVisible id rest
25-
| [] -> false
26-
27-
[<TailCall>]
28-
let rec getTopLevelParent args index =
29-
let parents = List.rev (args.GetParents index)
30-
match parents with
31-
| AstNode.ModuleDeclaration (SynModuleDecl.Let _) :: rest -> rest
32-
| _ -> getTopLevelParent args (index + 1)
33-
3410
let runner args =
3511
let isStringFormat (identifiers: List<Ident>) =
3612
"String" = identifiers.[0].idText && "Format" = identifiers.[1].idText
@@ -44,43 +20,38 @@ let runner args =
4420

4521
match args.AstNode with
4622
| AstNode.Expression(SynExpr.App(_, _, SynExpr.LongIdent(_, SynLongIdent(ids, _, _), _, _), paren, range)) when ids.Length = 2 && isStringFormat ids ->
47-
let isTopMember (text: string) =
48-
moduleIdentifiers.Contains text
4923
match paren with
50-
| SynExpr.Paren(SynExpr.Tuple(_, [SynExpr.Const(SynConst.String(_), _); _], _, _), _, _, _) ->
24+
| SynExpr.Paren(SynExpr.Tuple(_, SynExpr.Const(SynConst.String(_), _) :: _args, _, _), _, _, _) ->
5125
emitViolation range
52-
| SynExpr.Paren(SynExpr.Tuple(_, [SynExpr.Ident identifier; _], _, _), _, _, range) ->
53-
54-
if isTopMember identifier.idText then
26+
| SynExpr.Paren(SynExpr.Tuple(_, SynExpr.Ident(identifier) :: _args, _, _), _, _, _range) ->
27+
let isBindingOfIdentifierToTemplate binding =
28+
match binding with
29+
| SynBinding(_, _, _, _, _, _, _, SynPat.Named(SynIdent.SynIdent(bindingIdent, _), _, _, _), _, SynExpr.Const(SynConst.String(value, _, _), _), _, _, _) ->
30+
value.Contains "{0}" && bindingIdent.idText = identifier.idText
31+
| _ -> false
32+
33+
let hasDeclarationContainingBindingOfIdentifierToTemplate astNode =
34+
match astNode with
35+
| AstNode.ModuleOrNamespace(SynModuleOrNamespace(_, _, _, declarations, _, _, _, _, _)) ->
36+
declarations
37+
|> List.exists
38+
(fun decl ->
39+
match decl with
40+
| SynModuleDecl.Let(_, bindings, _) ->
41+
bindings |> List.exists isBindingOfIdentifierToTemplate
42+
| _ -> false)
43+
| _ -> false
44+
45+
let parents = args.GetParents args.NodeIndex
46+
if parents |> List.exists hasDeclarationContainingBindingOfIdentifierToTemplate then
5547
emitViolation range
5648
else
57-
if letIdentifiers.Contains identifier.idText && isVisible identifier (getTopLevelParent args 2) then
58-
emitViolation range
59-
else
60-
Array.empty
49+
Array.empty
6150
| _ -> Array.empty
62-
| AstNode.Binding(SynBinding(_, _, _, _, _, _, _, SynPat.Named(SynIdent.SynIdent(identifier, _), _, _, _), _, SynExpr.Const(SynConst.String(value, _, _), _), _, _, _)) when value.Contains "{0}" ->
63-
let parents = args.GetParents 1
64-
match parents with
65-
| AstNode.ModuleDeclaration(SynModuleDecl.Let _) :: _ ->
66-
moduleIdentifiers <- moduleIdentifiers.Add(identifier.idText)
67-
| _ -> letIdentifiers <- letIdentifiers.Add(identifier.idText)
68-
Array.empty
69-
| AstNode.ModuleDeclaration(SynModuleDecl.Let _) ->
70-
letIdentifiers <- Set.empty
71-
Array.empty
72-
| AstNode.ModuleDeclaration(SynModuleDecl.NestedModule _) ->
73-
moduleIdentifiers <- Set.empty
74-
letIdentifiers <- Set.empty
75-
Array.empty
7651
| _ -> Array.empty
7752

78-
let cleanup () =
79-
moduleIdentifiers <- Set.empty
80-
letIdentifiers <- Set.empty
81-
8253
let rule =
8354
{ Name = "PreferStringInterpolationWithSprintf"
8455
Identifier = Identifiers.PreferStringInterpolationWithSprintf
85-
RuleConfig = { AstNodeRuleConfig.Runner = runner; Cleanup = cleanup } }
56+
RuleConfig = { AstNodeRuleConfig.Runner = runner; Cleanup = ignore } }
8657
|> AstNodeRule

0 commit comments

Comments
 (0)