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 bb85395..d7e4db3 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,8 +32,7 @@ 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, @@ -49,7 +48,9 @@ enum CoreRule { OPERATION_ALWAYS_EVALUATES_TO_SELF_VALUE(RuleFactory.createRule(9, "This operation always evaluates to the same value", RuleKind.CODE_SMELL)), SELF_ASSIGNMENT(RuleFactory.createRule(10, - "This variable is assigned to itself", RuleKind.CODE_SMELL)); + "This variable is assigned to itself", RuleKind.CODE_SMELL)), + UNUSED_PRIVATE_CLASS_FIELD(RuleFactory.createRule(11, + "Unused class private fields", 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 5af096f..e9762fd 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 @@ -19,6 +19,7 @@ package io.ballerina.scan.internal; import io.ballerina.compiler.api.SemanticModel; +import io.ballerina.compiler.api.symbols.ClassFieldSymbol; import io.ballerina.compiler.api.symbols.ObjectTypeSymbol; import io.ballerina.compiler.api.symbols.Qualifier; import io.ballerina.compiler.api.symbols.Symbol; @@ -40,6 +41,7 @@ import io.ballerina.compiler.syntax.tree.Node; import io.ballerina.compiler.syntax.tree.NodeList; 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; @@ -52,6 +54,7 @@ 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.ISOLATED_KEYWORD; import static io.ballerina.compiler.syntax.tree.SyntaxKind.PUBLIC_KEYWORD; import static io.ballerina.scan.utils.Constants.INIT_FUNCTION; @@ -92,9 +95,23 @@ public void visit(CheckExpressionNode checkExpressionNode) { if (checkExpressionNode.checkKeyword().kind().equals(SyntaxKind.CHECKPANIC_KEYWORD)) { reportIssue(checkExpressionNode, CoreRule.AVOID_CHECKPANIC); } + this.visitSyntaxNode(checkExpressionNode); } @Override + public void visit(ObjectFieldNode objectFieldNode) { + semanticModel.symbol(objectFieldNode).ifPresent(symbol -> { + if (symbol instanceof ClassFieldSymbol classFieldSymbol) { + List qualifiers = classFieldSymbol.qualifiers(); + if (hasQualifier(qualifiers, PRIVATE_KEYWORD)) { + if (semanticModel.references(symbol).size() == 1) { + reportIssue(objectFieldNode, CoreRule.UNUSED_PRIVATE_CLASS_FIELD); + } + } + } + }); + this.visitSyntaxNode(objectFieldNode); + } public void visit(BinaryExpressionNode binaryExpressionNode) { reportIssuesWithTrivialOperations(binaryExpressionNode); filterSameReferenceIssueBasedOnOperandType(binaryExpressionNode.operator()).ifPresent(rule -> { @@ -397,7 +414,7 @@ private boolean isUnusedNode(Node node) { Optional symbol = semanticModel.symbol(node); return symbol.filter(value -> semanticModel.references(value).size() == 1).isPresent(); } - + 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 5da964b..45379b2 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(), 10); + Assert.assertEquals(CoreRule.rules().size(), 11); } @Test(description = "test checkpanic rule") @@ -68,6 +68,14 @@ void testUnusedFunctionParameterRule() { Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL); } + @Test(description = "test unused class fields rule") + void testUnusedClassFieldsRule() { + Rule rule = CoreRule.UNUSED_PRIVATE_CLASS_FIELD.rule(); + Assert.assertEquals(rule.id(), "ballerina:11"); + Assert.assertEquals(rule.numericId(), 11); + Assert.assertEquals(rule.description(), "Unused class private fields"); + } + @Test(description = "test always true evaluates") void testTrueEvaluates() { Rule rule = CoreRule.OPERATION_ALWAYS_EVALUATES_TO_TRUE.rule(); diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/Rule011Test.java b/scan-command/src/test/java/io/ballerina/scan/internal/Rule011Test.java new file mode 100644 index 0000000..33e60e1 --- /dev/null +++ b/scan-command/src/test/java/io/ballerina/scan/internal/Rule011Test.java @@ -0,0 +1,57 @@ +/* + * 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; + +/** + * Unused private class fields analyzer tests. + * + * @since 0.5.0 + */ +public class Rule011Test extends StaticCodeAnalyzerTest { + private static final String UNUSED_CLASS_PRIVATE_FIELDS = "Unused class private fields"; + + @Test(description = "test unused private class fields") + void testUnusedPrivateFieldsAnalyzer() { + String documentName = "rule011_unused_class_fields.bal"; + Document document = loadDocument(documentName); + ScannerContextImpl scannerContext = new ScannerContextImpl(List.of(CoreRule.UNUSED_PRIVATE_CLASS_FIELD.rule())); + StaticCodeAnalyzer staticCodeAnalyzer = new StaticCodeAnalyzer(document, scannerContext, + document.module().getCompilation().getSemanticModel()); + staticCodeAnalyzer.analyze(); + List issues = scannerContext.getReporter().getIssues(); + Assert.assertEquals(issues.size(), 5); + assertIssue(issues.get(0), documentName, 17, 4, 17, 26, "ballerina:11", 11, + UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(1), documentName, 23, 4, 23, 26, "ballerina:11", 11, + UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(2), documentName, 31, 4, 31, 30, "ballerina:11", 11, + UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(3), documentName, 33, 4, 33, 30, "ballerina:11", 11, + UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(4), documentName, 75, 4, 75, 26, "ballerina:11", 11, + UNUSED_CLASS_PRIVATE_FIELDS, 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 dd47e97..b86ea92 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 @@ -11,6 +11,7 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-proj ballerina:8 | CODE_SMELL | This operation always evaluates to false 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/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 dd47e97..b86ea92 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 @@ -11,6 +11,7 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-proj ballerina:8 | CODE_SMELL | This operation always evaluates to false 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/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 346c9e3..6215c08 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 @@ -11,6 +11,7 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-proj ballerina:8 | CODE_SMELL | This operation always evaluates to false 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/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 346c9e3..6215c08 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 @@ -11,6 +11,7 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-proj ballerina:8 | CODE_SMELL | This operation always evaluates to false 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/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/rule011_unused_class_fields.bal b/scan-command/src/test/resources/test-resources/core-rules/rule011_unused_class_fields.bal new file mode 100644 index 0000000..81bda44 --- /dev/null +++ b/scan-command/src/test/resources/test-resources/core-rules/rule011_unused_class_fields.bal @@ -0,0 +1,87 @@ +// 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. + +class A { + private string c = ""; // warning +} + +public isolated class A2 { + int a = 1; + public string b = ""; + private string c = ""; // warning +} + +public isolated class A3 { + int a; + public string b; + private string c; + private boolean d = true; + private boolean? e = true; // warning + private int|boolean f = 0; + private boolean g = false; // warning + public string h = ""; + + function init() { + self.a = 0; + self.b = ""; + self.c = ""; + } + + public isolated function test(boolean d) { + self.d = d; + } + + public isolated function test2(boolean? e) { + if (e is ()) { + return; + } + self.d = e; + } + + public isolated function test3(boolean e) { + if (self.f is int) { + return; + } + self.d = e; + } +} + +class A4 { + private string c = ""; + private string d = ""; + + function init() { + self.test(self.d); + } + + function test(string s) { + self.c = s; + } +} + +service class SA { + private string b = ""; // warning + private string c = ""; + private string d = ""; + + function init() { + self.test(self.d); + } + + isolated function test(string s) { + self.c = s; + } +} diff --git a/scan-command/src/test/resources/testng.xml b/scan-command/src/test/resources/testng.xml index 6a42a0a..c0c9fa0 100644 --- a/scan-command/src/test/resources/testng.xml +++ b/scan-command/src/test/resources/testng.xml @@ -31,12 +31,13 @@ under the License. + + - - +