Skip to content

Commit

Permalink
Merge pull request #41494 from nipunayf/fix-resource-path-completion
Browse files Browse the repository at this point in the history
Fix invalid completions for a precise resource parameter type
  • Loading branch information
KavinduZoysa authored Dec 18, 2023
2 parents fe783fc + 4a2b7a0 commit 05b6ad2
Show file tree
Hide file tree
Showing 14 changed files with 494 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.ballerina.compiler.api.symbols.ResourceMethodSymbol;
import io.ballerina.compiler.api.symbols.TypeDescKind;
import io.ballerina.compiler.api.symbols.TypeSymbol;
import io.ballerina.compiler.api.symbols.UnionTypeSymbol;
import io.ballerina.compiler.api.symbols.resourcepath.ResourcePath;
import io.ballerina.compiler.api.symbols.resourcepath.util.NamedPathSegment;
import io.ballerina.compiler.api.symbols.resourcepath.util.PathSegment;
Expand Down Expand Up @@ -106,7 +107,7 @@ public static List<Pair<String, String>> getResourceAccessInfo(ResourceMethodSym
signatureWithComputedResourceSegments.append("/").append(resourceAccessPart.computedPathSignature);
insertTextWithComputedResourceSegments.append("/").append(resourceAccessPart.computedPathInsertText);

if (resourceAccessPart.isStringPathPram) {
if (resourceAccessPart.isStringPathParam) {
isStringPathParamsAvailable = true;
signatureWithNamedSegments.append("/").append(resourceAccessPart.namedPathSignature);
insertTextWithNamedSegments.append("/").append(resourceAccessPart.namedPathInsertText);
Expand Down Expand Up @@ -191,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;
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 @@ -211,20 +242,52 @@ private static ResourceAccessPathPart getResourceAccessPartForSegment(PathSegmen
String paramType = FunctionCompletionItemBuilder
.getFunctionParameterSyntax(pathParameterSymbol, context).orElse("");
String computedSignature = "[" + paramType + "]";
String computedInsertText = "[${" + placeHolderIndex + ":" + defaultValue.orElse("") + "}]";
String computedInsertText =
typeSymbol.typeKind().equals(TypeDescKind.SINGLETON) ? "[" + defaultValue.orElse("") + "]" :
"[${" + 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() &&
context.currentSemanticModel().get().types().STRING.subtypeOf(typeSymbol)) {
resourceAccessPathPart.namedPathSignature = "<" +
(pathParameterSymbol.getName().isPresent() ? pathParameterSymbol.getName().get() : "path") + ">";
resourceAccessPathPart.namedPathInsertText = "${" + placeHolderIndex + ":" + "path" + "}";
resourceAccessPathPart.computedPathInsertText = "[${" + placeHolderIndex + ":" + "\"path\"" + "}]";
resourceAccessPathPart.isStringPathPram = true;
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.namedPathInsertText = "${" + placeHolderIndex + ":" + "path" + "}";
resourceAccessPathPart.computedPathInsertText = "[${" + placeHolderIndex + ":" + "\"path\"" + "}]";
}
resourceAccessPathPart.isStringPathParam = true;
}
return resourceAccessPathPart;
}

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 (isStringSubtype(childSymbol, stringType)) {
return true;
}
}
return false;
case SINGLETON:
case TYPE_REFERENCE:
return paramType.subtypeOf(stringType);
default:
return false;
}
}

