-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Improve arity error message to prefer generic type when both generic and non-generic exist #80707
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
base: main
Are you sure you want to change the base?
Conversation
…and non-generic exist Co-authored-by: CyrusNajmabadi <[email protected]>
Co-authored-by: CyrusNajmabadi <[email protected]>
} | ||
} | ||
|
||
private static bool PreferGenericOverNonGeneric(Symbol currentSymbol, Symbol newSymbol) |
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 make this a static local function inside MergeEqual
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.
Done in commit 7b5a8c6.
[Fact] | ||
public void TestGenericAndNonGenericType() | ||
{ | ||
var text = @" |
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 make all these test strings into raw string literals.
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.
Done in commit a2c4d9f.
{ | ||
public class ImprovedArityErrorTests : CSharpTestBase | ||
{ | ||
[Fact] |
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.
add an appropriate WorkItem attribute to all these tests.
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.
Done in commit a2c4d9f.
Co-authored-by: CyrusNajmabadi <[email protected]>
Co-authored-by: CyrusNajmabadi <[email protected]>
@dotnet/roslyn-ide ptal. |
….com/dotnet/roslyn into copilot/fix-inaccurate-error-message
@dotnet/roslyn-compiler ptal. |
1 similar comment
@dotnet/roslyn-compiler ptal. |
Summary
This PR fixes an inaccurate error message that occurs when a user supplies type arguments to a type name that has both generic and non-generic versions.
Problem
When attempting to use a type with the wrong number of type arguments, if both a generic and non-generic version exist with the same name, the compiler incorrectly reports an error about the non-generic version instead of the generic one.
For example, consider this code:
Before this fix, the compiler reported:
This is confusing because:
MyExpression<T>
does existAfter this fix, the compiler reports:
This is much clearer because:
Implementation
The fix modifies the lookup result merging logic in
LookupResult.MergeEqual(SingleLookupResult)
to add special handling forWrongArity
results. When both a generic type (arity > 0) and a non-generic type (arity == 0) are found with wrong arity, the logic now prefers reporting the error about the generic type.The implementation is minimal and surgical:
PreferGenericOverNonGeneric
static local function to determine which symbol to preferMergeEqual
method to use this logic when both results haveLookupResultKind.WrongArity
Testing
Added 5 comprehensive test cases in
ImprovedArityErrorTests.cs
covering:Updated 1 existing test in
BindingTests.cs
that was validating the old behaviorAll existing arity-related tests pass (1,293 tests)
No regressions found in semantic error tests
Fixes #366
Fixes #24406
Original prompt
Fixes #24406
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.