From 500a4fd90084bf1886fdd75863268b532a6907e1 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Tue, 1 Nov 2022 16:51:16 +0100 Subject: [PATCH 1/2] Use more accurate range to only report obsolete field as a obsolete rather that making the whole record construction expression obsolete --- src/Compiler/Checking/CheckExpressions.fs | 8 ++++---- .../Language/ObsoleteAttributeCheckingTests.fs | 16 +++++++++------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 02cdd360e86..e84379fe665 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -1822,19 +1822,19 @@ let BuildFieldMap (cenv: cenv) env isPartial ty (flds: ((Ident list * Ident) * ' rfinfo1.TypeInst, rfinfo1.TyconRef let fldsmap, rfldsList = - ((Map.empty, []), fldResolutions) ||> List.fold (fun (fs, rfldsList) (fld, frefs, fldExpr) -> + ((Map.empty, []), fldResolutions) ||> List.fold (fun (fs, rfldsList) ((_, ident), frefs, fldExpr) -> match frefs |> List.filter (fun (FieldResolution(rfinfo2, _)) -> tyconRefEq g tcref rfinfo2.TyconRef) with | [FieldResolution(rfinfo2, showDeprecated)] -> // Record the precise resolution of the field for intellisense let item = Item.RecdField(rfinfo2) - CallNameResolutionSink cenv.tcSink ((snd fld).idRange, env.NameEnv, item, emptyTyparInst, ItemOccurence.Use, ad) + CallNameResolutionSink cenv.tcSink (ident.idRange, env.NameEnv, item, emptyTyparInst, ItemOccurence.Use, ad) let fref2 = rfinfo2.RecdFieldRef - CheckRecdFieldAccessible cenv.amap m env.eAccessRights fref2 |> ignore + CheckRecdFieldAccessible cenv.amap ident.idRange env.eAccessRights fref2 |> ignore - CheckFSharpAttributes g fref2.PropertyAttribs m |> CommitOperationResult + CheckFSharpAttributes g fref2.PropertyAttribs ident.idRange |> CommitOperationResult if Map.containsKey fref2.FieldName fs then errorR (Error(FSComp.SR.tcFieldAppearsTwiceInRecord(fref2.FieldName), m)) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/ObsoleteAttributeCheckingTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/ObsoleteAttributeCheckingTests.fs index 20e785370a3..8ccbe23126c 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/ObsoleteAttributeCheckingTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/ObsoleteAttributeCheckingTests.fs @@ -165,7 +165,7 @@ let c = { X = 0 } |> compile |> shouldFail |> withDiagnostics [ - (Error 101, Line 7, Col 9, Line 7, Col 18, "This construct is deprecated. Use B instead") + (Error 101, Line 7, Col 11, Line 7, Col 12, "This construct is deprecated. Use B instead") ] [] @@ -373,7 +373,7 @@ type ButtonExtensions = |> compile |> shouldFail |> withDiagnostics [ - (Error 101, Line 13, Col 9, Line 13, Col 34, "This construct is deprecated. Use B instead") + (Error 101, Line 13, Col 21, Line 13, Col 25, "This construct is deprecated. Use B instead") ] [] @@ -397,7 +397,7 @@ type ButtonExtensions = |> shouldFail |> withDiagnostics [ (Error 101, Line 12, Col 37, Line 12, Col 43, "This construct is deprecated. Use B instead"); - (Error 101, Line 13, Col 9, Line 13, Col 34, "This construct is deprecated. Use B instead") + (Error 101, Line 13, Col 21, Line 13, Col 25, "This construct is deprecated. Use B instead") ] [] @@ -416,7 +416,7 @@ module Button = |> compile |> shouldFail |> withDiagnostics [ - (Error 101, Line 9, Col 20, Line 9, Col 36, "This construct is deprecated. Use B instead") + (Error 101, Line 9, Col 22, Line 9, Col 26, "This construct is deprecated. Use B instead") ] @@ -437,7 +437,7 @@ module Button = |> compile |> shouldFail |> withDiagnostics [ - (Error 101, Line 10, Col 20, Line 10, Col 36, "This construct is deprecated. Use B instead") + (Error 101, Line 10, Col 22, Line 10, Col 26, "This construct is deprecated. Use B instead") ] [] @@ -462,7 +462,7 @@ type ButtonExtensions = |> compile |> shouldFail |> withDiagnostics [ - (Error 101, Line 9, Col 20, Line 9, Col 36, "This construct is deprecated. Use B instead") + (Error 101, Line 9, Col 22, Line 9, Col 26, "This construct is deprecated. Use B instead") ] [] @@ -607,9 +607,10 @@ let a = { DeprecatedField= "23" ; JustField = "" } |> compile |> shouldFail |> withDiagnostics [ - (Error 101, Line 4, Col 9, Line 4, Col 51, "This construct is deprecated. Deprecated Field") + (Error 101, Line 4, Col 11, Line 4, Col 26, "This construct is deprecated. Deprecated Field") ] + // This should only report one warning but instead show two. Related issue https://github.com/dotnet/fsharp/issues/14203 [] let ``Obsolete attribute warning is taken into account when used in one the record properties`` () = Fsx """ @@ -620,6 +621,7 @@ let a = { DeprecatedField= "23" ; JustField = "" } |> compile |> shouldFail |> withDiagnostics [ + (Warning 44, Line 4, Col 11, Line 4, Col 26, "This construct is deprecated. Deprecated Field") (Warning 44, Line 4, Col 9, Line 4, Col 51, "This construct is deprecated. Deprecated Field") ] From 5927bb598a506c822f93326a513467477d495c0e Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Tue, 1 Nov 2022 21:36:00 +0100 Subject: [PATCH 2/2] Use record expression range for CheckRecdFieldAccessible --- src/Compiler/Checking/CheckExpressions.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index e84379fe665..7d9cff68b37 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -1832,7 +1832,7 @@ let BuildFieldMap (cenv: cenv) env isPartial ty (flds: ((Ident list * Ident) * ' let fref2 = rfinfo2.RecdFieldRef - CheckRecdFieldAccessible cenv.amap ident.idRange env.eAccessRights fref2 |> ignore + CheckRecdFieldAccessible cenv.amap m env.eAccessRights fref2 |> ignore CheckFSharpAttributes g fref2.PropertyAttribs ident.idRange |> CommitOperationResult