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

Warn on uppercase identifiers in patterns. #15816

Open
wants to merge 68 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
60127ac
Better error for match with a patterns referring to an out of scope i…
edgarfgp Aug 16, 2023
b852634
Lets find out
edgarfgp Aug 16, 2023
f599bca
Ok now I wish I did not know about this
edgarfgp Aug 16, 2023
27f2a3c
update test
edgarfgp Aug 16, 2023
c8294ec
last one ?
edgarfgp Aug 16, 2023
5d37156
Update error message to clarify the details of the historical hack.
edgarfgp Aug 18, 2023
bef94a2
Update more tests
edgarfgp Aug 18, 2023
58834dc
update tests
edgarfgp Aug 18, 2023
48fb2ff
more tests
edgarfgp Aug 19, 2023
b927f77
Merge branch 'main' into another-pat-matching-fix
edgarfgp Aug 21, 2023
050c8c5
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 17, 2024
fea45d5
comment id.idText.Length >= 3 to see what breaks
edgarfgp Oct 17, 2024
47c287e
revert back the error message
edgarfgp Oct 17, 2024
885b5ab
update tests
edgarfgp Oct 17, 2024
bd76c9d
release notes
edgarfgp Oct 17, 2024
d68843e
Add LanguageFeature flag
edgarfgp Oct 17, 2024
110d417
release notes
edgarfgp Oct 17, 2024
869acd5
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 17, 2024
6d4af0a
Merge branch 'another-pat-matching-fix' of github.com:edgarfgp/fsharp…
edgarfgp Oct 17, 2024
7bf5163
format code
edgarfgp Oct 17, 2024
7934bf3
reduce diff
edgarfgp Oct 17, 2024
1554db9
LanguageFeature.WarnOnUppercaseIdentifiersInPatterns
edgarfgp Oct 17, 2024
5a3cf90
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 21, 2024
a3f4ae1
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 23, 2024
8f4643a
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 23, 2024
88f13a9
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 24, 2024
b01b957
simplify tests
edgarfgp Oct 25, 2024
0183998
more tests
edgarfgp Oct 25, 2024
3d4cbb3
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 25, 2024
5236633
3874,chkVariablePatternUppercase,"Variable patterns should be lowerca…
edgarfgp Oct 25, 2024
23b3a53
Merge branch 'another-pat-matching-fix' of github.com:edgarfgp/fsharp…
edgarfgp Oct 25, 2024
68039fa
hasConstructorShape
edgarfgp Oct 25, 2024
3be7320
Improve error message for match cases labels and update tests
edgarfgp Oct 27, 2024
2f5d878
update fsharpqa tests
edgarfgp Oct 27, 2024
bf5ca00
more tests
edgarfgp Oct 27, 2024
2865c6a
release notes entry
edgarfgp Oct 27, 2024
0c50d4c
reduce diff
edgarfgp Oct 27, 2024
8f69e67
update tests
edgarfgp Oct 27, 2024
d8b2f50
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 27, 2024
2f360bf
update tests
edgarfgp Oct 27, 2024
1d3402a
Merge branch 'another-pat-matching-fix' of github.com:edgarfgp/fsharp…
edgarfgp Oct 27, 2024
724dc0b
more tests
edgarfgp Oct 27, 2024
d4720cb
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 29, 2024
573a1a0
Do't want for WarnOnUpperVariablePatterns in preview
edgarfgp Oct 29, 2024
cd0521c
revert changes from MutableTuple
edgarfgp Oct 29, 2024
dd8a79c
revert changes from Query.fs
edgarfgp Oct 29, 2024
792249a
Revert remaining changes
edgarfgp Oct 29, 2024
34b42f9
format code
edgarfgp Oct 29, 2024
b8682b6
use TcTrueMatchClause instead of bool
edgarfgp Oct 29, 2024
cb71b1c
revert test changes
edgarfgp Oct 29, 2024
a294904
reverte changes on Query.fs
edgarfgp Oct 29, 2024
1cb2c74
revert changes
edgarfgp Oct 29, 2024
5597e60
allow all identifier pattern regardless its length in preview
edgarfgp Oct 30, 2024
21bd69c
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 30, 2024
2330290
Preview: Don't warn on as named pattern
edgarfgp Oct 30, 2024
7a44ead
Merge branch 'another-pat-matching-fix' of github.com:edgarfgp/fsharp…
edgarfgp Oct 30, 2024
490b665
revert error message changes
edgarfgp Oct 31, 2024
e126d0a
Revert "update tests"
edgarfgp Oct 31, 2024
1ec5c87
Revert "update fsharpqa tests"
edgarfgp Oct 31, 2024
f34f9a3
Rename LanguageFeature
edgarfgp Oct 31, 2024
8244124
format code
edgarfgp Oct 31, 2024
63c4c5d
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 31, 2024
fbcf91f
Merge branch 'main' into another-pat-matching-fix
edgarfgp Oct 31, 2024
698e3f2
Update src/Compiler/Checking/CheckPatterns.fs
edgarfgp Nov 2, 2024
3adf463
PR review
edgarfgp Nov 3, 2024
1379605
Add xml comments
edgarfgp Nov 4, 2024
7a7ac5d
Merge branch 'main' into another-pat-matching-fix
edgarfgp Nov 4, 2024
3484b51
Merge branch 'main' into another-pat-matching-fix
edgarfgp Nov 5, 2024
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
1 change: 0 additions & 1 deletion src/Compiler/Checking/NameResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3239,7 +3239,6 @@ let rec ResolvePatternLongIdentPrim sink (ncenv: NameResolver) fullyQualified wa
if
not newDef
&& warnOnUpper = WarnOnUpperCase
&& id.idText.Length >= 3
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
&& System.Char.ToLowerInvariant id.idText[0] <> id.idText[0]
then
warning(UpperCaseIdentifierInPattern m)
Expand Down
44 changes: 22 additions & 22 deletions src/FSharp.Core/Query.fs
Original file line number Diff line number Diff line change
Expand Up @@ -452,14 +452,14 @@ module Query =
let FuncExprToLinqFunc2 (srcTy, targetTy, v, body) =
FuncExprToDelegateExpr(srcTy, targetTy, v, body) |> LeafExpressionConverter.EvaluateQuotation

