Skip to content

Commit

Permalink
Merge PR #661 from knocte/typePrefixingModes
Browse files Browse the repository at this point in the history
New typePrefixing modes "Always" and "Never" (default: "Hybrid").
  • Loading branch information
knocte authored Jan 9, 2024
2 parents e3139c5 + e3ea85b commit a263185
Show file tree
Hide file tree
Showing 9 changed files with 320 additions and 35 deletions.
90 changes: 90 additions & 0 deletions FSharpLint.sln
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,94 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
build.fsx = build.fsx
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "docs", "docs", "{E1E03FFE-30DF-4522-83DA-9089147B431E}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "rules", "rules", "{AEBB56D7-30B4-40D7-B065-54B8BE960298}"
ProjectSection(SolutionItems) = preProject
docs\content\how-tos\rules\FL0001.md = docs\content\how-tos\rules\FL0001.md
docs\content\how-tos\rules\FL0002.md = docs\content\how-tos\rules\FL0002.md
docs\content\how-tos\rules\FL0003.md = docs\content\how-tos\rules\FL0003.md
docs\content\how-tos\rules\FL0004.md = docs\content\how-tos\rules\FL0004.md
docs\content\how-tos\rules\FL0005.md = docs\content\how-tos\rules\FL0005.md
docs\content\how-tos\rules\FL0006.md = docs\content\how-tos\rules\FL0006.md
docs\content\how-tos\rules\FL0007.md = docs\content\how-tos\rules\FL0007.md
docs\content\how-tos\rules\FL0008.md = docs\content\how-tos\rules\FL0008.md
docs\content\how-tos\rules\FL0009.md = docs\content\how-tos\rules\FL0009.md
docs\content\how-tos\rules\FL0010.md = docs\content\how-tos\rules\FL0010.md
docs\content\how-tos\rules\FL0011.md = docs\content\how-tos\rules\FL0011.md
docs\content\how-tos\rules\FL0012.md = docs\content\how-tos\rules\FL0012.md
docs\content\how-tos\rules\FL0013.md = docs\content\how-tos\rules\FL0013.md
docs\content\how-tos\rules\FL0014.md = docs\content\how-tos\rules\FL0014.md
docs\content\how-tos\rules\FL0015.md = docs\content\how-tos\rules\FL0015.md
docs\content\how-tos\rules\FL0016.md = docs\content\how-tos\rules\FL0016.md
docs\content\how-tos\rules\FL0017.md = docs\content\how-tos\rules\FL0017.md
docs\content\how-tos\rules\FL0018.md = docs\content\how-tos\rules\FL0018.md
docs\content\how-tos\rules\FL0019.md = docs\content\how-tos\rules\FL0019.md
docs\content\how-tos\rules\FL0020.md = docs\content\how-tos\rules\FL0020.md
docs\content\how-tos\rules\FL0021.md = docs\content\how-tos\rules\FL0021.md
docs\content\how-tos\rules\FL0022.md = docs\content\how-tos\rules\FL0022.md
docs\content\how-tos\rules\FL0023.md = docs\content\how-tos\rules\FL0023.md
docs\content\how-tos\rules\FL0024.md = docs\content\how-tos\rules\FL0024.md
docs\content\how-tos\rules\FL0025.md = docs\content\how-tos\rules\FL0025.md
docs\content\how-tos\rules\FL0026.md = docs\content\how-tos\rules\FL0026.md
docs\content\how-tos\rules\FL0027.md = docs\content\how-tos\rules\FL0027.md
docs\content\how-tos\rules\FL0028.md = docs\content\how-tos\rules\FL0028.md
docs\content\how-tos\rules\FL0029.md = docs\content\how-tos\rules\FL0029.md
docs\content\how-tos\rules\FL0030.md = docs\content\how-tos\rules\FL0030.md
docs\content\how-tos\rules\FL0031.md = docs\content\how-tos\rules\FL0031.md
docs\content\how-tos\rules\FL0032.md = docs\content\how-tos\rules\FL0032.md
docs\content\how-tos\rules\FL0033.md = docs\content\how-tos\rules\FL0033.md
docs\content\how-tos\rules\FL0034.md = docs\content\how-tos\rules\FL0034.md
docs\content\how-tos\rules\FL0035.md = docs\content\how-tos\rules\FL0035.md
docs\content\how-tos\rules\FL0036.md = docs\content\how-tos\rules\FL0036.md
docs\content\how-tos\rules\FL0037.md = docs\content\how-tos\rules\FL0037.md
docs\content\how-tos\rules\FL0038.md = docs\content\how-tos\rules\FL0038.md
docs\content\how-tos\rules\FL0039.md = docs\content\how-tos\rules\FL0039.md
docs\content\how-tos\rules\FL0040.md = docs\content\how-tos\rules\FL0040.md
docs\content\how-tos\rules\FL0041.md = docs\content\how-tos\rules\FL0041.md
docs\content\how-tos\rules\FL0042.md = docs\content\how-tos\rules\FL0042.md
docs\content\how-tos\rules\FL0043.md = docs\content\how-tos\rules\FL0043.md
docs\content\how-tos\rules\FL0044.md = docs\content\how-tos\rules\FL0044.md
docs\content\how-tos\rules\FL0045.md = docs\content\how-tos\rules\FL0045.md
docs\content\how-tos\rules\FL0046.md = docs\content\how-tos\rules\FL0046.md
docs\content\how-tos\rules\FL0047.md = docs\content\how-tos\rules\FL0047.md
docs\content\how-tos\rules\FL0048.md = docs\content\how-tos\rules\FL0048.md
docs\content\how-tos\rules\FL0049.md = docs\content\how-tos\rules\FL0049.md
docs\content\how-tos\rules\FL0050.md = docs\content\how-tos\rules\FL0050.md
docs\content\how-tos\rules\FL0051.md = docs\content\how-tos\rules\FL0051.md
docs\content\how-tos\rules\FL0052.md = docs\content\how-tos\rules\FL0052.md
docs\content\how-tos\rules\FL0053.md = docs\content\how-tos\rules\FL0053.md
docs\content\how-tos\rules\FL0054.md = docs\content\how-tos\rules\FL0054.md
docs\content\how-tos\rules\FL0055.md = docs\content\how-tos\rules\FL0055.md
docs\content\how-tos\rules\FL0056.md = docs\content\how-tos\rules\FL0056.md
docs\content\how-tos\rules\FL0057.md = docs\content\how-tos\rules\FL0057.md
docs\content\how-tos\rules\FL0058.md = docs\content\how-tos\rules\FL0058.md
docs\content\how-tos\rules\FL0059.md = docs\content\how-tos\rules\FL0059.md
docs\content\how-tos\rules\FL0060.md = docs\content\how-tos\rules\FL0060.md
docs\content\how-tos\rules\FL0061.md = docs\content\how-tos\rules\FL0061.md
docs\content\how-tos\rules\FL0062.md = docs\content\how-tos\rules\FL0062.md
docs\content\how-tos\rules\FL0063.md = docs\content\how-tos\rules\FL0063.md
docs\content\how-tos\rules\FL0064.md = docs\content\how-tos\rules\FL0064.md
docs\content\how-tos\rules\FL0065.md = docs\content\how-tos\rules\FL0065.md
docs\content\how-tos\rules\FL0066.md = docs\content\how-tos\rules\FL0066.md
docs\content\how-tos\rules\FL0067.md = docs\content\how-tos\rules\FL0067.md
docs\content\how-tos\rules\FL0068.md = docs\content\how-tos\rules\FL0068.md
docs\content\how-tos\rules\FL0069.md = docs\content\how-tos\rules\FL0069.md
docs\content\how-tos\rules\FL0070.md = docs\content\how-tos\rules\FL0070.md
docs\content\how-tos\rules\FL0071.md = docs\content\how-tos\rules\FL0071.md
docs\content\how-tos\rules\FL0072.md = docs\content\how-tos\rules\FL0072.md
docs\content\how-tos\rules\FL0073.md = docs\content\how-tos\rules\FL0073.md
docs\content\how-tos\rules\FL0074.md = docs\content\how-tos\rules\FL0074.md
docs\content\how-tos\rules\FL0075.md = docs\content\how-tos\rules\FL0075.md
docs\content\how-tos\rules\FL0076.md = docs\content\how-tos\rules\FL0076.md
docs\content\how-tos\rules\FL0077.md = docs\content\how-tos\rules\FL0077.md
docs\content\how-tos\rules\FL0078.md = docs\content\how-tos\rules\FL0078.md
docs\content\how-tos\rules\FL0079.md = docs\content\how-tos\rules\FL0079.md
docs\content\how-tos\rules\FL0080.md = docs\content\how-tos\rules\FL0080.md
docs\content\how-tos\rules\FL0081.md = docs\content\how-tos\rules\FL0081.md
docs\content\how-tos\rules\FL0082.md = docs\content\how-tos\rules\FL0082.md
EndProjectSection
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -117,6 +205,8 @@ Global
{0DE70980-49E9-4613-84D4-FCF6ACABA5EE} = {1CD44876-BCDC-4C93-9DC2-C45244BD62AE}
{73AD322D-2E3F-45C4-8DB2-179571DECCD0} = {1CD44876-BCDC-4C93-9DC2-C45244BD62AE}
{B4A92AC6-F74A-4709-B2F7-6C5BABBFDEB0} = {1CD44876-BCDC-4C93-9DC2-C45244BD62AE}
{E1E03FFE-30DF-4522-83DA-9089147B431E} = {270E691D-ECA1-4BC5-B851-C5431A64E9FA}
{AEBB56D7-30B4-40D7-B065-54B8BE960298} = {E1E03FFE-30DF-4522-83DA-9089147B431E}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {B54B4B7D-F019-48A3-BB5B-635B68FE41C3}
Expand Down
2 changes: 1 addition & 1 deletion docs/content/how-tos/rules/FL0010.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ Update typed item to use configured spacing.
}
}

