From 746b001cb9ce00d337e498b2e3887f648858ebe1 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 14:18:22 +0800 Subject: [PATCH 01/17] tests/Console: rename some vars with confusing names --- tests/FSharpLint.Console.Tests/TestApp.fs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 1402915b8..7b0467a08 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -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 @@ -35,12 +35,12 @@ let main input = type TestConsoleApplication() = [] 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 |] @@ -62,14 +62,14 @@ type TestConsoleApplication() = [] 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 = From 99a6642810107fcfa83a3bcebdf40b53d82e5ce2 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 14:33:37 +0800 Subject: [PATCH 02/17] tests/Console: regression test for typePrefixing As we're going to change how the configuration of this rule is structured, we first create a test to make sure the old config works. --- tests/FSharpLint.Console.Tests/TestApp.fs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 7b0467a08..68b69ab64 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -95,3 +95,25 @@ type TestConsoleApplication() = Assert.AreEqual(0, returnCode) Assert.AreEqual(Set.empty, errors) + + [] + 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) From a7a36e2847a2dc6ea37e474d17867f1074293034 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 14:45:05 +0800 Subject: [PATCH 03/17] sln: add docs (to edit rules info in IDE) --- FSharpLint.sln | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/FSharpLint.sln b/FSharpLint.sln index 99b625527..95809fec0 100644 --- a/FSharpLint.sln +++ b/FSharpLint.sln @@ -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 @@ -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} From 310b7b0266b3cc0a51bd96a89f3915c3470cd2cd Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 14:45:16 +0800 Subject: [PATCH 04/17] docs(rule10): fix typo [no ci] --- docs/content/how-tos/rules/FL0010.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/how-tos/rules/FL0010.md b/docs/content/how-tos/rules/FL0010.md index 7e1d0b9f0..26d82c9d6 100644 --- a/docs/content/how-tos/rules/FL0010.md +++ b/docs/content/how-tos/rules/FL0010.md @@ -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" From 3108e3672b15a4388d1f60385dcf18ee7a7cd1a9 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 14:46:05 +0800 Subject: [PATCH 05/17] fsharplint.json: fix cosmetic double-space --- src/FSharpLint.Core/fsharplint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/FSharpLint.Core/fsharplint.json b/src/FSharpLint.Core/fsharplint.json index dfe9984cd..7aab9ef96 100644 --- a/src/FSharpLint.Core/fsharplint.json +++ b/src/FSharpLint.Core/fsharplint.json @@ -144,7 +144,7 @@ }, "recordFieldNames": { "enabled": true, - "config": { + "config": { "naming": "PascalCase", "underscores": "None" } From a48777643e7e5145e64ed21b578660de3bd07308 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 15:12:36 +0800 Subject: [PATCH 06/17] Core.{Rules,Tests},docs: prepare for TypePrefixing new cfg --- docs/content/how-tos/rules/FL0011.md | 7 ++++++- .../Application/Configuration.fs | 8 ++++---- .../Rules/Formatting/TypePrefixing.fs | 18 +++++++++++++----- src/FSharpLint.Core/fsharplint.json | 7 ++++++- tests/FSharpLint.Console.Tests/TestApp.fs | 2 ++ .../Rules/Formatting/TypePrefixing.fs | 5 +++-- 6 files changed, 34 insertions(+), 13 deletions(-) diff --git a/docs/content/how-tos/rules/FL0011.md b/docs/content/how-tos/rules/FL0011.md index f0489e402..b71601b6a 100644 --- a/docs/content/how-tos/rules/FL0011.md +++ b/docs/content/how-tos/rules/FL0011.md @@ -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") diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index b3e23affd..983bc604d 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -165,7 +165,7 @@ with type FormattingConfig = { typedItemSpacing:RuleConfig option - typePrefixing:EnabledConfig option + typePrefixing:RuleConfig option unionDefinitionIndentation:EnabledConfig option moduleDeclSpacing:EnabledConfig option classMemberSpacing:EnabledConfig option @@ -175,7 +175,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 (constructRuleWithConfig 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 @@ -389,7 +389,7 @@ type Configuration = ignoreFiles:string [] option Hints:HintConfig option TypedItemSpacing:RuleConfig option - TypePrefixing:EnabledConfig option + TypePrefixing:RuleConfig option UnionDefinitionIndentation:EnabledConfig option ModuleDeclSpacing:EnabledConfig option ClassMemberSpacing:EnabledConfig option @@ -628,7 +628,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 (constructRuleWithConfig TypePrefixing.rule) config.UnionDefinitionIndentation |> Option.bind (constructRuleIfEnabled UnionDefinitionIndentation.rule) config.ModuleDeclSpacing |> Option.bind (constructRuleIfEnabled ModuleDeclSpacing.rule) config.ClassMemberSpacing |> Option.bind (constructRuleIfEnabled ClassMemberSpacing.rule) diff --git a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs index 88519b99d..5cac27be6 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs @@ -8,7 +8,15 @@ 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 + +[] +type Config = { Mode: Mode } + +let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName typeArgs isPostfix = match typeName with | SynType.LongIdent lid -> match lid |> longIdentWithDotsToString with @@ -55,18 +63,18 @@ let checkTypePrefixing (args:AstNodeRuleParams) range typeName typeArgs isPostfi | _ -> 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 | _ -> 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 diff --git a/src/FSharpLint.Core/fsharplint.json b/src/FSharpLint.Core/fsharplint.json index 7aab9ef96..0cc859f0a 100644 --- a/src/FSharpLint.Core/fsharplint.json +++ b/src/FSharpLint.Core/fsharplint.json @@ -11,7 +11,12 @@ "typedItemStyle": "NoSpaces" } }, - "typePrefixing": { "enabled": false }, + "typePrefixing": { + "enabled": false, + "config": { + "mode": "Hybrid" + } + }, "unionDefinitionIndentation": { "enabled": false }, "moduleDeclSpacing": { "enabled": false }, "classMemberSpacing": { "enabled": false }, diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 68b69ab64..409163124 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -96,7 +96,9 @@ type TestConsoleApplication() = Assert.AreEqual(0, returnCode) Assert.AreEqual(Set.empty, errors) +(* disabling regression test for now until I finish the development of new config [] +*) member __.``Regression test: typePrefixing rule with old config format should still work``() = let fileContent = """ { diff --git a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs index 3cefcf197..f2008aa07 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs @@ -2,10 +2,11 @@ module FSharpLint.Core.Tests.Rules.Formatting.TypePrefixing open NUnit.Framework open FSharpLint.Rules +open FSharpLint.Rules.TypePrefixing [] -type TestFormattingTypePrefixing() = - inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule) +type TestFormattingHybridTypePrefixing() = + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule { Config.Mode = Mode.Hybrid }) [] member this.``Error for F# List type prefix syntax``() = From 6957536af72988c029dc29e9ad7bba43cef126b3 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 15:44:58 +0800 Subject: [PATCH 07/17] tests(TypePrefixing): 1st two tests for Always mode (should pass already) The 'Always' mode behaves in the same way for this case. --- .../Rules/Formatting/TypePrefixing.fs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs index f2008aa07..5cafcf690 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs @@ -237,3 +237,27 @@ type T = Generic this.Parse source Assert.AreEqual(expected, this.ApplyQuickFix source) +[] +type TestFormattingAlwaysTypePrefixing() = + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule { Config.Mode = Mode.Always }) + + [] + member this.``Error for generic type postfix syntax (like hybrid)``() = + this.Parse """ + module Program + + type X = int Generic + """ + + Assert.IsTrue this.ErrorsExist + + [] + member this.``No error for generic type prefix syntax (like hybrid)``() = + this.Parse """ + module Program + + type X = Generic + """ + + Assert.IsTrue this.NoErrorsExist + From 1e474d37cfac36620c27b0200e79f8d85765d03c Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 16:03:16 +0800 Subject: [PATCH 08/17] tests(TypePrefixing): add 2 failing tests for Always cfg --- .../Rules/Formatting/TypePrefixing.fs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs index 5cafcf690..076bc7811 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs @@ -261,3 +261,22 @@ type TestFormattingAlwaysTypePrefixing() = Assert.IsTrue this.NoErrorsExist + [] + member this.``No error for F# Option type prefix syntax``() = + this.Parse """ + module Program + + type T = Option + """ + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Error for F# Option type postfix syntax``() = + this.Parse """ + module Program + + type T = int option + """ + + Assert.IsTrue this.ErrorsExist From 0264faf805abbf3ef3e2bf4584523ba0896b026f Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 16:04:58 +0800 Subject: [PATCH 09/17] TypePrefixing(Always): make tests pass --- .../Rules/Formatting/TypePrefixing.fs | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs index 5cac27be6..57ce4050d 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs @@ -19,6 +19,15 @@ type Config = { Mode: Mode } let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName typeArgs isPostfix = 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" @@ -26,8 +35,9 @@ let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName t | "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( @@ -38,7 +48,10 @@ let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName t SuggestedFix = Some suggestedFix TypeChecks = [] } |> Some else - None + if isPostfix && config.Mode = Mode.Always then + prefixSuggestion typeName + else + None | "array" -> // Prefer special postfix (e.g. int []). let suggestedFix = lazy( @@ -51,13 +64,7 @@ let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName t | 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 + ">" })) - { Range = range - Message = Resources.GetString("RulesFormattingGenericPrefixError") - SuggestedFix = Some suggestedFix - TypeChecks = [] } |> Some + prefixSuggestion typeName else None | _ -> From c6c04888c0835da9ece6a5918016e7ec19171658 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 16:13:23 +0800 Subject: [PATCH 10/17] tests(TypePrefixing): 1st Never Mode tests (should pass already) --- .../Rules/Formatting/TypePrefixing.fs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs index 076bc7811..e18c6008e 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs @@ -280,3 +280,27 @@ type TestFormattingAlwaysTypePrefixing() = """ Assert.IsTrue this.ErrorsExist + +[] +type TestFormattingNeverTypePrefixing() = + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule { Config.Mode = Mode.Never }) + + [] + member this.``Error for F# Option type prefix syntax (like hybrid)``() = + this.Parse """ + module Program + + type T = Option + """ + + Assert.IsTrue this.ErrorsExist + + [] + member this.``No error for F# Option type postfix syntax (like hybrid)``() = + this.Parse """ + module Program + + type T = int option + """ + + Assert.IsTrue this.NoErrorsExist From 2fc75d8dffe6ed3b16f3fd9602531b739b0a9011 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 16:21:46 +0800 Subject: [PATCH 11/17] tests(TypePrefixing): 2 failing tests for Never mode --- .../Rules/Formatting/TypePrefixing.fs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs index e18c6008e..223955097 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs @@ -304,3 +304,24 @@ type TestFormattingNeverTypePrefixing() = """ Assert.IsTrue this.NoErrorsExist + + [] + member this.``No error for generic type postfix syntax (unlike any other mode)``() = + this.Parse """ + module Program + + type X = int Generic + """ + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Error for generic type prefix syntax (unlike any other mode)``() = + this.Parse """ + module Program + + type X = Generic + """ + + Assert.IsTrue this.ErrorsExist + From 23750966e22a2ecb7e62a20bf1f05043df218a82 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 16:33:06 +0800 Subject: [PATCH 12/17] TypePrefixing: make Never tests pass --- .../Rules/Formatting/TypePrefixing.fs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs index 57ce4050d..5ba64bf30 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs @@ -17,6 +17,7 @@ type Mode = 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 = @@ -39,12 +40,11 @@ let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName t // Prefer postfix. 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 @@ -62,10 +62,18 @@ let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName t SuggestedFix = Some suggestedFix TypeChecks = [] } |> Some | typeName -> - // Prefer prefix. - if isPostfix then + match (isPostfix, config.Mode) with + | true, Mode.Never -> + None + | true, _ -> prefixSuggestion typeName - else + | false, Mode.Never -> + { Range = range + Message = String.Format(recommendPostfixErrMsg.Value, typeName) + // TODO + SuggestedFix = None + TypeChecks = List.Empty } |> Some + | false, _ -> None | _ -> None From 18f9f96d067d0d68e5e002aa84c260d28cec495f Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 16:50:34 +0800 Subject: [PATCH 13/17] tests(TypePrefixing): 2 failing tests for array(Always) Also editing an existing Hybrid mode one to make sure we don't break it. --- .../Rules/Formatting/TypePrefixing.fs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs index 223955097..49908741a 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs @@ -87,6 +87,7 @@ type T = int array """ Assert.IsTrue(this.ErrorExistsAt(4, 9)) + Assert.IsTrue (this.ErrorWithMessageExistsAt("Use special postfix syntax for F# type array.", 4, 9)) [] member this.``No error for F# array type special postfix syntax``() = @@ -281,6 +282,27 @@ type TestFormattingAlwaysTypePrefixing() = Assert.IsTrue this.ErrorsExist + [] + member this.``No error for F# array type prefix syntax``() = + this.Parse """ +module Program + +type T = array +""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Error for F# array type standard postfix syntax``() = + this.Parse """ +module Program + +type T = int array +""" + + Assert.IsTrue this.ErrorsExist + Assert.IsTrue (this.ErrorWithMessageExistsAt("Use prefix syntax for generic type.", 4, 9)) + [] type TestFormattingNeverTypePrefixing() = inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule { Config.Mode = Mode.Never }) From 3e1d4d7e6b2a1546a293095988e1ec56750a6682 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 16:51:45 +0800 Subject: [PATCH 14/17] TypePrefixing: make new Always array tests pass --- src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs index 5ba64bf30..dfa245b8a 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs @@ -52,7 +52,8 @@ let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName t prefixSuggestion typeName else None - | "array" -> + + | "array" when config.Mode <> Mode.Always -> // Prefer special postfix (e.g. int []). let suggestedFix = lazy( (ExpressionUtilities.tryFindTextOfRange range args.FileContent, typeArgs) @@ -61,6 +62,7 @@ let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName t Message = Resources.GetString("RulesFormattingF#ArrayPostfixError") SuggestedFix = Some suggestedFix TypeChecks = [] } |> Some + | typeName -> match (isPostfix, config.Mode) with | true, Mode.Never -> From 9e62473b38fb5ebf9931a6274e4c00e3ec3106c2 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 30 Dec 2023 17:49:44 +0800 Subject: [PATCH 15/17] tests(TypePrefixing): failing test for [] in Always mode --- .../Rules/Formatting/TypePrefixing.fs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs index 49908741a..c3791a2fe 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs @@ -303,6 +303,17 @@ type T = int array Assert.IsTrue this.ErrorsExist Assert.IsTrue (this.ErrorWithMessageExistsAt("Use prefix syntax for generic type.", 4, 9)) + [] + member this.``Error for F# array type special postfix syntax``() = + this.Parse """ +module Program + +type T = int [] +""" + + Assert.IsTrue this.ErrorsExist + Assert.IsTrue (this.ErrorWithMessageExistsAt("Use prefix syntax for generic type (array<'Foo>).", 4, 9)) + [] type TestFormattingNeverTypePrefixing() = inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule { Config.Mode = Mode.Never }) From 5e68892143faa587ae31b6d1b7226a243fef146e Mon Sep 17 00:00:00 2001 From: webwarrior Date: Tue, 2 Jan 2024 10:58:17 +0100 Subject: [PATCH 16/17] TypePrefixing: make new Always array test pass Handle case of array types like `foo[]` in Always mode. --- src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs | 6 ++++++ src/FSharpLint.Core/Text.resx | 3 +++ 2 files changed, 9 insertions(+) diff --git a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs index dfa245b8a..f9b15e5f2 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs @@ -86,6 +86,12 @@ let runner (config:Config) args = let typeArgs = typeArgsToString args.FileContent typeArgs 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 diff --git a/src/FSharpLint.Core/Text.resx b/src/FSharpLint.Core/Text.resx index 108959a45..41f711eaa 100644 --- a/src/FSharpLint.Core/Text.resx +++ b/src/FSharpLint.Core/Text.resx @@ -273,6 +273,9 @@ Use postfix syntax for F# type {0}. + + Use prefix syntax for generic type (array<'Foo>). + Use prefix syntax for generic type. From e3ea85bc38332d02e2a720f119521ca248229e96 Mon Sep 17 00:00:00 2001 From: webwarrior Date: Tue, 2 Jan 2024 11:43:58 +0100 Subject: [PATCH 17/17] Configuration,Tests.Console: re-enabled regression test Re-enabled regression test for TypePrefixing rule with old config format and made it pass. --- src/FSharpLint.Core/Application/Configuration.fs | 11 +++++++++-- tests/FSharpLint.Console.Tests/TestApp.fs | 2 -- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index 983bc604d..c79890059 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -137,6 +137,13 @@ let constructRuleWithConfig rule ruleConfig = else None +let constructTypePrefixingRuleWithConfig rule (ruleConfig: RuleConfig) = + 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 @@ -175,7 +182,7 @@ with member this.Flatten() = [| this.typedItemSpacing |> Option.bind (constructRuleWithConfig TypedItemSpacing.rule) |> Option.toArray - this.typePrefixing |> Option.bind (constructRuleWithConfig 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 @@ -628,7 +635,7 @@ let flattenConfig (config:Configuration) = let allRules = [| config.TypedItemSpacing |> Option.bind (constructRuleWithConfig TypedItemSpacing.rule) - config.TypePrefixing |> Option.bind (constructRuleWithConfig 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) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 409163124..68b69ab64 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -96,9 +96,7 @@ type TestConsoleApplication() = Assert.AreEqual(0, returnCode) Assert.AreEqual(Set.empty, errors) -(* disabling regression test for now until I finish the development of new config [] -*) member __.``Regression test: typePrefixing rule with old config format should still work``() = let fileContent = """ {