-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Field-backed properties: report diagnostic for variable named field declared in accessor #76671
base: main
Are you sure you want to change the base?
Conversation
…eclared in accessor
0ada606
to
b40643e
Compare
@@ -6886,8 +6886,11 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ | |||
<data name="WRN_FieldIsAmbiguous_Title" xml:space="preserve"> | |||
<value>The 'field' keyword binds to a synthesized backing field for the property.</value> | |||
</data> | |||
<data name="INF_IdentifierConflictWithContextualKeyword_Title" xml:space="preserve"> |
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.
ca99664
to
e89a75b
Compare
aad5168
to
bd70829
Compare
<data name="INF_IdentifierConflictWithContextualKeyword_Title" xml:space="preserve"> | ||
<value>Identifier is a contextual keyword, with a specific meaning, in a later language version.</value> | ||
<data name="ERR_VariableDeclarationNamedField" xml:space="preserve"> | ||
<value>In language version {0}, the 'field' keyword binds to a synthesized backing field for the property. Unescaped references to 'field' within the scope of this variable will refer to the synthesized field rather than this variable.</value> |
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.
I think a message like the following would be better:
field
is a keyword in this context. Rename the variable or use the identifier@field
instead.
IMO, this gets the point across more effectively, and focuses the message on what the user needs to know (how to unblock). I think users who get this error on upgrade will already see that this is a new thing in the new language version.
@@ -561,6 +561,8 @@ private BoundStatement BindLocalFunctionStatement(LocalFunctionStatementSyntax n | |||
var hasErrors = localSymbol.ScopeBinder | |||
.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics); | |||
|
|||
ReportFieldContextualKeywordConflictIfAny(localSymbol, node, node.Identifier, diagnostics); |
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.
I was curious if there was any more "central" location to do this check, like where local symbols are gathered up into a scope, or some such. It looks like probably not. Places like LocalScopeBinder.MakeLocal
are not really designed for reporting diagnostics.
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.
I haven't found a central location to catch these cases.
Report an error for a declaration of a local or parameter named
field
within an accessor, sincefield
will not bind to that variable. See item 2 in #76031.Based on the variable declaration checks reverted in #74164.