From c006174550eb43230d3c3182664e9e87e50f176b Mon Sep 17 00:00:00 2001 From: dawe Date: Mon, 23 Oct 2023 09:02:22 +0200 Subject: [PATCH 1/5] Add tests for FS3365 --- .../ErrorMessages/IndexingSyntax.fs | 59 +++++++++++++++++++ .../FSharp.Compiler.ComponentTests.fsproj | 1 + 2 files changed, 60 insertions(+) create mode 100644 tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs diff --git a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs new file mode 100644 index 00000000000..87c50590744 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs @@ -0,0 +1,59 @@ +namespace FSharp.Compiler.ComponentTests.ErrorMessages + +open FSharp.Test.Compiler +open FSharp.Test.Compiler.Assertions.StructuredResultsAsserts + +module ``Indexing Syntax`` = + + [] + let ``Warn successfully for SynExpr.Ident app`` () = + """ +namespace N + + module M = + + let f (a: int list) = a + + let g () = f[1] + """ + |> FSharp + |> withLangVersion70 + |> compile + |> shouldFail + |> withResults [ + { Error = Information 3365 + Range = { StartLine = 8 + StartColumn = 20 + EndLine = 8 + EndColumn = 24 } + Message = + "The syntax 'expr1[expr2]' is used for indexing. Consider adding a type annotation to enable indexing, or if calling a function add a space, e.g. 'expr1 [expr2]'." } + ] + + [] + let ``Warn successfully for SynExpr.LongIdent app`` () = + """ +namespace N + + module N = + + type C () = + member _.MyFunc (inputList: int list) = inputList + + let g () = + let c = C() + c.MyFunc[42] + """ + |> FSharp + |> withLangVersion70 + |> compile + |> shouldFail + |> withResults [ + { Error = Information 3365 + Range = { StartLine = 11 + StartColumn = 13 + EndLine = 11 + EndColumn = 25 } + Message = + "The syntax 'expr1[expr2]' is used for indexing. Consider adding a type annotation to enable indexing, or if calling a function add a space, e.g. 'expr1 [expr2]'." } + ] diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index aefeb44cc50..99fbfe55d88 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -167,6 +167,7 @@ + From e71eef1c793f48d7d6671fde33be68f83c90add4 Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 3 Nov 2023 18:08:09 +0100 Subject: [PATCH 2/5] improve tests --- .../ErrorMessages/IndexingSyntax.fs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs index 87c50590744..6864cb21f74 100644 --- a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs +++ b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs @@ -14,7 +14,9 @@ namespace N let f (a: int list) = a - let g () = f[1] + let g () = f [1] // should not warn + + let h () = f[1] // should warn """ |> FSharp |> withLangVersion70 @@ -22,9 +24,9 @@ namespace N |> shouldFail |> withResults [ { Error = Information 3365 - Range = { StartLine = 8 + Range = { StartLine = 10 StartColumn = 20 - EndLine = 8 + EndLine = 10 EndColumn = 24 } Message = "The syntax 'expr1[expr2]' is used for indexing. Consider adding a type annotation to enable indexing, or if calling a function add a space, e.g. 'expr1 [expr2]'." } @@ -42,7 +44,8 @@ namespace N let g () = let c = C() - c.MyFunc[42] + let _ = c.MyFunc [23] // should not warn + c.MyFunc[42] // should warn """ |> FSharp |> withLangVersion70 @@ -50,9 +53,9 @@ namespace N |> shouldFail |> withResults [ { Error = Information 3365 - Range = { StartLine = 11 + Range = { StartLine = 12 StartColumn = 13 - EndLine = 11 + EndLine = 12 EndColumn = 25 } Message = "The syntax 'expr1[expr2]' is used for indexing. Consider adding a type annotation to enable indexing, or if calling a function add a space, e.g. 'expr1 [expr2]'." } From 55fdea29eb325c23e1c375778e9995dab99a8d21 Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 3 Nov 2023 18:09:18 +0100 Subject: [PATCH 3/5] add missing warning for 3365 --- src/Compiler/Checking/CheckExpressions.fs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 527fc3233ae..bb952458d20 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -9366,6 +9366,15 @@ and TcMethodApplicationThen // Nb. args is always of List.length <= 1 except for indexed setters, when it is 2 let mWholeExpr = (m, args) ||> List.fold (fun m arg -> unionRanges m arg.Range) + // c.atomicLeftMethExpr[idx] as application gives a warning + match args, atomicFlag with + | ([SynExpr.ArrayOrList (false, _, _)] | [SynExpr.ArrayOrListComputed (false, _, _)]), ExprAtomicFlag.Atomic -> + if g.langVersion.SupportsFeature LanguageFeature.IndexerNotationWithoutDot then + informationalWarning(Error(FSComp.SR.tcHighPrecedenceFunctionApplicationToListDeprecated(), mWholeExpr)) + elif not (g.langVersion.IsExplicitlySpecifiedAs50OrBefore()) then + informationalWarning(Error(FSComp.SR.tcHighPrecedenceFunctionApplicationToListReserved(), mWholeExpr)) + | _ -> () + // Work out if we know anything about the return type of the overall expression. If there are any delayed // lookups then we don't know anything. let exprTy = if isNil delayed then overallTy else MustEqual (NewInferenceType g) From eb73c06d6a30b838084677db76c91e4e23690d73 Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 3 Nov 2023 18:15:37 +0100 Subject: [PATCH 4/5] format --- .../ErrorMessages/IndexingSyntax.fs | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs index 6864cb21f74..e0704dc52fd 100644 --- a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs +++ b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/IndexingSyntax.fs @@ -22,15 +22,21 @@ namespace N |> withLangVersion70 |> compile |> shouldFail - |> withResults [ - { Error = Information 3365 - Range = { StartLine = 10 - StartColumn = 20 - EndLine = 10 - EndColumn = 24 } - Message = - "The syntax 'expr1[expr2]' is used for indexing. Consider adding a type annotation to enable indexing, or if calling a function add a space, e.g. 'expr1 [expr2]'." } - ] + |> withResults + [ + { + Error = Information 3365 + Range = + { + StartLine = 10 + StartColumn = 20 + EndLine = 10 + EndColumn = 24 + } + Message = + "The syntax 'expr1[expr2]' is used for indexing. Consider adding a type annotation to enable indexing, or if calling a function add a space, e.g. 'expr1 [expr2]'." + } + ] [] let ``Warn successfully for SynExpr.LongIdent app`` () = @@ -51,12 +57,18 @@ namespace N |> withLangVersion70 |> compile |> shouldFail - |> withResults [ - { Error = Information 3365 - Range = { StartLine = 12 - StartColumn = 13 - EndLine = 12 - EndColumn = 25 } - Message = - "The syntax 'expr1[expr2]' is used for indexing. Consider adding a type annotation to enable indexing, or if calling a function add a space, e.g. 'expr1 [expr2]'." } - ] + |> withResults + [ + { + Error = Information 3365 + Range = + { + StartLine = 12 + StartColumn = 13 + EndLine = 12 + EndColumn = 25 + } + Message = + "The syntax 'expr1[expr2]' is used for indexing. Consider adding a type annotation to enable indexing, or if calling a function add a space, e.g. 'expr1 [expr2]'." + } + ] From 9026e316bed8ecc081d5567c1fc87dfbe64dba75 Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 3 Nov 2023 18:57:07 +0100 Subject: [PATCH 5/5] refactor --- src/Compiler/Checking/CheckExpressions.fs | 26 +++++++++++------------ 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index bb952458d20..b2469920e69 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -3950,6 +3950,16 @@ let GetInstanceMemberThisVariable (vspec: Val, expr) = else None +/// c.atomicLeftMethExpr[idx] and atomicLeftExpr[idx] as applications give warnings +let checkHighPrecedenceFunctionApplicationToList (g: TcGlobals) args atomicFlag exprRange = + match args, atomicFlag with + | ([SynExpr.ArrayOrList (false, _, _)] | [SynExpr.ArrayOrListComputed (false, _, _)]), ExprAtomicFlag.Atomic -> + if g.langVersion.SupportsFeature LanguageFeature.IndexerNotationWithoutDot then + informationalWarning(Error(FSComp.SR.tcHighPrecedenceFunctionApplicationToListDeprecated(), exprRange)) + elif not (g.langVersion.IsExplicitlySpecifiedAs50OrBefore()) then + informationalWarning(Error(FSComp.SR.tcHighPrecedenceFunctionApplicationToListReserved(), exprRange)) + | _ -> () + /// Indicates whether a syntactic type is allowed to include new type variables /// not declared anywhere, e.g. `let f (x: 'T option) = x.Value` type ImplicitlyBoundTyparsAllowed = @@ -8202,13 +8212,7 @@ and TcApplicationThen (cenv: cenv) (overallTy: OverallTy) env tpenv mExprAndArg // atomicLeftExpr[idx] unifying as application gives a warning if not isSugar then - match synArg, atomicFlag with - | (SynExpr.ArrayOrList (false, _, _) | SynExpr.ArrayOrListComputed (false, _, _)), ExprAtomicFlag.Atomic -> - if g.langVersion.SupportsFeature LanguageFeature.IndexerNotationWithoutDot then - informationalWarning(Error(FSComp.SR.tcHighPrecedenceFunctionApplicationToListDeprecated(), mExprAndArg)) - elif not (g.langVersion.IsExplicitlySpecifiedAs50OrBefore()) then - informationalWarning(Error(FSComp.SR.tcHighPrecedenceFunctionApplicationToListReserved(), mExprAndArg)) - | _ -> () + checkHighPrecedenceFunctionApplicationToList g [synArg] atomicFlag mExprAndArg match leftExpr with | ApplicableExpr(expr=NameOfExpr g _) when g.langVersion.SupportsFeature LanguageFeature.NameOf -> @@ -9367,13 +9371,7 @@ and TcMethodApplicationThen let mWholeExpr = (m, args) ||> List.fold (fun m arg -> unionRanges m arg.Range) // c.atomicLeftMethExpr[idx] as application gives a warning - match args, atomicFlag with - | ([SynExpr.ArrayOrList (false, _, _)] | [SynExpr.ArrayOrListComputed (false, _, _)]), ExprAtomicFlag.Atomic -> - if g.langVersion.SupportsFeature LanguageFeature.IndexerNotationWithoutDot then - informationalWarning(Error(FSComp.SR.tcHighPrecedenceFunctionApplicationToListDeprecated(), mWholeExpr)) - elif not (g.langVersion.IsExplicitlySpecifiedAs50OrBefore()) then - informationalWarning(Error(FSComp.SR.tcHighPrecedenceFunctionApplicationToListReserved(), mWholeExpr)) - | _ -> () + checkHighPrecedenceFunctionApplicationToList g args atomicFlag mWholeExpr // Work out if we know anything about the return type of the overall expression. If there are any delayed // lookups then we don't know anything.