Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add static rule for non isolated public constructs #35

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add static rule for non isolated public constructs #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
665d674
Add rule for non isolated public construct
SasinduDilshara Oct 1, 2024
a9e43f6
Merge branch 'static-code-analysis-tool' of https://github.com/baller…
SasinduDilshara Oct 9, 2024
a2e5611
Update scan command tests for public constructs
SasinduDilshara Oct 9, 2024
ad37667
Merge branch 'static-code-analysis-tool' of https://github.com/baller…
SasinduDilshara Oct 31, 2024
8f89ef7
Add static rule warning for public isolated constructs
SasinduDilshara Nov 1, 2024
22fd8a2
Fix windows build issue with non isolated public construct static rule
SasinduDilshara Nov 1, 2024
3741490
Refactor utill methods
SasinduDilshara Feb 2, 2025
4341403
Merge branch 'main' of https://github.com/ballerina-platform/static-c…
SasinduDilshara Feb 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions scan-command/src/main/java/io/ballerina/scan/internal/CoreRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@
* */
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_FUNCTIONS_OR_METHOD_CONSTRUCT(RuleFactory.createRule(3,
"Non isolated public function or method", RuleKind.CODE_SMELL)),
PUBLIC_NON_ISOLATED_CLASS_OR_OBJECT_CONSTRUCT(RuleFactory.createRule(4,
"Non isolated public class or object", RuleKind.CODE_SMELL));

private final Rule rule;

Expand Down
120 changes: 120 additions & 0 deletions scan-command/src/main/java/io/ballerina/scan/internal/StaticCodeAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,14 @@
package io.ballerina.scan.internal;

import io.ballerina.compiler.api.SemanticModel;
import io.ballerina.compiler.api.symbols.ObjectTypeSymbol;
import io.ballerina.compiler.api.symbols.Qualifier;
import io.ballerina.compiler.api.symbols.Symbol;
import io.ballerina.compiler.api.symbols.SymbolKind;
import io.ballerina.compiler.api.symbols.TypeDefinitionSymbol;
import io.ballerina.compiler.api.symbols.TypeSymbol;
import io.ballerina.compiler.syntax.tree.CheckExpressionNode;
import io.ballerina.compiler.syntax.tree.ClassDefinitionNode;
import io.ballerina.compiler.syntax.tree.ExplicitAnonymousFunctionExpressionNode;
import io.ballerina.compiler.syntax.tree.FunctionDefinitionNode;
import io.ballerina.compiler.syntax.tree.FunctionSignatureNode;
Expand All @@ -29,15 +35,25 @@
import io.ballerina.compiler.syntax.tree.IncludedRecordParameterNode;
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.NodeVisitor;
import io.ballerina.compiler.syntax.tree.ObjectTypeDescriptorNode;
import io.ballerina.compiler.syntax.tree.SimpleNameReferenceNode;
import io.ballerina.compiler.syntax.tree.SyntaxKind;
import io.ballerina.compiler.syntax.tree.SyntaxTree;
import io.ballerina.compiler.syntax.tree.Token;
import io.ballerina.compiler.syntax.tree.TypeDefinitionNode;
import io.ballerina.projects.Document;
import io.ballerina.scan.ScannerContext;

import java.util.List;
import java.util.Optional;

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;
import static io.ballerina.scan.utils.Constants.MAIN_FUNCTION;

