Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ var symbolInfo = semanticModel.GetSymbolInfo(expression);
- Test utilities in `Microsoft.CodeAnalysis.Test.Utilities`
- Language-specific test bases: `CSharpTestBase`, `VisualBasicTestBase`
- Add `[WorkItem("https://github.com/dotnet/roslyn/issues/issueNumber")]` attribute to tests that fix specific GitHub issues
- Prefer raw string literals (`"""..."""`) over verbatim strings (`@"..."`) when creating test source code
- Avoid unnecessary intermediary assertions - tests should do the minimal amount of work to validate just the core issue being addressed
Copy link
Member

Choose a reason for hiding this comment

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

@copilot. Update this. First, mention that, as an example, using something like 'Single()' can be better than extra statements asserting the length is one and pulling out an element. Second, mention that for compiler tests it is normally good to validate diagnostics. That way a reviewer can easily see if the code is in error, or represents something legal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated copilot-instructions.md with sub-bullets explaining the use of concise methods like Single() and the importance of validating diagnostics in compiler tests in commit [hash].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated copilot-instructions.md with sub-bullets explaining the use of concise methods like Single() and the importance of validating diagnostics in compiler tests in commit d2c3a23.

- In tests, use concise methods like `.Single()` instead of asserting count and extracting elements
- For compiler tests, validate diagnostics (e.g., `comp.VerifyEmitDiagnostics()`) so reviewers can easily see if the code is in error or represents something legal

## Critical Integration Points

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7460,5 +7460,88 @@ 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/41733")]
public void TypeParameter_BaseType_ReturnsNull()
{
var source = """
abstract class Base
{
public abstract void Method();
}

class Derived<T> where T : Base
{
}
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics();

var typeParameter = (ITypeParameterSymbol)comp.GetTypeByMetadataName("Derived`1").TypeParameters[0].GetPublicSymbol();
Assert.Null(typeParameter.BaseType);
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Oct 20, 2025

Choose a reason for hiding this comment

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

@copilot please update the copilot-instructions file to mention that tests should strive to avoid unnecessary intermediary asserts, and do the minimal amount of work to validate just the core issue being addressed by the PR. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated copilot-instructions.md to include guidance about avoiding unnecessary intermediary assertions in commit [hash].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated copilot-instructions.md to include guidance about avoiding unnecessary intermediary assertions in commit 0e3430a.

}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/41733")]
public void MethodTypeParameter_BaseType_ReturnsNull()
{
var source = """
class C
{
void M<T>() where T : class
{
}
}
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics();

var typeParameter = (ITypeParameterSymbol)comp.GetTypeByMetadataName("C").GetMethod("M").TypeParameters[0].GetPublicSymbol();
Assert.Null(typeParameter.BaseType);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/41733")]
public void ObjectType_BaseType_ReturnsNull()
{
var comp = CreateCompilation("");
comp.VerifyDiagnostics();

var objectType = (INamedTypeSymbol)comp.GetSpecialType(SpecialType.System_Object).GetPublicSymbol();
Assert.Null(objectType.BaseType);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/41733")]
public void InterfaceType_BaseType_ReturnsNull()
{
var source = """
interface IMyInterface
{
void Method();
}
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics();

var interfaceType = (INamedTypeSymbol)comp.GetTypeByMetadataName("IMyInterface").GetPublicSymbol();
Assert.Null(interfaceType.BaseType);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/41733")]
public void PointerType_BaseType_ReturnsNull()
{
var source = """
unsafe class C
{
int* ptr;
}
""";
var comp = CreateCompilation(source, options: TestOptions.UnsafeDebugDll);
Copy link
Member

Choose a reason for hiding this comment

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

@copilot Restore the VerifyEmitDiagnostics() checks on compilations in every test you added. If the compilation had errors (not necessarily trivial for human reviewers to spot) that might cause the behavior observed below (null BaseType) for incorrect reasons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restored VerifyDiagnostics() checks to all 5 tests in commit [hash]. Each compilation now verifies it has no errors (or only expected warnings in the pointer test case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restored VerifyDiagnostics() checks to all 5 tests in commit 3dd27c2. Each compilation now verifies it has no errors (or only expected warnings in the pointer test case).

comp.VerifyDiagnostics(
// (3,10): warning CS0169: The field 'C.ptr' is never used
// int* ptr;
Diagnostic(ErrorCode.WRN_UnreferencedField, "ptr").WithArguments("C.ptr").WithLocation(3, 10));

var pointerType = (IPointerTypeSymbol)comp.GetTypeByMetadataName("C").GetField("ptr").Type.GetPublicSymbol();
Assert.Null(pointerType.BaseType);
}
}
}
3 changes: 1 addition & 2 deletions src/Compilers/Core/Portable/Symbols/ITypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests for all cases in commit [hash]. The new tests cover:

  • MethodTypeParameter_BaseType_ReturnsNull: method type parameters
  • ObjectType_BaseType_ReturnsNull: System.Object
  • InterfaceType_BaseType_ReturnsNull: interface types
  • PointerType_BaseType_ReturnsNull: pointer types

All 5 tests pass successfully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests for all cases in commit e915694. The new tests cover:

  • MethodTypeParameter_BaseType_ReturnsNull: method type parameters
  • ObjectType_BaseType_ReturnsNull: System.Object
  • InterfaceType_BaseType_ReturnsNull: interface types
  • PointerType_BaseType_ReturnsNull: pointer types

All 5 tests pass successfully.

/// </summary>
INamedTypeSymbol? BaseType { get; }

Expand Down