Skip to content

Commit

Permalink
Address review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
nipunayf committed Dec 14, 2023
1 parent 44d4e65 commit 4a2b7a0
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,36 @@ public static String getFilterTextForClientResourceAccessAction(ResourceMethodSy
+ "|" + resourceMethodSymbol.getName().orElse("");
}

/**
* Check if the given expression node is a valid type for the given parameter node.
*
* @param paramType Type of the parameter
* @param exprType Type of the expression
* @param exprValue Value of the expression
* @return Returns true if the expression is a valid type for the parameter
*/
public static boolean checkSubtype(TypeSymbol paramType, TypeSymbol exprType, String exprValue) {
if (exprType.subtypeOf(paramType)) {
return true;
}
switch (paramType.typeKind()) {
case UNION:
for (TypeSymbol childSymbol : ((UnionTypeSymbol) paramType).memberTypeDescriptors()) {
if (checkSubtype(childSymbol, exprType, exprValue)) {
return true;
}
}
return false;

Check warning on line 214 in language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/completions/builder/ResourcePathCompletionUtil.java

View check run for this annotation

Codecov / codecov/patch

language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/completions/builder/ResourcePathCompletionUtil.java#L213-L214

Added lines #L213 - L214 were not covered by tests
case SINGLETON:
return paramType.subtypeOf(exprType) && exprValue.equals(paramType.signature());
case TYPE_REFERENCE:
return paramType.subtypeOf(exprType);
default:
return false;
}
}