/**
* {@code StaticCodeAnalyzer} contains the logic to perform core static code analysis on Ballerina documents.
*
Expand Down Expand Up @@ -75,6 +91,11 @@ public void visit(CheckExpressionNode checkExpressionNode) {
@Override
public void visit(FunctionDefinitionNode functionDefinitionNode) {
checkUnusedFunctionParameters(functionDefinitionNode.functionSignature());
String functionName = functionDefinitionNode.functionName().text();
if (!functionName.equals(MAIN_FUNCTION) &&
!functionName.equals(INIT_FUNCTION)) {
checkNonIsolatedPublicFunctions(functionDefinitionNode);
}
this.visitSyntaxNode(functionDefinitionNode);
}

Expand All @@ -100,6 +121,105 @@ public void visit(ImplicitAnonymousFunctionExpressionNode implicitAnonymousFunct
this.visitSyntaxNode(implicitAnonymousFunctionExpressionNode.expression());
}

@Override
public void visit(ClassDefinitionNode classDefinitionNode) {
checkNonIsolatedPublicClassDefinition(classDefinitionNode);
checkNonIsolatedPublicClassMembers(classDefinitionNode);
}

@Override
public void visit(TypeDefinitionNode typeDefinitionNode) {
semanticModel.symbol(typeDefinitionNode).ifPresent(symbol -> {
if (symbol instanceof TypeDefinitionSymbol typeDefinitionSymbol) {
TypeSymbol typeSymbol = typeDefinitionSymbol.typeDescriptor();
if (typeSymbol instanceof ObjectTypeSymbol objectTypeSymbol) {
List<Qualifier> qualifiers = objectTypeSymbol.qualifiers();
List<Qualifier> typeDefQualifiers = typeDefinitionSymbol.qualifiers();
if (hasQualifier(typeDefQualifiers, PUBLIC_KEYWORD) &&
!hasQualifier(qualifiers, ISOLATED_KEYWORD)) {
reportIssue(typeDefinitionNode, CoreRule.PUBLIC_NON_ISOLATED_CLASS_OR_OBJECT_CONSTRUCT);
}
}
}
});
}

private void checkNonIsolatedPublicClassDefinition(ClassDefinitionNode classDefinitionNode) {
semanticModel.symbol(classDefinitionNode).ifPresent(symbol -> {
if (symbol instanceof ObjectTypeSymbol objectTypeSymbol) {
List<Qualifier> qualifiers = objectTypeSymbol.qualifiers();
if (hasQualifier(qualifiers, PUBLIC_KEYWORD) &&
!hasQualifier(qualifiers, ISOLATED_KEYWORD)) {
reportIssue(classDefinitionNode, CoreRule.PUBLIC_NON_ISOLATED_CLASS_OR_OBJECT_CONSTRUCT);
}
}
});
}

private void checkNonIsolatedPublicClassMembers(ClassDefinitionNode classDefinitionNode) {
semanticModel.symbol(classDefinitionNode).ifPresent(classSymbol -> {
if (classSymbol instanceof ObjectTypeSymbol objectTypeSymbol) {
boolean isPublicObjectTypeSymbol = hasQualifier(objectTypeSymbol.qualifiers(), PUBLIC_KEYWORD);
classDefinitionNode.members().forEach(member -> {
semanticModel.symbol(member).ifPresent(memberSymbol -> {
if (isPublicObjectTypeSymbol && memberSymbol.kind() == SymbolKind.METHOD) {
checkNonIsolatedPublicMethods((FunctionDefinitionNode) member);
}
member.accept(this);
});
});
}
});
}

private void checkNonIsolatedPublicObjectMembers(ObjectTypeDescriptorNode objectTypeDescriptorNode) {
objectTypeDescriptorNode.members().forEach(member -> {
semanticModel.symbol(member).ifPresent(memberSymbol -> {
if (memberSymbol.kind() == SymbolKind.METHOD) {
checkNonIsolatedPublicMethods((FunctionDefinitionNode) member);
}
member.accept(this);
});
});
}

private void checkNonIsolatedPublicMethods(FunctionDefinitionNode member) {
if (hasQualifier(member.qualifierList(), PUBLIC_KEYWORD) &&
!hasQualifier(member.qualifierList(), ISOLATED_KEYWORD)) {
reportIssue(member, CoreRule.PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT);
}
}

private void checkNonIsolatedPublicFunctions(FunctionDefinitionNode functionDefinitionNode) {
semanticModel.symbol(functionDefinitionNode).ifPresent(symbol -> {
if (symbol.kind() != SymbolKind.METHOD) {
NodeList<Token> qualifiers = functionDefinitionNode.qualifierList();
if (hasQualifier(qualifiers, PUBLIC_KEYWORD) && !hasQualifier(qualifiers, ISOLATED_KEYWORD)) {
reportIssue(functionDefinitionNode, CoreRule.PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT);
}
}
});
}

private boolean hasQualifier(List<Qualifier> qualifierList, SyntaxKind qualifierValue) {
String qualifierValueStr = qualifierValue.stringValue();
for (Qualifier qualifier : qualifierList) {
if (qualifier.getValue().equals(qualifierValueStr)) {
return true;
}
}
return false;
}

private boolean hasQualifier(NodeList<Token> qualifierList, SyntaxKind qualifier) {
for (Token token : qualifierList) {
if (qualifier == token.kind()) {
return true;
}
}
return false;
}

private void checkUnusedFunctionParameters(FunctionSignatureNode functionSignatureNode) {
functionSignatureNode.parameters().forEach(parameter -> {
if (parameter instanceof IncludedRecordParameterNode includedRecordParameterNode) {
Expand Down
2 changes: 2 additions & 0 deletions scan-command/src/main/java/io/ballerina/scan/utils/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ public class Constants {
static final String RULE_KIND_COLUMN = "Rule Kind";
static final String RULE_DESCRIPTION_COLUMN = "Rule Description";
static final String[] RULE_PRIORITY_LIST = {"ballerina", "ballerina/", "ballerinax/", "wso2/"};
public static final String MAIN_FUNCTION = "main";
public static final String INIT_FUNCTION = "init";

private Constants() {
}
Expand Down
23 changes: 22 additions & 1 deletion scan-command/src/test/java/io/ballerina/scan/internal/CoreRuleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,13 @@
public class CoreRuleTest {
public static final String AVOID_CHECKPANIC = "Avoid checkpanic";
public static final String UNUSED_FUNCTION_PARAMETER = "Unused function parameter";
public static final String PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT =
"Non isolated public function or method";
public static final String PUBLIC_NON_ISOLATED_CLASS_OR_OBJECT_CONSTRUCT = "Non isolated public class or object";

@Test(description = "test all rules")
void testAllRules() {
Assert.assertEquals(CoreRule.rules().size(), 2);
Assert.assertEquals(CoreRule.rules().size(), 3);
}

@Test(description = "test checkpanic rule")
Expand All @@ -54,4 +57,22 @@ void testUnusedFunctionParameterRule() {
Assert.assertEquals(rule.description(), UNUSED_FUNCTION_PARAMETER);
Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL);
}

@Test(description = "test non isolated public functions or methods")
void testNonIsolatedPublicFunctionOrMethodConstructsRule() {
Rule rule = CoreRule.PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT.rule();
Assert.assertEquals(rule.id(), "ballerina:3");
Assert.assertEquals(rule.numericId(), 3);
Assert.assertEquals(rule.description(), PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT);
Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL);
}

@Test(description = "test non isolated public class or object constructs")
void testNonIsolatedPublicClassOrObjectConstructsRule() {
Rule rule = CoreRule.PUBLIC_NON_ISOLATED_CLASS_OR_OBJECT_CONSTRUCT.rule();
Assert.assertEquals(rule.id(), "ballerina:4");
Assert.assertEquals(rule.numericId(), 4);
Assert.assertEquals(rule.description(), PUBLIC_NON_ISOLATED_CLASS_OR_OBJECT_CONSTRUCT);
Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL);
}
}
18 changes: 9 additions & 9 deletions scan-command/src/test/java/io/ballerina/scan/internal/Rule002Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ void testUnusedFunctionParameter() {
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(11), documentName, 72, 19, 72, 24, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(12), documentName, 76, 18, 76, 23, "ballerina:2", 2,
assertIssue(issues.get(12), documentName, 76, 11, 76, 16, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(13), documentName, 76, 32, 76, 37, "ballerina:2", 2,
assertIssue(issues.get(13), documentName, 76, 25, 76, 30, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(14), documentName, 77, 22, 77, 28, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
Expand All @@ -93,31 +93,31 @@ void testUnusedAnonymousFunctionParameter() {
List<Issue> issues = scannerContext.getReporter().getIssues();
Assert.assertEquals(issues.size(), 16);

assertIssue(issues.get(0), documentName, 16, 34, 16, 39, "ballerina:2", 2,
assertIssue(issues.get(0), documentName, 16, 27, 16, 32, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(1), documentName, 16, 41, 16, 46, "ballerina:2", 2,
assertIssue(issues.get(1), documentName, 16, 34, 16, 39, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(2), documentName, 17, 17, 17, 24, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(3), documentName, 25, 26, 25, 31, "ballerina:2", 2,
assertIssue(issues.get(3), documentName, 25, 19, 25, 24, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(4), documentName, 27, 50, 27, 57, "ballerina:2", 2,
assertIssue(issues.get(4), documentName, 27, 43, 27, 50, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(5), documentName, 29, 48, 29, 49, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(6), documentName, 31, 61, 31, 66, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(7), documentName, 33, 26, 33, 31, "ballerina:2", 2,
assertIssue(issues.get(7), documentName, 33, 19, 33, 24, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(8), documentName, 33, 57, 33, 64, "ballerina:2", 2,
assertIssue(issues.get(8), documentName, 33, 50, 33, 57, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(9), documentName, 39, 26, 39, 31, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(10), documentName, 46, 12, 46, 13, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(11), documentName, 47, 17, 47, 22, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(12), documentName, 53, 28, 53, 33, "ballerina:2", 2,
assertIssue(issues.get(12), documentName, 53, 21, 53, 26, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
assertIssue(issues.get(13), documentName, 56, 19, 56, 26, "ballerina:2", 2,
UNUSED_FUNCTION_PARAMETER, RuleKind.CODE_SMELL);
Expand Down
81 changes: 81 additions & 0 deletions scan-command/src/test/java/io/ballerina/scan/internal/Rule003Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* 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;

/**
* Non isolated public constructs usage tests.
*
* @since 0.1.0
*/
public class Rule003Test extends StaticCodeAnalyzerTest {
public static final String PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT =
"Non isolated public function or method";
public static final String PUBLIC_NON_ISOLATED_CLASS_OR_OBJECT_CONSTRUCT =
"Non isolated public class or object";

@Test(description = "test non isolated public functions analyzer")
void testNonIsolatedPublicFunctionOrMethodConstructsUsage() {
String documentName = "rule003_rule_non_isolated_public_functions_or_methods.bal";
Document document = loadDocument(documentName);
ScannerContextImpl scannerContext = new ScannerContextImpl(List.of(
CoreRule.PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT.rule()));
StaticCodeAnalyzer staticCodeAnalyzer = new StaticCodeAnalyzer(document, scannerContext,
document.module().getCompilation().getSemanticModel());
staticCodeAnalyzer.analyze();

List<Issue> issues = scannerContext.getReporter().getIssues();
Assert.assertEquals(issues.size(), 6);
assertIssue(issues.get(0), documentName, 16, 0, 18, 1, "ballerina:3", 3,
PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(1), documentName, 63, 4, 65, 5, "ballerina:3", 3,
PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(2), documentName, 67, 4, 69, 5, "ballerina:3", 3,
PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(3), documentName, 75, 4, 77, 5, "ballerina:3", 3,
PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(4), documentName, 89, 4, 91, 5, "ballerina:3", 3,
PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(5), documentName, 97, 4, 99, 5, "ballerina:3", 3,
PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
}

@Test(description = "test non isolated public class or object analyzer")
void testNonIsolatedPublicClassOrObjectConstructsUsage() {
String documentName = "rule004_rule_non_isolated_public_classes_or_objects.bal";
Document document = loadDocument(documentName);
ScannerContextImpl scannerContext = new ScannerContextImpl(List.of(
CoreRule.PUBLIC_NON_ISOLATED_FUNCTIONS_OR_METHOD_CONSTRUCT.rule()));
StaticCodeAnalyzer staticCodeAnalyzer = new StaticCodeAnalyzer(document, scannerContext,
document.module().getCompilation().getSemanticModel());
staticCodeAnalyzer.analyze();

List<Issue> issues = scannerContext.getReporter().getIssues();
Assert.assertEquals(issues.size(), 2);
assertIssue(issues.get(0), documentName, 20, 0, 22, 1, "ballerina:4", 4,
PUBLIC_NON_ISOLATED_CLASS_OR_OBJECT_CONSTRUCT, RuleKind.CODE_SMELL);
}
}
5 changes: 3 additions & 2 deletions scan-command/src/test/resources/command-outputs/unix/list-rules-output.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
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:3 | CODE_SMELL | Non isolated public class or function/method
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
Expand Down
Loading
Loading