From 53ff48df3a7930e1cc1bb87c2f9517c0453c32e8 Mon Sep 17 00:00:00 2001 From: ijanus Date: Thu, 26 May 2022 09:38:06 +0100 Subject: [PATCH 1/6] GenericTypesNames: add a false negative test --- .../Conventions/Naming/GenericTypesNames.fs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs index 0df1fb958..1a5a073db 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs @@ -55,5 +55,37 @@ type Foo<'K, 'V> = Option<'K * 'V> member this.``generic type names shouldn't be camelCase (multiple generic types)``() = this.Parse """ type Foo<'T1, 'T2, 'T3, 'T4, 'T5, 'a, 'T6> = Option<'T1 * 'T2 * 'T3 * 'T4 * 'T5 * 'a * 'T6> +""" + Assert.IsTrue(this.ErrorsExist) + + [] + member this.``generic type names shouldn't be camelCase even for types in methods``() = + this.Parse """ +module PeerChannelEncryptorMonad = + type PeerChannelEncryptorComputation<'T> = + | PeerChannelEncryptorComputation of + (PeerChannelEncryptor -> Result<'T * PeerChannelEncryptor, PeerError>) + + let runP pcec initialState = + let (PeerChannelEncryptorComputation innerFn) = pcec + innerFn initialState + + let returnP x = + let innerFn state = + Ok(x, state) + + PeerChannelEncryptorComputation innerFn + + let bindP + (f: 'a -> PeerChannelEncryptorComputation<'b>) + (xT: PeerChannelEncryptorComputation<'a>) + : PeerChannelEncryptorComputation<'b> = + let innerFn state = + runP xT state + >>= fun (res, state2) -> + let h = runP (f res) state2 + h + + PeerChannelEncryptorComputation innerFn """ Assert.IsTrue(this.ErrorsExist) From 81d3f6e57782fc5e7e42b1660a80c794ebc04c07 Mon Sep 17 00:00:00 2001 From: ijanus Date: Thu, 26 May 2022 09:38:36 +0100 Subject: [PATCH 2/6] GenericTypesNames: fix some false negative Fixes https://github.com/fsprojects/FSharpLint/issues/551. --- .../Rules/Conventions/Naming/GenericTypesNames.fs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/FSharpLint.Core/Rules/Conventions/Naming/GenericTypesNames.fs b/src/FSharpLint.Core/Rules/Conventions/Naming/GenericTypesNames.fs index 95cab3b41..1b070055b 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Naming/GenericTypesNames.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Naming/GenericTypesNames.fs @@ -20,6 +20,8 @@ let private getIdentifiers (args: AstNodeRuleParams) = match componentInfo with | SynComponentInfo(_attrs, types, _, _identifier, _, _, _, _) -> checkTypes types |> Array.ofSeq + | AstNode.Type(SynType.Var(SynTypar(id, _, _), _)) when not (isPascalCase id.idText) -> + (id, id.idText, None) |> Array.singleton | _ -> Array.empty let rule config = From 0d95c1e1b6c71203c7df81aabc1d380d6b3b8d11 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Thu, 26 May 2022 12:00:30 +0200 Subject: [PATCH 3/6] tests: rather use ErrorsExistOnLine in GenericTypesNames --- .../Rules/Conventions/Naming/GenericTypesNames.fs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs index 1a5a073db..d4aed4af6 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs @@ -26,7 +26,7 @@ type Foo<'T> = Option<'T> type Foo<'a> = Option<'a> """ Assert.IsTrue(this.ErrorsExist) - Assert.IsTrue(this.ErrorExistsAt(2, 9)) + Assert.IsTrue(this.ErrorExistsOnLine 2) [] member this.``generic type names shouldn't be camelCase (2 generic types)``() = @@ -34,7 +34,7 @@ type Foo<'a> = Option<'a> type Foo<'a, 'T> = Option<'a * 'T> """ Assert.IsTrue(this.ErrorsExist) - Assert.IsTrue(this.ErrorExistsAt(2, 9)) + Assert.IsTrue(this.ErrorExistsOnLine 2) [] member this.``generic type names shouldn't be camelCase (2 generic types with different order)``() = @@ -42,7 +42,7 @@ type Foo<'a, 'T> = Option<'a * 'T> type Foo<'T, 'a> = Option<'T * 'a> """ Assert.IsTrue(this.ErrorsExist) - Assert.IsTrue(this.ErrorExistsAt(2, 13)) + Assert.IsTrue(this.ErrorExistsOnLine 2) [] member this.``generic type names are PascalCase``() = @@ -57,6 +57,7 @@ type Foo<'K, 'V> = Option<'K * 'V> type Foo<'T1, 'T2, 'T3, 'T4, 'T5, 'a, 'T6> = Option<'T1 * 'T2 * 'T3 * 'T4 * 'T5 * 'a * 'T6> """ Assert.IsTrue(this.ErrorsExist) + Assert.IsTrue(this.ErrorExistsOnLine 2) [] member this.``generic type names shouldn't be camelCase even for types in methods``() = @@ -89,3 +90,4 @@ module PeerChannelEncryptorMonad = PeerChannelEncryptorComputation innerFn """ Assert.IsTrue(this.ErrorsExist) + Assert.IsTrue(this.ErrorExistsOnLine 18) From 4251336ac104551759ed0106bcd84055e4ba8234 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Thu, 26 May 2022 10:54:33 +0200 Subject: [PATCH 4/6] Tests: inverse GenericTypeNames tests to demonstrate issue It seems the rule is hardcoding the function isPascalCase, which means that it's not really looking at the configuration of the rule. This theory will be demonstrated if this commit's CI status is red. --- .../Conventions/Naming/GenericTypesNames.fs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs index d4aed4af6..cbf837e3c 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs @@ -5,7 +5,7 @@ open FSharpLint.Framework.Rules open FSharpLint.Rules let config = - { NamingConfig.Naming = Some NamingCase.PascalCase + { NamingConfig.Naming = Some NamingCase.CamelCase Underscores = Some NamingUnderscores.None Prefix = None Suffix = None } @@ -14,22 +14,22 @@ type TestConventionsGenericTypesNames() = inherit TestAstNodeRuleBase.TestAstNodeRuleBase(GenericTypesNames.rule config) [] - member this.GenericTypeNameIsPascalCase() = + member this.GenericTypeNameIsCamelCase() = this.Parse """ -type Foo<'T> = Option<'T> +type Foo<'a> = Option<'a> """ this.AssertNoWarnings() [] - member this.``generic type name shouldn't be camelCase``() = + member this.``generic type name shouldn't be PascalCase``() = this.Parse """ -type Foo<'a> = Option<'a> +type Foo<'T> = Option<'T> """ Assert.IsTrue(this.ErrorsExist) Assert.IsTrue(this.ErrorExistsOnLine 2) [] - member this.``generic type names shouldn't be camelCase (2 generic types)``() = + member this.``generic type names shouldn't be PascalCase (2 generic types)``() = this.Parse """ type Foo<'a, 'T> = Option<'a * 'T> """ @@ -37,7 +37,7 @@ type Foo<'a, 'T> = Option<'a * 'T> Assert.IsTrue(this.ErrorExistsOnLine 2) [] - member this.``generic type names shouldn't be camelCase (2 generic types with different order)``() = + member this.``generic type names shouldn't be PascalCase (2 generic types with different order)``() = this.Parse """ type Foo<'T, 'a> = Option<'T * 'a> """ @@ -45,27 +45,27 @@ type Foo<'T, 'a> = Option<'T * 'a> Assert.IsTrue(this.ErrorExistsOnLine 2) [] - member this.``generic type names are PascalCase``() = + member this.``generic type names are camelCase``() = this.Parse """ -type Foo<'K, 'V> = Option<'K * 'V> +type Foo<'k, 'v> = Option<'k * 'v> """ this.AssertNoWarnings() [] - member this.``generic type names shouldn't be camelCase (multiple generic types)``() = + member this.``generic type names shouldn't be PascalCase (multiple generic types)``() = this.Parse """ -type Foo<'T1, 'T2, 'T3, 'T4, 'T5, 'a, 'T6> = Option<'T1 * 'T2 * 'T3 * 'T4 * 'T5 * 'a * 'T6> +type Foo<'a1, 'a2, 'a3, 'a4, 'a5, 'T, 'a6> = Option<'a1 * 'a2 * 'a3 * 'a4 * 'a5 * 'T * 'a6> """ Assert.IsTrue(this.ErrorsExist) Assert.IsTrue(this.ErrorExistsOnLine 2) [] - member this.``generic type names shouldn't be camelCase even for types in methods``() = + member this.``generic type names shouldn't be PascalCase even for types in methods``() = this.Parse """ module PeerChannelEncryptorMonad = - type PeerChannelEncryptorComputation<'T> = + type PeerChannelEncryptorComputation<'a> = | PeerChannelEncryptorComputation of - (PeerChannelEncryptor -> Result<'T * PeerChannelEncryptor, PeerError>) + (PeerChannelEncryptor -> Result<'a * PeerChannelEncryptor, PeerError>) let runP pcec initialState = let (PeerChannelEncryptorComputation innerFn) = pcec @@ -78,9 +78,9 @@ module PeerChannelEncryptorMonad = PeerChannelEncryptorComputation innerFn let bindP - (f: 'a -> PeerChannelEncryptorComputation<'b>) - (xT: PeerChannelEncryptorComputation<'a>) - : PeerChannelEncryptorComputation<'b> = + (f: 'T1 -> PeerChannelEncryptorComputation<'T2>) + (xT: PeerChannelEncryptorComputation<'T1>) + : PeerChannelEncryptorComputation<'T2> = let innerFn state = runP xT state >>= fun (res, state2) -> From 4543354e54fb4275370378a8f9581a02383765b3 Mon Sep 17 00:00:00 2001 From: ijanus Date: Thu, 26 May 2022 12:15:04 +0200 Subject: [PATCH 5/6] GenericTypesNames: honor config instead of hardcoding --- .../Rules/Conventions/Naming/GenericTypesNames.fs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/FSharpLint.Core/Rules/Conventions/Naming/GenericTypesNames.fs b/src/FSharpLint.Core/Rules/Conventions/Naming/GenericTypesNames.fs index 1b070055b..23d0be791 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Naming/GenericTypesNames.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Naming/GenericTypesNames.fs @@ -10,17 +10,14 @@ let private getIdentifiers (args: AstNodeRuleParams) = | AstNode.TypeDefinition(SynTypeDefn(componentInfo, _typeDef, _, _, _)) -> let checkTypes types = seq { - for SynTyparDecl(_attr, synTypeDecl) in types do - match synTypeDecl with - | SynTypar(id, _, _) when not (isPascalCase id.idText) -> - yield (id, id.idText, None) - | _ -> () + for SynTyparDecl(_attr, SynTypar(id, _, _)) in types do + yield (id, id.idText, None) } match componentInfo with | SynComponentInfo(_attrs, types, _, _identifier, _, _, _, _) -> checkTypes types |> Array.ofSeq - | AstNode.Type(SynType.Var(SynTypar(id, _, _), _)) when not (isPascalCase id.idText) -> + | AstNode.Type(SynType.Var(SynTypar(id, _, _), _)) -> (id, id.idText, None) |> Array.singleton | _ -> Array.empty From 4580673779bf04e46c5603618ef708ddd638cdd0 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Thu, 26 May 2022 12:18:29 +0200 Subject: [PATCH 6/6] Revert "Tests: inverse GenericTypeNames tests to demonstrate issue" This reverts commit 8ed70d280755da4faf9a3ff59a0bc14736b7d451. --- .../Conventions/Naming/GenericTypesNames.fs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs index cbf837e3c..d4aed4af6 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/GenericTypesNames.fs @@ -5,7 +5,7 @@ open FSharpLint.Framework.Rules open FSharpLint.Rules let config = - { NamingConfig.Naming = Some NamingCase.CamelCase + { NamingConfig.Naming = Some NamingCase.PascalCase Underscores = Some NamingUnderscores.None Prefix = None Suffix = None } @@ -14,22 +14,22 @@ type TestConventionsGenericTypesNames() = inherit TestAstNodeRuleBase.TestAstNodeRuleBase(GenericTypesNames.rule config) [] - member this.GenericTypeNameIsCamelCase() = + member this.GenericTypeNameIsPascalCase() = this.Parse """ -type Foo<'a> = Option<'a> +type Foo<'T> = Option<'T> """ this.AssertNoWarnings() [] - member this.``generic type name shouldn't be PascalCase``() = + member this.``generic type name shouldn't be camelCase``() = this.Parse """ -type Foo<'T> = Option<'T> +type Foo<'a> = Option<'a> """ Assert.IsTrue(this.ErrorsExist) Assert.IsTrue(this.ErrorExistsOnLine 2) [] - member this.``generic type names shouldn't be PascalCase (2 generic types)``() = + member this.``generic type names shouldn't be camelCase (2 generic types)``() = this.Parse """ type Foo<'a, 'T> = Option<'a * 'T> """ @@ -37,7 +37,7 @@ type Foo<'a, 'T> = Option<'a * 'T> Assert.IsTrue(this.ErrorExistsOnLine 2) [] - member this.``generic type names shouldn't be PascalCase (2 generic types with different order)``() = + member this.``generic type names shouldn't be camelCase (2 generic types with different order)``() = this.Parse """ type Foo<'T, 'a> = Option<'T * 'a> """ @@ -45,27 +45,27 @@ type Foo<'T, 'a> = Option<'T * 'a> Assert.IsTrue(this.ErrorExistsOnLine 2) [] - member this.``generic type names are camelCase``() = + member this.``generic type names are PascalCase``() = this.Parse """ -type Foo<'k, 'v> = Option<'k * 'v> +type Foo<'K, 'V> = Option<'K * 'V> """ this.AssertNoWarnings() [] - member this.``generic type names shouldn't be PascalCase (multiple generic types)``() = + member this.``generic type names shouldn't be camelCase (multiple generic types)``() = this.Parse """ -type Foo<'a1, 'a2, 'a3, 'a4, 'a5, 'T, 'a6> = Option<'a1 * 'a2 * 'a3 * 'a4 * 'a5 * 'T * 'a6> +type Foo<'T1, 'T2, 'T3, 'T4, 'T5, 'a, 'T6> = Option<'T1 * 'T2 * 'T3 * 'T4 * 'T5 * 'a * 'T6> """ Assert.IsTrue(this.ErrorsExist) Assert.IsTrue(this.ErrorExistsOnLine 2) [] - member this.``generic type names shouldn't be PascalCase even for types in methods``() = + member this.``generic type names shouldn't be camelCase even for types in methods``() = this.Parse """ module PeerChannelEncryptorMonad = - type PeerChannelEncryptorComputation<'a> = + type PeerChannelEncryptorComputation<'T> = | PeerChannelEncryptorComputation of - (PeerChannelEncryptor -> Result<'a * PeerChannelEncryptor, PeerError>) + (PeerChannelEncryptor -> Result<'T * PeerChannelEncryptor, PeerError>) let runP pcec initialState = let (PeerChannelEncryptorComputation innerFn) = pcec @@ -78,9 +78,9 @@ module PeerChannelEncryptorMonad = PeerChannelEncryptorComputation innerFn let bindP - (f: 'T1 -> PeerChannelEncryptorComputation<'T2>) - (xT: PeerChannelEncryptorComputation<'T1>) - : PeerChannelEncryptorComputation<'T2> = + (f: 'a -> PeerChannelEncryptorComputation<'b>) + (xT: PeerChannelEncryptorComputation<'a>) + : PeerChannelEncryptorComputation<'b> = let innerFn state = runP xT state >>= fun (res, state2) ->