-
Notifications
You must be signed in to change notification settings - Fork 789
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
Disallow attributes on type extensions #7481
Conversation
@KevinRansom @dsyme Could you guys take a look at this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Perhaps change the error text to "Attributes cannot be applied to type extensions"
@nelson-wu, can you change the error message per @dsyme, then I will merge the change. Thanks Kevin |
@KevinRansom @dsyme Thanks for the review! I've made the changes |
@nelson-wu, thanks mate, when it goes green, I'll merge it. |
@@ -16429,6 +16429,11 @@ module TcDeclarations = | |||
if not (isNil members) && tcref.IsTypeAbbrev then | |||
errorR(Error(FSComp.SR.tcTypeAbbreviationsCannotHaveAugmentations(), tyDeclRange)) | |||
|
|||
let (ComponentInfo (attributes, _, _, _, _, _, _, _)) = synTyconInfo | |||
if not (List.isEmpty attributes) && (declKind = ExtrinsicExtensionBinding || declKind = IntrinsicExtensionBinding) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
* add test for feature * add error logic to typechecker * update strings * address comments * update tests
Fixes #7394
Before, when attributes get added to type extensions, they can be parsed, no errors are thrown, but the attributes are silently dropped inside the type checker. This PR explicitly disallows that.