Add Static rules for invalid range expression

.github/workflows/central-publish.yml

@@ -30,7 +30,7 @@ jobs:
       - name: Set Up Ballerina
         uses: ballerina-platform/[email protected]
         with:
-          version: latest
+          version: 2201.12.0
 
       - name: Build with Gradle
         env:

.github/workflows/full_build.yml

@@ -26,7 +26,7 @@ jobs:
       - name: Set Up Ballerina
         uses: ballerina-platform/[email protected]
         with:
-          version: latest
+          version: 2201.12.0
 
       - name: Grant execute permission for gradlew
         run: chmod +x gradlew
@@ -66,7 +66,7 @@ jobs:
       - name: Set Up Ballerina
         uses: ballerina-platform/[email protected]
         with:
-          version: latest
+          version: 2201.12.0
 
       - name: Build with Gradle
         env:

.github/workflows/publish-release.yml

@@ -22,7 +22,7 @@ jobs:
       - name: Set Up Ballerina
         uses: ballerina-platform/[email protected]
         with:
-          version: latest
+          version: 2201.12.0
 
       - name: Set version env variable
         run: echo "VERSION=$((grep -w "version" | cut -d= -f2) < gradle.properties | rev | cut --complement -d- -f1 | rev)" >> $GITHUB_ENV

scan-command/src/main/java/io/ballerina/scan/internal/CoreRule.java

