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

Disallow types named 'scoped' #62968

Merged
merged 1 commit into from
Jul 28, 2022
Merged

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Jul 27, 2022

@jcouv jcouv changed the title Move more code over Disallow types named 'scoped' Jul 27, 2022
@alrz
Copy link
Member

alrz commented Jul 27, 2022

All lowercase types are already disallowed, no?

@jcouv
Copy link
Member Author

jcouv commented Jul 27, 2022

All lowercase types are already disallowed, no?

We only produce a warning wave warning for those. So we escalate to an error when we actually start using a lowercase word as contextual keyword.

@jcouv jcouv marked this pull request as ready for review July 27, 2022 16:01
@jcouv jcouv requested a review from a team as a code owner July 27, 2022 16:01
@@ -478,7 +478,8 @@ internal static void ReportReservedTypeName(string? name, CSharpCompilation comp

if (reportIfContextual(SyntaxKind.RecordKeyword, MessageID.IDS_FeatureRecords, ErrorCode.WRN_RecordNamedDisallowed)
Copy link
Member

Choose a reason for hiding this comment

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

Curious why records is warning and other contextual keywords are errors?

Copy link
Member Author

Choose a reason for hiding this comment

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

Would have to dig up the PR and/or LDM notes. Most likely this was evolution of our philosophy towards such breaks. Records came before we added the warning wave warning.

@jcouv jcouv merged commit 498db64 into dotnet:release/dev17.4 Jul 28, 2022
@jcouv jcouv deleted the reserve-scoped branch July 28, 2022 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants