Support basic cases in UseRangeOperator #45693
Conversation
| return false; | ||
| } | ||
|
|
||
| if (_methodToMemberInfo.TryGetValue(method, out memberInfo)) |
There was a problem hiding this comment.
| if (_methodToMemberInfo.TryGetValue(method, out memberInfo)) | |
| if (!_methodToMemberInfo.TryGetValue(method, out memberInfo)) |
then have hte logic to try to produce the value in the if-statement.
|
|
||
| /// <summary> | ||
| /// Can be null, if we are dealing with one-argument call to a slice-like method. | ||
| /// </summary> | ||
| public readonly IOperation Op2; |
There was a problem hiding this comment.
| public readonly IOperation Op2; | |
| public readonly IOperation? Op2; |
| var targetMethod = invocation.TargetMethod; | ||
|
|
||
| // Try to see if we're calling into some sort of Slice method with a matching | ||
| // indexer or overload |
There was a problem hiding this comment.
i'm not finding tehse comments helpful. having real world examples of the before/after would be more helpful here imo.
There was a problem hiding this comment.
Agreed. I re-worded the comment.
| targetMethod, | ||
| memberInfo, | ||
| startOperation, | ||
| null); // secondOperation is null: the range will run to the end. |
| null); // secondOperation is null: the range will run to the end. | ||
| } | ||
|
|
||
| private static Result? AnalyzeInvocationWithTwoArguments( |
There was a problem hiding this comment.
| private static Result? AnalyzeInvocationWithTwoArguments( | |
| private static Result? AnalyzeTwoArgumentInvocation( |
There was a problem hiding this comment.
Also renamed AnalyzeInvocationWithOneArgument to AnalyzeOneArgumentInvocation.
| // From: https://github.com/dotnet/csharplang/blob/master/proposals/csharp-8.0/ranges.md#decisions-made-during-implementation | ||
| // | ||
| // When looking for the pattern members, we look for original definitions, not | ||
| // constructed members |
There was a problem hiding this comment.
just make this a normal method. breaking up an expression with complex comments like this just makes things too hard to read.
| // of the range. i.e. `start..` | ||
| if (IsInstanceLengthCheck(lengthLikeProperty, instance, endOperation)) | ||
| var endFromEnd = false; | ||
| ExpressionSyntax endExpr; |
There was a problem hiding this comment.
can we just make this null, and then just set it in the case where we have it?
| var overloadWithTwoArguments = method.ContainingType | ||
| .GetMembers(method.Name) | ||
| .OfType<IMethodSymbol>() | ||
| .FirstOrDefault(s => IsSliceLikeMethod(s)); |
There was a problem hiding this comment.
should we rename IsSliceLikeMethod to IsTwoArgumentSliceLikeMethod?
There was a problem hiding this comment.
Fixed; also renamed IsSliceLikeMethodWithOneArgument to IsOneArgumentSliceLikeMethod.
|
Nice start! |
|
Feel free to ping me when this passes, and i'll merge in. Thanks! |
|
@CyrusNajmabadi thanks for the feedback! It’s ready for merging now. |
|
Thank you for the contribution! |
ghost
added this to the 
jinujoseph
added
Area-IDE
Community
|
@mahalex Nice! |
Fixes #45637