Skip to content

Disallow attributes on type extensions - #7481

Merged
cartermp merged 5 commits into
dotnet:masterfrom
nelson-wu:dev/type-extension-attrs
Sep 4, 2019
Merged

cartermp merged 5 commits into
dotnet:masterfrom
nelson-wu:dev/type-extension-attrs

Conversation

@nelson-wu

@nelson-wu nelson-wu commented Sep 3, 2019

Copy link
Copy Markdown
Contributor

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.

@dnfclas

dnfclas commented Sep 3, 2019

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@nelson-wu
nelson-wu marked this pull request as ready for review September 3, 2019 22:53
@nelson-wu

Copy link
Copy Markdown
Contributor Author

@KevinRansom @dsyme Could you guys take a look at this PR?

@dsyme dsyme left a comment

Copy link
Copy Markdown
Contributor

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"

@KevinRansom

KevinRansom commented Sep 4, 2019

Copy link
Copy Markdown
Contributor

@nelson-wu, can you change the error message per @dsyme, then I will merge the change.

Thanks

Kevin

@nelson-wu

Copy link
Copy Markdown
Contributor Author

@KevinRansom @dsyme Thanks for the review! I've made the changes

Comment thread tests/fsharp/Compiler/Language/TypeAttributeTests.fs Outdated
@KevinRansom

Copy link
Copy Markdown
Contributor

@nelson-wu, thanks mate, when it goes green, I'll merge it.

@cartermp
cartermp merged commit 49eeb63 into dotnet:master Sep 4, 2019
Comment thread src/fsharp/TypeChecker.fs
errorR(Error(FSComp.SR.tcTypeAbbreviationsCannotHaveAugmentations(), tyDeclRange))

let (ComponentInfo (attributes, _, _, _, _, _, _, _)) = synTyconInfo
if not (List.isEmpty attributes) && (declKind = ExtrinsicExtensionBinding || declKind = IntrinsicExtensionBinding) then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
* add test for feature

* add error logic to typechecker

* update strings

* address comments

* update tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Arbitrary nonsense attributes can be added to type extensions

6 participants