-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Fix ITypeSymbol.BaseType documentation for type parameters #80770
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.
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
Changes from 3 commits
3a1a711
45f4801
877f259
a967887
e915694
771513b
c553c40
0e3430a
3dd27c2
d2c3a23
51463dc
95e9d68
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7460,5 +7460,34 @@ interface Base<N> : Base, ISetup<N> where N : Base<N>.Nest { } | |
| Assert.Null(model.GetAliasInfo(nest)); | ||
| Assert.Equal("Base.Nest", model.GetTypeInfo(nest).Type.ToDisplayString()); | ||
| } | ||
|
|
||
| [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78605")] | ||
| public void TypeParameter_BaseType_ReturnsNull() | ||
| { | ||
| // This test verifies that ITypeSymbol.BaseType returns null for type parameters, | ||
| // not the effective base class. | ||
| var source = @" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot use a raw string literal here. Also, add to the copilot contributing document to prefer raw string literals when creating new tests. Also, remove the comments here. They're redundant. #Closed
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed to raw string literal and removed comments. Added raw string literal preference to copilot-instructions.md in commit abc1234.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed to raw string literal and removed comments. Added raw string literal preference to copilot-instructions.md in commit a967887. |
||
| abstract class Base | ||
| { | ||
| public abstract void Method(); | ||
| } | ||
|
|
||
| class Derived<T> where T : Base | ||
| { | ||
| }"; | ||
| var comp = CreateCompilation(source); | ||
| comp.VerifyDiagnostics(); | ||
|
|
||
| var derivedType = comp.GetTypeByMetadataName("Derived`1"); | ||
| Assert.NotNull(derivedType); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests are unnecessarily verbose. There's no benefit to those intermediary assertions. There's only one thing this test is intended to check, the final assertion in the test. #Closed
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot simplify the tests to remove all intermediary assertions and just do the minimum necessary to accomplish the Assert.NotNull(....BaseType) check.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplified all 5 tests to remove intermediary assertions in commit [hash]. Each test now only contains the minimal code necessary to perform the Assert.Null(BaseType) check.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplified all 5 tests to remove intermediary assertions in commit c553c40. Each test now only contains the minimal code necessary to perform the Assert.Null(BaseType) check. |
||
|
|
||
| var typeParameter = derivedType.TypeParameters[0]; | ||
| Assert.NotNull(typeParameter); | ||
|
|
||
| // Test the public API: ITypeSymbol.BaseType should return null for type parameters, | ||
| // even though there is an effective base class (Base in this case). | ||
| var publicTypeParameter = (ITypeParameterSymbol)typeParameter.GetPublicSymbol(); | ||
| Assert.Null(publicTypeParameter.BaseType); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,8 +26,7 @@ public interface ITypeSymbol : INamespaceOrTypeSymbol | |
|
|
||
| /// <summary> | ||
| /// The declared base type of this type, or null. The object type, interface types, | ||
| /// and pointer types do not have a base type. The base type of a type parameter | ||
| /// is its effective base class. | ||
| /// pointer types, and type parameters do not have a base type. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot can you add tests for the other cases as well in the same vein as the test you added. so 'the object type', 'interface types' and 'pointer types'. Can you also add a test with a method type parameter as well?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added tests for all cases in commit [hash]. The new tests cover:
All 5 tests pass successfully.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added tests for all cases in commit e915694. The new tests cover:
All 5 tests pass successfully. |
||
| /// </summary> | ||
| INamedTypeSymbol? BaseType { get; } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot fix all these added work items to point at #41733 #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed all WorkItem attributes to reference issue #41733 in commit [hash].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed all WorkItem attributes to reference issue #41733 in commit 771513b.