From 4a2b7a0389464c40924e914176bbde6d525d79b7 Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Thu, 9 Nov 2023 13:38:27 +0530 Subject: [PATCH] Address review suggestions --- .../builder/ResourcePathCompletionUtil.java | 44 ++++++++++++++++--- ...ClientResourceAccessActionNodeContext.java | 28 ++---------- ...lient_resource_access_action_config38.json | 4 +- ...lient_resource_access_action_config39.json | 6 +-- ...lient_resource_access_action_config40.json | 6 +-- 5 files changed, 51 insertions(+), 37 deletions(-) diff --git a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/completions/builder/ResourcePathCompletionUtil.java b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/completions/builder/ResourcePathCompletionUtil.java index 36b316e385fe..55b8bf722607 100644 --- a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/completions/builder/ResourcePathCompletionUtil.java +++ b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/completions/builder/ResourcePathCompletionUtil.java @@ -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; + 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) { @@ -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\"" + "}]"; } @@ -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; } } diff --git a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/completions/providers/context/ClientResourceAccessActionNodeContext.java b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/completions/providers/context/ClientResourceAccessActionNodeContext.java index 1647eb4c51dc..2c8224524e2d 100644 --- a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/completions/providers/context/ClientResourceAccessActionNodeContext.java +++ b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/completions/providers/context/ClientResourceAccessActionNodeContext.java @@ -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; @@ -347,7 +346,8 @@ private Pair, Boolean> completableSegmentList(ResourceMethodSy if (node.kind() == SyntaxKind.COMPUTED_RESOURCE_ACCESS_SEGMENT) { ExpressionNode exprNode = ((ComputedResourceAccessSegmentNode) node).expression(); Optional 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 @@ -356,7 +356,8 @@ private Pair, 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; @@ -373,27 +374,6 @@ private Pair, 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(); diff --git a/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config38.json b/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config38.json index 2b70e6b1194c..804b8dcaf1b5 100644 --- a/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config38.json +++ b/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config38.json @@ -16,7 +16,7 @@ "value": "**Package:** _._ \n \n \n**Params** \n- `\"name\"` a" } }, - "sortText": "CC", + "sortText": "CD", "filterText": "|accessor", "insertText": "/[\"name\"].accessor;", "insertTextFormat": "Snippet" @@ -31,7 +31,7 @@ "value": "**Package:** _._ \n \n \n**Params** \n- `\"name\"` a" } }, - "sortText": "CC", + "sortText": "CE", "filterText": "|accessor", "insertText": "/name.accessor;", "insertTextFormat": "Snippet" diff --git a/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config39.json b/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config39.json index 8aeb4c53682a..7a2ccc8530e7 100644 --- a/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config39.json +++ b/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config39.json @@ -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": "/.accessor", + "label": "/.accessor", "kind": "Function", "detail": "()", "documentation": { @@ -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" diff --git a/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config40.json b/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config40.json index 1ee1ab76a2dc..9596deff949f 100644 --- a/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config40.json +++ b/language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_resource_access_action_config40.json @@ -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": "/.accessor", + "label": "/.accessor", "kind": "Function", "detail": "()", "documentation": { @@ -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"