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

Jump to bottom

Disallow declaration of indexers in absence of proper DefaultMemberAttribute. #75099

Merged
merged 5 commits into from
Sep 30, 2024
Merged

Disallow declaration of indexers in absence of proper DefaultMemberAttribute. #75099

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
1c90a61
Suppress an assert when an indexer is declared, but DefaultMemberName…
AlekseyTs Sep 12, 2024
fbe35b9
Adjust test
AlekseyTs Sep 17, 2024
9aeb480
Adjust test
AlekseyTs Sep 17, 2024
4009e2e
Merge remote-tracking branch 'upstream/main' into Issue75032
AlekseyTs Sep 27, 2024
691e812
Disallow declaration of indexers in absence of proper DefaultMemberAt…
AlekseyTs Sep 27, 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
12 changes: 12 additions & 0 deletions docs/compilers/CSharp/Compiler Breaking Changes - DotNet 9.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,15 @@ class C
}
}
```


## Declaration of indexers in absence of proper declaration of DefaultMemberAttribute is no longer allowed.

***Introduced in Visual Studio 2022 version 17.13***

```cs
public interface I1
{
public I1 this[I1 args] { get; } // error CS0656: Missing compiler required member 'System.Reflection.DefaultMemberAttribute..ctor'
}
```
31 changes: 17 additions & 14 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1796,27 +1796,30 @@ protected override void AfterMembersCompletedChecks(BindingDiagnosticBag diagnos
Debug.Assert(ObsoleteKind != ObsoleteAttributeKind.Uninitialized);
Debug.Assert(GetMembers().All(m => m.ObsoleteKind != ObsoleteAttributeKind.Uninitialized));

if (ObsoleteKind != ObsoleteAttributeKind.None
|| GetMembers().All(m => m is not MethodSymbol { MethodKind: MethodKind.Constructor, ObsoleteKind: ObsoleteAttributeKind.None } method
if (ObsoleteKind == ObsoleteAttributeKind.None
&& !GetMembers().All(m => m is not MethodSymbol { MethodKind: MethodKind.Constructor, ObsoleteKind: ObsoleteAttributeKind.None } method
|| !method.ShouldCheckRequiredMembers()))
{
return;
}

foreach (var member in GetMembers())
{
if (!member.IsRequired())
foreach (var member in GetMembers())
{
continue;
}
if (!member.IsRequired())
{
continue;
}

if (member.ObsoleteKind != ObsoleteAttributeKind.None)
{
// Required member '{0}' should not be attributed with 'ObsoleteAttribute' unless the containing type is obsolete or all constructors are obsolete.
diagnostics.Add(ErrorCode.WRN_ObsoleteMembersShouldNotBeRequired, member.GetFirstLocation(), member);
if (member.ObsoleteKind != ObsoleteAttributeKind.None)
{
// Required member '{0}' should not be attributed with 'ObsoleteAttribute' unless the containing type is obsolete or all constructors are obsolete.
diagnostics.Add(ErrorCode.WRN_ObsoleteMembersShouldNotBeRequired, member.GetFirstLocation(), member);
}
}
}

if (Indexers.FirstOrDefault() is PropertySymbol indexerSymbol)
{
Binder.GetWellKnownTypeMember(DeclaringCompilation, WellKnownMember.System_Reflection_DefaultMemberAttribute__ctor, diagnostics, indexerSymbol.TryGetFirstLocation() ?? GetFirstLocation());
Copy link
Member

@jcouv jcouv Sep 30, 2024 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

indexerSymbol.TryGetFirstLocation() ?? GetFirstLocation()

nit: I didn't follow the intricacy of the Location logic. Do we have a test where indexerSymbol.TryGetFirstLocation() is null? #Resolved

Copy link
Contributor Author

@AlekseyTs AlekseyTs Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is a defense in depth, theoretically the API can return null, even though we don't expect it to. This pattern is also used elsewhere in this function.

}

if (TypeKind == TypeKind.Struct && !IsRecordStruct && HasInlineArrayAttribute(out _))
{
if (Layout.Kind == LayoutKind.Explicit)
Expand Down
76 changes: 76 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/IndexerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.CSharp.UnitTests.Emit;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Test.Utilities;
using Xunit;
Expand Down Expand Up @@ -1185,5 +1186,80 @@ static void Main()
}

#endregion Lowering

[Fact]
[WorkItem("https://github.com/dotnet/roslyn/issues/75032")]
public void MissingDefaultMemberAttribute_01()
{
var text1 = @"
public interface I1
{
public I1 this[I1 args] { get; }
}
";
var comp1 = CreateCompilation(text1);
comp1.MakeMemberMissing(WellKnownMember.System_Reflection_DefaultMemberAttribute__ctor);

comp1.VerifyDiagnostics(
// (4,15): error CS0656: Missing compiler required member 'System.Reflection.DefaultMemberAttribute..ctor'
// public I1 this[I1 args] { get; }
Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "this").WithArguments("System.Reflection.DefaultMemberAttribute", ".ctor").WithLocation(4, 15)
);

comp1 = CreateCompilation(text1);
comp1.MakeTypeMissing(WellKnownType.System_Reflection_DefaultMemberAttribute);

comp1.VerifyDiagnostics(
// (4,15): error CS0656: Missing compiler required member 'System.Reflection.DefaultMemberAttribute..ctor'
// public I1 this[I1 args] { get; }
Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "this").WithArguments("System.Reflection.DefaultMemberAttribute", ".ctor").WithLocation(4, 15)
);

comp1 = CreateCompilation(text1);
comp1.VerifyEmitDiagnostics();
}

[Fact]
[WorkItem("https://github.com/dotnet/roslyn/issues/75032")]
public void MissingDefaultMemberAttribute_02()
{
var text1 = @"
public interface I1
{
public I1 this[I1 args] { get; }
}
";
var comp1 = CreateCompilation(text1);

var text2 = @"
class C : I1
{
I1 I1.this[I1 args] => throw null;
}
";
var comp2 = CreateCompilation(text2, references: [comp1.ToMetadataReference()]);
comp2.MakeMemberMissing(WellKnownMember.System_Reflection_DefaultMemberAttribute__ctor);
comp2.VerifyEmitDiagnostics();
}

[Fact]
[WorkItem("https://github.com/dotnet/roslyn/issues/75032")]
public void MissingDefaultMemberAttribute_03()
{
var text1 = @"
using System.Collections.Generic;

class Program
{
static IEnumerable<int> Test()
{
return [123];
}
}
";
var comp1 = CreateCompilation(text1);
comp1.MakeMemberMissing(WellKnownMember.System_Reflection_DefaultMemberAttribute__ctor);
CompileAndVerify(comp1).VerifyDiagnostics();
}
}
}
9 changes: 8 additions & 1 deletion src/Compilers/CSharp/Test/Emit/Emit/InAttributeModifierTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1565,6 +1565,9 @@ class Test
}";

CreateEmptyCompilation(code).VerifyDiagnostics(
// (9,27): error CS0656: Missing compiler required member 'System.Reflection.DefaultMemberAttribute..ctor'
// public virtual object this[in object p] => null;
Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "this").WithArguments("System.Reflection.DefaultMemberAttribute", ".ctor").WithLocation(9, 27),
// (9,32): error CS0518: Predefined type 'System.Runtime.InteropServices.InAttribute' is not defined or imported
// public virtual object this[in object p] => null;
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "in object p").WithArguments("System.Runtime.InteropServices.InAttribute").WithLocation(9, 32));
Expand All @@ -1588,7 +1591,11 @@ class Test
CreateEmptyCompilation(code).VerifyDiagnostics(
// (10,20): error CS0518: Predefined type 'System.Runtime.InteropServices.InAttribute' is not defined or imported
// public virtual ref readonly object this[object p] => ref value;
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "ref readonly object").WithArguments("System.Runtime.InteropServices.InAttribute").WithLocation(10, 20));
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "ref readonly object").WithArguments("System.Runtime.InteropServices.InAttribute").WithLocation(10, 20),
// (10,40): error CS0656: Missing compiler required member 'System.Reflection.DefaultMemberAttribute..ctor'
// public virtual ref readonly object this[object p] => ref value;
Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "this").WithArguments("System.Reflection.DefaultMemberAttribute", ".ctor").WithLocation(10, 40)
);
}

[Fact]
Expand Down
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IArgument.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2588,6 +2588,9 @@ void M1()
// (6,37): error CS0518: Predefined type 'System.Int32' is not defined or imported
// public int this[int x, int? y = 5]
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "5").WithArguments("System.Int32").WithLocation(6, 37),
// (6,16): error CS0656: Missing compiler required member 'System.Reflection.DefaultMemberAttribute..ctor'
// public int this[int x, int? y = 5]
Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "this").WithArguments("System.Reflection.DefaultMemberAttribute", ".ctor").WithLocation(6, 16),
// (5,27): error CS0518: Predefined type 'System.Int32' is not defined or imported
// private int _number = 0;
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "0").WithArguments("System.Int32").WithLocation(5, 27),
Expand Down Expand Up @@ -2766,6 +2769,9 @@ void M1()
// (5,12): error CS0518: Predefined type 'System.Int32' is not defined or imported
// public int this[int x, int? y = null]
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "int").WithArguments("System.Int32").WithLocation(5, 12),
// (5,16): error CS0656: Missing compiler required member 'System.Reflection.DefaultMemberAttribute..ctor'
// public int this[int x, int? y = null]
Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "this").WithArguments("System.Reflection.DefaultMemberAttribute", ".ctor").WithLocation(5, 16),
// (5,21): error CS0518: Predefined type 'System.Int32' is not defined or imported
// public int this[int x, int? y = null]
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "int").WithArguments("System.Int32").WithLocation(5, 21),
Expand Down Expand Up @@ -3829,6 +3835,9 @@ P M1()
// (5,37): error CS0518: Predefined type 'System.Int32' is not defined or imported
// public int this[int x, int? y = 0]
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "0").WithArguments("System.Int32").WithLocation(5, 37),
// (5,16): error CS0656: Missing compiler required member 'System.Reflection.DefaultMemberAttribute..ctor'
// public int this[int x, int? y = 0]
Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "this").WithArguments("System.Reflection.DefaultMemberAttribute", ".ctor").WithLocation(5, 16),
// (4,27): error CS0518: Predefined type 'System.Int32' is not defined or imported
// private int _number = 0;
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "0").WithArguments("System.Int32").WithLocation(4, 27),
Expand Down Expand Up @@ -3903,6 +3912,9 @@ P M1()
// (5,12): error CS0518: Predefined type 'System.Int32' is not defined or imported
// public int this[int x, int? y = null]
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "int").WithArguments("System.Int32").WithLocation(5, 12),
// (5,16): error CS0656: Missing compiler required member 'System.Reflection.DefaultMemberAttribute..ctor'
// public int this[int x, int? y = null]
Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "this").WithArguments("System.Reflection.DefaultMemberAttribute", ".ctor").WithLocation(5, 16),
// (5,21): error CS0518: Predefined type 'System.Int32' is not defined or imported
// public int this[int x, int? y = null]
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "int").WithArguments("System.Int32").WithLocation(5, 21),
Expand Down
11 changes: 9 additions & 2 deletions src/Compilers/CSharp/Test/Semantic/Semantics/InitOnlyMemberTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#nullable disable

using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
Expand Down Expand Up @@ -3462,7 +3463,10 @@ void M(C c)
";

var reference = CreateMetadataReferenceFromIlSource(il);
var comp = CreateCompilation(source, references: new[] { reference }, parseOptions: TestOptions.Regular9);
var comp = CreateCompilation(source, references: new[] { reference }, parseOptions: TestOptions.Regular9,
options: TestOptions.ReleaseDll.WithSpecificDiagnosticOptions(
// warning CS1685: The predefined type 'DefaultMemberAttribute' is defined in multiple assemblies
ImmutableDictionary<string, ReportDiagnostic>.Empty.Add("CS1685", ReportDiagnostic.Suppress)));
comp.VerifyEmitDiagnostics(
// (4,25): error CS0569: 'Derived.this[int]': cannot override 'C.this[int]' because it is not supported by the language
// public override int this[int i] { set { throw null; } }
Expand Down Expand Up @@ -3558,7 +3562,10 @@ public class Derived2 : C
";

var reference = CreateMetadataReferenceFromIlSource(il);
var comp = CreateCompilation(source, references: new[] { reference }, parseOptions: TestOptions.Regular9);
var comp = CreateCompilation(source, references: new[] { reference }, parseOptions: TestOptions.Regular9,
options: TestOptions.ReleaseDll.WithSpecificDiagnosticOptions(
// warning CS1685: The predefined type 'DefaultMemberAttribute' is defined in multiple assemblies
ImmutableDictionary<string, ReportDiagnostic>.Empty.Add("CS1685", ReportDiagnostic.Suppress)));
comp.VerifyEmitDiagnostics(
// (4,39): error CS0570: 'C.this[int].set' is not supported by the language
// public override int this[int i] { set { throw null; } }
Expand Down