From f49638ea2fd6426b7d2a586b23f36df62e613aae Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Thu, 10 Oct 2024 23:27:33 -0700 Subject: [PATCH] Field-backed properties: [field:] should not be considered a backing field reference (#75461) --- .../Symbols/Source/SourcePropertySymbol.cs | 36 +++++++-- .../CSharp/Test/Emit3/FieldKeywordTests.cs | 81 +++++++++++++++++++ 2 files changed, 111 insertions(+), 6 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index a76fc7bd75edf..0f82cb5a6c5b1 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -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); @@ -264,7 +267,7 @@ private static void GetAccessorDeclarations( throw ExceptionUtilities.UnexpectedValue(accessor.Kind()); } - usesFieldKeyword = usesFieldKeyword || containsFieldKeyword(accessor); + usesFieldKeyword = usesFieldKeyword || containsFieldExpressionInAccessor(accessor); } } else @@ -272,7 +275,7 @@ private static void GetAccessorDeclarations( 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. } @@ -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; diff --git a/src/Compilers/CSharp/Test/Emit3/FieldKeywordTests.cs b/src/Compilers/CSharp/Test/Emit3/FieldKeywordTests.cs index 6e7364596a931..3ffc8cd8c38cd 100644 --- a/src/Compilers/CSharp/Test/Emit3/FieldKeywordTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/FieldKeywordTests.cs @@ -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("C"); + var actualFields = containingType.GetMembers().OfType().ToImmutableArray(); + AssertEx.Equal([], actualFields); + + var actualProperties = containingType.GetMembers().OfType().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() {