Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,15 @@
* */
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,
"Non isolated public method", RuleKind.CODE_SMELL)),
PUBLIC_NON_ISOLATED_CLASS_CONSTRUCT(RuleFactory.createRule(5,
"Non isolated public class", RuleKind.CODE_SMELL)),
PUBLIC_NON_ISOLATED_OBJECT_CONSTRUCT(RuleFactory.createRule(6,
"Non isolated public object", RuleKind.CODE_SMELL));

private final Rule rule;

Expand Down
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,24 @@
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.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 +90,10 @@ 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)) {
checkNonIsolatedPublicFunction(functionDefinitionNode);
}
this.visitSyntaxNode(functionDefinitionNode);
}

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

@Override
public void visit(ClassDefinitionNode classDefinitionNode) {
checkNonIsolatedPublicClassDefinition(classDefinitionNode);
checkNonIsolatedPublicClassMethod(classDefinitionNode);
this.visitSyntaxNode(classDefinitionNode);
}

@Override
public void visit(TypeDefinitionNode typeDefinitionNode) {
checkNonIsolatedConstructsInTypeDefinition(typeDefinitionNode);
this.visitSyntaxNode(typeDefinitionNode);
}

private void checkNonIsolatedConstructsInTypeDefinition(TypeDefinitionNode typeDefinitionNode) {
semanticModel.symbol(typeDefinitionNode).ifPresent(symbol -> {
Comment thread
SasinduDilshara marked this conversation as resolved.
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_OBJECT_CONSTRUCT);
}
}
}
});
}

private void checkNonIsolatedPublicClassDefinition(ClassDefinitionNode classDefinitionNode) {
semanticModel.symbol(classDefinitionNode).ifPresent(symbol -> {
if (symbol instanceof ObjectTypeSymbol objectTypeSymbol) {
List<Qualifier> qualifiers = objectTypeSymbol.qualifiers();
if (isPublicIsolatedConstruct(qualifiers)) {
reportIssue(classDefinitionNode, CoreRule.PUBLIC_NON_ISOLATED_CLASS_CONSTRUCT);
}
}
});
}

private void checkNonIsolatedPublicClassMethod(ClassDefinitionNode classDefinitionNode) {
semanticModel.symbol(classDefinitionNode).ifPresent(classSymbol -> {
if (classSymbol instanceof ObjectTypeSymbol objectTypeSymbol) {
boolean isPublicObjectTypeSymbol = hasQualifier(objectTypeSymbol.qualifiers(), PUBLIC_KEYWORD);
classDefinitionNode.members().forEach(member -> {
Comment thread
SasinduDilshara marked this conversation as resolved.
semanticModel.symbol(member).ifPresent(memberSymbol -> {
if (isPublicObjectTypeSymbol && memberSymbol.kind() == SymbolKind.METHOD) {
checkNonIsolatedPublicMethod((FunctionDefinitionNode) member);
}
});
});
}
});
}

private void checkNonIsolatedPublicMethod(FunctionDefinitionNode member) {
if (isPublicIsolatedConstruct(member.qualifierList())) {
reportIssue(member, CoreRule.PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT);
}
}

