From ca5fc1e0feae9365944f17e79b1b019ba3c07b3e Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Fri, 19 Jan 2024 11:43:48 +0800 Subject: [PATCH 1/9] Console.Tests: new typePrefixing regression test We plan to split Hybrid mode in two modes -> HybridWeak and HybridStrict; but if user is still using old "Hybrid" mode name we want it to make it still work (and assume it's the default). --- tests/FSharpLint.Console.Tests/TestApp.fs | 25 +++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 68b69ab64..fc1c69020 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -117,3 +117,28 @@ type TestConsoleApplication() = Assert.AreEqual(-1, returnCode) Assert.AreEqual(set ["Use prefix syntax for generic type."], errors) + + [] + member __.``TypePrefixing rule Hybrid mode should still work (after it gets renamed to HybridWeak)``() = + let fileContent = """ + { + "typePrefixing": { + "enabled": true, + "config": { + "mode": "Hybrid" + } + } + } + """ + 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 16b71500f6de9206118c6d8c60e14aa3f6764c5c Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Fri, 19 Jan 2024 11:59:47 +0800 Subject: [PATCH 2/9] Console.Tests: acceptance test for HybridStrict For now it maps to Hybrid mode, as HybridStrict doesn't exist yet. CI of this build should be green already. --- tests/FSharpLint.Console.Tests/TestApp.fs | 27 +++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index fc1c69020..355a64001 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -142,3 +142,30 @@ type TestConsoleApplication() = Assert.AreEqual(-1, returnCode) Assert.AreEqual(set ["Use prefix syntax for generic type."], errors) + + [] + member __.``TypePrefixing rule HybridStrict mode should complain about List``() = + let fileContent = """ + { + "typePrefixing": { + "enabled": true, + "config": { + "mode": "Hybrid" + } + } + } + """ + use config = new TemporaryFile(fileContent, "json") + + let input = """ + module Program + + type X = List + """ + + let (returnCode, errors) = main [| "lint"; "--lint-config"; config.FileName; input |] + + Assert.AreEqual(-1, returnCode, "Return code of HybridStrict against List should not be zero") + Assert.AreNotEqual(0, errors.Count, "Number of errors for HybridStrict mode against List should not be zero") + Assert.AreEqual("Use postfix syntax for F# type List.", errors.MaximumElement) + From c87fd6a97882b250a015c3a9e381eaf1d66429be Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Fri, 19 Jan 2024 12:06:09 +0800 Subject: [PATCH 3/9] Console.Tests: another HybridStrict acceptance test This time for arrays, not lists. It should already be green. --- tests/FSharpLint.Console.Tests/TestApp.fs | 26 +++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 355a64001..1c4cc04a3 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -169,3 +169,29 @@ type TestConsoleApplication() = Assert.AreNotEqual(0, errors.Count, "Number of errors for HybridStrict mode against List should not be zero") Assert.AreEqual("Use postfix syntax for F# type List.", errors.MaximumElement) + [] + member __.``TypePrefixing rule HybridStrict mode should complain about array``() = + let fileContent = """ + { + "typePrefixing": { + "enabled": true, + "config": { + "mode": "Hybrid" + } + } + } + """ + use config = new TemporaryFile(fileContent, "json") + + let input = """ + module Program + + type X = array + """ + + let (returnCode, errors) = main [| "lint"; "--lint-config"; config.FileName; input |] + + Assert.AreEqual(-1, returnCode, "Return code of HybridStrict against array should not be zero") + Assert.AreNotEqual(0, errors.Count, "Number of errors for HybridStrict mode against array should not be zero") + Assert.AreEqual("Use special postfix syntax for F# type array.", errors.MaximumElement) + From fff4abf8062a9b2d07ed441746938d581619329d Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Fri, 19 Jan 2024 12:12:37 +0800 Subject: [PATCH 4/9] Console.Tests: acceptance test for HybridWeak It should be red at the moment. --- tests/FSharpLint.Console.Tests/TestApp.fs | 27 +++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 1c4cc04a3..0ae6a8ce1 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -195,3 +195,30 @@ type TestConsoleApplication() = Assert.AreNotEqual(0, errors.Count, "Number of errors for HybridStrict mode against array should not be zero") Assert.AreEqual("Use special postfix syntax for F# type array.", errors.MaximumElement) + [] + member __.``TypePrefixing rule HybridWeak mode should not complain about List``() = + let fileContent = """ + { + "typePrefixing": { + "enabled": true, + "config": { + "mode": "Hybrid" + } + } + } + """ + use config = new TemporaryFile(fileContent, "json") + + let input = """ + module Program + + type X = List + """ + + let (returnCode, errors) = main [| "lint"; "--lint-config"; config.FileName; input |] + + Assert.AreEqual(0, returnCode) + if errors.Count = 0 then + Assert.AreEqual(0, errors.Count, "Return code of HybridWeak against List should not be zero (=success; no suggestions)") + else + Assert.AreEqual(0, errors.Count, "No errors should happen, but we got at least one: " + errors.MaximumElement) From 9c78be347487f79f5e74835bdcfeb61cceeefd25 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Fri, 19 Jan 2024 12:28:20 +0800 Subject: [PATCH 5/9] TypePrefixing: implement new Hybrid sub-modes CI should be green. --- src/FSharpLint.Core/Application/Configuration.fs | 2 +- src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs | 10 +++++++--- src/FSharpLint.Core/fsharplint.json | 2 +- tests/FSharpLint.Console.Tests/TestApp.fs | 8 +++++--- .../Rules/Formatting/TypePrefixing.fs | 2 +- 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index f2745b966..4e76bec7e 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -136,7 +136,7 @@ let constructRuleWithConfig rule ruleConfig = let constructTypePrefixingRuleWithConfig rule (ruleConfig: RuleConfig) = if ruleConfig.Enabled then - let config = ruleConfig.Config |> Option.defaultValue { Mode = TypePrefixing.Mode.Hybrid } + let config = ruleConfig.Config |> Option.defaultValue { Mode = TypePrefixing.Mode.HybridWeak } Some(rule config) else None diff --git a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs index f9b15e5f2..43d8a2731 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs @@ -9,9 +9,13 @@ open FSharpLint.Framework.Rules open FSharpLint.Framework.ExpressionUtilities type Mode = - | Hybrid = 0 + | HybridWeak = 0 | Always = 1 | Never = 2 + | HybridStrict = 3 + + /// obsolete: use HybridStrict or HybridWeak instead (default: HybridWeak) + ///| Hybrid = 0 [] type Config = { Mode: Mode } @@ -38,7 +42,7 @@ let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName t | "Ref" as typeName -> // Prefer postfix. - if not isPostfix && config.Mode <> Mode.Always + if not isPostfix && (config.Mode = Mode.HybridStrict || config.Mode = Mode.Never) then let suggestedFix = lazy( (ExpressionUtilities.tryFindTextOfRange range args.FileContent, typeArgs) @@ -53,7 +57,7 @@ let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName t else None - | "array" when config.Mode <> Mode.Always -> + | "array" when config.Mode = Mode.HybridStrict || config.Mode = Mode.Never -> // Prefer special postfix (e.g. int []). let suggestedFix = lazy( (ExpressionUtilities.tryFindTextOfRange range args.FileContent, typeArgs) diff --git a/src/FSharpLint.Core/fsharplint.json b/src/FSharpLint.Core/fsharplint.json index 25a8be5d4..5f8cce7df 100644 --- a/src/FSharpLint.Core/fsharplint.json +++ b/src/FSharpLint.Core/fsharplint.json @@ -14,7 +14,7 @@ "typePrefixing": { "enabled": false, "config": { - "mode": "Hybrid" + "mode": "HybridWeak" } }, "unionDefinitionIndentation": { "enabled": false }, diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 0ae6a8ce1..96621ee31 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -118,7 +118,9 @@ type TestConsoleApplication() = Assert.AreEqual(-1, returnCode) Assert.AreEqual(set ["Use prefix syntax for generic type."], errors) +(* temporarily disabled until PR is finished [] +*) member __.``TypePrefixing rule Hybrid mode should still work (after it gets renamed to HybridWeak)``() = let fileContent = """ { @@ -150,7 +152,7 @@ type TestConsoleApplication() = "typePrefixing": { "enabled": true, "config": { - "mode": "Hybrid" + "mode": "HybridStrict" } } } @@ -176,7 +178,7 @@ type TestConsoleApplication() = "typePrefixing": { "enabled": true, "config": { - "mode": "Hybrid" + "mode": "HybridStrict" } } } @@ -202,7 +204,7 @@ type TestConsoleApplication() = "typePrefixing": { "enabled": true, "config": { - "mode": "Hybrid" + "mode": "HybridWeak" } } } diff --git a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs index 7cf4faf7c..4508b48b4 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs @@ -6,7 +6,7 @@ open FSharpLint.Rules.TypePrefixing [] type TestFormattingHybridTypePrefixing() = - inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule { Config.Mode = Mode.Hybrid }) + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule { Config.Mode = Mode.HybridStrict }) [] member this.``Error for F# List type prefix syntax``() = From 3e0485dea634e3fd8ffe21f6af01c677d54ca3d6 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Fri, 19 Jan 2024 12:48:54 +0800 Subject: [PATCH 6/9] Core.Tests: HybridWeak TypePrefixing unit tests --- .../Rules/Formatting/TypePrefixing.fs | 116 +++++++++++++++++- 1 file changed, 115 insertions(+), 1 deletion(-) diff --git a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs index 4508b48b4..22c8abc10 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Formatting/TypePrefixing.fs @@ -5,7 +5,7 @@ open FSharpLint.Rules open FSharpLint.Rules.TypePrefixing [] -type TestFormattingHybridTypePrefixing() = +type TestFormattingHybridStrictTypePrefixing() = inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule { Config.Mode = Mode.HybridStrict }) [] @@ -238,6 +238,120 @@ type T = Generic this.Parse source Assert.AreEqual(expected, this.ApplyQuickFix source) +[] +type TestFormattingHybridWeakTypePrefixing() = + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule { Config.Mode = Mode.HybridWeak }) + + [] + member this.``Ignore F# List type prefix syntax``() = + this.Parse """ +module Program + +type T = list +""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Ignore for F# List type postfix syntax``() = + this.Parse """ +module Program + +type T = int list +""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Ignore F# Option type prefix syntax``() = + this.Parse """ +module Program + +type T = Option +""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Ignore F# Option type postfix syntax``() = + this.Parse """ +module Program + +type T = int option +""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Ignore F# ref type prefix syntax``() = + this.Parse """ +module Program + +type T = ref +""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Ignore F# ref type postfix syntax``() = + this.Parse """ +module Program + +type T = int ref +""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Ignore F# array type prefix syntax``() = + this.Parse """ +module Program + +type T = array +""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Ignore F# array type standard postfix syntax``() = + this.Parse """ +module Program + +type T = int array +""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Ignore F# array type special postfix syntax``() = + this.Parse """ +module Program + +type T = int [] +""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.``Error for generic type postfix syntax``() = + this.Parse """ +module Program + +type X = int Generic +""" + + Assert.IsTrue(this.ErrorExistsAt(4, 9)) + + [] + member this.``No error for generic type prefix syntax``() = + this.Parse """ +module Program + +type X = Generic +""" + + Assert.IsTrue this.NoErrorsExist + [] type TestFormattingAlwaysTypePrefixing() = inherit TestAstNodeRuleBase.TestAstNodeRuleBase(TypePrefixing.rule { Config.Mode = Mode.Always }) From 1b8bd021880a6cfcdc2411090bb14f91e423a5d1 Mon Sep 17 00:00:00 2001 From: webwarrior Date: Fri, 19 Jan 2024 11:19:12 +0100 Subject: [PATCH 7/9] TypePrefixing: fix for array in HybridWeak mode Fixed TypePrefixing rule for array type in HybridWeak mode. --- src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs index 43d8a2731..c67def591 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs @@ -67,6 +67,9 @@ let checkTypePrefixing (config:Config) (args:AstNodeRuleParams) range typeName t SuggestedFix = Some suggestedFix TypeChecks = [] } |> Some + | "array" when config.Mode = Mode.HybridWeak -> + None + | typeName -> match (isPostfix, config.Mode) with | true, Mode.Never -> From ca09dc6dc3bdfb76b20ec94ff35630def6eebaf2 Mon Sep 17 00:00:00 2001 From: webwarrior Date: Fri, 19 Jan 2024 11:54:33 +0100 Subject: [PATCH 8/9] Tests.Console: re-enable test for Hybrid mode Re-enable test for TypePrefixing rule in deprecated Hybrid mode. --- tests/FSharpLint.Console.Tests/TestApp.fs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 96621ee31..1d10f6d7b 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -118,9 +118,7 @@ type TestConsoleApplication() = Assert.AreEqual(-1, returnCode) Assert.AreEqual(set ["Use prefix syntax for generic type."], errors) -(* temporarily disabled until PR is finished [] -*) member __.``TypePrefixing rule Hybrid mode should still work (after it gets renamed to HybridWeak)``() = let fileContent = """ { From 6f4e51d9353fbc59e08710efc6527f4a2afa63b4 Mon Sep 17 00:00:00 2001 From: webwarrior Date: Fri, 19 Jan 2024 12:41:51 +0100 Subject: [PATCH 9/9] Core: map Hybrid to HybridWeak Map deprecated Hybrid TypePrefixing.Mode in json configs to HybridWeak. --- src/FSharpLint.Core/Application/Configuration.fs | 7 +++++-- src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index 4e76bec7e..ec8befc99 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -136,8 +136,11 @@ let constructRuleWithConfig rule ruleConfig = let constructTypePrefixingRuleWithConfig rule (ruleConfig: RuleConfig) = if ruleConfig.Enabled then - let config = ruleConfig.Config |> Option.defaultValue { Mode = TypePrefixing.Mode.HybridWeak } - Some(rule config) + match ruleConfig.Config with + | Some { Mode = TypePrefixing.Mode.Hybrid } | None -> + Some(rule { TypePrefixing.Config.Mode = TypePrefixing.Mode.HybridWeak }) + | Some config -> + Some(rule config) else None diff --git a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs index c67def591..1fa8b68f0 100644 --- a/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs +++ b/src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs @@ -15,7 +15,7 @@ type Mode = | HybridStrict = 3 /// obsolete: use HybridStrict or HybridWeak instead (default: HybridWeak) - ///| Hybrid = 0 + | Hybrid = 128 [] type Config = { Mode: Mode }