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.

Sign up for GitHub

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

Report info diagnostic for field and value in property accessors regardless of language version #73952

Merged
merged 5 commits into from
Jun 13, 2024
Merged

Report info diagnostic for field and value in property accessors regardless of language version #73952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f14fb17
Update diagnostic message
cston Jun 12, 2024
4ab8762
Additional test
cston Jun 13, 2024
d3615b6
Report diagnostic regardless of language version
cston Jun 12, 2024
b724c68
Fix tests
cston Jun 13, 2024
15ce91b
Update diagnostic message
cston Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1750,10 +1750,7 @@ internal void ReportFieldOrValueContextualKeywordConflictIfAny(SyntaxNode syntax
case "value" when ContainingMember() is MethodSymbol { MethodKind: MethodKind.PropertySet or MethodKind.EventAdd or MethodKind.EventRemove }:
{
var requiredVersion = MessageID.IDS_FeatureFieldAndValueKeywords.RequiredVersion();
if (Compilation.LanguageVersion < requiredVersion)
{
diagnostics.Add(ErrorCode.INF_IdentifierConflictWithContextualKeyword, syntax, name, requiredVersion.ToDisplayString());
}
diagnostics.Add(ErrorCode.INF_IdentifierConflictWithContextualKeyword, syntax, name, requiredVersion.ToDisplayString());
}
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 in property accessors starting in language version {1}. Use '@{0}' instead.</value>
</data>
<data name="INF_IdentifierConflictWithContextualKeyword_Title" xml:space="preserve">
<value>Identifier is a contextual keyword, with a specific meaning, in a later language version.</value>
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; } }
Copy link
Contributor

Choose a reason for hiding this comment

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

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.)


static void Main()
{
Expand Down Expand Up @@ -1941,7 +1941,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; } }

static void Main()
{
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Emit2/Semantics/RecordTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5282,7 +5282,7 @@ record C1(int I1)
public int this[int i] => 0;
public int PropertyWithoutGetter { set { } }
public int P2 { get => 43; }
public ref int P3 { get => ref field; }
public ref int P3 { get => ref @field; }
public event System.Action a;

private int field1 = 100;
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/Semantic/Semantics/BindingAsyncTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2313,13 +2313,13 @@ public Task Prop
get
{
Meth(1);
return field;
return @field;
}
set
{
Bar();
Meth("""");
field = value;
@field = value;
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Test/Semantic/Semantics/InheritanceBindingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1907,17 +1907,17 @@ class Base
int field = 0;

public virtual int Proprty1 { get { return 0; } }
public virtual ref int Property2 { get { return ref field; } }
public virtual ref int Property3 { get { return ref field; } }
public virtual ref int Property2 { get { return ref @field; } }
public virtual ref int Property3 { get { return ref @field; } }
}

class Derived : Base
{
int field = 0;

public override ref int Proprty1 { get { return ref field; } }
public override ref int Proprty1 { get { return ref @field; } }
public override int Property2 { get { return 0; } }
public override ref int Property3 { get { return ref field; } }
public override ref int Property3 { get { return ref @field; } }
}
";
CreateCompilationWithMscorlib45(text).VerifyDiagnostics(
Expand Down
12 changes: 6 additions & 6 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LambdaTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4742,7 +4742,7 @@ public void NullableTypeArraySwitchPattern()
class C
{
object? field;
string Prop => field switch
string Prop => @field switch
{
string?[] a => ""a""
};
Expand All @@ -4752,9 +4752,9 @@ class C
// (4,13): warning CS0649: Field 'C.field' is never assigned to, and will always have its default value null
// object? field;
Diagnostic(ErrorCode.WRN_UnassignedInternalField, "field").WithArguments("C.field", "null").WithLocation(4, 13),
// (5,26): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '_' is not covered.
// string Prop => field switch
Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(5, 26));
// (5,27): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '_' is not covered.
// string Prop => @field switch
Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(5, 27));
}

[Fact, WorkItem(52827, "https://github.com/dotnet/roslyn/issues/52827")]
Expand Down Expand Up @@ -7344,7 +7344,7 @@ public void LambdaWithNonConstantDefault()
class C
{
object field;
public object Field => field;
public object Field => @field;

public C(object f) { field = f; }

Expand Down Expand Up @@ -7373,7 +7373,7 @@ public void AnonymousMethodWithNonConstantDefault()
class C
{
object field;
public object Field => field;
public object Field => @field;

public C(object f) { field = f; }

Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22615,7 +22615,7 @@ public void MemberNotNull_NotFound_PrivateInBase()
public class Base
{
private string? field;
public string? P { get { return field; } set { field = value; } }
public string? P { get { return @field; } set { @field = value; } }
}
public class C : Base
{
Expand Down Expand Up @@ -156308,7 +156308,7 @@ class C
public string? field = null;

[MemberNotNullWhen(true, ""field"")]
public System.Nullable<bool> Init => field != null;
public System.Nullable<bool> Init => @field != null;

public void M()
{
Expand Down Expand Up @@ -156349,7 +156349,7 @@ class C
public string? field = null;

[MemberNotNullWhen(true, ""field"")]
public bool Init => field != null;
public bool Init => @field != null;

public void M()
{
Expand Down Expand Up @@ -156387,7 +156387,7 @@ class C
public string? field = null;

[MemberNotNullWhen(true, ""field"")]
public System.Nullable<bool> Init => field != null;
public System.Nullable<bool> Init => @field != null;

public void M()
{
Expand Down
Loading