private static ResourceAccessPathPart getResourceAccessPartForSegment(PathSegment segment, int placeHolderIndex,
BallerinaCompletionContext context) {

Expand All @@ -217,15 +247,19 @@ private static ResourceAccessPathPart getResourceAccessPartForSegment(PathSegmen
"[${" + placeHolderIndex + ":" + defaultValue.orElse("") + "}]";
ResourceAccessPathPart resourceAccessPathPart =
new ResourceAccessPathPart(computedInsertText, computedSignature);

// A resource method parameter can be a singleton or a union with a `string`. As the node "text" is always
// resolved to `string`, we need to check if typeSymbol is either a supertype or a subtype of `string`.
if (context.currentSemanticModel().isPresent() &&
checkSubtype(typeSymbol, context.currentSemanticModel().get().types().STRING)) {
isStringSubtype(typeSymbol, context.currentSemanticModel().get().types().STRING)) {
if (typeSymbol.typeKind().equals(TypeDescKind.SINGLETON)) {
resourceAccessPathPart.namedPathSignature =
typeSymbol.signature().substring(1, typeSymbol.signature().length() - 1);
resourceAccessPathPart.namedPathInsertText = resourceAccessPathPart.namedPathSignature;
} else {
resourceAccessPathPart.namedPathSignature = "<" +
(pathParameterSymbol.getName().isPresent() ? pathParameterSymbol.getName().get() : "path") + ">";
resourceAccessPathPart.namedPathSignature = "<" +
(pathParameterSymbol.getName().isPresent() ? pathParameterSymbol.getName().get() : "path") +
">";
resourceAccessPathPart.namedPathInsertText = "${" + placeHolderIndex + ":" + "path" + "}";
resourceAccessPathPart.computedPathInsertText = "[${" + placeHolderIndex + ":" + "\"path\"" + "}]";
}
Expand All @@ -234,14 +268,14 @@ private static ResourceAccessPathPart getResourceAccessPartForSegment(PathSegmen
return resourceAccessPathPart;
}

private static boolean checkSubtype(TypeSymbol paramType, TypeSymbol stringType) {
private static boolean isStringSubtype(TypeSymbol paramType, TypeSymbol stringType) {
if (stringType.subtypeOf(paramType)) {
return true;
}
switch (paramType.typeKind()) {
case UNION:
for (TypeSymbol childSymbol : ((UnionTypeSymbol) paramType).memberTypeDescriptors()) {
if (checkSubtype(childSymbol, stringType)) {
if (isStringSubtype(childSymbol, stringType)) {
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import io.ballerina.compiler.api.symbols.Symbol;
import io.ballerina.compiler.api.symbols.SymbolKind;
import io.ballerina.compiler.api.symbols.TypeSymbol;
import io.ballerina.compiler.api.symbols.UnionTypeSymbol;
import io.ballerina.compiler.api.symbols.resourcepath.PathRestParam;
import io.ballerina.compiler.api.symbols.resourcepath.PathSegmentList;
import io.ballerina.compiler.api.symbols.resourcepath.ResourcePath;
Expand Down Expand Up @@ -347,7 +346,8 @@ private Pair<List<PathSegment>, Boolean> completableSegmentList(ResourceMethodSy
if (node.kind() == SyntaxKind.COMPUTED_RESOURCE_ACCESS_SEGMENT) {
ExpressionNode exprNode = ((ComputedResourceAccessSegmentNode) node).expression();
Optional<TypeSymbol> exprType = semanticModel.get().typeOf(exprNode);
if (exprType.isEmpty() || !checkSubtype(typeSymbol, exprType.get(), exprNode.toString())) {
if (exprType.isEmpty() || !ResourcePathCompletionUtil.checkSubtype(typeSymbol, exprType.get(),
exprNode.toString())) {
return Pair.of(Collections.emptyList(), false);
}
if (segment.pathSegmentKind() == PathSegment.Kind.PATH_REST_PARAMETER
Expand All @@ -356,7 +356,8 @@ private Pair<List<PathSegment>, Boolean> completableSegmentList(ResourceMethodSy
}
continue;
} else if (node.kind() == SyntaxKind.IDENTIFIER_TOKEN &&
checkSubtype(typeSymbol, semanticModel.get().types().STRING, "\"" + node + "\"")) {
ResourcePathCompletionUtil.checkSubtype(typeSymbol, semanticModel.get().types().STRING,
"\"" + ((IdentifierToken) node).text() + "\"")) {
if (segment.pathSegmentKind() == PathSegment.Kind.PATH_REST_PARAMETER
&& !ResourcePathCompletionUtil.isInMethodCallContext(accNode, context)) {
completableSegmentStartIndex -= 1;
Expand All @@ -373,27 +374,6 @@ private Pair<List<PathSegment>, Boolean> completableSegmentList(ResourceMethodSy
return Pair.of(completableSegments, true);
}

private boolean checkSubtype(TypeSymbol paramType, TypeSymbol exprType, String exprValue) {
if (exprType.subtypeOf(paramType)) {
return true;
}
switch (paramType.typeKind()) {
case UNION:
for (TypeSymbol childSymbol : ((UnionTypeSymbol) paramType).memberTypeDescriptors()) {
if (checkSubtype(childSymbol, exprType, exprValue)) {
return true;
}
}
return false;
case SINGLETON:
return paramType.subtypeOf(exprType) && exprValue.equals(paramType.signature());
case TYPE_REFERENCE:
return paramType.subtypeOf(exprType);
default:
return false;
}
}

private boolean onSuggestClients(ClientResourceAccessActionNode node, BallerinaCompletionContext context) {
int cursor = context.getCursorPositionInTree();
return cursor <= node.rightArrowToken().textRange().startOffset();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"value": "**Package:** _._ \n \n \n**Params** \n- `\"name\"` a"
}
},
"sortText": "CC",
"sortText": "CD",
"filterText": "|accessor",
"insertText": "/[\"name\"].accessor;",
"insertTextFormat": "Snippet"
Expand All @@ -31,7 +31,7 @@
"value": "**Package:** _._ \n \n \n**Params** \n- `\"name\"` a"
}
},
"sortText": "CC",
"sortText": "CE",
"filterText": "|accessor",
"insertText": "/name.accessor;",
"insertTextFormat": "Snippet"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
"value": "**Package:** _._ \n \n \n**Params** \n- `\"A1\"|\"A2\"` a"
}
},
"sortText": "CC",
"sortText": "CD",
"filterText": "|accessor",
"insertText": "/[${1:\"path\"}].accessor;",
"insertTextFormat": "Snippet"
},
{
"label": "/<path>.accessor",
"label": "/<a>.accessor",
"kind": "Function",
"detail": "()",
"documentation": {
Expand All @@ -31,7 +31,7 @@
"value": "**Package:** _._ \n \n \n**Params** \n- `\"A1\"|\"A2\"` a"
}
},
"sortText": "CC",
"sortText": "CE",
"filterText": "|accessor",
"insertText": "/${1:path}.accessor;",
"insertTextFormat": "Snippet"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
"value": "**Package:** _._ \n \n \n**Params** \n- `int|string:Char` a"
}
},
"sortText": "CC",
"sortText": "CD",
"filterText": "|accessor",
"insertText": "/[${1:\"path\"}].accessor;",
"insertTextFormat": "Snippet"
},
{
"label": "/<path>.accessor",
"label": "/<a>.accessor",
"kind": "Function",
"detail": "()",
"documentation": {
Expand All @@ -31,7 +31,7 @@
"value": "**Package:** _._ \n \n \n**Params** \n- `int|string:Char` a"
}
},
"sortText": "CC",
"sortText": "CE",
"filterText": "|accessor",
"insertText": "/${1:path}.accessor;",
"insertTextFormat": "Snippet"
Expand Down

0 comments on commit 4a2b7a0

Please sign in to comment.