@@ -32,7 +32,8 @@
 enum CoreRule {
 
     AVOID_CHECKPANIC(RuleFactory.createRule(1, "Avoid checkpanic", RuleKind.CODE_SMELL)),
-    UNUSED_FUNCTION_PARAMETER(RuleFactory.createRule(2, "Unused function parameter", RuleKind.CODE_SMELL)),
+    UNUSED_FUNCTION_PARAMETER(RuleFactory.createRule(2,
+            "Unused function parameter", RuleKind.CODE_SMELL)),
     PUBLIC_NON_ISOLATED_FUNCTION_CONSTRUCT(RuleFactory.createRule(3,
             "Non isolated public function", RuleKind.CODE_SMELL)),
     PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT(RuleFactory.createRule(4,
@@ -50,7 +51,9 @@ enum CoreRule {
     SELF_ASSIGNMENT(RuleFactory.createRule(10,
             "This variable is assigned to itself", RuleKind.CODE_SMELL)),
     UNUSED_PRIVATE_CLASS_FIELD(RuleFactory.createRule(11,
-            "Unused class private fields", RuleKind.CODE_SMELL));
+            "Unused class private fields", RuleKind.CODE_SMELL)),
+    INVALID_RANGE_EXPRESSION(RuleFactory.createRule(12, 
+    "Invalid range expression", RuleKind.CODE_SMELL));
 
     private final Rule rule;
 

scan-command/src/main/java/io/ballerina/scan/internal/StaticCodeAnalyzer.java

@@ -27,6 +27,7 @@
 import io.ballerina.compiler.api.symbols.TypeDefinitionSymbol;
 import io.ballerina.compiler.api.symbols.TypeSymbol;
 import io.ballerina.compiler.syntax.tree.AssignmentStatementNode;
+import io.ballerina.compiler.syntax.tree.BasicLiteralNode;
 import io.ballerina.compiler.syntax.tree.BinaryExpressionNode;
 import io.ballerina.compiler.syntax.tree.CheckExpressionNode;
 import io.ballerina.compiler.syntax.tree.ClassDefinitionNode;
@@ -40,22 +41,26 @@
 import io.ballerina.compiler.syntax.tree.ModulePartNode;
 import io.ballerina.compiler.syntax.tree.Node;
 import io.ballerina.compiler.syntax.tree.NodeList;
+import io.ballerina.compiler.syntax.tree.NodeLocation;
 import io.ballerina.compiler.syntax.tree.NodeVisitor;
 import io.ballerina.compiler.syntax.tree.ObjectFieldNode;
 import io.ballerina.compiler.syntax.tree.SimpleNameReferenceNode;
 import io.ballerina.compiler.syntax.tree.SyntaxKind;
 import io.ballerina.compiler.syntax.tree.SyntaxTree;
 import io.ballerina.compiler.syntax.tree.Token;
 import io.ballerina.compiler.syntax.tree.TypeDefinitionNode;
+import io.ballerina.compiler.syntax.tree.UnaryExpressionNode;
 import io.ballerina.projects.Document;
 import io.ballerina.scan.ScannerContext;
 import io.ballerina.scan.utils.Constants;
 
 import java.util.List;
 import java.util.Optional;
 
-import static io.ballerina.compiler.syntax.tree.SyntaxKind.PRIVATE_KEYWORD;
+import static io.ballerina.compiler.syntax.tree.SyntaxKind.DOUBLE_DOT_LT_TOKEN;
+import static io.ballerina.compiler.syntax.tree.SyntaxKind.ELLIPSIS_TOKEN;
 import static io.ballerina.compiler.syntax.tree.SyntaxKind.ISOLATED_KEYWORD;
+import static io.ballerina.compiler.syntax.tree.SyntaxKind.PRIVATE_KEYWORD;
 import static io.ballerina.compiler.syntax.tree.SyntaxKind.PUBLIC_KEYWORD;
 import static io.ballerina.scan.utils.Constants.INIT_FUNCTION;
 import static io.ballerina.scan.utils.Constants.MAIN_FUNCTION;
@@ -98,6 +103,51 @@ public void visit(CheckExpressionNode checkExpressionNode) {
         this.visitSyntaxNode(checkExpressionNode);
     }
 
+    public void visit(BinaryExpressionNode binaryExpressionNode) {
+        reportIssuesWithTrivialOperations(binaryExpressionNode);
+        filterSameReferenceIssueBasedOnOperandType(binaryExpressionNode.operator()).ifPresent(rule -> {
+            checkUsageOfSameOperandInBinaryExpr(binaryExpressionNode.lhsExpr(),
+                    binaryExpressionNode.rhsExpr(), rule, binaryExpressionNode);
+        });
+
+        SyntaxKind binaryOperatorKind = binaryExpressionNode.operator().kind();
+        if (binaryOperatorKind.equals(ELLIPSIS_TOKEN)
+                || binaryOperatorKind.equals(DOUBLE_DOT_LT_TOKEN)) {
+            validateRangeExpressionOperator(scannerContext, document, binaryExpressionNode.lhsExpr(),
+                    binaryExpressionNode.rhsExpr(), binaryExpressionNode.operator(), binaryExpressionNode.location());
+        }
+        this.visitSyntaxNode(binaryExpressionNode);
+    }
+
+    private void validateRangeExpressionOperator(ScannerContext scannerContext, Document document,
+                                                 Node lhsNode, Node rhsNode, Token operator, NodeLocation location) {
+        Optional<String> lhsExpr = getExprStringValueFromRangeExprNode(lhsNode);
+        Optional<String> rhsExpr = getExprStringValueFromRangeExprNode(rhsNode);
+
+        if (lhsExpr.isEmpty() || rhsExpr.isEmpty()) {
+            return;
+        }
+
+        // According to the spec, these literal tokens are integers. So no need to cast or check.
+        try {
+            int lhsValue = Integer.parseInt(lhsExpr.get());
+            int rhsValue = Integer.parseInt(rhsExpr.get());
+            if (operator.kind() == DOUBLE_DOT_LT_TOKEN) {
+                if (lhsValue >= rhsValue) {
+                    scannerContext.getReporter().reportIssue(document, location,
+                            CoreRule.INVALID_RANGE_EXPRESSION.rule());
+                }
+            } else if (operator.kind() == ELLIPSIS_TOKEN) {
+                if (lhsValue > rhsValue) {
+                    scannerContext.getReporter().reportIssue(document, location,
+                            CoreRule.INVALID_RANGE_EXPRESSION.rule());
+                }
+            }
+        } catch (NumberFormatException e) {
+            // ignore
+        }
+    }
+
     @Override
     public void visit(ObjectFieldNode objectFieldNode) {
         semanticModel.symbol(objectFieldNode).ifPresent(symbol -> {
@@ -112,13 +162,6 @@ public void visit(ObjectFieldNode objectFieldNode) {
         });
         this.visitSyntaxNode(objectFieldNode);
     }
-    public void visit(BinaryExpressionNode binaryExpressionNode) {
-        reportIssuesWithTrivialOperations(binaryExpressionNode);
-        filterSameReferenceIssueBasedOnOperandType(binaryExpressionNode.operator()).ifPresent(rule -> {
-            checkUsageOfSameOperandInBinaryExpr(binaryExpressionNode.lhsExpr(),
-                    binaryExpressionNode.rhsExpr(), rule, binaryExpressionNode);
-        });
-    }
 
     @Override
     public void visit(AssignmentStatementNode assignmentStatementNode) {
@@ -414,6 +457,27 @@ private boolean isUnusedNode(Node node) {
         Optional<Symbol> symbol = semanticModel.symbol(node);
         return symbol.filter(value -> semanticModel.references(value).size() == 1).isPresent();
     }
+
+    private Optional<String> getExprStringValueFromRangeExprNode(Node rangeExprNode) {
+        boolean isMinusOperatorPresent = false;
+
+        if (rangeExprNode instanceof UnaryExpressionNode unaryNode) {
+            if (unaryNode.unaryOperator().kind() == SyntaxKind.MINUS_TOKEN) {
+                isMinusOperatorPresent = true;
+            }
+            rangeExprNode = unaryNode.expression();
+        }
+
+        if (rangeExprNode instanceof BasicLiteralNode literalNode) {
+            String expr = literalNode.literalToken().text();
+            if (isMinusOperatorPresent) {
+                expr = "-" + expr;
+            }
+            return Optional.of(expr);
+        }
+
+        return Optional.empty();
+    }
 
     private boolean isPublicIsolatedConstruct(NodeList<Token> qualifiers) {
         return hasQualifier(qualifiers, PUBLIC_KEYWORD) && !hasQualifier(qualifiers, ISOLATED_KEYWORD);

scan-command/src/test/java/io/ballerina/scan/internal/CoreRuleTest.java

@@ -47,7 +47,7 @@ public class CoreRuleTest {
 
     @Test(description = "test all rules")
     void testAllRules() {
-        Assert.assertEquals(CoreRule.rules().size(), 11);
+        Assert.assertEquals(CoreRule.rules().size(), 12);
     }
 
     @Test(description = "test checkpanic rule")

scan-command/src/test/java/io/ballerina/scan/internal/Rule012Test.java

@@ -0,0 +1,72 @@
+/*
+ *  Copyright (c) 2024, WSO2 LLC. (https://www.wso2.com).
+ *
+ *  WSO2 LLC. licenses this file to you under the Apache License,
+ *  Version 2.0 (the "License"); you may not use this file except
+ *  in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied. See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package io.ballerina.scan.internal;
+
+import io.ballerina.projects.Document;
+import io.ballerina.scan.Issue;
+import io.ballerina.scan.RuleKind;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import java.util.List;
+
+/**
+ * Invalid range expression analyzer tests.
+ *
+ * @since 0.1.0
+ */
+public class Rule012Test extends StaticCodeAnalyzerTest {
+    private static final String INVALID_RANGE_EXPRESSION = "Invalid range expression";
+
+    @Test(description = "test invalid range expresion")
+    void testRangeOperatorAnalyzer() {
+        String documentName = "rule012_invalid_range_expression.bal";
+        Document document = loadDocument(documentName);
+        ScannerContextImpl scannerContext = new ScannerContextImpl(List.of(CoreRule.INVALID_RANGE_EXPRESSION.rule()));
+        StaticCodeAnalyzer staticCodeAnalyzer = new StaticCodeAnalyzer(document,
+                scannerContext, document.module().getCompilation().getSemanticModel());
+        staticCodeAnalyzer.analyze();
+        List<Issue> issues = scannerContext.getReporter().getIssues();
+        Assert.assertEquals(issues.size(), 12);
+        assertIssue(issues.get(0), documentName, 25, 21, 25, 27, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(1), documentName, 29, 21, 29, 27, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(2), documentName, 33, 21, 33, 28, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(3), documentName, 57, 21, 57, 27, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(4), documentName, 61, 21, 61, 27, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(5), documentName, 65, 21, 65, 28, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(6), documentName, 69, 21, 69, 26, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(7), documentName, 73, 21, 73, 28, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(8), documentName, 77, 21, 77, 28, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(9), documentName, 81, 22, 81, 29, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(10), documentName, 83, 22, 83, 29, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+        assertIssue(issues.get(11), documentName, 87, 14, 87, 22, "ballerina:12", 12,
+                INVALID_RANGE_EXPRESSION, RuleKind.CODE_SMELL);
+    }
+}

scan-command/src/test/resources/command-outputs/unix/list-rules-output.txt

@@ -12,6 +12,7 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-proj
 	ballerina:9                                      | CODE_SMELL    | This operation always evaluates to the same value
 	ballerina:10                                     | CODE_SMELL    | This variable is assigned to itself
 	ballerina:11                                     | CODE_SMELL    | Unused class private fields
+	ballerina:12                                     | CODE_SMELL    | Invalid range expression
 	ballerina/example_module_static_code_analyzer:1  | CODE_SMELL    | rule 1
 	ballerina/example_module_static_code_analyzer:2  | BUG           | rule 2
 	ballerina/example_module_static_code_analyzer:3  | VULNERABILITY | rule 3

scan-command/src/test/resources/command-outputs/unix/print-rules-to-console.txt

@@ -12,6 +12,7 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-proj
 	ballerina:9                                      | CODE_SMELL    | This operation always evaluates to the same value
 	ballerina:10                                     | CODE_SMELL    | This variable is assigned to itself
 	ballerina:11                                     | CODE_SMELL    | Unused class private fields
+	ballerina:12                                     | CODE_SMELL    | Invalid range expression
 	ballerina/example_module_static_code_analyzer:1  | CODE_SMELL    | rule 1
 	ballerina/example_module_static_code_analyzer:2  | BUG           | rule 2
 	ballerina/example_module_static_code_analyzer:3  | VULNERABILITY | rule 3

scan-command/src/test/resources/command-outputs/windows/list-rules-output.txt

@@ -12,6 +12,7 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-proj
 	ballerina:9                                      | CODE_SMELL    | This operation always evaluates to the same value
 	ballerina:10                                     | CODE_SMELL    | This variable is assigned to itself
 	ballerina:11                                     | CODE_SMELL    | Unused class private fields
+	ballerina:12                                     | CODE_SMELL    | Invalid range expression
 	ballerina/example_module_static_code_analyzer:1  | CODE_SMELL    | rule 1
 	ballerina/example_module_static_code_analyzer:2  | BUG           | rule 2
 	ballerina/example_module_static_code_analyzer:3  | VULNERABILITY | rule 3

scan-command/src/test/resources/command-outputs/windows/print-rules-to-console.txt

@@ -12,6 +12,7 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-proj
 	ballerina:9                                      | CODE_SMELL    | This operation always evaluates to the same value
 	ballerina:10                                     | CODE_SMELL    | This variable is assigned to itself
 	ballerina:11                                     | CODE_SMELL    | Unused class private fields
+	ballerina:12                                     | CODE_SMELL    | Invalid range expression
 	ballerina/example_module_static_code_analyzer:1  | CODE_SMELL    | rule 1
 	ballerina/example_module_static_code_analyzer:2  | BUG           | rule 2
 	ballerina/example_module_static_code_analyzer:3  | VULNERABILITY | rule 3

scan-command/src/test/resources/test-resources/core-rules/rule012_invalid_range_expression.bal

@@ -0,0 +1,89 @@
+// Copyright (c) 2024, WSO2 LLC. (https://www.wso2.com).
+//
+// WSO2 Inc. licenses this file to you under the Apache License,
+// Version 2.0 (the "License"); you may not use this file except
+// in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+function testRangeOperator() {
+    foreach int i in 0...10 {
+        // code
+    }
+
+    foreach int i in -5...-2 {
+        // code
+    }
+
+    foreach int i in 0...-1 { // warning
+        // code
+    }
+
+    foreach int i in 10...9 { // warning
+            // code
+    }
+
+    foreach int i in -2...-3 { // warning
+            // code
+    }
+
+    foreach int i in 0...0 {
+        // code
+    }
+
+    foreach int i in 10...10 {
+        // code
+    }
+
+    foreach int i in -2...-2 {
+        // code
+    }
+
+    foreach int i in 0..<10 {
+        // code
+    }
+
+    foreach int i in -5..<-2 {
+        // code
+    }
+
+    foreach int i in 0..<-1 { // warning
+        // code
+    }
+
+    foreach int i in 10..<9 { // warning
+            // code
+    }
+
+    foreach int i in -2..<-3 { // warning
+            // code
+    }
+
+    foreach int i in 0..<0 { // warning
+        // code
+    }
+
+    foreach int i in 10..<10 { // warning
+        // code
+    }
+
+    foreach int i in -2..<-2 { // warning
+        // code
+    }
+
+    _ = from int i in 10..<10 select [1, 2, 3]; // warning
+
+    _ = from int i in -2..<-2 select 1; // warning
+
+    _ = from int i in -2..<2 select 1;
+
+    var val = 10 ... 3; // warning
+}

scan-command/src/test/resources/testng.xml

@@ -38,6 +38,7 @@ under the License.
             <class name="io.ballerina.scan.internal.Rule009Test"/>
             <class name="io.ballerina.scan.internal.Rule010Test"/>
             <class name="io.ballerina.scan.internal.Rule011Test"/>
+            <class name="io.ballerina.scan.internal.Rule012Test"/>
             <class name="io.ballerina.scan.internal.ProjectAnalyzerTest"/>
             <class name="io.ballerina.scan.utils.ScanUtilsTest"/>
             <class name="io.ballerina.scan.utils.DiagnosticLogTest"/>