List-patterns: report error when no suitable length member #59475
Conversation
| @@ -359,7 +359,12 @@ private bool IsCountableAndIndexable(SyntaxNode node, TypeSymbol inputType, out | |||
| } | |||
| else | |||
| { | |||
| hasErrors |= !TryBindLengthOrCount(node, receiverPlaceholder, out lengthAccess, diagnostics); | |||
There was a problem hiding this comment.
Surprised that this wasn't sufficient. Pretty sure we had tests with missing members. How this was still possible? #Resolved
There was a problem hiding this comment.
The tests with missing/bad length all used an int indexer, so produced other errors. But when we have a System.Index indexer, we'd produce a bound tree with an error but no diagnostics.
jcouv
force-pushed
the
missing-length
branch
from
75078d8 to
8e2a3a8
Compare
|
@333fred @dotnet/roslyn-compiler for review. Thanks |
| @@ -359,7 +359,12 @@ private bool IsCountableAndIndexable(SyntaxNode node, TypeSymbol inputType, out | |||
| } | |||
| else | |||
| { | |||
| hasErrors |= !TryBindLengthOrCount(node, receiverPlaceholder, out lengthAccess, diagnostics); | |||
| var foundLengthOrCount = TryBindLengthOrCount(node, receiverPlaceholder, out lengthAccess, diagnostics); | |||
| if (!foundLengthOrCount && !hasErrors) | |||
There was a problem hiding this comment.
It looks like the only situation when hasErrors can be true here is when inputType.IsDynamic(). Why even trying to locate Length/Count in that case, we know it is going to fail. It might make sense to refactor the code according to this, i.e. base the logic on the type, rather than on hasErrors.
| @@ -3651,7 +3657,7 @@ public void ListPattern_StringLength() | |||
| class C | |||
| { | |||
| public string Length => throw null; | |||
| public int this[int i] => throw null; | |||
| public int this[System.Index i] => throw null; | |||
| @@ -7727,7 +7745,7 @@ public void ListPattern_SetOnlyIndexers() | |||
|
|
|||
| class C | |||
| { | |||
| public int Length { set { } } | |||
| public int Length => 0; | |||
| // (2,16): error CS8985: List patterns may not be used for a value of type 'S'. No suitable 'Length' or 'Count' property was found. | ||
| // _ = new S() is []; | ||
| Diagnostic(ErrorCode.ERR_ListPatternRequiresLength, "[]").WithArguments("S").WithLocation(2, 16), | ||
| // (2,16): error CS1503: Argument 1: cannot convert from 'System.Index' to 'int' |
There was a problem hiding this comment.
Agreed the error could be confusing.
FWIW, is the same error you get if you try to index with System.Index into this type:
_ = new S()[^1]; // error CS1503: Argument 1: cannot convert from 'System.Index' to 'int'
struct S
{
public int this[int i] => i;
}
I'm not planning to address in this PR, feel free to open an issue.
| } | ||
|
|
||
| [Fact, WorkItem(59465, "https://github.com/dotnet/roslyn/issues/59465")] | ||
| public void MissingLength_PrivateLength() |
There was a problem hiding this comment.
Yes, all the previous tests for those scenarios (bad Length, such as ListPattern_StringLength, SlicePattern_VoidReturn, SlicePattern_LengthAndIndexAndSliceAreStatic, ListPattern_UintCount, ListPattern_LengthFieldNotApplicable, ...) used int Indexer. That's why this problem was missed.
|
Done with review pass (commit 2) |
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
ghost
added this to the 
Fixes #59465
Relates to test plan #51289
FYI @alrz