Add new rule PreferStringInterpolationWithSprintf

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

@@ -131,3 +131,4 @@ The following rules can be specified for linting.
 - [IndexerAccessorStyleConsistency (FL0088)](rules/FL0088.html)
 - [FavourSingleton (FL0089)](rules/FL0089.html)
 - [NoAsyncRunSynchronouslyInLibrary (FL0090)](rules/FL0090.html)
+- [PreferStringInterpolationWithSprintf (FL0091)](rules/FL0091.html)

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

@@ -0,0 +1,29 @@
+---
+title: FL0091
+category: how-to
+hide_menu: true
+---
+
+# PreferStringInterpolationWithSprintf (FL0091)
+
+*Introduced in `0.26.12`*
+
+## Cause
+
+String interpolation is done with String.Format.
+
+## Rationale
+
+sprintf is statically type checked and with sprintf F# compiler will complain when too few arguments are supplied (unlike with String.Format).
+
+## How To Fix
+
+Use sprintf instead of String.Format.
+
+## Rule Settings
+
+    {
+        "preferStringInterpolationWithSprintf": {
+            "enabled": false
+        }
+    }

src/FSharpLint.Core/Application/Configuration.fs

@@ -366,6 +366,7 @@ type ConventionsConfig =
     { recursiveAsyncFunction:EnabledConfig option
       avoidTooShortNames:EnabledConfig option
       indexerAccessorStyleConsistency: RuleConfig<IndexerAccessorStyleConsistency.Config> option
+      preferStringInterpolationWithSprintf:EnabledConfig option
       redundantNewKeyword:EnabledConfig option
       favourStaticEmptyFields:EnabledConfig option
       asyncExceptionWithoutReturn:EnabledConfig option
@@ -413,6 +414,7 @@ with
                 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
+                this.preferStringInterpolationWithSprintf |> Option.bind (constructRuleIfEnabled PreferStringInterpolationWithSprintf.rule) |> Option.toArray
             |]
 
 [<Obsolete(ObsoleteMsg, ObsoleteWarnTreatAsError)>]
@@ -482,6 +484,7 @@ type Configuration =
       RecursiveAsyncFunction:EnabledConfig option
       AvoidTooShortNames:EnabledConfig option
       IndexerAccessorStyleConsistency:RuleConfig<IndexerAccessorStyleConsistency.Config> option
+      PreferStringInterpolationWithSprintf:EnabledConfig option
       RedundantNewKeyword:EnabledConfig option
       FavourNonMutablePropertyInitialization:EnabledConfig option
       FavourReRaise:EnabledConfig option
@@ -584,6 +587,7 @@ with
         RecursiveAsyncFunction = None
         AvoidTooShortNames = None
         IndexerAccessorStyleConsistency = None
+        PreferStringInterpolationWithSprintf = None
         RedundantNewKeyword = None
         FavourNonMutablePropertyInitialization = None
         FavourReRaise = None

src/FSharpLint.Core/FSharpLint.Core.fsproj

@@ -60,6 +60,7 @@
     <Compile Include="Rules\Conventions\EnsureTailCallDiagnosticsInRecursiveFunctions.fs" />
     <Compile Include="Rules\Conventions\FailwithBadUsage.fs" />
     <Compile Include="Rules\Conventions\FavourSingleton.fs" />
+    <Compile Include="Rules\Conventions\PreferStringInterpolationWithSprintf.fs" />
     <Compile Include="Rules\Conventions\SourceLength\SourceLengthHelper.fs" />
     <Compile Include="Rules\Conventions\SourceLength\MaxLinesInLambdaFunction.fs" />
     <Compile Include="Rules\Conventions\SourceLength\MaxLinesInMatchLambdaFunction.fs" />

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

