-
Notifications
You must be signed in to change notification settings - Fork 753
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
Fix Resource access that are not ambiguous are reported as ambiguous access
#43589
base: master
Are you sure you want to change the base?
Conversation
.../src/test/java/io/ballerina/semantic/api/test/actions/SymbolsInResourceAccessActionTest.java
Outdated
Show resolved
Hide resolved
…emantic/api/test/actions/SymbolsInResourceAccessActionTest.java
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #43589 +/- ##
=========================================
Coverage 77.55% 77.56%
- Complexity 58742 58753 +11
=========================================
Files 3448 3448
Lines 219709 219735 +26
Branches 28915 28923 +8
=========================================
+ Hits 170401 170430 +29
+ Misses 39920 39909 -11
- Partials 9388 9396 +8 ☔ View full report in Codecov by Sentry. |
Optional<BResourceFunction> first = resourceFunctions.stream().filter(func -> func.pathSegmentSymbols | ||
.get(func.pathSegmentSymbols.size() - 1).kind == SymbolKind.RESOURCE_PATH_IDENTIFIER_SEGMENT) | ||
.findFirst(); |
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.
This will only handle the test case in the issue right? We need to handle the bellow cases as well
case1:
public client class SimpleClient {
resource function get metadata/a() returns string {
return "";
}
resource function get [string id]/a() returns string {
return "";
}
}
case2:
public client class SimpleClient {
resource function get metadata/[string]() returns string {
return "";
}
resource function get [string]/a() returns string {
return "";
}
}
sampleClient->/metadata/a; // which method to invoke. Should be give priority to first match in the path?
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.
Case 1 is handled by the fix. case 2 should be ambiguous, because both resource methods are candidates, right?
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.
Yes, 2nd is ambiguous. Case one wont work if we change the order of the resource methods
public client class SimpleClient {
resource function get [string]/a/[string]/c() returns string {
return "";
}
resource function get metadata/[string]/b/c() returns string {
return "";
}
}
sampleClient->/metadata/a/b/c; We need to think wether we give ambiguous error for this or not. Second resource method maches with three singleton types while first matches with only two. Hence should we pick the second from the compile time? We also needs to add test to check wether the correct method is invoked and results are returned |
Purpose
Fixes #40606
Approach
Samples
Remarks
Check List