Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New typePrefixing modes "Always" and "Never" (default: "Hybrid") #661

Merged
merged 17 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -389,7 +396,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 @@ -628,7 +635,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
Loading