Skip to content

Commit

Permalink
Field-backed properties: [field:] should not be considered a backing …
Browse files Browse the repository at this point in the history 
…field reference (#75461)
  • Loading branch information
@cston
cston authored Oct 11, 2024
1 parent c9dd07d commit f49638e
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 6 deletions.
36 changes: 30 additions & 6 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ private static SourcePropertySymbol Create(
out var getSyntax,
out var setSyntax);

Debug.Assert(!usesFieldKeyword ||
((CSharpParseOptions)syntax.SyntaxTree.Options).IsFeatureEnabled(MessageID.IDS_FeatureFieldKeyword));

bool accessorsHaveImplementation = hasGetAccessorImplementation || hasSetAccessorImplementation;

var explicitInterfaceSpecifier = GetExplicitInterfaceSpecifier(syntax);
Expand Down Expand Up @@ -264,15 +267,15 @@ private static void GetAccessorDeclarations(
throw ExceptionUtilities.UnexpectedValue(accessor.Kind());
}

usesFieldKeyword = usesFieldKeyword || containsFieldKeyword(accessor);
usesFieldKeyword = usesFieldKeyword || containsFieldExpressionInAccessor(accessor);
}
}
else
{
var body = GetArrowExpression(syntax);
hasGetAccessorImplementation = body is object;
hasSetAccessorImplementation = false;
usesFieldKeyword = body is { } && containsFieldKeyword(body);
usesFieldKeyword = body is { } && containsFieldExpressionInGreenNode(body.Green);
Debug.Assert(hasGetAccessorImplementation); // it's not clear how this even parsed as a property if it has no accessor list and no arrow expression.
}

Expand All @@ -282,13 +285,34 @@ static bool hasImplementation(AccessorDeclarationSyntax accessor)
return body != null;
}

static bool containsFieldKeyword(SyntaxNode syntax)
static bool containsFieldExpressionInAccessor(AccessorDeclarationSyntax syntax)
{
var accessorDeclaration = (Syntax.InternalSyntax.AccessorDeclarationSyntax)syntax.Green;
foreach (var attributeList in accessorDeclaration.AttributeLists)
{
var attributes = attributeList.Attributes;
for (int i = 0; i < attributes.Count; i++)
{
if (containsFieldExpressionInGreenNode(attributes[i]))
{
return true;
}
}
}
return containsFieldExpressionInGreenNode(accessorDeclaration.Body) ||
containsFieldExpressionInGreenNode(accessorDeclaration.ExpressionBody);
}

static bool containsFieldExpressionInGreenNode(GreenNode? green)
{
foreach (var node in syntax.Green.EnumerateNodes())
if (green is { })
{
if (node.RawKind == (int)SyntaxKind.FieldKeyword)
foreach (var node in green.EnumerateNodes())
{
return true;
if (node.RawKind == (int)SyntaxKind.FieldExpression)
{
return true;
}
}
}
return false;
Expand Down
81 changes: 81 additions & 0 deletions src/Compilers/CSharp/Test/Emit3/FieldKeywordTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,87 @@ static void ReportMember(MemberInfo member)
"""));
}

[WorkItem("https://github.com/dotnet/roslyn/issues/75459")]
[Theory]
[CombinatorialData]
public void FieldAttribute_NotAutoProperty(
[CombinatorialValues(LanguageVersion.CSharp13, LanguageVersionFacts.CSharpNext)] LanguageVersion languageVersion,
bool useInit)
{
string setter = useInit ? "init" : "set";
string source = $$"""
using System;
using System.Reflection;

class A : Attribute { }

class C
{
[field: A] object P1 { get => null; }
[field: A] object P2 { {{setter}} { } }
[field: A] object P3 { get => null; {{setter}} { } }
object P4 { [field: A] get => null; }
object P5 { [field: A] {{setter}} { } }
object P6 { [field: A] get => null; {{setter}} { } }
object P7 { get => null; [field: A] {{setter}} { } }
}

class Program
{
static void Main()
{
foreach (var field in typeof(C).GetFields(BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static))
Console.WriteLine("{0}", field.Name);
}
}
""";
var verifier = CompileAndVerify(
source,
parseOptions: TestOptions.Regular.WithLanguageVersion(languageVersion),
targetFramework: GetTargetFramework(useInit),
verify: Verification.Skipped,
expectedOutput: IncludeExpectedOutput(useInit, ""));
verifier.VerifyDiagnostics(
// (8,6): warning CS0657: 'field' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'property'. All attributes in this block will be ignored.
// [field: A] object P1 { get => null; }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "property").WithLocation(8, 6),
// (9,6): warning CS0657: 'field' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'property'. All attributes in this block will be ignored.
// [field: A] object P2 { set { } }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "property").WithLocation(9, 6),
// (10,6): warning CS0657: 'field' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'property'. All attributes in this block will be ignored.
// [field: A] object P3 { get => null; set { } }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "property").WithLocation(10, 6),
// (11,18): warning CS0657: 'field' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'method, return'. All attributes in this block will be ignored.
// object P4 { [field: A] get => null; }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "method, return").WithLocation(11, 18),
// (12,18): warning CS0657: 'field' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'method, param, return'. All attributes in this block will be ignored.
// object P5 { [field: A] set { } }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "method, param, return").WithLocation(12, 18),
// (13,18): warning CS0657: 'field' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'method, return'. All attributes in this block will be ignored.
// object P6 { [field: A] get => null; set { } }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "method, return").WithLocation(13, 18),
// (14,31): warning CS0657: 'field' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'method, param, return'. All attributes in this block will be ignored.
// object P7 { get => null; [field: A] set { } }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "method, param, return").WithLocation(14, 31));

var comp = (CSharpCompilation)verifier.Compilation;
var containingType = comp.GetMember<NamedTypeSymbol>("C");
var actualFields = containingType.GetMembers().OfType<FieldSymbol>().ToImmutableArray();
AssertEx.Equal([], actualFields);

var actualProperties = containingType.GetMembers().OfType<PropertySymbol>().ToImmutableArray();
Assert.Equal(7, actualProperties.Length);
Assert.True(actualProperties[0] is SourcePropertySymbol { Name: "P1", IsAutoProperty: false, UsesFieldKeyword: false, BackingField: null });
Assert.True(actualProperties[1] is SourcePropertySymbol { Name: "P2", IsAutoProperty: false, UsesFieldKeyword: false, BackingField: null });
Assert.True(actualProperties[2] is SourcePropertySymbol { Name: "P3", IsAutoProperty: false, UsesFieldKeyword: false, BackingField: null });
Assert.True(actualProperties[3] is SourcePropertySymbol { Name: "P4", IsAutoProperty: false, UsesFieldKeyword: false, BackingField: null });
Assert.True(actualProperties[4] is SourcePropertySymbol { Name: "P5", IsAutoProperty: false, UsesFieldKeyword: false, BackingField: null });
Assert.True(actualProperties[5] is SourcePropertySymbol { Name: "P6", IsAutoProperty: false, UsesFieldKeyword: false, BackingField: null });
Assert.True(actualProperties[6] is SourcePropertySymbol { Name: "P7", IsAutoProperty: false, UsesFieldKeyword: false, BackingField: null });

VerifyMergedProperties(actualProperties, actualFields);
}

[Fact]
public void ObsoleteAttribute()
{
Expand Down

0 comments on commit f49638e

Please sign in to comment.