private void checkNonIsolatedPublicFunction(FunctionDefinitionNode functionDefinitionNode) {
semanticModel.symbol(functionDefinitionNode).ifPresent(symbol -> {
if (symbol.kind() != SymbolKind.METHOD) {
NodeList<Token> qualifiers = functionDefinitionNode.qualifierList();
if (isPublicIsolatedConstruct(qualifiers)) {
reportIssue(functionDefinitionNode, CoreRule.PUBLIC_NON_ISOLATED_FUNCTION_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 Expand Up @@ -127,4 +237,12 @@ private boolean isUnusedNode(Node node) {
Optional<Symbol> symbol = semanticModel.symbol(node);
return symbol.filter(value -> semanticModel.references(value).size() == 1).isPresent();
}

private boolean isPublicIsolatedConstruct(NodeList<Token> qualifiers) {
return hasQualifier(qualifiers, PUBLIC_KEYWORD) && !hasQualifier(qualifiers, ISOLATED_KEYWORD);
}

private boolean isPublicIsolatedConstruct(List<Qualifier> qualifiers) {
return hasQualifier(qualifiers, PUBLIC_KEYWORD) && !hasQualifier(qualifiers, ISOLATED_KEYWORD);
}
}
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", "ballerinax", "wso2"};
public static final String MAIN_FUNCTION = "main";
public static final String INIT_FUNCTION = "init";
Comment thread
SasinduDilshara marked this conversation as resolved.

private Constants() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,18 @@
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_FUNCTION_CONSTRUCT =
"Non isolated public function";
public static final String PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT =
"Non isolated public method";
public static final String PUBLIC_NON_ISOLATED_CLASS_CONSTRUCT =
"Non isolated public class";
public static final String PUBLIC_NON_ISOLATED_OBJECT_CONSTRUCT =
"Non isolated public object";

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

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

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

@Test(description = "test non isolated public methods")
void testNonIsolatedPublicMethodConstructsRule() {
Rule rule = CoreRule.PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT.rule();
Assert.assertEquals(rule.id(), "ballerina:4");
Assert.assertEquals(rule.numericId(), 4);
Assert.assertEquals(rule.description(), PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT);
Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL);
}

@Test(description = "test non isolated public classes")
void testNonIsolatedPublicClassConstructsRule() {
Rule rule = CoreRule.PUBLIC_NON_ISOLATED_CLASS_CONSTRUCT.rule();
Assert.assertEquals(rule.id(), "ballerina:5");
Assert.assertEquals(rule.numericId(), 5);
Assert.assertEquals(rule.description(), PUBLIC_NON_ISOLATED_CLASS_CONSTRUCT);
Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL);
}

@Test(description = "test non isolated public objects")
void testNonIsolatedPublicObjectConstructsRule() {
Rule rule = CoreRule.PUBLIC_NON_ISOLATED_OBJECT_CONSTRUCT.rule();
Assert.assertEquals(rule.id(), "ballerina:6");
Assert.assertEquals(rule.numericId(), 6);
Assert.assertEquals(rule.description(), PUBLIC_NON_ISOLATED_OBJECT_CONSTRUCT);
Assert.assertEquals(rule.kind(), RuleKind.CODE_SMELL);
}
}
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
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 Rule003And004Test extends StaticCodeAnalyzerTest {
public static final String PUBLIC_NON_ISOLATED_FUNCTION_CONSTRUCT =
"Non isolated public function";
public static final String PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT =
"Non isolated public method";
public static final String PUBLIC_NON_ISOLATED_CLASS_CONSTRUCT =
"Non isolated public class";

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

List<Issue> issues = scannerContext.getReporter().getIssues();
Assert.assertEquals(issues.size(), 12);
assertIssue(issues.get(0), documentName, 16, 0, 18, 1, "ballerina:3", 3,
PUBLIC_NON_ISOLATED_FUNCTION_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(1), documentName, 63, 4, 65, 5, "ballerina:4", 4,
PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(2), documentName, 67, 4, 69, 5, "ballerina:4", 4,
PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(3), documentName, 75, 4, 77, 5, "ballerina:4", 4,
PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(4), documentName, 89, 4, 91, 5, "ballerina:4", 4,
PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(5), documentName, 97, 4, 99, 5, "ballerina:4", 4,
PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(6), documentName, 102, 0, 122, 1, "ballerina:5", 5,
PUBLIC_NON_ISOLATED_CLASS_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(7), documentName, 111, 4, 113, 5, "ballerina:4", 4,
PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(8), documentName, 133, 4, 135, 5, "ballerina:4", 4,
PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(9), documentName, 146, 0, 166, 1, "ballerina:5", 5,
PUBLIC_NON_ISOLATED_CLASS_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(10), documentName, 155, 4, 157, 5, "ballerina:4", 4,
PUBLIC_NON_ISOLATED_METHOD_CONSTRUCT, RuleKind.CODE_SMELL);
assertIssue(issues.get(11), documentName, 180, 0, 184, 1, "ballerina:5", 5,
PUBLIC_NON_ISOLATED_CLASS_CONSTRUCT, RuleKind.CODE_SMELL);
}
}
Loading
Loading