diff --git a/docs/content/how-tos/rule-configuration.md b/docs/content/how-tos/rule-configuration.md index f53668bf9..e230245c7 100644 --- a/docs/content/how-tos/rule-configuration.md +++ b/docs/content/how-tos/rule-configuration.md @@ -117,3 +117,4 @@ The following rules can be specified for linting. - [FavourConsistentThis (FL0074)](rules/FL0074.html) - [AvoidTooShortNames (FL0075)](rules/FL0075.html) - [FavourStaticEmptyFields (FL0076)](rules/FL0076.html) +- [PreferStringInterpolationWithSprintf (FL0077)](rules/FL0077.html) diff --git a/docs/content/how-tos/rules/FL0077.md b/docs/content/how-tos/rules/FL0077.md new file mode 100644 index 000000000..14d75809f --- /dev/null +++ b/docs/content/how-tos/rules/FL0077.md @@ -0,0 +1,29 @@ +--- +title: FL0077 +category: how-to +hide_menu: true +--- + +# PreferStringInterpolationWithSprintf (FL0077) + +*Introduced in `0.21.3`* + +## 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 + } + } \ No newline at end of file diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index 7762fe4f7..0b25684db 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -307,6 +307,7 @@ with type ConventionsConfig = { recursiveAsyncFunction:EnabledConfig option avoidTooShortNames:EnabledConfig option + preferStringInterpolationWithSprintf:EnabledConfig option redundantNewKeyword:EnabledConfig option favourStaticEmptyFields:EnabledConfig option nestedStatements:RuleConfig option @@ -325,6 +326,7 @@ with [| this.recursiveAsyncFunction |> Option.bind (constructRuleIfEnabled RecursiveAsyncFunction.rule) |> Option.toArray this.avoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule) |> Option.toArray + this.preferStringInterpolationWithSprintf |> Option.bind (constructRuleIfEnabled PreferStringInterpolationWithSprintf.rule) |> Option.toArray this.redundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule) |> Option.toArray this.favourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule) |> Option.toArray this.favourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule) |> Option.toArray @@ -394,6 +396,7 @@ type Configuration = PatternMatchExpressionIndentation:EnabledConfig option RecursiveAsyncFunction:EnabledConfig option AvoidTooShortNames:EnabledConfig option + PreferStringInterpolationWithSprintf:EnabledConfig option RedundantNewKeyword:EnabledConfig option FavourReRaise:EnabledConfig option FavourStaticEmptyFields:EnabledConfig option @@ -478,6 +481,7 @@ with PatternMatchExpressionIndentation = None RecursiveAsyncFunction = None AvoidTooShortNames = None + PreferStringInterpolationWithSprintf = None RedundantNewKeyword = None FavourReRaise = None FavourStaticEmptyFields = None @@ -625,6 +629,7 @@ let flattenConfig (config:Configuration) = config.PatternMatchExpressionIndentation |> Option.bind (constructRuleIfEnabled PatternMatchExpressionIndentation.rule) config.RecursiveAsyncFunction |> Option.bind (constructRuleIfEnabled RecursiveAsyncFunction.rule) config.AvoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule) + config.PreferStringInterpolationWithSprintf |> Option.bind (constructRuleIfEnabled PreferStringInterpolationWithSprintf.rule) config.RedundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule) config.FavourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule) config.FavourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule) diff --git a/src/FSharpLint.Core/FSharpLint.Core.fsproj b/src/FSharpLint.Core/FSharpLint.Core.fsproj index aa55c8d30..55dd179fb 100644 --- a/src/FSharpLint.Core/FSharpLint.Core.fsproj +++ b/src/FSharpLint.Core/FSharpLint.Core.fsproj @@ -50,6 +50,7 @@ + diff --git a/src/FSharpLint.Core/Rules/Conventions/PreferStringInterpolationWithSprintf.fs b/src/FSharpLint.Core/Rules/Conventions/PreferStringInterpolationWithSprintf.fs new file mode 100644 index 000000000..bb092e393 --- /dev/null +++ b/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) = + "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(_, LongIdentWithDots(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(_, 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(_, identifier, _, _, _), _, SynExpr.Const(SynConst.String(value, _, _), _), range, _)) 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 diff --git a/src/FSharpLint.Core/Rules/Identifiers.fs b/src/FSharpLint.Core/Rules/Identifiers.fs index 95fd12259..686a2ad4c 100644 --- a/src/FSharpLint.Core/Rules/Identifiers.fs +++ b/src/FSharpLint.Core/Rules/Identifiers.fs @@ -81,3 +81,4 @@ let FavourReRaise = identifier 73 let FavourConsistentThis = identifier 74 let AvoidTooShortNames = identifier 75 let FavourStaticEmptyFields = identifier 76 +let PreferStringInterpolationWithSprintf = identifier 77 diff --git a/src/FSharpLint.Core/Text.resx b/src/FSharpLint.Core/Text.resx index 1c90e1adc..ce7fc2ff6 100644 --- a/src/FSharpLint.Core/Text.resx +++ b/src/FSharpLint.Core/Text.resx @@ -345,4 +345,7 @@ Consider using 'Array.empty' instead. + + Use sprintf instead of String.Format. + diff --git a/src/FSharpLint.Core/fsharplint.json b/src/FSharpLint.Core/fsharplint.json index 06cf0a47e..3bc6ab80e 100644 --- a/src/FSharpLint.Core/fsharplint.json +++ b/src/FSharpLint.Core/fsharplint.json @@ -277,6 +277,9 @@ "indentation": { "enabled": false }, + "preferStringInterpolationWithSprintf": { + "enabled": false + }, "maxCharactersOnLine": { "enabled": false, "config": { diff --git a/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj b/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj index abc0ec7ca..78063886c 100644 --- a/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj +++ b/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj @@ -37,6 +37,7 @@ + diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/PreferStringInterpolationWithSprintf.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/PreferStringInterpolationWithSprintf.fs new file mode 100644 index 000000000..835c83638 --- /dev/null +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/PreferStringInterpolationWithSprintf.fs @@ -0,0 +1,80 @@ +module FSharpLint.Core.Tests.Rules.Conventions.PreferStringInterpolationWithSprintf + +open NUnit.Framework +open FSharpLint.Rules + +[] +type TestConventionsPreferStringInterpolationWithSprintf() = + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(PreferStringInterpolationWithSprintf.rule) + + [] + member this.StringInterpolationWithSprintfShouldNotProduceError() = + this.Parse """ +let someString = sprintf "Hello %s" world""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.StringInterpolationWithStringFormatShouldProduceError() = + this.Parse """ +let someString = String.Format("Hello {0}", world)""" + + Assert.IsTrue this.ErrorsExist + + + [] + member this.StringInterpolationWithStringFormatAndExternalTemplateShouldNotProduceError() = + this.Parse """ +let someFunction someTemplate = + Console.WriteLine(String.Format(someTemplate, world))""" + + Assert.IsTrue this.NoErrorsExist + + + [] + member this.StringInterpolationWithStringFormatAndLocalVariableShouldProduceError() = + this.Parse """ +let someTemplate = "Hello {0}" +let someString = String.Format(someTemplate, world)""" + + Assert.IsTrue this.ErrorsExist + + + [] + 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 + + + [] + 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 + + + [] + 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