Merge pull request #40890 from TharushiJay/fix40823

Fix ignoring comment minutiae in add import code action

language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/codeaction/providers/imports/ImportModuleCodeAction.java

@@ -18,14 +18,10 @@
 import io.ballerina.compiler.api.symbols.ModuleSymbol;
 import io.ballerina.compiler.syntax.tree.ImportDeclarationNode;
 import io.ballerina.compiler.syntax.tree.ImportPrefixNode;
-import io.ballerina.compiler.syntax.tree.Minutiae;
-import io.ballerina.compiler.syntax.tree.ModulePartNode;
-import io.ballerina.compiler.syntax.tree.NodeList;
 import io.ballerina.compiler.syntax.tree.NodeVisitor;
 import io.ballerina.compiler.syntax.tree.NonTerminalNode;
 import io.ballerina.compiler.syntax.tree.QualifiedNameReferenceNode;
 import io.ballerina.compiler.syntax.tree.SyntaxKind;
-import io.ballerina.compiler.syntax.tree.SyntaxTree;
 import io.ballerina.compiler.syntax.tree.Token;
 import io.ballerina.tools.diagnostics.Diagnostic;
 import org.ballerinalang.annotation.JavaSPIService;
@@ -148,7 +144,7 @@ public List<CodeAction> getCodeActions(Diagnostic diagnostic,
                     String orgName = pkgEntry.packageOrg().value();
                     String pkgName = pkgEntry.packageName().value();
                     String moduleName = ModuleUtil.escapeModuleName(pkgName);
-                    Position insertPos = getImportPosition(context);
+                    Position insertPos = CommonUtil.getImportPosition(context);
                     String importText = orgName.isEmpty() ?
                             String.format("%s %s;%n", ItemResolverConstants.IMPORT, moduleName)
                             : String.format("%s %s/%s;%n", ItemResolverConstants.IMPORT, orgName, moduleName);
@@ -169,39 +165,6 @@ public String getName() {
         return NAME;
     }
 
-    private static Position getImportPosition(CodeActionContext context) {
-        // Calculate initial import insertion line
-        Optional<SyntaxTree> syntaxTree = context.currentSyntaxTree();
-        ModulePartNode modulePartNode = syntaxTree.orElseThrow().rootNode();
-        NodeList<ImportDeclarationNode> imports = modulePartNode.imports();
-        // If there is already an import, add the new import after the last import
-        if (!imports.isEmpty()) {
-            ImportDeclarationNode lastImport = imports.get(imports.size() - 1);
-            return new Position(lastImport.lineRange().endLine().line() + 1, 0);
-        }
-
-        // If the module part has no children, add the import at the beginning of the file
-        if (modulePartNode.members().isEmpty()) {
-            return new Position(0, 0);
-        }
-
-        Position insertPosition = new Position(0, 0);
-        for (Minutiae minutiae : modulePartNode.leadingMinutiae()) {
-            if (minutiae.kind() == SyntaxKind.END_OF_LINE_MINUTIAE
-                    && minutiae.lineRange().startLine().offset() == 0) {
-                // If we find a new line character with offset 0 (a blank line), add the import after that
-                // And no further processing is required
-                insertPosition = new Position(minutiae.lineRange().startLine().line(), 0);
-                break;
-            } else if (minutiae.kind() == SyntaxKind.COMMENT_MINUTIAE) {
-                // If we find a comment, consider the import's position to be the next line
-                insertPosition = new Position(minutiae.lineRange().endLine().line() + 1, 0);
-            }
-        }
-
-        return insertPosition;
-    }
-
     /**
      * A visitor to find the qualified name reference node within an expression.
      */

language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/common/utils/CommonUtil.java

@@ -36,6 +36,7 @@
 import io.ballerina.compiler.syntax.tree.ModulePartNode;
 import io.ballerina.compiler.syntax.tree.ModuleVariableDeclarationNode;
 import io.ballerina.compiler.syntax.tree.Node;
+import io.ballerina.compiler.syntax.tree.NodeList;
 import io.ballerina.compiler.syntax.tree.NonTerminalNode;
 import io.ballerina.compiler.syntax.tree.ObjectFieldNode;
 import io.ballerina.compiler.syntax.tree.ObjectTypeDescriptorNode;
@@ -173,10 +174,6 @@ public static List<TextEdit> getAutoImportTextEdits(@Nonnull String orgName, Str
      */
     public static List<TextEdit> getAutoImportTextEdits(@Nonnull String orgName, String pkgName, String alias,
                                                         DocumentServiceContext context) {
-        Map<ImportDeclarationNode, ModuleSymbol> currentDocImports = context.currentDocImportsMap();
-        Optional<ImportDeclarationNode> last = CommonUtil.getLastItem(new ArrayList<>(currentDocImports.keySet()));
-        int endLine = last.map(node -> node.lineRange().endLine().line()).orElse(0);
-        Position start = new Position(endLine, 0);
 
         StringBuilder builder = new StringBuilder(ItemResolverConstants.IMPORT + " "
                 + (!orgName.isEmpty() ? orgName + SLASH_KEYWORD_KEY : orgName)
@@ -186,7 +183,38 @@ public static List<TextEdit> getAutoImportTextEdits(@Nonnull String orgName, Str
         }
         builder.append(SEMI_COLON_SYMBOL_KEY).append(CommonUtil.LINE_SEPARATOR);
 
-        return Collections.singletonList(new TextEdit(new Range(start, start), builder.toString()));
+        Position insertPos = getImportPosition(context);
+        return Collections.singletonList(new TextEdit(new Range(insertPos, insertPos), builder.toString()));
+    }
+
+    public static Position getImportPosition(DocumentServiceContext context) {
+        // Calculate initial import insertion line
+        Optional<SyntaxTree> syntaxTree = context.currentSyntaxTree();
+        ModulePartNode modulePartNode = syntaxTree.orElseThrow().rootNode();
+        NodeList<ImportDeclarationNode> imports = modulePartNode.imports();
+        // If there is already an import, add the new import after the last import
+        if (!imports.isEmpty()) {
+            ImportDeclarationNode lastImport = imports.get(imports.size() - 1);
+            return new Position(lastImport.lineRange().endLine().line() + 1, 0);
+        }
+
+        // If the module part has no children, add the import at the beginning of the file
+        if (modulePartNode.members().isEmpty()) {
+            return new Position(0, 0);
+        }
+
+        Position insertPosition = new Position(0, 0);
+        for (Minutiae minutiae : modulePartNode.leadingMinutiae()) {
+            if (minutiae.kind() == SyntaxKind.END_OF_LINE_MINUTIAE
+                    && minutiae.lineRange().startLine().offset() == 0) {
+                // If we find a new line character with offset 0 (a blank line), add the import after that
+                // And no further processing is required
+                insertPosition = new Position(minutiae.lineRange().startLine().line(), 0);
+                break;
+            }
+        }
+
+        return insertPosition;
     }
 
     /**

language-server/modules/langserver-core/src/test/java/org/ballerinalang/langserver/codeaction/ImportModuleCodeActionTest.java

@@ -66,7 +66,8 @@ public Object[][] dataProvider() {
                 {"importModuleWithMultipleModAliases2.json"},
                 {"importModuleWithLicenceHeader1.json"},
                 {"importModuleWithLicenceHeader2.json"},
-                {"importModuleWithIgnoredImport.json"}
+                {"importModuleWithIgnoredImport.json"},
+                {"importModuleWithTopLevelComment.json"}
         };
     }
 

language-server/modules/langserver-core/src/test/resources/codeaction/import-module/config/importModuleWithTopLevelComment.json

@@ -0,0 +1,28 @@
+{
+  "position": {
+    "line": 6,
+    "character": 24
+  },
+  "source": "importModuleWithTopLevelComment.bal",
+  "expected": [
+    {
+      "title": "Import module 'ballerina/lang.array'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 0,
+              "character": 0
+            },
+            "end": {
+              "line": 0,
+              "character": 0
+            }
+          },
+          "newText": "import ballerina/lang.array;\n"
+        }
+      ]
+    }
+  ]
+}

language-server/modules/langserver-core/src/test/resources/codeaction/import-module/source/importModuleWithTopLevelComment.bal

@@ -0,0 +1,8 @@
+// Foo is a record
+type Foo record {|
+   string name;
+|};
+
+public function main() {
+   int length = array:length([1,2,3]);
+}

language-server/modules/langserver-core/src/test/resources/completion/action_node_context/config/client_remote_action_config1.json

@@ -4,6 +4,7 @@
     "character": 13
   },
   "source": "action_node_context/source/client_remote_action_source1.bal",
+  "description": "",
   "items": [
     {
       "label": "module1",
@@ -26,11 +27,11 @@
         {
           "range": {
             "start": {
-              "line": 0,
+              "line": 1,
               "character": 0
             },
             "end": {
-              "line": 0,
+              "line": 1,
               "character": 0
             }
           },
@@ -50,11 +51,11 @@
         {
           "range": {
             "start": {
-              "line": 0,
+              "line": 1,
               "character": 0
             },
             "end": {
-              "line": 0,
+              "line": 1,
               "character": 0
             }
           },
@@ -74,11 +75,11 @@
         {
           "range": {
             "start": {
-              "line": 0,
+              "line": 1,
               "character": 0
             },
             "end": {
-              "line": 0,
+              "line": 1,
               "character": 0
             }
           },
@@ -98,11 +99,11 @@
         {
           "range": {
             "start": {
-              "line": 0,
+              "line": 1,
               "character": 0
             },
             "end": {
-              "line": 0,
+              "line": 1,
               "character": 0
             }
           },
@@ -122,11 +123,11 @@
         {
           "range": {
             "start": {
-              "line": 0,
+              "line": 1,
               "character": 0
             },
             "end": {
-              "line": 0,
+              "line": 1,
               "character": 0
             }
           },
@@ -487,11 +488,11 @@
         {
           "range": {
             "start": {
-              "line": 0,
+              "line": 1,
               "character": 0
             },
             "end": {
-              "line": 0,
+              "line": 1,
               "character": 0
             }
           },
@@ -511,11 +512,11 @@
         {
           "range": {
             "start": {
-              "line": 0,
+              "line": 1,
               "character": 0
             },
             "end": {
-              "line": 0,
+              "line": 1,
               "character": 0
             }
           },
@@ -535,11 +536,11 @@
         {
           "range": {
             "start": {
-              "line": 0,
+              "line": 1,
               "character": 0
             },
             "end": {
-              "line": 0,
+              "line": 1,
               "character": 0
             }
           },
@@ -559,11 +560,11 @@
         {
           "range": {
             "start": {
-              "line": 0,
+              "line": 1,
               "character": 0
             },
             "end": {
-              "line": 0,
+              "line": 1,
               "character": 0
             }
           },
@@ -583,11 +584,11 @@
         {
           "range": {
             "start": {
-              "line": 0,
+              "line": 1,
               "character": 0
             },
             "end": {
-              "line": 0,
+              "line": 1,
               "character": 0
             }
           },