Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix Resource access that are not ambiguous are reported as ambiguous access #43589

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix Resource access that are not ambiguous are reported as ambiguous access #43589

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1445180
Fix #40606
gimantha Nov 17, 2024
8b3ca8e
Fix checkstyle issue
gimantha Nov 17, 2024
06a71a1
Fix test failure
gimantha Nov 17, 2024
d0b026e
Update tests/ballerina-compiler-api-test/src/test/java/io/ballerina/s…
gimantha Nov 17, 2024
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
28 changes: 19 additions & 9 deletions ...na-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4085,17 +4085,27 @@ public void visit(BLangInvocation.BLangResourceAccessInvocation resourceAccessIn
handleResourceAccessError(resourceAccessInvocation.resourceAccessPathSegments,
resourceAccessInvocation.name.pos, DiagnosticErrorCode.UNDEFINED_RESOURCE_METHOD, data,
resourceAccessInvocation.name, lhsExprType);
return;
} else if (targetResourceFuncCount > 1) {
handleResourceAccessError(resourceAccessInvocation.resourceAccessPathSegments, resourceAccessInvocation.pos,
DiagnosticErrorCode.AMBIGUOUS_RESOURCE_ACCESS_NOT_YET_SUPPORTED, data, lhsExprType);
} else {
BResourceFunction targetResourceFunc = resourceFunctions.get(0);
checkExpr(resourceAccessInvocation.resourceAccessPathSegments,
getResourcePathType(targetResourceFunc.pathSegmentSymbols), data);
resourceAccessInvocation.symbol = targetResourceFunc.symbol;
resourceAccessInvocation.targetResourceFunc = targetResourceFunc;
checkResourceAccessParamAndReturnType(resourceAccessInvocation, targetResourceFunc, data);
//Filter the resource function with identifier segment
Optional<BResourceFunction> first = resourceFunctions.stream().filter(func -> func.pathSegmentSymbols
.get(func.pathSegmentSymbols.size() - 1).kind == SymbolKind.RESOURCE_PATH_IDENTIFIER_SEGMENT)
.findFirst();
Comment on lines +4091 to +4093
Copy link
Contributor

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?

Copy link
Contributor Author

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?

Copy link
Contributor

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

if (first.isPresent()) {
resourceFunctions = new ArrayList<>(List.of(first.get()));
} else {
handleResourceAccessError(resourceAccessInvocation.resourceAccessPathSegments,
resourceAccessInvocation.pos, DiagnosticErrorCode.AMBIGUOUS_RESOURCE_ACCESS_NOT_YET_SUPPORTED,
data, lhsExprType);
return;
}
}
BResourceFunction targetResourceFunc = resourceFunctions.get(0);
checkExpr(resourceAccessInvocation.resourceAccessPathSegments,
getResourcePathType(targetResourceFunc.pathSegmentSymbols), data);
resourceAccessInvocation.symbol = targetResourceFunc.symbol;
resourceAccessInvocation.targetResourceFunc = targetResourceFunc;
checkResourceAccessParamAndReturnType(resourceAccessInvocation, targetResourceFunc, data);
}

private void handleResourceAccessError(BLangListConstructorExpr resourceAccessPathSegments,
Expand Down
2 changes: 1 addition & 1 deletion ...c/test/java/io/ballerina/semantic/api/test/actions/SymbolsInResourceAccessActionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public Object[][] getResourceAccessActionPathPos() {
@Test
public void testPathSegmentOfAmbiguousResourceFunction() {
Optional<Symbol> symbol = model.symbol(srcFile, LinePosition.from(91, 9));
assertTrue(symbol.isEmpty()); // Resource method is ambiguous
assertTrue(symbol.isPresent()); // Resource method is ambiguous
gimantha marked this conversation as resolved.
Show resolved Hide resolved
}

// Utils
Expand Down
8 changes: 0 additions & 8 deletions ...t/src/test/java/org/ballerinalang/test/action/ClientResourceAccessActionNegativeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,18 +124,10 @@ public void testClientResourceCallNegative() {
"too many arguments in call to 'post()'", 90, 13);
validateError(clientResourceAccessNegative, index++,
"too many arguments in call to 'post()'", 91, 13);
validateError(clientResourceAccessNegative, index++, "client resource access action is not yet " +
"supported when the corresponding resource method is ambiguous", 145, 13);
validateError(clientResourceAccessNegative, index++, "client resource access action is not yet " +
"supported when the corresponding resource method is ambiguous", 146, 13);
validateError(clientResourceAccessNegative, index++, "client resource access action is not yet " +
"supported when the corresponding resource method is ambiguous", 147, 13);
validateError(clientResourceAccessNegative, index++, "client resource access action is not yet " +
"supported when the corresponding resource method is ambiguous", 148, 13);
validateError(clientResourceAccessNegative, index++, "client resource access action is not yet " +
"supported when the corresponding resource method is ambiguous", 149, 13);
validateError(clientResourceAccessNegative, index++, "client resource access action is not yet " +
"supported when the corresponding resource method is ambiguous", 150, 13);
validateError(clientResourceAccessNegative, index++, "client resource access action is not yet " +
"supported when the corresponding resource method is ambiguous", 151, 13);
validateError(clientResourceAccessNegative, index++, "client resource access action is not yet " +
Expand Down
Loading