let MakersCallers F = CallGenericStaticMethod F, MakeGenericStaticMethod F
let MakersCallers f = CallGenericStaticMethod f, MakeGenericStaticMethod f

let MakersCallersInstance F = CallGenericInstanceMethod F, MakeGenericInstanceMethod F
let MakersCallersInstance f = CallGenericInstanceMethod f, MakeGenericInstanceMethod f

let MakersCallers2 FQ FE = MakersCallers FQ, MakersCallers FE
let MakersCallers2 fQ fE = MakersCallers fQ, MakersCallers fE

let MakeOrCallContainsOrElementAt FQ FE =
let (CQ, MQ), (CE, ME) = MakersCallers2 FQ FE
let MakeOrCallContainsOrElementAt fQ fE =
let (CQ, MQ), (CE, ME) = MakersCallers2 fQ fE
let Make (isIQ, srcItemTy: Type, src: Expr, key: Expr) =
if isIQ then
//let key = MakeImplicitExpressionConversion key
Expand All @@ -483,8 +483,8 @@ module Query =
let FE = methodhandleof (fun (x, y) -> Enumerable.ElementAt(x, y))
MakeOrCallContainsOrElementAt FQ FE

let MakeOrCallMinByOrMaxBy FQ FE =
let (CQ, MQ), (CE, ME) = MakersCallers2 FQ FE
let MakeOrCallMinByOrMaxBy fQ fE =
let (CQ, MQ), (CE, ME) = MakersCallers2 fQ fE
let Make (isIQ, src: Expr, v: Var, valSelector: Expr) =
let srcItemTy = v.Type
let keyElemTy = valSelector.Type
Expand Down Expand Up @@ -527,8 +527,8 @@ module Query =
let FE = methodhandleof (fun (x, y: Func<_, 'Result>) -> Enumerable.Max(x, y))
MakeOrCallMinByOrMaxBy FQ FE

let MakeOrCallAnyOrAllOrFirstFind FQ FE =
let (CQ, MQ), (CE, ME) = MakersCallers2 FQ FE
let MakeOrCallAnyOrAllOrFirstFind fQ fE =
let (CQ, MQ), (CE, ME) = MakersCallers2 fQ fE
let Make (isIQ, src: Expr, v: Var, predicate: Expr) =
let srcItemTy= v.Type
let predicate = FuncExprToDelegateExpr (srcItemTy, boolTy, v, predicate)
Expand Down Expand Up @@ -563,14 +563,14 @@ module Query =
let FE = methodhandleof (fun (x, y: Func<_, _>) -> Enumerable.First(x, y))
MakeOrCallAnyOrAllOrFirstFind FQ FE

let MakeOrCallAverageByOrSumByGeneric (isNullable, fq_double, fq_single, fq_decimal, fq_int32, fq_int64, fe_double, fe_single, fe_decimal, fe_int32, fe_int64, FE) =
let MakeOrCallAverageByOrSumByGeneric (isNullable, fq_double, fq_single, fq_decimal, fq_int32, fq_int64, fe_double, fe_single, fe_decimal, fe_int32, fe_int64, fE) =
let (cq_double, mq_double), (ce_double, me_double) = MakersCallers2 fq_double fe_double
let (cq_single, mq_single), (ce_single, me_single) = MakersCallers2 fq_single fe_single
let (cq_decimal, mq_decimal), (ce_decimal, me_decimal) = MakersCallers2 fq_decimal fe_decimal
let (cq_int32, mq_int32), (ce_int32, me_int32) = MakersCallers2 fq_int32 fe_int32
let (cq_int64, mq_int64), (ce_int64, me_int64) = MakersCallers2 fq_int64 fe_int64
// The F# implementation is an instance method on QueryBuilder
let (CE, ME) = MakersCallersInstance FE
let (CE, ME) = MakersCallersInstance fE
let failDueToUnsupportedInputTypeInSumByOrAverageBy() = invalidOp (SR.GetString(SR.failDueToUnsupportedInputTypeInSumByOrAverageBy))

let Make (qb: Expr, isIQ, src: Expr, v: Var, res: Expr) =
Expand Down Expand Up @@ -711,8 +711,8 @@ module Query =
let FE = methodhandleof (fun (query:QueryBuilder, arg1: QuerySource<_, _>, arg2:_->Nullable<double>) -> query.SumByNullable(arg1, arg2))
MakeOrCallAverageByOrSumByGeneric (true, FQ_double, FQ_single, FQ_decimal, FQ_int32, FQ_int64, FE_double, FE_single, FE_decimal, FE_int32, FE_int64, FE)

let MakeOrCallSimpleOp FQ FE =
let (CQ, MQ), (CE, ME) = MakersCallers2 FQ FE
let MakeOrCallSimpleOp fQ fE =
let (CQ, MQ), (CE, ME) = MakersCallers2 fQ fE
let Make (isIQ, srcItemTy, src: Expr) =
if isIQ then
MQ ([srcItemTy], [src])
Expand Down Expand Up @@ -816,16 +816,16 @@ module Query =
else
FE ([v.Type], [src; selector])

let MakeOrderByOrThenBy FQ FE =
let MakeOrderByOrThenBy fQ fE =
fun (isIQ, src: Expr, v: Var, keySelector: Expr) ->
let srcItemTy = v.Type
let keyItemTy = keySelector.Type
let selector = Expr.NewDelegate (MakeQueryFuncTy(srcItemTy, keyItemTy), [v], keySelector)
if isIQ then
let selector = MakeImplicitExpressionConversion selector
FQ ([srcItemTy; keyItemTy], [src; selector])
fQ ([srcItemTy; keyItemTy], [src; selector])
else
FE ([srcItemTy; keyItemTy], [src; selector])
fE ([srcItemTy; keyItemTy], [src; selector])

let MakeOrderBy =
let FQ = MakeGenericStaticMethod (methodhandleof (fun (x, y: Expression<Func<_, _>>) -> System.Linq.Queryable.OrderBy(x, y)))
Expand Down Expand Up @@ -856,9 +856,9 @@ module Query =

let MakeThenByNullableDescending = MakeThenByDescending

let GenMakeSkipWhileOrTakeWhile FQ FE =
let FQ = MakeGenericStaticMethod FQ
let FE = MakeGenericStaticMethod FE
let GenMakeSkipWhileOrTakeWhile fQ fE =
let FQ = MakeGenericStaticMethod fQ
let FE = MakeGenericStaticMethod fE
fun (isIQ, src: Expr, v: Var, predicate) ->
let srcItemTy = v.Type
let selector = Expr.NewDelegate (MakeQueryFuncTy(srcItemTy, boolTy), [v], predicate)
Expand All @@ -868,9 +868,9 @@ module Query =
else
FE ([srcItemTy], [src; selector])

let MakeSkipOrTake FQ FE =
let FQ = MakeGenericStaticMethod FQ
let FE = MakeGenericStaticMethod FE
let MakeSkipOrTake fQ fE =
let FQ = MakeGenericStaticMethod fQ
let FE = MakeGenericStaticMethod fE
fun (isIQ, srcItemTy, src: Expr, count) ->
if isIQ then
FQ ([srcItemTy], [src; count])
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
module DUs =
type Countries =
| CA
| US
| GB
let x = DUs.CA

open DUs

let output =
match x with
| US -> "US"
| CA -> "CA"
| U -> "U"
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module DUs =
type Countries =
| CA
| US
| GB
let x = DUs.CA

let output =
match x with
| US -> "US"
| CA -> "CA"
| U -> "U"
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module DUs =
type Countries =
| CA
| US
| GB
let x = DUs.CA

let output =
match x with
| uSA -> "US"
| AAA -> "CA"
| uAA -> "U"
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module DUs =
type Countries =
| CA
| US
| GB
let x = DUs.CA

let output =
match x with
| US -> "US"
| CA -> "CA"
| _ -> "U"
Original file line number Diff line number Diff line change
Expand Up @@ -209,4 +209,57 @@ but here has type
|> shouldFail
|> withDiagnostics [
(Warning 26, Line 8, Col 7, Line 8, Col 55, "This rule will never be matched")
]
]

[<Theory; Directory(__SOURCE_DIRECTORY__, Includes = [|"E_UnionPattern12.fs"|])>]
let ``Union - E_UnionPattern12_fs - --test:ErrorRanges`` compilation =
compilation
|> asFs
|> withOptions ["--test:ErrorRanges"]
|> typecheck
|> shouldFail
|> withDiagnostics [
(Warning 49, Line 14, Col 7, Line 14, Col 8, "Uppercase variable identifiers should not generally be used in patterns, and may indicate a missing open declaration or a misspelt pattern name.")
]

[<Theory; Directory(__SOURCE_DIRECTORY__, Includes = [|"E_UnionPattern13.fs"|])>]
let ``Union - E_UnionPattern13_fs - --test:ErrorRanges`` compilation =
compilation
|> asFs
|> withOptions ["--test:ErrorRanges"]
|> typecheck
|> shouldFail
|> withDiagnostics [
(Warning 49, Line 10, Col 7, Line 10, Col 9, "Uppercase variable identifiers should not generally be used in patterns, and may indicate a missing open declaration or a misspelt pattern name.")
(Warning 49, Line 11, Col 7, Line 11, Col 9, "Uppercase variable identifiers should not generally be used in patterns, and may indicate a missing open declaration or a misspelt pattern name.")
(Warning 49, Line 12, Col 7, Line 12, Col 8, "Uppercase variable identifiers should not generally be used in patterns, and may indicate a missing open declaration or a misspelt pattern name.")
(Warning 26, Line 11, Col 7, Line 11, Col 9, "This rule will never be matched")
(Warning 26, Line 12, Col 7, Line 12, Col 8, "This rule will never be matched")
]

[<Theory; Directory(__SOURCE_DIRECTORY__, Includes = [|"E_UnionPattern14.fs"|])>]
let ``Union - E_UnionPattern14_fs - --test:ErrorRanges`` compilation =
compilation
|> asFs
|> withOptions ["--test:ErrorRanges"]
|> typecheck
|> shouldFail
|> withDiagnostics [
(Warning 49, Line 11, Col 7, Line 11, Col 10, "Uppercase variable identifiers should not generally be used in patterns, and may indicate a missing open declaration or a misspelt pattern name.")
(Warning 26, Line 11, Col 7, Line 11, Col 10, "This rule will never be matched")
(Warning 26, Line 12, Col 7, Line 12, Col 10, "This rule will never be matched")
]

[<Theory; Directory(__SOURCE_DIRECTORY__, Includes = [|"E_UnionPattern15.fs"|])>]
let ``Union - E_UnionPattern15_fs - --test:ErrorRanges`` compilation =
compilation
|> asFs
|> withOptions ["--test:ErrorRanges"]
|> typecheck
|> shouldFail
|> withDiagnostics [
(Warning 49, Line 10, Col 7, Line 10, Col 9, "Uppercase variable identifiers should not generally be used in patterns, and may indicate a missing open declaration or a misspelt pattern name.")
(Warning 49, Line 11, Col 7, Line 11, Col 9, "Uppercase variable identifiers should not generally be used in patterns, and may indicate a missing open declaration or a misspelt pattern name.")
(Warning 26, Line 11, Col 7, Line 11, Col 9, "This rule will never be matched")
(Warning 26, Line 12, Col 7, Line 12, Col 8, "This rule will never be matched")
]
4 changes: 2 additions & 2 deletions tests/fsharp/core/auto-widen/5.0/test.fsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ module ConvertToSealedViaOpImplicit =
[<Sealed>]
type C() =
static member op_Implicit(x:int) = C()
static member M1(C:C) = 1
static member M1(c:C) = 1
let x = C.M1(2)

module ConvertNoOverloadin =
Expand Down Expand Up @@ -161,7 +161,7 @@ module ConvertViaOpImplicit3 =
module ConvertViaOpImplicit4 =
type C() =
static member op_Implicit(x:int) = C()
static member M1(C:C) = 1
static member M1(c:C) = 1
let x = C.M1(2)

module ConvertViaOpImplicit5 =
Expand Down
10 changes: 5 additions & 5 deletions tests/fsharp/core/auto-widen/preview-default-warns/test.fsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ module ConvertToSealedViaOpImplicit =
[<Sealed>]
type C() =
static member op_Implicit(x:int) = C()
static member M1(C:C) = 1
static member M1(c:C) = 1
let x = C.M1(2)

module ConvertNoOverloadin =
Expand Down Expand Up @@ -161,7 +161,7 @@ module ConvertViaOpImplicit3 =
module ConvertViaOpImplicit4 =
type C() =
static member op_Implicit(x:int) = C()
static member M1(C:C) = 1
static member M1(c:C) = 1
let x = C.M1(2)

module ConvertViaOpImplicit5 =
Expand Down Expand Up @@ -459,15 +459,15 @@ module TestAcessibilityOfOpImplicit =
[<Sealed>]
type C() =
static member private op_Implicit(x:int) = C()
static member M1(C:C) = 1
static member M1(c:C) = 1
let x = C.M1(2)

module TestObsoleteOfOpImplicit =
[<Sealed>]
type C() =
[<System.Obsolete("nope")>]
static member op_Implicit(x:int) = C()
static member M1(C:C) = 1
static member M1(c:C) = 1
let x = C.M1(2)

module TestAmbiguousOpImplicit =
Expand All @@ -477,7 +477,7 @@ module TestAmbiguousOpImplicit =

and [<Sealed>] C(x:int) =
static member op_Implicit(x:B) = C(1)
static member M1(C:C) = 1
static member M1(c:C) = 1
member _.Value = x
let x = C.M1(B())

Expand Down
10 changes: 5 additions & 5 deletions tests/fsharp/core/auto-widen/preview/test.fsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ module ConvertToSealedViaOpImplicit =
[<Sealed>]
type C() =
static member op_Implicit(x:int) = C()
static member M1(C:C) = 1
static member M1(c:C) = 1
let x = C.M1(2)

module ConvertNoOverloadin =
Expand Down Expand Up @@ -161,7 +161,7 @@ module ConvertViaOpImplicit3 =
module ConvertViaOpImplicit4 =
type C() =
static member op_Implicit(x:int) = C()
static member M1(C:C) = 1
static member M1(c:C) = 1
let x = C.M1(2)

module ConvertViaOpImplicit5 =
Expand Down Expand Up @@ -459,15 +459,15 @@ module TestAcessibilityOfOpImplicit =
[<Sealed>]
type C() =
static member private op_Implicit(x:int) = C()
static member M1(C:C) = 1
static member M1(c:C) = 1
let x = C.M1(2)

module TestObsoleteOfOpImplicit =
[<Sealed>]
type C() =
[<System.Obsolete("nope")>]
static member op_Implicit(x:int) = C()
static member M1(C:C) = 1
static member M1(c:C) = 1
let x = C.M1(2)

module TestAmbiguousOpImplicit =
Expand All @@ -477,7 +477,7 @@ module TestAmbiguousOpImplicit =

and [<Sealed>] C(x:int) =
static member op_Implicit(x:B) = C(1)
static member M1(C:C) = 1
static member M1(c:C) = 1
member _.Value = x
let x = C.M1(B())

Expand Down
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
Binary file not shown.
Loading