* *typedItemSpacing* - style of spacing: "NoSpaces", "SpaceAfter", "SpacesAround"
* *typedItemStyle* - style of spacing: "NoSpaces", "SpaceAfter", "SpacesAround"
7 changes: 6 additions & 1 deletion docs/content/how-tos/rules/FL0011.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ Update higher order type to have correct formatting as per guide linked above.

{
"typePrefixing": {
"enabled": false
"enabled": false,
"config": {
"mode": "Hybrid"
}
}
}

* *mode* - how to enforce the rule ("Hybrid" or "Always" or "Never")
15 changes: 11 additions & 4 deletions src/FSharpLint.Core/Application/Configuration.fs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,13 @@ let constructRuleWithConfig rule ruleConfig =
else
None

let constructTypePrefixingRuleWithConfig rule (ruleConfig: RuleConfig<TypePrefixing.Config>) =
if ruleConfig.Enabled then
let config = ruleConfig.Config |> Option.defaultValue { Mode = TypePrefixing.Mode.Hybrid }
Some(rule config)
else
None

type TupleFormattingConfig =
{ tupleCommaSpacing:EnabledConfig option
tupleIndentation:EnabledConfig option
Expand Down Expand Up @@ -165,7 +172,7 @@ with

type FormattingConfig =
{ typedItemSpacing:RuleConfig<TypedItemSpacing.Config> option
typePrefixing:EnabledConfig option
typePrefixing:RuleConfig<TypePrefixing.Config> option
unionDefinitionIndentation:EnabledConfig option
moduleDeclSpacing:EnabledConfig option
classMemberSpacing:EnabledConfig option
Expand All @@ -175,7 +182,7 @@ with
member this.Flatten() =
[|
this.typedItemSpacing |> Option.bind (constructRuleWithConfig TypedItemSpacing.rule) |> Option.toArray
this.typePrefixing |> Option.bind (constructRuleIfEnabled TypePrefixing.rule) |> Option.toArray
this.typePrefixing |> Option.bind (constructTypePrefixingRuleWithConfig TypePrefixing.rule) |> Option.toArray
this.unionDefinitionIndentation |> Option.bind (constructRuleIfEnabled UnionDefinitionIndentation.rule) |> Option.toArray
this.moduleDeclSpacing |> Option.bind (constructRuleIfEnabled ModuleDeclSpacing.rule) |> Option.toArray
this.classMemberSpacing |> Option.bind (constructRuleIfEnabled ClassMemberSpacing.rule) |> Option.toArray
Expand Down Expand Up @@ -391,7 +398,7 @@ type Configuration =
ignoreFiles:string [] option
Hints:HintConfig option
TypedItemSpacing:RuleConfig<TypedItemSpacing.Config> option
TypePrefixing:EnabledConfig option
TypePrefixing:RuleConfig<TypePrefixing.Config> option
UnionDefinitionIndentation:EnabledConfig option
ModuleDeclSpacing:EnabledConfig option
ClassMemberSpacing:EnabledConfig option
Expand Down Expand Up @@ -632,7 +639,7 @@ let flattenConfig (config:Configuration) =
let allRules =
[|
config.TypedItemSpacing |> Option.bind (constructRuleWithConfig TypedItemSpacing.rule)
config.TypePrefixing |> Option.bind (constructRuleIfEnabled TypePrefixing.rule)
config.TypePrefixing |> Option.bind (constructTypePrefixingRuleWithConfig TypePrefixing.rule)
config.UnionDefinitionIndentation |> Option.bind (constructRuleIfEnabled UnionDefinitionIndentation.rule)
config.ModuleDeclSpacing |> Option.bind (constructRuleIfEnabled ModuleDeclSpacing.rule)
config.ClassMemberSpacing |> Option.bind (constructRuleIfEnabled ClassMemberSpacing.rule)
Expand Down
69 changes: 50 additions & 19 deletions src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,52 @@ open FSharpLint.Framework.Ast
open FSharpLint.Framework.Rules
open FSharpLint.Framework.ExpressionUtilities

let checkTypePrefixing (args:AstNodeRuleParams) range typeName typeArgs isPostfix =
type Mode =
| Hybrid = 0
| Always = 1
| Never = 2

[<RequireQualifiedAccess>]
type Config = { Mode: Mode }

let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName typeArgs isPostfix =
let recommendPostfixErrMsg = lazy(Resources.GetString("RulesFormattingF#PostfixGenericError"))
match typeName with
| SynType.LongIdent lid ->
let prefixSuggestion typeName =
let suggestedFix = lazy(
(ExpressionUtilities.tryFindTextOfRange range args.FileContent, typeArgs)
||> Option.map2 (fun fromText typeArgs -> { FromText = fromText; FromRange = range; ToText = typeName + "<" + typeArgs + ">" }))
{ Range = range
Message = Resources.GetString("RulesFormattingGenericPrefixError")
SuggestedFix = Some suggestedFix
TypeChecks = [] } |> Some

match lid |> longIdentWithDotsToString with
| "list"
| "List"
| "option"
| "Option"
| "ref"
| "Ref" as typeName ->

// Prefer postfix.
if not isPostfix
if not isPostfix && config.Mode <> Mode.Always
then
let errorFormatString = Resources.GetString("RulesFormattingF#PostfixGenericError")
let suggestedFix = lazy(
(ExpressionUtilities.tryFindTextOfRange range args.FileContent, typeArgs)
||> Option.map2 (fun fromText typeArgs -> { FromText = fromText; FromRange = range; ToText = typeArgs + " " + typeName }))
{ Range = range
Message = String.Format(errorFormatString, typeName)
Message = String.Format(recommendPostfixErrMsg.Value, typeName)
SuggestedFix = Some suggestedFix
TypeChecks = [] } |> Some
else
None
| "array" ->
if isPostfix && config.Mode = Mode.Always then
prefixSuggestion typeName
else
None

| "array" when config.Mode <> Mode.Always ->
// Prefer special postfix (e.g. int []).
let suggestedFix = lazy(
(ExpressionUtilities.tryFindTextOfRange range args.FileContent, typeArgs)
Expand All @@ -40,33 +62,42 @@ let checkTypePrefixing (args:AstNodeRuleParams) range typeName typeArgs isPostfi
Message = Resources.GetString("RulesFormattingF#ArrayPostfixError")
SuggestedFix = Some suggestedFix
TypeChecks = [] } |> Some

| typeName ->
// Prefer prefix.
if isPostfix then
let suggestedFix = lazy(
(ExpressionUtilities.tryFindTextOfRange range args.FileContent, typeArgs)
||> Option.map2 (fun fromText typeArgs -> { FromText = fromText; FromRange = range; ToText = typeName + "<" + typeArgs + ">" }))
match (isPostfix, config.Mode) with
| true, Mode.Never ->
None
| true, _ ->
prefixSuggestion typeName
| false, Mode.Never ->
{ Range = range
Message = Resources.GetString("RulesFormattingGenericPrefixError")
SuggestedFix = Some suggestedFix
TypeChecks = [] } |> Some
else
Message = String.Format(recommendPostfixErrMsg.Value, typeName)
// TODO
SuggestedFix = None
TypeChecks = List.Empty } |> Some
| false, _ ->
None
| _ ->
None

let runner args =
let runner (config:Config) args =
match args.AstNode with
| AstNode.Type (SynType.App (typeName, _, typeArgs, _, _, isPostfix, range)) ->
let typeArgs = typeArgsToString args.FileContent typeArgs
checkTypePrefixing args range typeName typeArgs isPostfix
checkTypePrefixing config args range typeName typeArgs isPostfix
|> Option.toArray
| AstNode.Type (SynType.Array (1, _elementType, range)) when config.Mode = Mode.Always ->
{ Range = range
Message = Resources.GetString("RulesFormattingF#ArrayPrefixError")
SuggestedFix = None
TypeChecks = List.Empty }
|> Array.singleton
| _ ->
Array.empty


let rule =
let rule config =
{ Name = "TypePrefixing"
Identifier = Identifiers.TypePrefixing
RuleConfig = { AstNodeRuleConfig.Runner = runner; Cleanup = ignore } }
RuleConfig = { AstNodeRuleConfig.Runner = runner config; Cleanup = ignore } }
|> AstNodeRule
3 changes: 3 additions & 0 deletions src/FSharpLint.Core/Text.resx
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@
<data name="RulesFormattingF#PostfixGenericError" xml:space="preserve">
<value>Use postfix syntax for F# type {0}.</value>
</data>
<data name="RulesFormattingF#ArrayPrefixError" xml:space="preserve">
<value>Use prefix syntax for generic type (array&lt;'Foo&gt;).</value>
</data>
<data name="RulesFormattingGenericPrefixError" xml:space="preserve">
<value>Use prefix syntax for generic type.</value>
</data>
Expand Down
9 changes: 7 additions & 2 deletions src/FSharpLint.Core/fsharplint.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@
"typedItemStyle": "NoSpaces"
}
},
"typePrefixing": { "enabled": false },
"typePrefixing": {
"enabled": false,
"config": {
"mode": "Hybrid"
}
},
"unionDefinitionIndentation": { "enabled": false },
"moduleDeclSpacing": { "enabled": false },
"classMemberSpacing": { "enabled": false },
Expand Down Expand Up @@ -144,7 +149,7 @@
},
"recordFieldNames": {
"enabled": true,
"config": {
"config": {
"naming": "PascalCase",
"underscores": "None"
}
Expand Down
34 changes: 28 additions & 6 deletions tests/FSharpLint.Console.Tests/TestApp.fs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ let getErrorsFromOutput (output:string) =
set [ for i in 1..splitOutput.Length - 1 do
if splitOutput.[i].StartsWith "Error" then yield splitOutput.[i - 1] ]

type TemporaryFile(config, extension) =
type TemporaryFile(fileContent, extension) =
let filename = Path.ChangeExtension(Path.GetTempFileName(), extension)
do
File.WriteAllText(filename, config)
File.WriteAllText(filename, fileContent)

member __.FileName = filename

Expand All @@ -35,12 +35,12 @@ let main input =
type TestConsoleApplication() =
[<Test>]
member __.``Lint file, expected rules are triggered.``() =
let config = """
let fileContent = """
type Signature =
abstract member Encoded : string
abstract member PathName : string
"""
use input = new TemporaryFile(config, "fs")
use input = new TemporaryFile(fileContent, "fs")

let (returnCode, errors) = main [| "lint"; input.FileName |]

Expand All @@ -62,14 +62,14 @@ type TestConsoleApplication() =

[<Test>]
member __.``Lint source with valid config to disable rule, disabled rule is not triggered for given source.``() =
let config = """
let fileContent = """
{
"InterfaceNames": {
"enabled": false
}
}
"""
use config = new TemporaryFile(config, "json")
use config = new TemporaryFile(fileContent, "json")

let input = """
type Signature =
Expand All @@ -95,3 +95,25 @@ type TestConsoleApplication() =

Assert.AreEqual(0, returnCode)
Assert.AreEqual(Set.empty, errors)

[<Test>]
member __.``Regression test: typePrefixing rule with old config format should still work``() =
let fileContent = """
{
"typePrefixing": {
"enabled": true
}
}
"""
use config = new TemporaryFile(fileContent, "json")

let input = """
module Program
type X = int Generic
"""

let (returnCode, errors) = main [| "lint"; "--lint-config"; config.FileName; input |]

Assert.AreEqual(-1, returnCode)
Assert.AreEqual(set ["Use prefix syntax for generic type."], errors)
Loading

0 comments on commit a263185

Please sign in to comment.