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

Attribute targets on records #17207

Merged
merged 24 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4720fac
Initial test
edgarfgp May 21, 2024
e14eb37
Update record check for targets
edgarfgp May 21, 2024
f0cd5d5
More tests
edgarfgp May 21, 2024
f42a88c
Merge branch 'main' into attribute-targets-on-records
edgarfgp May 22, 2024
ffb497b
Check Record fields
edgarfgp May 22, 2024
669ee19
release notes
edgarfgp May 22, 2024
670972a
Merge branch 'main' into attribute-targets-on-records
edgarfgp May 22, 2024
d43956f
Extend DefaultValueAttribute to use AttributeTargets.Property
edgarfgp May 22, 2024
cd582b6
revert DefaultValueAttribute changes
edgarfgp May 23, 2024
74e1342
TcFieldDecl
edgarfgp May 23, 2024
071611a
update tests
edgarfgp May 23, 2024
476f236
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 10, 2024
3ea80b3
update tests
edgarfgp Jun 10, 2024
6e555a5
update test
edgarfgp Jun 11, 2024
5a0b1c1
Update StructAttribute to also AttributeTargets.Class
edgarfgp Jun 11, 2024
389439c
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 12, 2024
0062acb
Update 8.0.400.md
psfinaki Jun 12, 2024
a0f3acf
Add clarifying comment
edgarfgp Jun 13, 2024
a63a322
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 13, 2024
cc7cc4a
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 13, 2024
207ac3b
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 14, 2024
993958e
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 17, 2024
be11382
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jul 3, 2024
5c6c7c8
We do not check for RQA attribute
edgarfgp Jul 3, 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: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/8.0.400.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
### Fixed

* Enforce `AttributeTargets` on records. ([PR #17207](https://github.com/dotnet/fsharp/pull/17207))
* Fix internal error when dotting into delegates with multiple type parameters. ([PR #17227](https://github.com/dotnet/fsharp/pull/17227))
* Error for partial implementation of interface with static and non-static abstract members. ([Issue #17138](https://github.com/dotnet/fsharp/issues/17138), [PR #17160](https://github.com/dotnet/fsharp/pull/17160))
* Optimize simple mappings with preludes in computed collections. ([PR #17067](https://github.com/dotnet/fsharp/pull/17067))
Expand Down
12 changes: 12 additions & 0 deletions src/Compiler/Checking/CheckDeclarations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ module TcRecdUnionAndEnumDeclarations =
let g = cenv.g
let m = id.idRange
let attrs, _ = TcAttributesWithPossibleTargets false cenv env AttributeTargets.FieldDecl synAttrs

let attrsForProperty, attrsForField = attrs |> List.partition (fun (attrTargets, _) -> (attrTargets &&& AttributeTargets.Property) <> enum 0)
let attrsForProperty = (List.map snd attrsForProperty)
let attrsForField = (List.map snd attrsForField)
Expand Down Expand Up @@ -2840,6 +2841,8 @@ module EstablishTypeDefinitionCores =
// Allow failure of constructor resolution because Vals for members in the same recursive group are not yet available
let attrs, getFinalAttrs = TcAttributesCanFail cenv envinner AttributeTargets.TyconDecl synAttrs
let hasMeasureAttr = HasFSharpAttribute g g.attrib_MeasureAttribute attrs
let hasStructAttr = HasFSharpAttribute g g.attrib_StructAttribute attrs
let hasRQAAttr = HasFSharpAttribute g g.attrib_RequireQualifiedAccessAttribute attrs

let isStructRecordOrUnionType =
match synTyconRepr with
Expand Down Expand Up @@ -2896,6 +2899,15 @@ module EstablishTypeDefinitionCores =

// Run InferTyconKind to raise errors on inconsistent attribute sets
InferTyconKind g (SynTypeDefnKind.Record, attrs, [], [], inSig, true, m) |> ignore

if g.langVersion.SupportsFeature(LanguageFeature.EnforceAttributeTargets) then
if hasStructAttr then
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
if hasRQAAttr then
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
TcAttributesWithPossibleTargets false cenv envinner (AttributeTargets.Class ||| AttributeTargets.Struct) synAttrs |> ignore
else
TcAttributesWithPossibleTargets false cenv envinner AttributeTargets.Struct synAttrs |> ignore
else
TcAttributesWithPossibleTargets false cenv envinner AttributeTargets.Class synAttrs |> ignore

// Note: the table of record fields is initially empty
TFSharpTyconRepr (Construct.NewEmptyFSharpTyconData TFSharpRecord)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,11 @@ type SemanticClassificationItem =
type ILTableName(idx: int) =
member __.Index = idx
static member FromIndex n = ILTableName n

[<RequireQualifiedAccess>]
[<CustomClass>]
type Record = { Prop: string }

[<RequireQualifiedAccess>]
[<Struct>]
type StructRecord = { Prop: string }
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ type PropertyLevelAttribute() =

type U =
| [<PropertyLevel>] A
| [<PropertyLevel>] B
| [<PropertyLevel>] B
Original file line number Diff line number Diff line change
Expand Up @@ -638,4 +638,25 @@ type InterruptibleLazy<'T> private (valueFactory: unit -> 'T) =
(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")
]

// SOURCE= E_AttributeTargetIsClass02.fs # E_AttributeTargetIsClass02.fs
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"E_AttributeTargetIsClass02.fs"|])>]
let ``E_AttributeTargetIsClass02_fs`` compilation =
compilation
|> verifyCompile
|> shouldSucceed

// SOURCE=E_AttributeTargetIsClass02.fs # E_AttributeTargetIsClass02.fs
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"E_AttributeTargetIsClass02.fs"|])>]
let ``E_AttributeTargetIsClass02_fs preview`` compilation =
compilation
|> withLangVersionPreview
|> verifyCompile
|> shouldFail
|> withDiagnostics [
(Error 842, Line 15, Col 3, Line 15, Col 18, "This attribute is not valid for use on this language element")
(Error 842, Line 16, Col 3, Line 16, Col 15, "This attribute is not valid for use on this language element")
(Error 842, Line 20, Col 3, Line 20, Col 14, "This attribute is not valid for use on this language element")
(Error 842, Line 21, Col 3, Line 21, Col 18, "This attribute is not valid for use on this language element")
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
open System

[<AttributeUsage(AttributeTargets.Class)>]
type ClassTargetAttribute() =
inherit Attribute()

[<AttributeUsage(AttributeTargets.Interface)>]
type InterfaceTargetAttribute() =
inherit Attribute()

[<AttributeUsage(AttributeTargets.Struct)>]
type StructTargetAttribute() =
inherit Attribute()

[<InterfaceTarget>]
[<StructTarget>]
[<ClassTarget>]
type Record = { Prop: string }

[<ClassTarget>]
[<InterfaceTarget>]
[<Struct>]
psfinaki marked this conversation as resolved.
Show resolved Hide resolved
type StructRecord = { Prop: string }
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ type PropertyOrFieldLevelAttribute() =

type SomeUnion =
| [<PropertyLevel>] Case1 of int // Should fail
| [<PropertyOrFieldLevel>] Case2 of int // Should fail
| [<PropertyOrFieldLevel>] Case2 of int // Should fail

Loading