private static class ResourceAccessPathPart {

private String computedPathInsertText;
Expand All @@ -233,7 +296,7 @@ private static class ResourceAccessPathPart {
private String namedPathSignature;
private String namedPathInsertText;

boolean isStringPathPram = false;
boolean isStringPathParam = false;

ResourceAccessPathPart(String computedPathInsertText, String computedPathSignature) {
this.computedPathInsertText = computedPathInsertText;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import io.ballerina.compiler.api.symbols.resourcepath.util.PathSegment;
import io.ballerina.compiler.syntax.tree.ClientResourceAccessActionNode;
import io.ballerina.compiler.syntax.tree.ComputedResourceAccessSegmentNode;
import io.ballerina.compiler.syntax.tree.ExpressionNode;
import io.ballerina.compiler.syntax.tree.IdentifierToken;
import io.ballerina.compiler.syntax.tree.Node;
import io.ballerina.compiler.syntax.tree.ParenthesizedArgList;
Expand Down Expand Up @@ -343,18 +344,20 @@ private Pair<List<PathSegment>, Boolean> completableSegmentList(ResourceMethodSy
return Pair.of(Collections.emptyList(), false);
}
if (node.kind() == SyntaxKind.COMPUTED_RESOURCE_ACCESS_SEGMENT) {
Optional<TypeSymbol> exprType =
semanticModel.get().typeOf(((ComputedResourceAccessSegmentNode) node).expression());
if (exprType.isEmpty() || !exprType.get().subtypeOf(typeSymbol)) {
ExpressionNode exprNode = ((ComputedResourceAccessSegmentNode) node).expression();
Optional<TypeSymbol> exprType = semanticModel.get().typeOf(exprNode);
if (exprType.isEmpty() || !ResourcePathCompletionUtil.checkSubtype(typeSymbol, exprType.get(),
exprNode.toString())) {
return Pair.of(Collections.emptyList(), false);
}
if (segment.pathSegmentKind() == PathSegment.Kind.PATH_REST_PARAMETER
&& !ResourcePathCompletionUtil.isInMethodCallContext(accNode, context)) {
completableSegmentStartIndex -= 1;
}
continue;
} else if (node.kind() == SyntaxKind.IDENTIFIER_TOKEN
&& semanticModel.get().types().STRING.subtypeOf(typeSymbol)) {
} else if (node.kind() == SyntaxKind.IDENTIFIER_TOKEN &&
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 Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
"position": {
"line": 8,
"character": 9
},
"source": "action_node_context/source/client_resource_access_action_source31.bal",
"description": "Test completions in singleton resource param",
"items": [
{
"label": "/[\"name\" a].accessor",
"kind": "Function",
"detail": "()",
"documentation": {
"right": {
"kind": "markdown",
"value": "**Package:** _._ \n \n \n**Params** \n- `\"name\"` a"
}
},
"sortText": "CD",
"filterText": "|accessor",
"insertText": "/[\"name\"].accessor;",
"insertTextFormat": "Snippet"
},
{
"label": "/name.accessor",
"kind": "Function",
"detail": "()",
"documentation": {
"right": {
"kind": "markdown",
"value": "**Package:** _._ \n \n \n**Params** \n- `\"name\"` a"
}
},
"sortText": "CE",
"filterText": "|accessor",
"insertText": "/name.accessor;",
"insertTextFormat": "Snippet"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
"position": {
"line": 8,
"character": 9
},
"source": "action_node_context/source/client_resource_access_action_source32.bal",
"description": "Test completions in union of singleton resource params",
"items": [
{
"label": "/[\"A1\"|\"A2\" a].accessor",
"kind": "Function",
"detail": "()",
"documentation": {
"right": {
"kind": "markdown",
"value": "**Package:** _._ \n \n \n**Params** \n- `\"A1\"|\"A2\"` a"
}
},
"sortText": "CD",
"filterText": "|accessor",
"insertText": "/[${1:\"path\"}].accessor;",
"insertTextFormat": "Snippet"
},
{
"label": "/<a>.accessor",
"kind": "Function",
"detail": "()",
"documentation": {
"right": {
"kind": "markdown",
"value": "**Package:** _._ \n \n \n**Params** \n- `\"A1\"|\"A2\"` a"
}
},
"sortText": "CE",
"filterText": "|accessor",
"insertText": "/${1:path}.accessor;",
"insertTextFormat": "Snippet"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
"position": {
"line": 8,
"character": 9
},
"source": "action_node_context/source/client_resource_access_action_source33.bal",
"description": "Test completions in resource param with a subtype of expression",
"items": [
{
"label": "/[int|string:Char a].accessor",
"kind": "Function",
"detail": "()",
"documentation": {
"right": {
"kind": "markdown",
"value": "**Package:** _._ \n \n \n**Params** \n- `int|string:Char` a"
}
},
"sortText": "CD",
"filterText": "|accessor",
"insertText": "/[${1:\"path\"}].accessor;",
"insertTextFormat": "Snippet"
},
{
"label": "/<a>.accessor",
"kind": "Function",
"detail": "()",
"documentation": {
"right": {
"kind": "markdown",
"value": "**Package:** _._ \n \n \n**Params** \n- `int|string:Char` a"
}
},
"sortText": "CE",
"filterText": "|accessor",
"insertText": "/${1:path}.accessor;",
"insertTextFormat": "Snippet"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
{
"position": {
"line": 16,
"character": 18
},
"source": "action_node_context/source/client_resource_access_action_source34.bal",
"description": "Test completions in multiple singleton resource access functions, and accessing with a field value",
"items": [
{
"label": ".accessor",
"kind": "Function",
"detail": "()",
"documentation": {
"right": {
"kind": "markdown",
"value": "**Package:** _._ \n \n \n**Params** \n- `\"A31\"` a"
}
},
"sortText": "C",
"filterText": "accessor",
"insertText": ".accessor",
"insertTextFormat": "Snippet",
"additionalTextEdits": [
{
"range": {
"start": {
"line": 16,
"character": 17
},
"end": {
"line": 16,
"character": 18
}
},
"newText": ""
}
]
},
{
"label": ".put",
"kind": "Function",
"detail": "()",
"documentation": {
"right": {
"kind": "markdown",
"value": "**Package:** _._ \n \n \n**Params** \n- `\"A31\"|\"A32\"` a"
}
},
"sortText": "C",
"filterText": "put",
"insertText": ".put",
"insertTextFormat": "Snippet",
"additionalTextEdits": [
{
"range": {
"start": {
"line": 16,
"character": 17
},
"end": {
"line": 16,
"character": 18
}
},
"newText": ""
}
]
}
]
}
Loading

0 comments on commit 05b6ad2

Please sign in to comment.