@@ -0,0 +1,88 @@
+module FSharpLint.Rules.PreferStringInterpolationWithSprintf
+
+open System
+open FSharpLint.Framework
+open FSharpLint.Framework.Suggestion
+open FSharp.Compiler.Syntax
+open FSharpLint.Framework.Ast
+open FSharpLint.Framework.Rules
+
+let mutable moduleIdentifiers = Set.empty
+let mutable letIdentifiers = Set.empty
+
+let private isStringFormat (identifiers: List<Ident>) =
+    "String" = identifiers.[0].idText && "Format" = identifiers.[1].idText
+
+let private genereateErrorMessage range =
+    { Range = range
+      Message = Resources.GetString "RulesPreferStringInterpolationWithSprintf"
+      SuggestedFix = None
+      TypeChecks = List.empty }
+    |> Array.singleton
+
+let runner args =
+    match args.AstNode with
+    | AstNode.Expression(SynExpr.App(_, _, SynExpr.LongIdent(_, SynLongIdent(ids, _, _), _, _), paren, range)) when ids.Length = 2 && isStringFormat ids ->
+        let isTopMember (text: string) =
+            moduleIdentifiers.Contains text
+        match paren with
+        | SynExpr.Paren(SynExpr.Tuple(_, [SynExpr.Const(SynConst.String(_), _); _], _, _), _, _, _) ->
+            genereateErrorMessage range
+        | SynExpr.Paren(SynExpr.Tuple(_, [SynExpr.Ident identifier; _], _, _), _, _, range)  ->
+
+            if isTopMember identifier.idText then
+                genereateErrorMessage range
+            else
+                let isNamedBinding binding =
+                    match binding with
+                    | SynBinding(_, _, _, _, _, _, _, SynPat.Named(SynIdent.SynIdent(ident, _), _, _, _), _, _, _, _, _) ->
+                        identifier.idText = ident.idText
+                    | _ -> false
+
+                let isVisible asts =
+                    let rec loop asts =
+                        match asts with
+                        | AstNode.Expression (SynExpr.LetOrUse (_, _, [binding], _, _, _)) :: rest ->
+                            isNamedBinding binding || loop rest
+                        | _ :: rest -> loop rest
+                        | [] -> false
+                    loop asts
+
+                let getTopLevelParent index =
+                    let rec loop index =
+                        let parents = List.rev (args.GetParents index)
+                        match parents with
+                        | AstNode.ModuleDeclaration (SynModuleDecl.Let _) :: rest -> rest
+                        | _ -> loop (index + 1)
+                    loop index
+
+                if letIdentifiers.Contains identifier.idText && isVisible (getTopLevelParent 2) then
+                    genereateErrorMessage range
+                else
+                    Array.empty
+        | _ -> Array.empty
+    | AstNode.Binding(SynBinding(_, _, _, _, _, _, _, SynPat.Named(SynIdent.SynIdent(identifier, _), _, _, _), _, SynExpr.Const(SynConst.String(value, _, _), _), _, _, _)) when value.Contains "{0}" ->
+        let parents = args.GetParents 1
+        match parents with
+        | AstNode.ModuleDeclaration(SynModuleDecl.Let _) :: _ ->
+            moduleIdentifiers <- moduleIdentifiers.Add(identifier.idText)
+        | _ -> letIdentifiers <- letIdentifiers.Add(identifier.idText)
+        Array.empty
+    | AstNode.ModuleDeclaration(SynModuleDecl.Let _) ->
+        letIdentifiers <- Set.empty
+        Array.empty
+    | AstNode.ModuleDeclaration(SynModuleDecl.NestedModule _) ->
+        moduleIdentifiers <- Set.empty
+        letIdentifiers <- Set.empty
+        Array.empty
+    | _ -> Array.empty
+
+let cleanup () =
+    moduleIdentifiers <- Set.empty
+    letIdentifiers <- Set.empty
+
+let rule =
+    { Name = "PreferStringInterpolationWithSprintf"
+      Identifier = Identifiers.PreferStringInterpolationWithSprintf
+      RuleConfig = { AstNodeRuleConfig.Runner = runner; Cleanup = cleanup } }
+    |> AstNodeRule

src/FSharpLint.Core/Rules/Identifiers.fs

@@ -95,3 +95,4 @@ let InterpolatedStringWithNoSubstitution = identifier 87
 let IndexerAccessorStyleConsistency = identifier 88
 let FavourSingleton = identifier 89
 let NoAsyncRunSynchronouslyInLibrary = identifier 90
+let PreferStringInterpolationWithSprintf = identifier 91

src/FSharpLint.Core/Text.resx

