Add fix return type for expressions

ballerina-platform · Jan 31, 2024 · dc1df0e · dc1df0e
1 parent e4482fd
commit dc1df0e
Show file tree

Hide file tree

Showing 19 changed files with 560 additions and 10 deletions.
diff --git a/...na-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java b/...na-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java
@@ -5293,11 +5293,11 @@ private BType getXMLConstituents(BType bType) {
 
     public void visit(BLangElvisExpr elvisExpr, AnalyzerData data) {
         BType lhsType = checkExpr(elvisExpr.lhsExpr, data);
-        BType actualType = lhsType == symTable.semanticError ?
+        BType lhsActualType = lhsType == symTable.semanticError ?
                 symTable.semanticError : validateElvisExprLhsExpr(elvisExpr, lhsType);
         BType rhsActualType = silentTypeCheckExpr(elvisExpr.rhsExpr, symTable.noType, data);
         BType rhsReturnType = checkExpr(elvisExpr.rhsExpr, data.expType, data);
-        BType lhsReturnType = types.checkType(elvisExpr.lhsExpr.pos, actualType, data.expType,
+        BType lhsReturnType = types.checkType(elvisExpr.lhsExpr.pos, lhsActualType, data.expType,
                 DiagnosticErrorCode.INCOMPATIBLE_TYPES);
         if (rhsReturnType == symTable.semanticError || lhsReturnType == symTable.semanticError) {
             data.resultType = symTable.semanticError;
@@ -5307,7 +5307,7 @@ public void visit(BLangElvisExpr elvisExpr, AnalyzerData data) {
             data.resultType = data.expType;
         }
 
-        elvisExpr.setDeterminedType(getConditionalExprType(actualType, rhsActualType));
+        elvisExpr.setDeterminedType(getConditionalExprType(lhsActualType, rhsActualType));
     }
 
     @Override

diff --git a/...org/ballerinalang/langserver/codeaction/providers/changetype/FixReturnTypeCodeAction.java b/...org/ballerinalang/langserver/codeaction/providers/changetype/FixReturnTypeCodeAction.java
@@ -69,14 +69,37 @@ public boolean validate(Diagnostic diagnostic, DiagBasedPositionDetails position
             return false;
         }
 
-        //Suggest the code action only if the immediate parent of the matched node is either of return statement,
-        //check expression, check action.
+        // Suggest code action if the node is aligned with a check keyword.
         NonTerminalNode matchedNode = positionDetails.matchedNode();
-        if (matchedNode.parent() != null && matchedNode.parent().kind() != SyntaxKind.RETURN_STATEMENT &&
-                matchedNode.kind() != SyntaxKind.CHECK_EXPRESSION && 
-                matchedNode.kind() != SyntaxKind.CHECK_ACTION) {
+        if (matchedNode.kind() == SyntaxKind.CHECK_ACTION || matchedNode.kind() == SyntaxKind.CHECK_EXPRESSION) {
+            return CodeActionNodeValidator.validate(context.nodeAtRange());
+        }
+
+        // Suggest code action if the node is an expression inside a return statement.
+        NonTerminalNode parentNode = matchedNode.parent();
+        if (parentNode == null) {
+            return false;
+        }
+        if (parentNode.kind() == SyntaxKind.RETURN_KEYWORD) {
+            return CodeActionNodeValidator.validate(context.nodeAtRange());
+        }
+
+        // Suggest code action if the node is an expression nested into a return statement.
+        NonTerminalNode grandParentNode = parentNode.parent();
+        if (grandParentNode == null) {
             return false;
         }
+        if (grandParentNode.kind() == SyntaxKind.RETURN_STATEMENT) {
+            return CodeActionNodeValidator.validate(context.nodeAtRange());
+        }
+
+        // Suggest code action if the node is an expression inside a collect clause.
+        if (matchedNode.kind() == SyntaxKind.COLLECT_CLAUSE) {
+            NonTerminalNode ancestorNode = grandParentNode.parent();
+            if (ancestorNode == null || ancestorNode.kind() != SyntaxKind.RETURN_STATEMENT) {
+                return false;
+            }
+        }
 
         return CodeActionNodeValidator.validate(context.nodeAtRange());
     }

diff --git a/...gserver-core/src/test/java/org/ballerinalang/langserver/codeaction/FixReturnTypeTest.java b/...gserver-core/src/test/java/org/ballerinalang/langserver/codeaction/FixReturnTypeTest.java
@@ -67,7 +67,19 @@ public Object[][] dataProvider() {
                 {"fixReturnTypeInUnionContext3.json"},
                 {"fixReturnTypeInUnionContext4.json"},
                 {"fixReturnTypeInCommitAction.json"},
-                {"fixReturnTypeInMain1.json"}
+                {"fixReturnTypeInMain1.json"},
+                {"fixReturnTypeForConditionalExpr1.json"},
+                {"fixReturnTypeForConditionalExpr2.json"},
+                {"fixReturnTypeForConditionalExpr3.json"},
+                {"fixReturnTypeForConditionalExpr4.json"},
+                {"fixReturnTypeForConditionalExpr5.json"},
+                {"fixReturnTypeForConditionalExpr6.json"},
+                {"fixReturnTypeForConditionalExpr7.json"},
+                {"fixReturnTypeForConditionalExpr8.json"},
+                {"fixReturnTypeForQueryExpr1.json"},
+                {"fixReturnTypeForQueryExpr2.json"},
+                {"fixReturnTypeForQueryExpr3.json"},
+                {"fixReturnTypeForQueryExpr4.json"}
         };
     }
 

diff --git a/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr1.json b/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr1.json
@@ -0,0 +1,29 @@
+{
+  "position": {
+    "line": 6,
+    "character": 39
+  },
+  "source": "fixReturnTypeForConditionalExpr.bal",
+  "expected": [
+    {
+      "title": "Change return type to 'boolean|int'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 5,
+              "character": 76
+            },
+            "end": {
+              "line": 5,
+              "character": 76
+            }
+          },
+          "newText": " returns boolean|int"
+        }
+      ],
+      "resolvable": false
+    }
+  ]
+}
diff --git a/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr2.json b/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr2.json
@@ -0,0 +1,29 @@
+{
+  "position": {
+    "line": 6,
+    "character": 51
+  },
+  "source": "fixReturnTypeForConditionalExpr.bal",
+  "expected": [
+    {
+      "title": "Change return type to 'boolean|int'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 5,
+              "character": 76
+            },
+            "end": {
+              "line": 5,
+              "character": 76
+            }
+          },
+          "newText": " returns boolean|int"
+        }
+      ],
+      "resolvable": false
+    }
+  ]
+}
diff --git a/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr3.json b/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr3.json
@@ -0,0 +1,29 @@
+{
+  "position": {
+    "line": 11,
+    "character": 72
+  },
+  "source": "fixReturnTypeForConditionalExpr.bal",
+  "expected": [
+    {
+      "title": "Change return type to 'module1:TestRecord1|int'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 9,
+              "character": 76
+            },
+            "end": {
+              "line": 9,
+              "character": 76
+            }
+          },
+          "newText": " returns module1:TestRecord1|int"
+        }
+      ],
+      "resolvable": false
+    }
+  ]
+}
diff --git a/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr4.json b/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr4.json
@@ -0,0 +1,29 @@
+{
+  "position": {
+    "line": 11,
+    "character": 81
+  },
+  "source": "fixReturnTypeForConditionalExpr.bal",
+  "expected": [
+    {
+      "title": "Change return type to 'module1:TestRecord1|int'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 9,
+              "character": 76
+            },
+            "end": {
+              "line": 9,
+              "character": 76
+            }
+          },
+          "newText": " returns module1:TestRecord1|int"
+        }
+      ],
+      "resolvable": false
+    }
+  ]
+}
diff --git a/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr5.json b/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr5.json
@@ -0,0 +1,29 @@
+{
+  "position": {
+    "line": 18,
+    "character": 18
+  },
+  "source": "fixReturnTypeForConditionalExpr.bal",
+  "expected": [
+    {
+      "title": "Change return type to 'string|boolean'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 17,
+              "character": 50
+            },
+            "end": {
+              "line": 17,
+              "character": 50
+            }
+          },
+          "newText": " returns string|boolean"
+        }
+      ],
+      "resolvable": false
+    }
+  ]
+}
diff --git a/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr6.json b/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr6.json
@@ -0,0 +1,29 @@
+{
+  "position": {
+    "line": 18,
+    "character": 28
+  },
+  "source": "fixReturnTypeForConditionalExpr.bal",
+  "expected": [
+    {
+      "title": "Change return type to 'string|boolean'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 17,
+              "character": 50
+            },
+            "end": {
+              "line": 17,
+              "character": 50
+            }
+          },
+          "newText": " returns string|boolean"
+        }
+      ],
+      "resolvable": false
+    }
+  ]
+}
diff --git a/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr7.json b/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr7.json
@@ -0,0 +1,29 @@
+{
+  "position": {
+    "line": 23,
+    "character": 74
+  },
+  "source": "fixReturnTypeForConditionalExpr.bal",
+  "expected": [
+    {
+      "title": "Change return type to 'string|int|boolean|module1:TestRecord1|float'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 21,
+              "character": 91
+            },
+            "end": {
+              "line": 21,
+              "character": 91
+            }
+          },
+          "newText": " returns string|int|boolean|module1:TestRecord1|float"
+        }
+      ],
+      "resolvable": false
+    }
+  ]
+}
diff --git a/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr8.json b/...rc/test/resources/codeaction/fix-return-type/config/fixReturnTypeForConditionalExpr8.json
@@ -0,0 +1,29 @@
+{
+  "position": {
+    "line": 23,
+    "character": 86
+  },
+  "source": "fixReturnTypeForConditionalExpr.bal",
+  "expected": [
+    {
+      "title": "Change return type to 'string|int|boolean|module1:TestRecord1|float'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 21,
+              "character": 91
+            },
+            "end": {
+              "line": 21,
+              "character": 91
+            }
+          },
+          "newText": " returns string|int|boolean|module1:TestRecord1|float"
+        }
+      ],
+      "resolvable": false
+    }
+  ]
+}
diff --git a/...core/src/test/resources/codeaction/fix-return-type/config/fixReturnTypeForQueryExpr1.json b/...core/src/test/resources/codeaction/fix-return-type/config/fixReturnTypeForQueryExpr1.json
@@ -0,0 +1,69 @@
+{
+  "position": {
+    "line": 29,
+    "character": 11
+  },
+  "source": "fixReturnTypeForQueryExpr.bal",
+  "expected": [
+    {
+      "title": "Change return type to 'map<any>[]'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 28,
+              "character": 32
+            },
+            "end": {
+              "line": 28,
+              "character": 32
+            }
+          },
+          "newText": " returns map<any>[]"
+        }
+      ],
+      "resolvable": false
+    },
+    {
+      "title": "Change return type to 'record {|string name; float gpa;|}[]'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 28,
+              "character": 32
+            },
+            "end": {
+              "line": 28,
+              "character": 32
+            }
+          },
+          "newText": " returns record {|string name; float gpa;|}[]"
+        }
+      ],
+      "resolvable": false
+    },
+    {
+      "title": "Change return type to 'json[]'",
+      "kind": "quickfix",
+      "edits": [
+        {
+          "range": {
+            "start": {
+              "line": 28,
+              "character": 32
+            },
+            "end": {
+              "line": 28,
+              "character": 32
+            }
+          },
+          "newText": " returns json[]"
+        }
+      ],
+      "resolvable": false
+    }
+  ]
+}