Report info diagnostic for field and value in property accessors regardless of language version
#73952
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
cston
force-pushed
the
keyword-diagnostic-more
branch
from
9f69b45
to
d3615b6
Compare
cston
changed the title
field in property accessors regardless of language versionfield and value in property accessors regardless of language version
| @@ -6866,7 +6866,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ | |||
| <value>Compiling requires binding the lambda expression many times. Consider declaring the lambda expression with explicit parameter types, or if the containing method call is generic, consider using explicit type arguments.</value> | |||
| </data> | |||
| <data name="INF_IdentifierConflictWithContextualKeyword" xml:space="preserve"> | |||
| <value>'{0}' is a contextual keyword, with a specific meaning, starting in language version {1}. Use '@{0}' to avoid a breaking change when compiling with language version {1} or later.</value> | |||
| <value>'{0}' is a contextual keyword, with a specific meaning, in language version {1} or later. Use '@{0}' to avoid a breaking change when compiling with different language versions.</value> | |||
There was a problem hiding this comment.
Not sure I understand the second half of this message changing; I actually find it clearer than the new wording.
There was a problem hiding this comment.
I wanted wording that made sense regardless of whether the code was compiled with -langversion:12 (or earlier) or -langversion:13 (or later). The previous message said "to avoid a breaking change when compiling with version 13 or later" but that seemed confusing when compiling with 13.
I'm open to suggestions on wording.
There was a problem hiding this comment.
I don't think the current message adequately describes the issue at all. Here's my workshop:
| <value>'{0}' is a contextual keyword, with a specific meaning, in language version {1} or later. Use '@{0}' to avoid a breaking change when compiling with different language versions.</value> | |
| <value>'{0}' is a contextual keyword, with a specific meaning in property accessors, in language version {1} or later. Use '@{0}' to avoid potential breaking changes in property accessors when compiling with different language versions.</value> |
There was a problem hiding this comment.
'field' is a contextual keyword in property accessors starting in C# 13. Use '@field' instead.
IMO this message gives all the key information and then stops, which is good for user comprehension. People who want more details on why exactly they're being asked to do this and what field and @field will mean in the future can hit F1.
There was a problem hiding this comment.
How about "to avoid breaking changes ..." rather than "to avoid potential breaking changes ..."? (We're only reporting the diagnostic when binding to a different symbol, so the diagnostic is quite likely identifying a breaking change.)
There was a problem hiding this comment.
I think I like Rikki's suggestion best.
| @@ -1906,7 +1906,7 @@ class C | |||
| static int field; | |||
| static long x { set { System.Console.WriteLine($""setX {value}""); } } | |||
| static string y { get; set; } | |||
| static ref int z { get { return ref field; } } | |||
| static ref int z { get { return ref @field; } } | |||
There was a problem hiding this comment.
No need to address in this PR. But, are we considering reporting anything in the following scenario, after field feature ships?
class C
{
int field;
public int Prop => field; // doesn't read 'C.field'.
}Essentially some kind of guardrail to try and avoid surprise here when field doesn't mean what the user thinks it means in the property accessor. For example, a nudge to declare int field as int @field, to signal they understand the new behavior in property accessors. (It feels like a warning on the usage of field when a symbol with that name is in scope is too cumbersome/hard to suppress.)
No description provided.