Field-backed properties: report diagnostic for variable named field declared in accessor #76671
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
…eclared in accessor
cston
force-pushed
the
variable-named-field
branch
from
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"> | |||
cston
force-pushed
the
variable-named-field
branch
from
ca99664 to
e89a75b
Compare
cston
force-pushed
the
variable-named-field
branch
from
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.
I think a message like the following would be better:
fieldis a keyword in this context. Rename the variable or use the identifier@fieldinstead.
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.
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. #ByDesign
There was a problem hiding this comment.
I haven't found a central location to catch these cases.
There was a problem hiding this comment.
Could we perhaps catch all parameters in ParameterHelpers.MakeParameters?
There was a problem hiding this comment.
ParameterHelpers.MakeParameters() will not work as is because that method is not used for lambda parameters (LambdaSymbol.MakeParameters() is used instead), and the method does not include a Binder which is used for reporting. I'll leave this as is.
| // The symbol should be locally declared. That is, it should be a symbol | ||
| // that would hide a backing field in earlier language versions. | ||
| // If a symbol is not provided, the caller is responsible for | ||
| // ensuring the identifier refers to a locally declared variable. |
There was a problem hiding this comment.
Perhaps make this a doc comment? #Resolved
| @@ -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.
Could we perhaps catch all parameters in ParameterHelpers.MakeParameters?
* upstream/main: (172 commits) Move impl of ILspWorkpace to EditorTestWorkspace/LspTestWorkspace (dotnet#76753) Field-backed properties: report diagnostic for variable named field declared in accessor (dotnet#76671) Add more tests Update 'use nameof instead of typeof' to support generic types Update dependencies from https://github.com/dotnet/arcade build 20250115.2 Add docs invert Update src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems Update options Fixup tests fixup options Update tests Add test Fix trivia Handle params Support modifiers with simple lambda parameters. (dotnet#75400) Handle params Use helper Add fixes Flesh out ...
Report an error for a declaration of a local or parameter named
fieldwithin an accessor, sincefieldwill not bind to that variable. See item 2 in #76031.See proposal: dotnet/csharplang#9038.
Based on the variable declaration checks reverted in #74164.