From 5c3ae27d67e83c6c908811c64841f945b9a2c841 Mon Sep 17 00:00:00 2001 From: Sasindu Alahakoon Date: Wed, 2 Oct 2024 11:16:30 +0530 Subject: [PATCH 1/9] Add static rule implementation for unused fields in class --- .../io/ballerina/scan/internal/CoreRule.java | 5 +- .../scan/internal/StaticCodeAnalyzer.java | 26 +++++++ .../io/ballerina/scan/utils/Constants.java | 8 +++ .../scan/utils/StaticCodeAnalyzerUtils.java | 24 +++++++ .../scan/internal/StaticCodeAnalyzerTest.java | 43 ++++++++--- .../core-rules/unused_class_fields.bal | 72 +++++++++++++++++++ 6 files changed, 168 insertions(+), 10 deletions(-) create mode 100644 scan-command/src/main/java/io/ballerina/scan/utils/StaticCodeAnalyzerUtils.java create mode 100644 scan-command/src/test/resources/test-resources/core-rules/unused_class_fields.bal 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 211049e3..877e1f9d 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 @@ -20,6 +20,7 @@ import io.ballerina.scan.Rule; import io.ballerina.scan.RuleKind; +import io.ballerina.scan.utils.Constants; import java.util.ArrayList; import java.util.List; @@ -30,7 +31,9 @@ * @since 0.1.0 * */ enum CoreRule { - AVOID_CHECKPANIC(RuleFactory.createRule(1, "Avoid checkpanic", RuleKind.CODE_SMELL)); + AVOID_CHECKPANIC(RuleFactory.createRule(1, Constants.RuleDescription.AVOID_CHECKPANIC, RuleKind.CODE_SMELL)), + UNUSED_PRIVATE_FIELDS(RuleFactory.createRule(3, + Constants.RuleDescription.UNUSED_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 d7cf2c0e..e9bea875 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 @@ -18,13 +18,22 @@ package io.ballerina.scan.internal; +import io.ballerina.compiler.api.SemanticModel; +import io.ballerina.compiler.api.symbols.ClassFieldSymbol; +import io.ballerina.compiler.api.symbols.Qualifier; import io.ballerina.compiler.syntax.tree.CheckExpressionNode; import io.ballerina.compiler.syntax.tree.ModulePartNode; import io.ballerina.compiler.syntax.tree.NodeVisitor; +import io.ballerina.compiler.syntax.tree.ObjectFieldNode; import io.ballerina.compiler.syntax.tree.SyntaxKind; import io.ballerina.compiler.syntax.tree.SyntaxTree; import io.ballerina.projects.Document; import io.ballerina.scan.ScannerContext; +import io.ballerina.scan.utils.StaticCodeAnalyzerUtils; + +import java.util.List; + +import static io.ballerina.compiler.syntax.tree.SyntaxKind.PRIVATE_KEYWORD; /** * {@code StaticCodeAnalyzer} contains the logic to perform core static code analysis on Ballerina documents. @@ -35,8 +44,10 @@ class StaticCodeAnalyzer extends NodeVisitor { private final Document document; private final SyntaxTree syntaxTree; private final ScannerContext scannerContext; + private final SemanticModel semanticModel; StaticCodeAnalyzer(Document document, ScannerContextImpl scannerContext) { + this.semanticModel = document.module().getCompilation().getSemanticModel(); this.document = document; this.syntaxTree = document.syntaxTree(); this.scannerContext = scannerContext; @@ -58,4 +69,19 @@ public void visit(CheckExpressionNode checkExpressionNode) { CoreRule.AVOID_CHECKPANIC.rule()); } } + + @Override + public void visit(ObjectFieldNode objectFieldNode) { + semanticModel.symbol(objectFieldNode).ifPresent(symbol -> { + if (symbol instanceof ClassFieldSymbol classFieldSymbol) { + List qualifiers = classFieldSymbol.qualifiers(); + if (StaticCodeAnalyzerUtils.getQualifier(qualifiers, PRIVATE_KEYWORD.stringValue())) { + if (semanticModel.references(symbol).size() == 1) { + StaticCodeAnalyzerUtils.reportIssue(scannerContext, document, objectFieldNode, + CoreRule.UNUSED_PRIVATE_FIELDS.rule()); + } + } + } + }); + } } diff --git a/scan-command/src/main/java/io/ballerina/scan/utils/Constants.java b/scan-command/src/main/java/io/ballerina/scan/utils/Constants.java index 372de85d..9544d71d 100644 --- a/scan-command/src/main/java/io/ballerina/scan/utils/Constants.java +++ b/scan-command/src/main/java/io/ballerina/scan/utils/Constants.java @@ -62,6 +62,14 @@ public class Constants { static final String RULE_DESCRIPTION_COLUMN = "Rule Description"; static final String[] RULE_PRIORITY_LIST = {"ballerina", "ballerina/", "ballerinax/", "wso2/"}; + public static class RuleDescription { + public static final String AVOID_CHECKPANIC = "Avoid checkpanic"; + public static final String UNUSED_PRIVATE_FIELDS = "Unused private fields"; + + private RuleDescription() { + } + } + private Constants() { } } diff --git a/scan-command/src/main/java/io/ballerina/scan/utils/StaticCodeAnalyzerUtils.java b/scan-command/src/main/java/io/ballerina/scan/utils/StaticCodeAnalyzerUtils.java new file mode 100644 index 00000000..be01f2c0 --- /dev/null +++ b/scan-command/src/main/java/io/ballerina/scan/utils/StaticCodeAnalyzerUtils.java @@ -0,0 +1,24 @@ +package io.ballerina.scan.utils; + +import io.ballerina.compiler.api.symbols.Qualifier; +import io.ballerina.compiler.syntax.tree.Node; +import io.ballerina.projects.Document; +import io.ballerina.scan.Rule; +import io.ballerina.scan.ScannerContext; + +import java.util.List; + +public class StaticCodeAnalyzerUtils { + public static boolean getQualifier(List qualifierList, String qualifierValue) { + for (Qualifier qualifier : qualifierList) { + if (qualifier.getValue() == qualifierValue) { + return true; + } + } + return false; + } + + public static void reportIssue(ScannerContext scannerContext, Document document, Node node, Rule rule) { + scannerContext.getReporter().reportIssue(document, node.location(), rule); + } +} diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java b/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java index 242b11e2..c460055e 100644 --- a/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java +++ b/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java @@ -27,6 +27,7 @@ import io.ballerina.scan.Rule; import io.ballerina.scan.RuleKind; import io.ballerina.scan.Source; +import io.ballerina.scan.utils.Constants; import io.ballerina.tools.text.LineRange; import org.testng.Assert; import org.testng.annotations.Test; @@ -57,18 +58,42 @@ void testCheckpanicAnalyzer() { staticCodeAnalyzer.analyze(); List issues = scannerContext.getReporter().getIssues(); Assert.assertEquals(issues.size(), 1); - Issue issue = issues.get(0); + assertIssue(issues.get(0), documentName, 20, 17, 20, 39, "ballerina:1", 1, + Constants.RuleDescription.AVOID_CHECKPANIC, RuleKind.CODE_SMELL); + } + + @Test(description = "test checkpanic analyzer") + void testUnusedClassFieldsAnalyzer() { + String documentName = "unused_class_fields.bal"; + Document document = loadDocument(documentName); + ScannerContextImpl scannerContext = new ScannerContextImpl(List.of(CoreRule.AVOID_CHECKPANIC.rule())); + StaticCodeAnalyzer staticCodeAnalyzer = new StaticCodeAnalyzer(document, scannerContext); + staticCodeAnalyzer.analyze(); + List issues = scannerContext.getReporter().getIssues(); + Assert.assertEquals(issues.size(), 4); + assertIssue(issues.get(0), documentName, 17, 4, 17, 21, "ballerina:3", 3, + Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(1), documentName, 23, 4, 23, 21, "ballerina:3", 3, + Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(2), documentName, 31, 4, 31, 23, "ballerina:3", 3, + Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(3), documentName, 33, 4, 33, 30, "ballerina:3", 3, + Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + } + + void assertIssue(Issue issue, String documentName, int startLine, int startOffset, int endLine, int endOffset, + String ruleId, int numericId, String description, RuleKind ruleKind) { Assert.assertEquals(issue.source(), Source.BUILT_IN); LineRange location = issue.location().lineRange(); Assert.assertEquals(location.fileName(), documentName); - Assert.assertEquals(location.startLine().line(), 20); - Assert.assertEquals(location.startLine().offset(), 17); - Assert.assertEquals(location.endLine().line(), 20); - Assert.assertEquals(location.endLine().offset(), 39); + Assert.assertEquals(location.startLine().line(), startLine); + Assert.assertEquals(location.startLine().offset(), startOffset); + Assert.assertEquals(location.endLine().line(), endLine); + Assert.assertEquals(location.endLine().offset(), endOffset); Rule rule = issue.rule(); - Assert.assertEquals(rule.id(), "ballerina:1"); - Assert.assertEquals(rule.numericId(), 1); - Assert.assertEquals(rule.description(), "Avoid checkpanic"); - Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL); + Assert.assertEquals(rule.id(), ruleId); + Assert.assertEquals(rule.numericId(), numericId); + Assert.assertEquals(rule.description(), description); + Assert.assertEquals(rule.kind(), ruleKind); } } diff --git a/scan-command/src/test/resources/test-resources/core-rules/unused_class_fields.bal b/scan-command/src/test/resources/test-resources/core-rules/unused_class_fields.bal new file mode 100644 index 00000000..d3970f7f --- /dev/null +++ b/scan-command/src/test/resources/test-resources/core-rules/unused_class_fields.bal @@ -0,0 +1,72 @@ +// 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 class A2 { + int a = 1; + public string b = ""; + private string c; // warning +} + +public class A3 { + int a; + public string b; + private string c; + private boolean d; + private boolean? e; // warning + private int|boolean f = 0; + private boolean g = false; // warning + + function init() { + self.a = 0; + self.b = ""; + self.c = ""; + } + + public function test(boolean d) { + self.d = d; + } + + public function test2(boolean? e) { + if (e is ()) { + return; + } + self.d = e; + } + + public 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); + } + + public function test(string s) { + self.c = s; + } +} From c922ceb27d14b3e6f822874b062d71250c4f5e48 Mon Sep 17 00:00:00 2001 From: Sasindu Alahakoon Date: Wed, 19 Feb 2025 01:42:47 +0530 Subject: [PATCH 2/9] Add service class tests for unused fields rule --- .../io/ballerina/scan/internal/CoreRule.java | 4 +-- .../scan/internal/StaticCodeAnalyzer.java | 2 +- .../ballerina/scan/internal/CoreRuleTest.java | 9 +++++ .../ballerina/scan/internal/Rule005Test.java | 33 +++++++++++++++++++ .../scan/internal/StaticCodeAnalyzerTest.java | 21 +----------- ...ds.bal => rule005_unused_class_fields.bal} | 15 +++++++++ scan-command/src/test/resources/testng.xml | 2 +- 7 files changed, 62 insertions(+), 24 deletions(-) create mode 100644 scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java rename scan-command/src/test/resources/test-resources/core-rules/{unused_class_fields.bal => rule005_unused_class_fields.bal} (86%) 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 877e1f9d..4bfe4063 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,8 @@ * */ enum CoreRule { AVOID_CHECKPANIC(RuleFactory.createRule(1, Constants.RuleDescription.AVOID_CHECKPANIC, RuleKind.CODE_SMELL)), - UNUSED_PRIVATE_FIELDS(RuleFactory.createRule(3, - Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL)); + UNUSED_CLASS_FIELDS(RuleFactory.createRule(5, + "Unused 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 e9bea875..cee5321d 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 @@ -78,7 +78,7 @@ public void visit(ObjectFieldNode objectFieldNode) { if (StaticCodeAnalyzerUtils.getQualifier(qualifiers, PRIVATE_KEYWORD.stringValue())) { if (semanticModel.references(symbol).size() == 1) { StaticCodeAnalyzerUtils.reportIssue(scannerContext, document, objectFieldNode, - CoreRule.UNUSED_PRIVATE_FIELDS.rule()); + CoreRule.UNUSED_CLASS_FIELDS.rule()); } } } 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 212b3596..12b2ba83 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 @@ -42,4 +42,13 @@ void testCheckpanicRule() { Assert.assertEquals(rule.description(), "Avoid checkpanic"); Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL); } + + @Test(description = "test unused class fields rule") + void testUnusedClassFieldsRule() { + Rule rule = CoreRule.UNUSED_CLASS_FIELDS.rule(); + Assert.assertEquals(rule.id(), "ballerina:5"); + Assert.assertEquals(rule.numericId(), 5); + Assert.assertEquals(rule.description(), "Unused private fields"); + Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL); + } } diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java b/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java new file mode 100644 index 00000000..e2a5256d --- /dev/null +++ b/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java @@ -0,0 +1,33 @@ +package io.ballerina.scan.internal; + +import io.ballerina.projects.Document; +import io.ballerina.scan.Issue; +import io.ballerina.scan.RuleKind; +import io.ballerina.scan.utils.Constants; +import org.testng.Assert; +import org.testng.annotations.Test; + +import java.util.List; + +public class Rule005Test extends StaticCodeAnalyzerTest { + @Test(description = "test checkpanic analyzer") + void testUnusedClassFieldsAnalyzer() { + String documentName = "rule005_unused_class_fields.bal"; + Document document = loadDocument(documentName); + ScannerContextImpl scannerContext = new ScannerContextImpl(List.of(CoreRule.UNUSED_CLASS_FIELDS.rule())); + StaticCodeAnalyzer staticCodeAnalyzer = new StaticCodeAnalyzer(document, scannerContext); + staticCodeAnalyzer.analyze(); + List issues = scannerContext.getReporter().getIssues(); + Assert.assertEquals(issues.size(), 5); + assertIssue(issues.get(0), documentName, 17, 4, 17, 21, "ballerina:5", 5, + Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(1), documentName, 23, 4, 23, 21, "ballerina:5", 5, + Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(2), documentName, 31, 4, 31, 23, "ballerina:5", 5, + Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(3), documentName, 33, 4, 33, 30, "ballerina:5", 5, + Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + assertIssue(issues.get(4), documentName, 75, 4, 75, 21, "ballerina:5", 5, + Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + } +} diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java b/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java index c460055e..22b7ea32 100644 --- a/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java +++ b/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java @@ -43,7 +43,7 @@ public class StaticCodeAnalyzerTest extends BaseTest { private final Path coreRuleBalFiles = testResources.resolve("test-resources").resolve("core-rules"); - private Document loadDocument(String documentName) { + protected Document loadDocument(String documentName) { Project project = SingleFileProject.load(coreRuleBalFiles.resolve(documentName)); Module defaultModule = project.currentPackage().getDefaultModule(); return defaultModule.document(defaultModule.documentIds().iterator().next()); @@ -62,25 +62,6 @@ void testCheckpanicAnalyzer() { Constants.RuleDescription.AVOID_CHECKPANIC, RuleKind.CODE_SMELL); } - @Test(description = "test checkpanic analyzer") - void testUnusedClassFieldsAnalyzer() { - String documentName = "unused_class_fields.bal"; - Document document = loadDocument(documentName); - ScannerContextImpl scannerContext = new ScannerContextImpl(List.of(CoreRule.AVOID_CHECKPANIC.rule())); - StaticCodeAnalyzer staticCodeAnalyzer = new StaticCodeAnalyzer(document, scannerContext); - staticCodeAnalyzer.analyze(); - List issues = scannerContext.getReporter().getIssues(); - Assert.assertEquals(issues.size(), 4); - assertIssue(issues.get(0), documentName, 17, 4, 17, 21, "ballerina:3", 3, - Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); - assertIssue(issues.get(1), documentName, 23, 4, 23, 21, "ballerina:3", 3, - Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); - assertIssue(issues.get(2), documentName, 31, 4, 31, 23, "ballerina:3", 3, - Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); - assertIssue(issues.get(3), documentName, 33, 4, 33, 30, "ballerina:3", 3, - Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); - } - void assertIssue(Issue issue, String documentName, int startLine, int startOffset, int endLine, int endOffset, String ruleId, int numericId, String description, RuleKind ruleKind) { Assert.assertEquals(issue.source(), Source.BUILT_IN); diff --git a/scan-command/src/test/resources/test-resources/core-rules/unused_class_fields.bal b/scan-command/src/test/resources/test-resources/core-rules/rule005_unused_class_fields.bal similarity index 86% rename from scan-command/src/test/resources/test-resources/core-rules/unused_class_fields.bal rename to scan-command/src/test/resources/test-resources/core-rules/rule005_unused_class_fields.bal index d3970f7f..157fe067 100644 --- a/scan-command/src/test/resources/test-resources/core-rules/unused_class_fields.bal +++ b/scan-command/src/test/resources/test-resources/core-rules/rule005_unused_class_fields.bal @@ -32,6 +32,7 @@ public class A3 { private boolean? e; // warning private int|boolean f = 0; private boolean g = false; // warning + public string h; function init() { self.a = 0; @@ -70,3 +71,17 @@ class A4 { self.c = s; } } + +service class SA { + private string b; // warning + private string c = ""; + private string d = ""; + + function init() { + self.test(self.d); + } + + public 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 2ec71776..7f749589 100644 --- a/scan-command/src/test/resources/testng.xml +++ b/scan-command/src/test/resources/testng.xml @@ -29,7 +29,7 @@ under the License. - + From 31a121089ebcf67765ad3ccc29213024d19433fa Mon Sep 17 00:00:00 2001 From: Sasindu Alahakoon Date: Thu, 20 Feb 2025 10:51:32 +0530 Subject: [PATCH 3/9] Refactor tests with rule 5 --- .../io/ballerina/scan/internal/CoreRule.java | 5 ++--- .../scan/internal/StaticCodeAnalyzer.java | 21 +++++++++++++------ .../io/ballerina/scan/utils/Constants.java | 8 ------- .../ballerina/scan/internal/CoreRuleTest.java | 4 ++-- .../ballerina/scan/internal/Rule005Test.java | 16 +++++++------- .../scan/internal/StaticCodeAnalyzerTest.java | 1 - .../unix/list-rules-output.txt | 1 + .../unix/print-rules-to-console.txt | 3 ++- .../windows/list-rules-output.txt | 1 + .../windows/print-rules-to-console.txt | 3 ++- 10 files changed, 34 insertions(+), 29 deletions(-) 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 6b0f124d..0fb67766 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 @@ -20,7 +20,6 @@ import io.ballerina.scan.Rule; import io.ballerina.scan.RuleKind; -import io.ballerina.scan.utils.Constants; import java.util.ArrayList; import java.util.List; @@ -31,9 +30,9 @@ * @since 0.1.0 * */ enum CoreRule { - AVOID_CHECKPANIC(RuleFactory.createRule(1, Constants.RuleDescription.AVOID_CHECKPANIC, RuleKind.CODE_SMELL)), + AVOID_CHECKPANIC(RuleFactory.createRule(1, "Avoid checkpanic", RuleKind.CODE_SMELL)), UNUSED_FUNCTION_PARAMETER(RuleFactory.createRule(2, "Unused function parameter", RuleKind.CODE_SMELL)), - UNUSED_CLASS_FIELDS(RuleFactory.createRule(5, "Unused private fields", RuleKind.CODE_SMELL)); + UNUSED_CLASS_FIELDS(RuleFactory.createRule(5, "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 45f7bff2..66a64419 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 @@ -38,14 +38,12 @@ import io.ballerina.compiler.syntax.tree.SyntaxTree; import io.ballerina.projects.Document; import io.ballerina.scan.ScannerContext; -import io.ballerina.scan.utils.StaticCodeAnalyzerUtils; import java.util.List; +import java.util.Optional; import static io.ballerina.compiler.syntax.tree.SyntaxKind.PRIVATE_KEYWORD; -import java.util.Optional; - /** * {@code StaticCodeAnalyzer} contains the logic to perform core static code analysis on Ballerina documents. * @@ -78,6 +76,7 @@ public void visit(CheckExpressionNode checkExpressionNode) { if (checkExpressionNode.checkKeyword().kind().equals(SyntaxKind.CHECKPANIC_KEYWORD)) { reportIssue(checkExpressionNode, CoreRule.AVOID_CHECKPANIC); } + this.visitSyntaxNode(checkExpressionNode); } @Override @@ -85,14 +84,14 @@ public void visit(ObjectFieldNode objectFieldNode) { semanticModel.symbol(objectFieldNode).ifPresent(symbol -> { if (symbol instanceof ClassFieldSymbol classFieldSymbol) { List qualifiers = classFieldSymbol.qualifiers(); - if (StaticCodeAnalyzerUtils.getQualifier(qualifiers, PRIVATE_KEYWORD.stringValue())) { + if (hasQualifier(qualifiers, PRIVATE_KEYWORD)) { if (semanticModel.references(symbol).size() == 1) { - StaticCodeAnalyzerUtils.reportIssue(scannerContext, document, objectFieldNode, - CoreRule.UNUSED_CLASS_FIELDS.rule()); + reportIssue(objectFieldNode, CoreRule.UNUSED_CLASS_FIELDS); } } } }); + this.visitSyntaxNode(objectFieldNode); } public void visit(FunctionDefinitionNode functionDefinitionNode) { checkUnusedFunctionParameters(functionDefinitionNode.functionSignature()); @@ -148,4 +147,14 @@ private boolean isUnusedNode(Node node) { Optional symbol = semanticModel.symbol(node); return symbol.filter(value -> semanticModel.references(value).size() == 1).isPresent(); } + + private boolean hasQualifier(List qualifierList, SyntaxKind qualifierValue) { + String qualifierValueStr = qualifierValue.stringValue(); + for (Qualifier qualifier : qualifierList) { + if (qualifier.getValue().equals(qualifierValueStr)) { + return true; + } + } + return false; + } } diff --git a/scan-command/src/main/java/io/ballerina/scan/utils/Constants.java b/scan-command/src/main/java/io/ballerina/scan/utils/Constants.java index 9544d71d..372de85d 100644 --- a/scan-command/src/main/java/io/ballerina/scan/utils/Constants.java +++ b/scan-command/src/main/java/io/ballerina/scan/utils/Constants.java @@ -62,14 +62,6 @@ public class Constants { static final String RULE_DESCRIPTION_COLUMN = "Rule Description"; static final String[] RULE_PRIORITY_LIST = {"ballerina", "ballerina/", "ballerinax/", "wso2/"}; - public static class RuleDescription { - public static final String AVOID_CHECKPANIC = "Avoid checkpanic"; - public static final String UNUSED_PRIVATE_FIELDS = "Unused private fields"; - - private RuleDescription() { - } - } - private Constants() { } } 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 acb5ddb4..8ff476f5 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 @@ -34,7 +34,7 @@ public class CoreRuleTest { @Test(description = "test all rules") void testAllRules() { - Assert.assertEquals(CoreRule.rules().size(), 2); + Assert.assertEquals(CoreRule.rules().size(), 3); } @Test(description = "test checkpanic rule") @@ -60,7 +60,7 @@ void testUnusedClassFieldsRule() { Rule rule = CoreRule.UNUSED_CLASS_FIELDS.rule(); Assert.assertEquals(rule.id(), "ballerina:5"); Assert.assertEquals(rule.numericId(), 5); - Assert.assertEquals(rule.description(), "Unused private fields"); + Assert.assertEquals(rule.description(), "Unused class private fields"); Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL); } } diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java b/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java index e2a5256d..b78f1971 100644 --- a/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java +++ b/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java @@ -3,31 +3,33 @@ import io.ballerina.projects.Document; import io.ballerina.scan.Issue; import io.ballerina.scan.RuleKind; -import io.ballerina.scan.utils.Constants; import org.testng.Assert; import org.testng.annotations.Test; import java.util.List; public class Rule005Test extends StaticCodeAnalyzerTest { + private static final String UNUSED_CLASS_PRIVATE_FIELDS = "Unused class private fields"; + @Test(description = "test checkpanic analyzer") void testUnusedClassFieldsAnalyzer() { String documentName = "rule005_unused_class_fields.bal"; Document document = loadDocument(documentName); ScannerContextImpl scannerContext = new ScannerContextImpl(List.of(CoreRule.UNUSED_CLASS_FIELDS.rule())); - StaticCodeAnalyzer staticCodeAnalyzer = new StaticCodeAnalyzer(document, scannerContext); + 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, 21, "ballerina:5", 5, - Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); assertIssue(issues.get(1), documentName, 23, 4, 23, 21, "ballerina:5", 5, - Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); assertIssue(issues.get(2), documentName, 31, 4, 31, 23, "ballerina:5", 5, - Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); assertIssue(issues.get(3), documentName, 33, 4, 33, 30, "ballerina:5", 5, - Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); assertIssue(issues.get(4), documentName, 75, 4, 75, 21, "ballerina:5", 5, - Constants.RuleDescription.UNUSED_PRIVATE_FIELDS, RuleKind.CODE_SMELL); + UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); } } diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java b/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java index aea7b850..32faf327 100644 --- a/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java +++ b/scan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.java @@ -27,7 +27,6 @@ import io.ballerina.scan.Rule; import io.ballerina.scan.RuleKind; import io.ballerina.scan.Source; -import io.ballerina.scan.utils.Constants; import io.ballerina.tools.text.LineRange; import org.testng.Assert; 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 5ad2e669..7dcaca3c 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 @@ -3,6 +3,7 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-proj --------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter + ballerina:5 | 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 431cad6f..7dcaca3c 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 @@ -3,6 +3,7 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-proj --------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter + ballerina:5 | 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 @@ -11,4 +12,4 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-proj ballerinax/example_module_static_code_analyzer:3 | VULNERABILITY | rule 3 exampleOrg/example_module_static_code_analyzer:1 | CODE_SMELL | rule 1 exampleOrg/example_module_static_code_analyzer:2 | BUG | rule 2 - exampleOrg/example_module_static_code_analyzer:3 | VULNERABILITY | rule 3 \ No newline at end of file + exampleOrg/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 e384b4ea..0a422951 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 @@ -3,6 +3,7 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-proj --------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter + ballerina:5 | 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 cab063a4..0a422951 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 @@ -3,6 +3,7 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-proj --------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter + ballerina:5 | 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 @@ -11,4 +12,4 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-proj ballerinax/example_module_static_code_analyzer:3 | VULNERABILITY | rule 3 exampleOrg/example_module_static_code_analyzer:1 | CODE_SMELL | rule 1 exampleOrg/example_module_static_code_analyzer:2 | BUG | rule 2 - exampleOrg/example_module_static_code_analyzer:3 | VULNERABILITY | rule 3 \ No newline at end of file + exampleOrg/example_module_static_code_analyzer:3 | VULNERABILITY | rule 3 From 5167c2827d8cd1f3541c07f5db9a704ec7dc817d Mon Sep 17 00:00:00 2001 From: Sasindu Alahakoon Date: Thu, 20 Feb 2025 13:27:02 +0530 Subject: [PATCH 4/9] Update assert tests reports --- .../test/resources/command-outputs/unix/list-rules-output.txt | 4 ++-- .../resources/command-outputs/unix/print-rules-to-console.txt | 4 ++-- .../resources/command-outputs/windows/list-rules-output.txt | 4 ++-- .../command-outputs/windows/print-rules-to-console.txt | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) 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 7dcaca3c..ac224862 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 @@ -1,6 +1,6 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-project-with-config-file/Scan.toml - RuleID | Rule Kind | Rule Description - --------------------------------------------------------------------------------------------- + RuleID | Rule Kind | Rule Description + ----------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter ballerina:5 | CODE_SMELL | Unused class private fields 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 7dcaca3c..ac224862 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 @@ -1,6 +1,6 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-project-with-config-file/Scan.toml - RuleID | Rule Kind | Rule Description - --------------------------------------------------------------------------------------------- + RuleID | Rule Kind | Rule Description + ----------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter ballerina:5 | CODE_SMELL | Unused class private fields 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 0a422951..053c767b 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 @@ -1,6 +1,6 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-project-with-config-file\Scan.toml - RuleID | Rule Kind | Rule Description - --------------------------------------------------------------------------------------------- + RuleID | Rule Kind | Rule Description + ----------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter ballerina:5 | CODE_SMELL | Unused class private fields 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 0a422951..053c767b 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 @@ -1,6 +1,6 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-project-with-config-file\Scan.toml - RuleID | Rule Kind | Rule Description - --------------------------------------------------------------------------------------------- + RuleID | Rule Kind | Rule Description + ----------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter ballerina:5 | CODE_SMELL | Unused class private fields From da4ca67903291f98a458bebc641d875512fd32ed Mon Sep 17 00:00:00 2001 From: Sasindu Alahakoon Date: Tue, 25 Feb 2025 15:07:15 +0530 Subject: [PATCH 5/9] Update unused private class fields tests --- .../io/ballerina/scan/internal/CoreRule.java | 2 +- .../scan/internal/StaticCodeAnalyzer.java | 2 +- .../ballerina/scan/internal/CoreRuleTest.java | 2 +- .../ballerina/scan/internal/Rule005Test.java | 34 +++++++++++++++---- .../rule005_unused_class_fields.bal | 12 +++---- 5 files changed, 37 insertions(+), 15 deletions(-) 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 0fb67766..df1edbb0 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,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_CLASS_FIELDS(RuleFactory.createRule(5, "Unused class private fields", RuleKind.CODE_SMELL)); + UNUSED_PRIVATE_CLASS_FIELD(RuleFactory.createRule(5, "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 66a64419..4b718e9d 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 @@ -86,7 +86,7 @@ public void visit(ObjectFieldNode objectFieldNode) { List qualifiers = classFieldSymbol.qualifiers(); if (hasQualifier(qualifiers, PRIVATE_KEYWORD)) { if (semanticModel.references(symbol).size() == 1) { - reportIssue(objectFieldNode, CoreRule.UNUSED_CLASS_FIELDS); + reportIssue(objectFieldNode, CoreRule.UNUSED_PRIVATE_CLASS_FIELD); } } } 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 8ff476f5..21fb6436 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 @@ -57,7 +57,7 @@ void testUnusedFunctionParameterRule() { @Test(description = "test unused class fields rule") void testUnusedClassFieldsRule() { - Rule rule = CoreRule.UNUSED_CLASS_FIELDS.rule(); + Rule rule = CoreRule.UNUSED_PRIVATE_CLASS_FIELD.rule(); Assert.assertEquals(rule.id(), "ballerina:5"); Assert.assertEquals(rule.numericId(), 5); Assert.assertEquals(rule.description(), "Unused class private fields"); diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java b/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java index b78f1971..c5b024ff 100644 --- a/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java +++ b/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java @@ -1,3 +1,20 @@ +/* + * 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; @@ -8,28 +25,33 @@ import java.util.List; +/** + * Unused private class fields analyzer tests. + * + * @since 0.5.0 + */ public class Rule005Test extends StaticCodeAnalyzerTest { private static final String UNUSED_CLASS_PRIVATE_FIELDS = "Unused class private fields"; - @Test(description = "test checkpanic analyzer") + @Test(description = "test unused private class fields") void testUnusedClassFieldsAnalyzer() { String documentName = "rule005_unused_class_fields.bal"; Document document = loadDocument(documentName); - ScannerContextImpl scannerContext = new ScannerContextImpl(List.of(CoreRule.UNUSED_CLASS_FIELDS.rule())); + 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, 21, "ballerina:5", 5, + assertIssue(issues.get(0), documentName, 17, 4, 17, 26, "ballerina:5", 5, UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); - assertIssue(issues.get(1), documentName, 23, 4, 23, 21, "ballerina:5", 5, + assertIssue(issues.get(1), documentName, 23, 4, 23, 26, "ballerina:5", 5, UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); - assertIssue(issues.get(2), documentName, 31, 4, 31, 23, "ballerina:5", 5, + assertIssue(issues.get(2), documentName, 31, 4, 31, 30, "ballerina:5", 5, UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); assertIssue(issues.get(3), documentName, 33, 4, 33, 30, "ballerina:5", 5, UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); - assertIssue(issues.get(4), documentName, 75, 4, 75, 21, "ballerina:5", 5, + assertIssue(issues.get(4), documentName, 75, 4, 75, 26, "ballerina:5", 5, UNUSED_CLASS_PRIVATE_FIELDS, RuleKind.CODE_SMELL); } } diff --git a/scan-command/src/test/resources/test-resources/core-rules/rule005_unused_class_fields.bal b/scan-command/src/test/resources/test-resources/core-rules/rule005_unused_class_fields.bal index 157fe067..41098349 100644 --- a/scan-command/src/test/resources/test-resources/core-rules/rule005_unused_class_fields.bal +++ b/scan-command/src/test/resources/test-resources/core-rules/rule005_unused_class_fields.bal @@ -15,24 +15,24 @@ // under the License. class A { - private string c; // warning + private string c = ""; // warning } public class A2 { int a = 1; public string b = ""; - private string c; // warning + private string c = ""; // warning } public class A3 { int a; public string b; private string c; - private boolean d; - private boolean? e; // warning + private boolean d = true; + private boolean? e = true; // warning private int|boolean f = 0; private boolean g = false; // warning - public string h; + public string h = ""; function init() { self.a = 0; @@ -73,7 +73,7 @@ class A4 { } service class SA { - private string b; // warning + private string b = ""; // warning private string c = ""; private string d = ""; From 03da8e3f90b66e8f55b24f2907848f539add1cf9 Mon Sep 17 00:00:00 2001 From: Sasindu Alahakoon Date: Sat, 15 Mar 2025 00:12:55 +0530 Subject: [PATCH 6/9] Rename the test method --- .../src/test/java/io/ballerina/scan/internal/Rule005Test.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java b/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java index c5b024ff..4c5da53c 100644 --- a/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java +++ b/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java @@ -34,7 +34,7 @@ public class Rule005Test extends StaticCodeAnalyzerTest { private static final String UNUSED_CLASS_PRIVATE_FIELDS = "Unused class private fields"; @Test(description = "test unused private class fields") - void testUnusedClassFieldsAnalyzer() { + void testUnusedPrivateFieldsAnalyzer() { String documentName = "rule005_unused_class_fields.bal"; Document document = loadDocument(documentName); ScannerContextImpl scannerContext = new ScannerContextImpl(List.of(CoreRule.UNUSED_PRIVATE_CLASS_FIELD.rule())); From d6e8a5c9fd697535654f2d3f8f5f738598576f4e Mon Sep 17 00:00:00 2001 From: Sasindu Alahakoon Date: Sat, 15 Mar 2025 23:21:20 +0530 Subject: [PATCH 7/9] Update rule ids --- .../java/io/ballerina/scan/internal/CoreRule.java | 2 +- .../io/ballerina/scan/internal/CoreRuleTest.java | 4 ++-- .../{Rule005Test.java => Rule011Test.java} | 14 +++++++------- ..._fields.bal => rule011_unused_class_fields.bal} | 0 scan-command/src/test/resources/testng.xml | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) rename scan-command/src/test/java/io/ballerina/scan/internal/{Rule005Test.java => Rule011Test.java} (91%) rename scan-command/src/test/resources/test-resources/core-rules/{rule005_unused_class_fields.bal => rule011_unused_class_fields.bal} (100%) 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 df1edbb0..a5fc57e4 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,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_PRIVATE_CLASS_FIELD(RuleFactory.createRule(5, "Unused class private fields", 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/test/java/io/ballerina/scan/internal/CoreRuleTest.java b/scan-command/src/test/java/io/ballerina/scan/internal/CoreRuleTest.java index 21fb6436..0e9900be 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 @@ -58,8 +58,8 @@ void testUnusedFunctionParameterRule() { @Test(description = "test unused class fields rule") void testUnusedClassFieldsRule() { Rule rule = CoreRule.UNUSED_PRIVATE_CLASS_FIELD.rule(); - Assert.assertEquals(rule.id(), "ballerina:5"); - Assert.assertEquals(rule.numericId(), 5); + Assert.assertEquals(rule.id(), "ballerina:11"); + Assert.assertEquals(rule.numericId(), 11); Assert.assertEquals(rule.description(), "Unused class private fields"); Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL); } diff --git a/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java b/scan-command/src/test/java/io/ballerina/scan/internal/Rule011Test.java similarity index 91% rename from scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java rename to scan-command/src/test/java/io/ballerina/scan/internal/Rule011Test.java index 4c5da53c..33e60e12 100644 --- a/scan-command/src/test/java/io/ballerina/scan/internal/Rule005Test.java +++ b/scan-command/src/test/java/io/ballerina/scan/internal/Rule011Test.java @@ -30,12 +30,12 @@ * * @since 0.5.0 */ -public class Rule005Test extends StaticCodeAnalyzerTest { +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 = "rule005_unused_class_fields.bal"; + 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, @@ -43,15 +43,15 @@ void testUnusedPrivateFieldsAnalyzer() { staticCodeAnalyzer.analyze(); List issues = scannerContext.getReporter().getIssues(); Assert.assertEquals(issues.size(), 5); - assertIssue(issues.get(0), documentName, 17, 4, 17, 26, "ballerina:5", 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:5", 5, + 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:5", 5, + 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:5", 5, + 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:5", 5, + 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/test-resources/core-rules/rule005_unused_class_fields.bal b/scan-command/src/test/resources/test-resources/core-rules/rule011_unused_class_fields.bal similarity index 100% rename from scan-command/src/test/resources/test-resources/core-rules/rule005_unused_class_fields.bal rename to scan-command/src/test/resources/test-resources/core-rules/rule011_unused_class_fields.bal diff --git a/scan-command/src/test/resources/testng.xml b/scan-command/src/test/resources/testng.xml index 16503d3b..df932174 100644 --- a/scan-command/src/test/resources/testng.xml +++ b/scan-command/src/test/resources/testng.xml @@ -31,7 +31,7 @@ under the License. - + From 5a6388201830a47e17ffc071ef97a6247ae21bd9 Mon Sep 17 00:00:00 2001 From: Sasindu Alahakoon Date: Sat, 15 Mar 2025 23:46:23 +0530 Subject: [PATCH 8/9] Update rule logs --- .../test/resources/command-outputs/unix/list-rules-output.txt | 2 +- .../resources/command-outputs/unix/print-rules-to-console.txt | 2 +- .../resources/command-outputs/windows/list-rules-output.txt | 2 +- .../command-outputs/windows/print-rules-to-console.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) 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 ac224862..1c41cee8 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 @@ -3,7 +3,7 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-proj ----------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter - ballerina:5 | CODE_SMELL | Unused class private fields + 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 ac224862..1c41cee8 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 @@ -3,7 +3,7 @@ Loading scan tool configurations from src/test/resources/test-resources/bal-proj ----------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter - ballerina:5 | CODE_SMELL | Unused class private fields + 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 053c767b..c9703f21 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 @@ -3,7 +3,7 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-proj ----------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter - ballerina:5 | CODE_SMELL | Unused class private fields + 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 053c767b..c9703f21 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 @@ -3,7 +3,7 @@ Loading scan tool configurations from src\test\resources\test-resources\bal-proj ----------------------------------------------------------------------------------------------- ballerina:1 | CODE_SMELL | Avoid checkpanic ballerina:2 | CODE_SMELL | Unused function parameter - ballerina:5 | CODE_SMELL | Unused class private fields + 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 From 882ce717987d497363787e60cbf72307beaecb1f Mon Sep 17 00:00:00 2001 From: Sasindu Alahakoon Date: Sun, 16 Mar 2025 11:01:00 +0530 Subject: [PATCH 9/9] Update unused field rules --- .../java/io/ballerina/scan/internal/CoreRule.java | 7 +++---- .../scan/internal/StaticCodeAnalyzer.java | 10 ---------- .../core-rules/rule011_unused_class_fields.bal | 14 +++++++------- 3 files changed, 10 insertions(+), 21 deletions(-) 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 3da77b73..d7e4db33 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 @@ -33,9 +33,6 @@ 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_PRIVATE_CLASS_FIELD(RuleFactory.createRule(11, "Unused class private fields", 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, @@ -51,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 69b8fec0..e9762fd9 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 @@ -414,16 +414,6 @@ private boolean isUnusedNode(Node node) { Optional symbol = semanticModel.symbol(node); return symbol.filter(value -> semanticModel.references(value).size() == 1).isPresent(); } - - private boolean hasQualifier(List qualifierList, SyntaxKind qualifierValue) { - String qualifierValueStr = qualifierValue.stringValue(); - for (Qualifier qualifier : qualifierList) { - if (qualifier.getValue().equals(qualifierValueStr)) { - return true; - } - } - return false; - } private boolean isPublicIsolatedConstruct(NodeList qualifiers) { return hasQualifier(qualifiers, PUBLIC_KEYWORD) && !hasQualifier(qualifiers, ISOLATED_KEYWORD); 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 index 41098349..81bda445 100644 --- 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 @@ -18,13 +18,13 @@ class A { private string c = ""; // warning } -public class A2 { +public isolated class A2 { int a = 1; public string b = ""; private string c = ""; // warning } -public class A3 { +public isolated class A3 { int a; public string b; private string c; @@ -40,18 +40,18 @@ public class A3 { self.c = ""; } - public function test(boolean d) { + public isolated function test(boolean d) { self.d = d; } - public function test2(boolean? e) { + public isolated function test2(boolean? e) { if (e is ()) { return; } self.d = e; } - public function test3(boolean e) { + public isolated function test3(boolean e) { if (self.f is int) { return; } @@ -67,7 +67,7 @@ class A4 { self.test(self.d); } - public function test(string s) { + function test(string s) { self.c = s; } } @@ -81,7 +81,7 @@ service class SA { self.test(self.d); } - public function test(string s) { + isolated function test(string s) { self.c = s; } }