From 10e8f3403d6ef2328b59d4491462f736fbcbfb43 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Fri, 17 May 2024 23:08:09 +0100 Subject: [PATCH 1/5] Enforce `AttributeTargets.Interface` --- src/Compiler/Checking/CheckDeclarations.fs | 5 +- .../AttributeTargetsIsInterface.fs | 17 ++++++ .../AttributeUsage/AttributeUsage.fs | 57 ++++++++++++++++++- .../E_AttributeTargetIsClass01.fs | 11 ++++ .../E_AttributeTargetIsInterface.fs | 22 +++++++ 5 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeTargetsIsInterface.fs create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_AttributeTargetIsClass01.fs create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_AttributeTargetIsInterface.fs diff --git a/src/Compiler/Checking/CheckDeclarations.fs b/src/Compiler/Checking/CheckDeclarations.fs index 1736132abe8..be083e74871 100644 --- a/src/Compiler/Checking/CheckDeclarations.fs +++ b/src/Compiler/Checking/CheckDeclarations.fs @@ -2912,7 +2912,10 @@ module EstablishTypeDefinitionCores = if g.langVersion.SupportsFeature(LanguageFeature.EnforceAttributeTargets) then TcAttributesWithPossibleTargets false cenv envinner AttributeTargets.Class synAttrs |> ignore TFSharpClass - | SynTypeDefnKind.Interface -> TFSharpInterface + | SynTypeDefnKind.Interface -> + if g.langVersion.SupportsFeature(LanguageFeature.EnforceAttributeTargets) then + TcAttributesWithPossibleTargets false cenv envinner AttributeTargets.Interface synAttrs |> ignore + TFSharpInterface | SynTypeDefnKind.Delegate _ -> if g.langVersion.SupportsFeature(LanguageFeature.EnforceAttributeTargets) then TcAttributesWithPossibleTargets false cenv envinner AttributeTargets.Delegate synAttrs |> ignore diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeTargetsIsInterface.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeTargetsIsInterface.fs new file mode 100644 index 00000000000..028bd8979f4 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeTargetsIsInterface.fs @@ -0,0 +1,17 @@ +open System + +[] +type CustomInterfaceAttribute() = + inherit Attribute() + +[] +type IFoo = interface end + +[] +type IFoo2 = + abstract A :int + +[] +[] +type IFoo3 = + abstract A :int diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs index d9ff7e9597c..1e68d9d5ee8 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs @@ -351,6 +351,13 @@ module CustomAttributes_AttributeUsage = |> verifyCompile |> shouldSucceed + // SOURCE=E_AttributeTargetIsClass01.fs # E_AttributeTargetIsClass01.fs + [] + let ``E_AttributeTargetIsClass01_fs`` compilation = + compilation + |> verifyCompile + |> shouldSucceed + // SOURCE=E_AttributeTargetIsClass.fs # E_AttributeTargetIsClass.fs [] let ``E_AttributeTargetIsClass_fs preview`` compilation = @@ -363,6 +370,18 @@ module CustomAttributes_AttributeUsage = (Error 842, Line 19, Col 3, Line 19, Col 15, "This attribute is not valid for use on this language element") (Error 842, Line 22, Col 10, Line 22, Col 22, "This attribute is not valid for use on this language element") ] + + // SOURCE=E_AttributeTargetIsClass01.fs # E_AttributeTargetIsClass01.fs + [] + let ``E_AttributeTargetIsClass01_fs preview`` compilation = + compilation + |> withLangVersionPreview + |> verifyCompile + |> shouldFail + |> withDiagnostics [ + (Error 842, Line 7, Col 3, Line 7, Col 18, "This attribute is not valid for use on this language element") + (Error 842, Line 10, Col 10, Line 10, Col 25, "This attribute is not valid for use on this language element") + ] // SOURCE=MarshalAsAttribute.fs # MarshalAsAttribute.fs [] @@ -583,4 +602,40 @@ type InterruptibleLazy<'T> private (valueFactory: unit -> 'T) = """ |> withLangVersionPreview |> compile - |> shouldSucceed \ No newline at end of file + |> shouldSucceed + + // SOURCE=AttributeTargetsIsInterface.fs # AttributeTargetsIsInterface.fs + [] + let ``AttributeTargetsIsInterface_fs`` compilation = + compilation + |> verifyCompile + |> shouldSucceed + + // SOURCE=AttributeTargetsIsInterface.fs # AttributeTargetsIsInterface.fs + [] + let ``AttributeTargetsIsInterface_fs preview`` compilation = + compilation + |> withLangVersionPreview + |> verifyCompile + |> shouldSucceed + + // SOURCE=E_AttributeTargetIsInterface.fs # E_AttributeTargetIsInterface.fs + [] + let ``E_AttributeTargetIsInterface_fs`` compilation = + compilation + |> verifyCompile + |> shouldSucceed + + // SOURCE=E_AttributeTargetIsInterface.fs # E_AttributeTargetIsInterface.fs + [] + let ``E_AttributeTargetIsInterface_fs preview`` compilation = + compilation + |> withLangVersionPreview + |> verifyCompile + |> shouldFail + |> withDiagnostics [ + (Error 842, Line 11, Col 3, Line 11, Col 14, "This attribute is not valid for use on this language element") + (Error 842, Line 14, Col 3, Line 14, Col 15, "This attribute is not valid for use on this language element") + (Error 842, Line 18, Col 3, Line 18, Col 14, "This attribute is not valid for use on this language element") + (Error 842, Line 19, Col 3, Line 19, Col 15, "This attribute is not valid for use on this language element") + ] \ No newline at end of file diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_AttributeTargetIsClass01.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_AttributeTargetIsClass01.fs new file mode 100644 index 00000000000..e3f94e20bf1 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_AttributeTargetIsClass01.fs @@ -0,0 +1,11 @@ +open System + +[] +type CustomInterfaceAttribute() = + inherit Attribute() + +[] +type Class(x: int) = class end + +[] +type Class3 = class end \ No newline at end of file diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_AttributeTargetIsInterface.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_AttributeTargetIsInterface.fs new file mode 100644 index 00000000000..5f4e2ff5ea0 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/E_AttributeTargetIsInterface.fs @@ -0,0 +1,22 @@ +open System + +[] +type CustomStructAttribute() = + inherit Attribute() + +[] +type CustomClassAttribute() = + inherit Attribute() + +[] +type IFoo = interface end + +[] +type IFoo2 = + abstract A :int + +[] +[] +[] +type IFoo3 = + abstract A :int \ No newline at end of file From c21e207f00b299ebae9a6da2df90f73f27030819 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sat, 18 May 2024 11:37:50 +0100 Subject: [PATCH 2/5] Update AllowNullLiteralAttribute AttributeTargets --- src/FSharp.Core/prim-types.fs | 2 +- src/FSharp.Core/prim-types.fsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/FSharp.Core/prim-types.fs b/src/FSharp.Core/prim-types.fs index 245a83380f5..556fb1bcde5 100644 --- a/src/FSharp.Core/prim-types.fs +++ b/src/FSharp.Core/prim-types.fs @@ -76,7 +76,7 @@ namespace Microsoft.FSharp.Core type ComparisonConditionalOnAttribute() = inherit Attribute() - [] + [] [] type AllowNullLiteralAttribute(value: bool) = inherit Attribute() diff --git a/src/FSharp.Core/prim-types.fsi b/src/FSharp.Core/prim-types.fsi index e7ceb9b6166..dee677aac31 100644 --- a/src/FSharp.Core/prim-types.fsi +++ b/src/FSharp.Core/prim-types.fsi @@ -263,7 +263,7 @@ namespace Microsoft.FSharp.Core /// interface types. /// /// Attributes - [] + [] [] type AllowNullLiteralAttribute = inherit Attribute From 85d80abe839e11b40bde960ec198d9841e97f157 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sat, 18 May 2024 14:05:57 +0100 Subject: [PATCH 3/5] Convert to AbstractClass --- vsintegration/src/FSharp.ProjectSystem.FSharp/Project.fs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/vsintegration/src/FSharp.ProjectSystem.FSharp/Project.fs b/vsintegration/src/FSharp.ProjectSystem.FSharp/Project.fs index c5e347878a4..cb30c2fc84d 100644 --- a/vsintegration/src/FSharp.ProjectSystem.FSharp/Project.fs +++ b/vsintegration/src/FSharp.ProjectSystem.FSharp/Project.fs @@ -161,12 +161,13 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem // Used to get us sorted appropriately with the other MSFT products in the splash screen and about box [] [] - [] - [] + [] [] [] + [] type public IVsMicrosoftInstalledProduct = - inherit IVsInstalledProduct + interface IVsInstalledProduct + abstract IdBmpSplashM : byref -> unit abstract OfficialNameM : on : byref -> unit abstract ProductIDM : pid : byref -> unit From 34353130af026bef06e670ba6afaae5f54cf6a04 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sat, 18 May 2024 20:28:04 +0100 Subject: [PATCH 4/5] release notes --- docs/release-notes/.FSharp.Compiler.Service/8.0.400.md | 2 +- docs/release-notes/.FSharp.Core/8.0.400.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md index 7765f188e0e..bccd5a0d762 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md @@ -20,7 +20,7 @@ * Generate new `Equals` overload to avoid boxing for structural comparison ([PR #16857](https://github.com/dotnet/fsharp/pull/16857)) ### Changed - +* Enforce `AttributeTargets.Interface` ([PR #17173](https://github.com/dotnet/fsharp/pull/17173)) * Minor compiler perf improvements. ([PR #17130](https://github.com/dotnet/fsharp/pull/17130)) * Improve error of Active Pattern case Argument Count Not Match ([PR #16846](https://github.com/dotnet/fsharp/pull/16846)) * AsyncLocal diagnostics context. ([PR #16779](https://github.com/dotnet/fsharp/pull/16779)) diff --git a/docs/release-notes/.FSharp.Core/8.0.400.md b/docs/release-notes/.FSharp.Core/8.0.400.md index 48a05231dc9..ee24d8d0a53 100644 --- a/docs/release-notes/.FSharp.Core/8.0.400.md +++ b/docs/release-notes/.FSharp.Core/8.0.400.md @@ -5,3 +5,4 @@ ### Changed * Cache delegate in query extensions. ([PR #17130](https://github.com/dotnet/fsharp/pull/17130)) +* Update `AllowNullLiteralAttribute` to also use `AttributeTargets.Interface` ([PR #17173](https://github.com/dotnet/fsharp/pull/17173)) From 5e05806f740cda11917fed0d0bc9239193e3e03f Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sat, 18 May 2024 20:29:58 +0100 Subject: [PATCH 5/5] Comment out IVsMicrosoftInstalledProduct for now --- .../FSharp.ProjectSystem.FSharp/Project.fs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/vsintegration/src/FSharp.ProjectSystem.FSharp/Project.fs b/vsintegration/src/FSharp.ProjectSystem.FSharp/Project.fs index cb30c2fc84d..991337acba3 100644 --- a/vsintegration/src/FSharp.ProjectSystem.FSharp/Project.fs +++ b/vsintegration/src/FSharp.ProjectSystem.FSharp/Project.fs @@ -158,22 +158,22 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem member this.Advise(callbackOwnerKey, callback) = notificationsDict.[callbackOwnerKey] <- callback - // Used to get us sorted appropriately with the other MSFT products in the splash screen and about box - [] - [] - [] - [] - [] - [] - type public IVsMicrosoftInstalledProduct = - interface IVsInstalledProduct - - abstract IdBmpSplashM : byref -> unit - abstract OfficialNameM : on : byref -> unit - abstract ProductIDM : pid : byref -> unit - abstract ProductDetailsM : pd : byref -> unit - abstract IdIcoLogoForAboutboxM : byref -> unit - abstract ProductRegistryName : prn : byref -> unit + // // Used to get us sorted appropriately with the other MSFT products in the splash screen and about box + // [] + // [] + // [] + // [] + // [] + // [] + // type public IVsMicrosoftInstalledProduct = + // interface IVsInstalledProduct + // + // abstract IdBmpSplashM : byref -> unit + // abstract OfficialNameM : on : byref -> unit + // abstract ProductIDM : pid : byref -> unit + // abstract ProductDetailsM : pd : byref -> unit + // abstract IdIcoLogoForAboutboxM : byref -> unit + // abstract ProductRegistryName : prn : byref -> unit exception internal ExitedOk exception internal ExitedWithError