@@ -393,4 +393,7 @@
   <data name="NoAsyncRunSynchronouslyInLibrary" xml:space="preserve">
     <value>Async.RunSynchronously should not be used in libraries.</value>
   </data>
+  <data name="RulesPreferStringInterpolationWithSprintf" xml:space="preserve">
+    <value>Use sprintf instead of String.Format.</value>
+  </data>
 </root>

src/FSharpLint.Core/fsharplint.json

@@ -302,6 +302,9 @@
     "indentation": {
         "enabled": false
     },
+    "preferStringInterpolationWithSprintf": {
+        "enabled": false
+    },
     "maxCharactersOnLine": {
         "enabled": false,
         "config": {

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

@@ -41,6 +41,7 @@
     <Compile Include="Rules\Conventions\SourceLength.fs" />
     <Compile Include="Rules\Conventions\NoPartialFunctions.fs" />
     <Compile Include="Rules\Conventions\FavourReRaise.fs" />
+    <Compile Include="Rules\Conventions\PreferStringInterpolationWithSprintf.fs" />
     <Compile Include="Rules\Conventions\FavourConsistentThis.fs" />
     <Compile Include="Rules\Conventions\AvoidTooShortNames.fs" />
     <Compile Include="Rules\Conventions\AvoidSinglePipeOperator.fs" />

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

@@ -0,0 +1,81 @@
+module FSharpLint.Core.Tests.Rules.Conventions.PreferStringInterpolationWithSprintf
+
+open NUnit.Framework
+open FSharpLint.Rules
+open FSharpLint.Core.Tests
+
+[<TestFixture>]
+type TestConventionsPreferStringInterpolationWithSprintf() =
+    inherit TestAstNodeRuleBase.TestAstNodeRuleBase(PreferStringInterpolationWithSprintf.rule)
+
+    [<Test>]
+    member this.StringInterpolationWithSprintfShouldNotProduceError() =
+        this.Parse """
+let someString = sprintf "Hello %s" world"""
+
+        Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.StringInterpolationWithStringFormatShouldProduceError() =
+        this.Parse """
+let someString = String.Format("Hello {0}", world)"""
+
+        Assert.IsTrue this.ErrorsExist
+
+
+    [<Test>]
+    member this.StringInterpolationWithStringFormatAndExternalTemplateShouldNotProduceError() =
+        this.Parse """
+let someFunction someTemplate =
+    Console.WriteLine(String.Format(someTemplate, world))"""
+
+        Assert.IsTrue this.NoErrorsExist
+
+
+    [<Test>]
+    member this.StringInterpolationWithStringFormatAndLocalVariableShouldProduceError() =
+        this.Parse """
+let someTemplate = "Hello {0}"
+let someString = String.Format(someTemplate, world)"""
+
+        Assert.IsTrue this.ErrorsExist
+
+
+    [<Test>]
+    member this.StringInterpolationWithMultipleModuleWithSameVariableNameShouldNotProduceError() =
+        this.Parse """
+module Foo =
+    let someTemplate = "Hello, this is not for String.Format actually"
+module Bar =
+    let someFunction someTemplate =
+        Console.WriteLine(String.Format(someTemplate, "world"))"""
+
+        Assert.IsTrue this.NoErrorsExist
+
+
+    [<Test>]
+    member this.StringInterpolationWithSameVariableNameInInnerLetShouldNotProduceError() =
+        this.Parse """
+module Bar =
+    let exampleFunction () =
+        let someTemplate = "Hello, this is not for String.Format actually"
+        someTemplate
+    let someFunction someTemplate =
+        let returnConstInt () =
+            89
+        Console.WriteLine(String.Format(someTemplate, "world"))"""
+
+        Assert.IsTrue this.NoErrorsExist
+
+
+    [<Test>]
+    member this.StringInterpolationWithSameVariableNameWithLocalLetShouldNotProduceError() =
+        this.Parse """
+module Bar =
+    let exampleFunction someTemplate =
+        let someResults =
+            let someTemplate = "Hello, this is not for String.Format actually"
+            someTemplate
+        Console.WriteLine(String.Format(someTemplate, "world"))"""
+
+        Assert.IsTrue this.NoErrorsExist