diff --git a/.github/workflows/central-publish.yml b/.github/workflows/central-publish.yml index 5d9f96c5..58ffc528 100644 --- a/.github/workflows/central-publish.yml +++ b/.github/workflows/central-publish.yml @@ -30,7 +30,7 @@ jobs: - name: Set Up Ballerina uses: ballerina-platform/setup-ballerina@v1.1.0 with: - version: latest + version: 2201.12.0 - name: Build with Gradle env: diff --git a/.github/workflows/full_build.yml b/.github/workflows/full_build.yml index ffcd020b..92e3dfb8 100644 --- a/.github/workflows/full_build.yml +++ b/.github/workflows/full_build.yml @@ -26,7 +26,7 @@ jobs: - name: Set Up Ballerina uses: ballerina-platform/setup-ballerina@v1.1.1 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/setup-ballerina@v1.1.1 with: - version: latest + version: 2201.12.0 - name: Build with Gradle env: diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index 8b60e959..cacc4da6 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -22,7 +22,7 @@ jobs: - name: Set Up Ballerina uses: ballerina-platform/setup-ballerina@v1.1.0 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 diff --git a/scan-command/src/main/java/io/ballerina/scan/internal/CoreRule.java b/scan-command/src/main/java/io/ballerina/scan/internal/CoreRule.java index d7e4db33..ec2d37cc 100644 --- a/scan-command/src/main/java/io/ballerina/scan/internal/CoreRule.java +++ b/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; diff --git a/scan-command/src/main/java/io/ballerina/scan/internal/StaticCodeAnalyzer.java b/scan-command/src/main/java/io/ballerina/scan/internal/StaticCodeAnalyzer.java index e9762fd9..58a602e1 100644 --- a/scan-command/src/main/java/io/ballerina/scan/internal/StaticCodeAnalyzer.java +++ b/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,6 +41,7 @@ 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; @@ -47,6 +49,7 @@ 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; @@ -54,8 +57,10 @@ 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 lhsExpr = getExprStringValueFromRangeExprNode(lhsNode); + Optional 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 = semanticModel.symbol(node); return symbol.filter(value -> semanticModel.references(value).size() == 1).isPresent(); } + + private Optional 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 qualifiers) { return hasQualifier(qualifiers, PUBLIC_KEYWORD) && !hasQualifier(qualifiers, ISOLATED_KEYWORD); diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/CoreRuleTest.java b/scan-command/src/test/java/io/ballerina/scan/internal/CoreRuleTest.java index 45379b20..7a6a3bfa 100644 --- a/scan-command/src/test/java/io/ballerina/scan/internal/CoreRuleTest.java +++ b/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") diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/Rule012Test.java b/scan-command/src/test/java/io/ballerina/scan/internal/Rule012Test.java new file mode 100644 index 00000000..3d3ed4d3 --- /dev/null +++ b/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 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); + } +} diff --git a/scan-command/src/test/resources/command-outputs/unix/list-rules-output.txt b/scan-command/src/test/resources/command-outputs/unix/list-rules-output.txt index b86ea92b..4df4d71c 100644 --- a/scan-command/src/test/resources/command-outputs/unix/list-rules-output.txt +++ b/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 diff --git a/scan-command/src/test/resources/command-outputs/unix/print-rules-to-console.txt b/scan-command/src/test/resources/command-outputs/unix/print-rules-to-console.txt index b86ea92b..4df4d71c 100644 --- a/scan-command/src/test/resources/command-outputs/unix/print-rules-to-console.txt +++ b/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 diff --git a/scan-command/src/test/resources/command-outputs/windows/list-rules-output.txt b/scan-command/src/test/resources/command-outputs/windows/list-rules-output.txt index 6215c081..3d764481 100644 --- a/scan-command/src/test/resources/command-outputs/windows/list-rules-output.txt +++ b/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 diff --git a/scan-command/src/test/resources/command-outputs/windows/print-rules-to-console.txt b/scan-command/src/test/resources/command-outputs/windows/print-rules-to-console.txt index 6215c081..3d764481 100644 --- a/scan-command/src/test/resources/command-outputs/windows/print-rules-to-console.txt +++ b/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 diff --git a/scan-command/src/test/resources/test-resources/core-rules/rule012_invalid_range_expression.bal b/scan-command/src/test/resources/test-resources/core-rules/rule012_invalid_range_expression.bal new file mode 100644 index 00000000..ad539b74 --- /dev/null +++ b/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 +} diff --git a/scan-command/src/test/resources/testng.xml b/scan-command/src/test/resources/testng.xml index c0c9fa0a..041a6b6d 100644 --- a/scan-command/src/test/resources/testng.xml +++ b/scan-command/src/test/resources/testng.xml @@ -38,6 +38,7 @@ under the License. +