From bfcf7fc029ee2e24e52bc388ebbc88e74493d7d7 Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Tue, 18 Feb 2025 14:11:11 +0530 Subject: [PATCH 01/15] [Automated] Update the native jar versions --- ballerina/Ballerina.toml | 10 +++++----- ballerina/CompilerPlugin.toml | 2 +- ballerina/Dependencies.toml | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ballerina/Ballerina.toml b/ballerina/Ballerina.toml index 9a0522db..714b1fb5 100644 --- a/ballerina/Ballerina.toml +++ b/ballerina/Ballerina.toml @@ -1,7 +1,7 @@ [package] org = "ballerina" name = "log" -version = "2.11.0" +version = "2.11.1" authors = ["Ballerina"] keywords = ["level", "format"] repository = "https://github.com/ballerina-platform/module-ballerina-log" @@ -15,12 +15,12 @@ graalvmCompatible = true [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "log-native" -version = "2.11.0" -path = "../native/build/libs/log-native-2.11.0.jar" +version = "2.11.1" +path = "../native/build/libs/log-native-2.11.1-SNAPSHOT.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "log-test-utils" -version = "2.11.0" -path = "../test-utils/build/libs/log-test-utils-2.11.0.jar" +version = "2.11.1" +path = "../test-utils/build/libs/log-test-utils-2.11.1-SNAPSHOT.jar" scope = "testOnly" diff --git a/ballerina/CompilerPlugin.toml b/ballerina/CompilerPlugin.toml index 9dd639d8..ea8987c5 100644 --- a/ballerina/CompilerPlugin.toml +++ b/ballerina/CompilerPlugin.toml @@ -3,4 +3,4 @@ id = "log-compiler-plugin" class = "io.ballerina.stdlib.log.plugin.LogCompilerPlugin" [[dependency]] -path = "../compiler-plugin/build/libs/log-compiler-plugin-2.11.0.jar" +path = "../compiler-plugin/build/libs/log-compiler-plugin-2.11.1-SNAPSHOT.jar" diff --git a/ballerina/Dependencies.toml b/ballerina/Dependencies.toml index 4213f4db..3279dc0d 100644 --- a/ballerina/Dependencies.toml +++ b/ballerina/Dependencies.toml @@ -76,7 +76,7 @@ modules = [ [[package]] org = "ballerina" name = "log" -version = "2.11.0" +version = "2.11.1" dependencies = [ {org = "ballerina", name = "io"}, {org = "ballerina", name = "jballerina.java"}, From 81f6934d62ae8c5aed31ab2bb3b96910933e66dd Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Wed, 19 Feb 2025 16:32:27 +0530 Subject: [PATCH 02/15] [Automated] Update the native jar versions --- ballerina/CompilerPlugin.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ballerina/CompilerPlugin.toml b/ballerina/CompilerPlugin.toml index ea8987c5..9b2ce5eb 100644 --- a/ballerina/CompilerPlugin.toml +++ b/ballerina/CompilerPlugin.toml @@ -1,6 +1,6 @@ [plugin] id = "log-compiler-plugin" -class = "io.ballerina.stdlib.log.plugin.LogCompilerPlugin" +class = "io.ballerina.stdlib.log.compiler.LogCompilerPlugin" [[dependency]] path = "../compiler-plugin/build/libs/log-compiler-plugin-2.11.1-SNAPSHOT.jar" From b52546ae5ba9900abd699059f22b52a9322508bf Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Thu, 20 Feb 2025 14:54:26 +0530 Subject: [PATCH 03/15] [Automated] Update the native jar versions --- ballerina/Ballerina.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ballerina/Ballerina.toml b/ballerina/Ballerina.toml index 714b1fb5..f412aa4c 100644 --- a/ballerina/Ballerina.toml +++ b/ballerina/Ballerina.toml @@ -18,6 +18,12 @@ artifactId = "log-native" version = "2.11.1" path = "../native/build/libs/log-native-2.11.1-SNAPSHOT.jar" +[[platform.java21.dependency]] +groupId = "io.ballerina.stdlib" +artifactId = "log-compiler-plugin" +version = "2.11.1" +path = "../native/build/libs/log-native-2.11.1-SNAPSHOT.jar" + [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "log-test-utils" From 491dc74879eef48fdcdf591ac1f65f0473f40a6b Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Thu, 20 Feb 2025 15:00:46 +0530 Subject: [PATCH 04/15] [Automated] Update the native jar versions --- ballerina/Ballerina.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ballerina/Ballerina.toml b/ballerina/Ballerina.toml index f412aa4c..b1add967 100644 --- a/ballerina/Ballerina.toml +++ b/ballerina/Ballerina.toml @@ -22,7 +22,7 @@ path = "../native/build/libs/log-native-2.11.1-SNAPSHOT.jar" groupId = "io.ballerina.stdlib" artifactId = "log-compiler-plugin" version = "2.11.1" -path = "../native/build/libs/log-native-2.11.1-SNAPSHOT.jar" +path = "../compiler-plugin/build/libs/log-compiler-plugin-2.11.1-SNAPSHOT.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" From 5a2b83aa19c03840a91e9b290905746264980037 Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Sat, 22 Feb 2025 23:09:05 +0530 Subject: [PATCH 05/15] Add bal scan rule for configurable variables --- changelog.md | 1 + .gitignore | 2 + build-config/resources/Ballerina.toml | 6 + build-config/resources/CompilerPlugin.toml | 2 +- compiler-plugin-tests/build.gradle | 126 +++++++++++++++ .../ProcessOutputGobbler.java | 64 ++++++++ .../StaticCodeAnalyzerTest.java | 132 ++++++++++++++++ .../ballerina_packages/rule1/Ballerina.toml | 8 + .../ballerina_packages/rule1/Config.toml | 1 + .../ballerina_packages/rule1/main.bal | 44 ++++++ .../expected_output/rule1.json | 102 ++++++++++++ .../src/test/resources/testng.xml | 29 ++++ compiler-plugin/build.gradle | 2 +- .../{plugin => compiler}/LogCodeModifier.java | 2 +- .../LogCompilerPlugin.java | 15 +- .../compiler/staticcodeanalyzer/LogRule.java | 50 ++++++ .../LogStatementAnalyzer.java | 147 ++++++++++++++++++ .../staticcodeanalyzer/RuleFactory.java | 31 ++++ .../compiler/staticcodeanalyzer/RuleImpl.java | 54 +++++++ .../StaticCodeAnalyzer.java | 41 +++++ .../src/main/java/module-info.java | 1 + compiler-plugin/src/main/resources/rules.json | 7 + gradle.properties | 1 + integration-tests/Ballerina.toml | 8 +- settings.gradle | 2 + 25 files changed, 868 insertions(+), 10 deletions(-) create mode 100644 compiler-plugin-tests/build.gradle create mode 100644 compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/ProcessOutputGobbler.java create mode 100644 compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java create mode 100644 compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml create mode 100644 compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal create mode 100644 compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json create mode 100644 compiler-plugin-tests/src/test/resources/testng.xml rename compiler-plugin/src/main/java/io/ballerina/stdlib/log/{plugin => compiler}/LogCodeModifier.java (99%) rename compiler-plugin/src/main/java/io/ballerina/stdlib/log/{plugin => compiler}/LogCompilerPlugin.java (61%) create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleFactory.java create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleImpl.java create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzer.java create mode 100644 compiler-plugin/src/main/resources/rules.json diff --git a/ changelog.md b/ changelog.md index 961e48c6..ec6ff1c5 100644 --- a/ changelog.md +++ b/ changelog.md @@ -5,6 +5,7 @@ This file contains all the notable changes done to the Ballerina TCP package thr ### Added - [Support logging raw template value in log APIs](https://github.com/ballerina-platform/ballerina-library/issues/3331) +- [Add static code rules](https://github.com/ballerina-platform/ballerina-library/issues/7283) ## [2.5.1] - 2023-01-04 diff --git a/.gitignore b/.gitignore index 2e53f73c..a927bbee 100644 --- a/.gitignore +++ b/.gitignore @@ -42,3 +42,5 @@ integration-tests/Dependencies.toml # Ballerina velocity.log* *Ballerina.lock + +compiler-plugin-tests/**/target diff --git a/build-config/resources/Ballerina.toml b/build-config/resources/Ballerina.toml index 7681323d..d6906b0f 100644 --- a/build-config/resources/Ballerina.toml +++ b/build-config/resources/Ballerina.toml @@ -18,6 +18,12 @@ artifactId = "log-native" version = "@toml.version@" path = "../native/build/libs/log-native-@project.version@.jar" +[[platform.java21.dependency]] +groupId = "io.ballerina.stdlib" +artifactId = "log-compiler-plugin" +version = "@toml.version@" +path = "../compiler-plugin/build/libs/log-compiler-plugin-@project.version@.jar" + [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "log-test-utils" diff --git a/build-config/resources/CompilerPlugin.toml b/build-config/resources/CompilerPlugin.toml index 2db40f06..472dbb75 100644 --- a/build-config/resources/CompilerPlugin.toml +++ b/build-config/resources/CompilerPlugin.toml @@ -1,6 +1,6 @@ [plugin] id = "log-compiler-plugin" -class = "io.ballerina.stdlib.log.plugin.LogCompilerPlugin" +class = "io.ballerina.stdlib.log.compiler.LogCompilerPlugin" [[dependency]] path = "../compiler-plugin/build/libs/log-compiler-plugin-@project.version@.jar" diff --git a/compiler-plugin-tests/build.gradle b/compiler-plugin-tests/build.gradle new file mode 100644 index 00000000..afae323e --- /dev/null +++ b/compiler-plugin-tests/build.gradle @@ -0,0 +1,126 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org). + * + * 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. + */ + +plugins { + id 'java' + id 'checkstyle' + id 'com.github.spotbugs' +} + +description = 'Ballerina - Cache Compiler Plugin Tests' + +dependencies { + checkstyle project(':checkstyle') + checkstyle "com.puppycrawl.tools:checkstyle:${checkstylePluginVersion}" + + implementation project(':log-compiler-plugin') + + testImplementation group: 'org.ballerinalang', name: 'ballerina-lang', version: "${ballerinaLangVersion}" + testImplementation group: 'org.ballerinalang', name: 'ballerina-tools-api', version: "${ballerinaLangVersion}" + testImplementation group: 'org.ballerinalang', name: 'ballerina-parser', version: "${ballerinaLangVersion}" + + implementation group: 'org.testng', name: 'testng', version: "${testngVersion}" +} + +tasks.withType(JavaCompile) { + options.encoding = 'UTF-8' +} + +sourceCompatibility = JavaVersion.VERSION_21 + +test { + systemProperty "ballerina.offline.flag", "true" + useTestNG() { + suites 'src/test/resources/testng.xml' + } + testLogging.showStandardStreams = true + testLogging { + events "PASSED", "FAILED", "SKIPPED" + afterSuite { desc, result -> + if (!desc.parent) { // will match the outermost suite + def output = "Results: ${result.resultType} (${result.testCount} tests, ${result.successfulTestCount} successes, ${result.failedTestCount} failures, ${result.skippedTestCount} skipped)" + def startItem = '| ', endItem = ' |' + def repeatLength = startItem.length() + output.length() + endItem.length() + println('\n' + ('-' * repeatLength) + '\n' + startItem + output + endItem + '\n' + ('-' * repeatLength)) + } + } + } +} + +spotbugsTest { + def classLoader = plugins["com.github.spotbugs"].class.classLoader + def SpotBugsConfidence = classLoader.findLoadedClass("com.github.spotbugs.snom.Confidence") + def SpotBugsEffort = classLoader.findLoadedClass("com.github.spotbugs.snom.Effort") + ignoreFailures = true + effort = SpotBugsEffort.MAX + reportLevel = SpotBugsConfidence.LOW + reportsDir = file("$project.buildDir/reports/spotbugs") + def excludeFile = file("${rootDir}/build-config/spotbugs-exclude.xml") + if (excludeFile.exists()) { + it.excludeFilter = excludeFile + } + reports { + text.enabled = true + } +} + +spotbugsMain { + enabled false +} + +task validateSpotbugs() { + doLast { + if (spotbugsMain.reports.size() > 0 && + spotbugsMain.reports[0].destination.exists() && + spotbugsMain.reports[0].destination.text.readLines().size() > 0) { + spotbugsMain.reports[0].destination?.eachLine { + println 'Failure: ' + it + } + throw new GradleException("Spotbugs rule violations were found."); + } + } +} + +tasks.withType(Checkstyle) { + exclude '**/module-info.java' +} + +checkstyle { + toolVersion "${project.checkstylePluginVersion}" + configFile rootProject.file("build-config/checkstyle/build/checkstyle.xml") + configProperties = ["suppressionFile": file("${rootDir}/build-config/checkstyle/build/suppressions.xml")] +} + +checkstyleMain { + enabled false +} + +spotbugsTest.finalizedBy validateSpotbugs +checkstyleTest.dependsOn ':checkstyle:downloadCheckstyleRuleFiles' + +compileJava { + doFirst { + options.compilerArgs = [ + '--module-path', classpath.asPath, + ] + classpath = files() + } +} + +test.dependsOn ":log-ballerina:build" +build.dependsOn ":log-ballerina:build" diff --git a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/ProcessOutputGobbler.java b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/ProcessOutputGobbler.java new file mode 100644 index 00000000..9c62e6ab --- /dev/null +++ b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/ProcessOutputGobbler.java @@ -0,0 +1,64 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org). + * + * 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.stdlib.log.compiler.staticcodeanalyzer; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; + +/** + * Helper class to consume the process streams. + */ +class ProcessOutputGobbler implements Runnable { + private final InputStream inputStream; + private final StringBuilder output; + private int exitCode; + + public ProcessOutputGobbler(InputStream inputStream) { + this.inputStream = inputStream; + this.output = new StringBuilder(); + } + + @Override + public void run() { + try (BufferedReader reader = new BufferedReader( + new InputStreamReader(inputStream, StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + output.append(line).append("\n"); + } + } catch (IOException e) { + this.output.append(e.getMessage()); + } + } + + public String getOutput() { + return output.toString(); + } + + public int getExitCode() { + return exitCode; + } + + public void setExitCode(int exitCode) { + this.exitCode = exitCode; + } +} diff --git a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java new file mode 100644 index 00000000..91cff480 --- /dev/null +++ b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java @@ -0,0 +1,132 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org). + * + * 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.stdlib.log.compiler.staticcodeanalyzer; + +import org.testng.Assert; +import org.testng.annotations.BeforeSuite; +import org.testng.annotations.Test; +import org.testng.internal.ExitCode; + +import java.io.IOException; +import java.io.PrintStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.Locale; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +/** + * This class includes tests for Ballerina Http static code analyzer. + */ +class StaticCodeAnalyzerTest { + + PrintStream outStream = System.out; + + private static final Path RESOURCE_PACKAGES_DIRECTORY = Paths + .get("src", "test", "resources", "static_code_analyzer", "ballerina_packages").toAbsolutePath(); + private static final Path EXPECTED_JSON_OUTPUT_DIRECTORY = Paths. + get("src", "test", "resources", "static_code_analyzer", "expected_output").toAbsolutePath(); + private static final Path BALLERINA_PATH = getBalCommandPath(); + private static final Path JSON_RULES_FILE_PATH = Paths + .get("../", "compiler-plugin", "src", "main", "resources", "rules.json").toAbsolutePath(); + private static final String SCAN_COMMAND = "scan"; + + private static Path getBalCommandPath() { + String balCommand = isWindows() ? "bal.bat" : "bal"; + return Paths.get("../", "target", "ballerina-runtime", "bin", balCommand).toAbsolutePath(); + } + + @BeforeSuite + public void pullScanTool() throws IOException, InterruptedException { + ProcessBuilder processBuilder = new ProcessBuilder(BALLERINA_PATH.toString(), "tool", "pull", SCAN_COMMAND); + ProcessOutputGobbler output = getOutput(processBuilder.start()); + if (Pattern.compile("tool 'scan:.+\\..+\\..+' successfully set as the active version\\.") + .matcher(output.getOutput()).find() || Pattern.compile("tool 'scan:.+\\..+\\..+' is already active\\.") + .matcher(output.getOutput()).find()) { + return; + } + Assert.assertFalse(ExitCode.hasFailure(output.getExitCode())); + } + + @Test + public void validateRulesJson() throws IOException { + String expectedRules = "[" + Arrays.stream(LogRule.values()) + .map(LogRule::toString).collect(Collectors.joining(",")) + "]"; + String actualRules = Files.readString(JSON_RULES_FILE_PATH); + assertJsonEqual(normalizeJson(actualRules), normalizeJson(expectedRules)); + } + + @Test + public void testStaticCodeRules() throws IOException, InterruptedException { + for (LogRule rule : LogRule.values()) { + String targetPackageName = "rule" + rule.getId(); + String actualJsonReport = StaticCodeAnalyzerTest.executeScanProcess(targetPackageName); + String expectedJsonReport = Files + .readString(EXPECTED_JSON_OUTPUT_DIRECTORY.resolve(targetPackageName + ".json")); + assertJsonEqual(actualJsonReport, expectedJsonReport); + } + } + + private static String executeScanProcess(String targetPackage) throws IOException, InterruptedException { + ProcessBuilder processBuilder = new ProcessBuilder(BALLERINA_PATH.toString(), SCAN_COMMAND); + processBuilder.directory(RESOURCE_PACKAGES_DIRECTORY.resolve(targetPackage).toFile()); + ProcessOutputGobbler output = getOutput(processBuilder.start()); + Assert.assertFalse(ExitCode.hasFailure(output.getExitCode())); + return Files.readString(RESOURCE_PACKAGES_DIRECTORY.resolve(targetPackage) + .resolve("target").resolve("report").resolve("scan_results.json")); + } + + private static ProcessOutputGobbler getOutput(Process process) throws InterruptedException { + ProcessOutputGobbler outputGobbler = new ProcessOutputGobbler(process.getInputStream()); + ProcessOutputGobbler errorGobbler = new ProcessOutputGobbler(process.getErrorStream()); + Thread outputThread = new Thread(outputGobbler); + Thread errorThread = new Thread(errorGobbler); + outputThread.start(); + errorThread.start(); + int exitCode = process.waitFor(); + outputGobbler.setExitCode(exitCode); + errorGobbler.setExitCode(exitCode); + outputThread.join(); + errorThread.join(); + return outputGobbler; + } + + private void assertJsonEqual(String actual, String expected) { + Assert.assertEquals(normalizeJson(actual), normalizeJson(expected)); + } + + private static String normalizeJson(String json) { + String normalizedJson = json.replaceAll("\\s*\"\\s*", "\"") + .replaceAll("\\s*:\\s*", ":") + .replaceAll("\\s*,\\s*", ",") + .replaceAll("\\s*\\{\\s*", "{") + .replaceAll("\\s*}\\s*", "}") + .replaceAll("\\s*\\[\\s*", "[") + .replaceAll("\\s*]\\s*", "]") + .replaceAll("\n", "") + .replaceAll(":\".*module-ballerina-http", ":\"module-ballerina-http"); + return isWindows() ? normalizedJson.replaceAll("/", "\\\\\\\\") : normalizedJson; + } + + private static boolean isWindows() { + return System.getProperty("os.name").toLowerCase(Locale.ENGLISH).startsWith("windows"); + } +} diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Ballerina.toml b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Ballerina.toml new file mode 100644 index 00000000..efbb839e --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Ballerina.toml @@ -0,0 +1,8 @@ +[package] +org = "ballerina" +name = "rule1" +version = "0.1.0" +distribution = "2201.10.3" + +[build-options] +observabilityIncluded = true diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml new file mode 100644 index 00000000..f04fc9a6 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml @@ -0,0 +1 @@ +password = "xyz" \ No newline at end of file diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal new file mode 100644 index 00000000..c106f83c --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal @@ -0,0 +1,44 @@ +// 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. + +import ballerina/log; +import ballerina/http; +import ballerina/file; + +public function main() { + log:printInfo(password); + log:printError(string `Error: ${password}`); + log:printError("Error " + password); + log:printWarn("Warning", password = password); +} + +function log() { + log:printInfo("Info"); +} + +service on new http:Listener(9090) { + resource function get test() { + log:printInfo("Info", password = password); + } +} + +service on new file:Listener({path: "/tmp", recursive: true}) { + remote function onCreate(file:FileEvent event) { + log:printInfo("File created: ", path = event.name); + } +} + +configurable string password = ?; diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json b/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json new file mode 100644 index 00000000..7b7c9618 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json @@ -0,0 +1,102 @@ +[ + { + "location": { + "filePath": "main.bal", + "startLine": 5, + "endLine": 5, + "startColumn": 4, + "endColumn": 28, + "startOffset": 98, + "length": 24 + }, + "rule": { + "id": "ballerina/log:1", + "numericId": 1, + "description": "Avoid logging configurable variables", + "ruleKind": "VULNERABILITY" + }, + "source": "BUILT_IN", + "fileName": "rule1/main.bal", + "filePath": "/Users/danesh/ballerina-platform/module-ballerina-log/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" + }, + { + "location": { + "filePath": "main.bal", + "startLine": 6, + "endLine": 6, + "startColumn": 4, + "endColumn": 48, + "startOffset": 127, + "length": 44 + }, + "rule": { + "id": "ballerina/log:1", + "numericId": 1, + "description": "Avoid logging configurable variables", + "ruleKind": "VULNERABILITY" + }, + "source": "BUILT_IN", + "fileName": "rule1/main.bal", + "filePath": "/Users/danesh/ballerina-platform/module-ballerina-log/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" + }, + { + "location": { + "filePath": "main.bal", + "startLine": 7, + "endLine": 7, + "startColumn": 4, + "endColumn": 40, + "startOffset": 176, + "length": 36 + }, + "rule": { + "id": "ballerina/log:1", + "numericId": 1, + "description": "Avoid logging configurable variables", + "ruleKind": "VULNERABILITY" + }, + "source": "BUILT_IN", + "fileName": "rule1/main.bal", + "filePath": "/Users/danesh/ballerina-platform/module-ballerina-log/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" + }, + { + "location": { + "filePath": "main.bal", + "startLine": 8, + "endLine": 8, + "startColumn": 4, + "endColumn": 50, + "startOffset": 217, + "length": 46 + }, + "rule": { + "id": "ballerina/log:1", + "numericId": 1, + "description": "Avoid logging configurable variables", + "ruleKind": "VULNERABILITY" + }, + "source": "BUILT_IN", + "fileName": "rule1/main.bal", + "filePath": "/Users/danesh/ballerina-platform/module-ballerina-log/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" + }, + { + "location": { + "filePath": "main.bal", + "startLine": 17, + "endLine": 17, + "startColumn": 8, + "endColumn": 51, + "startOffset": 394, + "length": 43 + }, + "rule": { + "id": "ballerina/log:1", + "numericId": 1, + "description": "Avoid logging configurable variables", + "ruleKind": "VULNERABILITY" + }, + "source": "BUILT_IN", + "fileName": "rule1/main.bal", + "filePath": "/Users/danesh/ballerina-platform/module-ballerina-log/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" + } +] diff --git a/compiler-plugin-tests/src/test/resources/testng.xml b/compiler-plugin-tests/src/test/resources/testng.xml new file mode 100644 index 00000000..0e632ae7 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/testng.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/compiler-plugin/build.gradle b/compiler-plugin/build.gradle index 23a09353..6cffee93 100644 --- a/compiler-plugin/build.gradle +++ b/compiler-plugin/build.gradle @@ -30,7 +30,7 @@ dependencies { implementation group: 'org.ballerinalang', name: 'ballerina-lang', version: "${ballerinaLangVersion}" implementation group: 'org.ballerinalang', name: 'ballerina-tools-api', version: "${ballerinaLangVersion}" implementation group: 'org.ballerinalang', name: 'ballerina-parser', version: "${ballerinaLangVersion}" - + implementation group: 'io.ballerina.scan', name: 'scan-command', version: "${balScanVersion}" implementation project(":log-native") } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/plugin/LogCodeModifier.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/LogCodeModifier.java similarity index 99% rename from compiler-plugin/src/main/java/io/ballerina/stdlib/log/plugin/LogCodeModifier.java rename to compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/LogCodeModifier.java index b875183b..8a3089cd 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/plugin/LogCodeModifier.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/LogCodeModifier.java @@ -16,7 +16,7 @@ * under the License. */ -package io.ballerina.stdlib.log.plugin; +package io.ballerina.stdlib.log.compiler; import io.ballerina.compiler.syntax.tree.AbstractNodeFactory; import io.ballerina.compiler.syntax.tree.FunctionArgumentNode; diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/plugin/LogCompilerPlugin.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/LogCompilerPlugin.java similarity index 61% rename from compiler-plugin/src/main/java/io/ballerina/stdlib/log/plugin/LogCompilerPlugin.java rename to compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/LogCompilerPlugin.java index b42bd27a..9676b93b 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/plugin/LogCompilerPlugin.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/LogCompilerPlugin.java @@ -16,18 +16,27 @@ * under the License. */ -package io.ballerina.stdlib.log.plugin; +package io.ballerina.stdlib.log.compiler; import io.ballerina.projects.plugins.CompilerPlugin; import io.ballerina.projects.plugins.CompilerPluginContext; +import io.ballerina.scan.ScannerContext; +import io.ballerina.stdlib.log.compiler.staticcodeanalyzer.StaticCodeAnalyzer; /** * log module Compiler plugin. */ public class LogCompilerPlugin extends CompilerPlugin { + private static final String SCANNER_CONTEXT = "ScannerContext"; + @Override - public void init(CompilerPluginContext compilerPluginContext) { -// compilerPluginContext.addCodeModifier(new LogCodeModifier()); + public void init(CompilerPluginContext context) { + + //context.addCodeModifier(new LogCodeModifier()); + Object object = context.userData().get(SCANNER_CONTEXT); + if (object instanceof ScannerContext scannerContext) { + context.addCodeAnalyzer(new StaticCodeAnalyzer(scannerContext.getReporter())); + } } } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java new file mode 100644 index 00000000..6b9cc25a --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org). + * + * 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.stdlib.log.compiler.staticcodeanalyzer; + +import io.ballerina.scan.Rule; + +import static io.ballerina.scan.RuleKind.VULNERABILITY; +import static io.ballerina.stdlib.log.compiler.staticcodeanalyzer.RuleFactory.createRule; + +public enum LogRule { + AVOID_LOGGING_CONFIGURABLE_VARIABLES(createRule(1, "Avoid logging configurable variables", VULNERABILITY)); + private final Rule rule; + + + LogRule(Rule rule) { + this.rule = rule; + } + + public int getId() { + return this.rule.numericId(); + } + + public Rule getRule() { + return this.rule; + } + + @Override + public String toString() { + return "{\"id\":" + this.getId() + ", \"kind\":\"" + this.rule.kind() + "\"," + + " \"description\" : \"" + this.rule.description() + "\"}"; + } + + +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java new file mode 100644 index 00000000..b5513cd6 --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java @@ -0,0 +1,147 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org). + * + * 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.stdlib.log.compiler.staticcodeanalyzer; + +import io.ballerina.compiler.api.SemanticModel; +import io.ballerina.compiler.api.symbols.Symbol; +import io.ballerina.compiler.api.symbols.VariableSymbol; +import io.ballerina.compiler.syntax.tree.BinaryExpressionNode; +import io.ballerina.compiler.syntax.tree.ChildNodeEntry; +import io.ballerina.compiler.syntax.tree.ExpressionNode; +import io.ballerina.compiler.syntax.tree.ExpressionStatementNode; +import io.ballerina.compiler.syntax.tree.InterpolationNode; +import io.ballerina.compiler.syntax.tree.NamedArgumentNode; +import io.ballerina.compiler.syntax.tree.Node; +import io.ballerina.compiler.syntax.tree.NodeList; +import io.ballerina.compiler.syntax.tree.PositionalArgumentNode; +import io.ballerina.compiler.syntax.tree.QualifiedNameReferenceNode; +import io.ballerina.compiler.syntax.tree.SimpleNameReferenceNode; +import io.ballerina.compiler.syntax.tree.TemplateExpressionNode; +import io.ballerina.projects.Document; +import io.ballerina.projects.plugins.AnalysisTask; +import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; +import io.ballerina.scan.Reporter; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static io.ballerina.stdlib.log.compiler.staticcodeanalyzer.LogRule.AVOID_LOGGING_CONFIGURABLE_VARIABLES; + +public class LogStatementAnalyzer implements AnalysisTask { + + public static final String CONFIGURABLE_QUALIFIER = "CONFIGURABLE"; + public static final String LOG_MODULE_PREFIX = "log"; + + final List logFunctions = Arrays.asList("printInfo", "printDebug", "printError", "printWarn"); + + List semanticModels = new ArrayList<>(); + + private final Reporter reporter; + + public LogStatementAnalyzer(Reporter reporter) { + this.reporter = reporter; + } + @Override + public void perform(SyntaxNodeAnalysisContext context) { + // If semantic model is empty, we get semantic models of all the modules in the package and save it in a list + if (semanticModels.isEmpty()) { + context.currentPackage().modules().forEach(module -> { + SemanticModel semanticModel = module.getCompilation().getSemanticModel(); + semanticModels.add(semanticModel); + }); + } + + if (context.node() instanceof ExpressionStatementNode expressionStatementNode) { + // Check if the log statement has a configurable qualifier + List childlist = expressionStatementNode.expression().childEntries().stream() + .toList(); + + if (childlist.size() < 4) { + return; + } + + Node firstChild = childlist.getFirst().node().orElse(null); + if (firstChild instanceof QualifiedNameReferenceNode qualifiedNameReferenceNode + && qualifiedNameReferenceNode.modulePrefix().text().equals(LOG_MODULE_PREFIX) + && logFunctions.contains(qualifiedNameReferenceNode.identifier().text())) { + + // The argument of the log function is the third child. second and fourth child are the parentheses + NodeList logArgumentNodeList = childlist.get(2).nodeList(); + // collect all the log function arguments. + List list = logArgumentNodeList.stream().filter(node -> node instanceof PositionalArgumentNode || + node instanceof NamedArgumentNode) + .toList(); + + for (Node node : list) { + if (node instanceof PositionalArgumentNode positionalArgumentNode) { + positionalArgumentNode.childEntries().forEach(childNodeEntry -> { + Node expression = childNodeEntry.node().orElse(null); + if (expression instanceof SimpleNameReferenceNode simpleNameReferenceNode) { + checkConfigurableQualifier(simpleNameReferenceNode, context, expressionStatementNode); + } else if (expression instanceof TemplateExpressionNode templateExpressionNode) { + templateExpressionNode.content().forEach(content -> { + if (content instanceof InterpolationNode interpolationNode) { + interpolationNode.childEntries().forEach(interpolationChild -> { + Node interpolationExpression = interpolationChild.node().orElse(null); + if (interpolationExpression instanceof SimpleNameReferenceNode + simpleNameReferenceNode) { + checkConfigurableQualifier(simpleNameReferenceNode, context, + expressionStatementNode); + } + }); + } + }); + } else if (expression instanceof BinaryExpressionNode binaryExpressionNode) { + binaryExpressionNode.childEntries().forEach(childEntry -> { + Node childNode = childEntry.node().orElse(null); + if (childNode instanceof SimpleNameReferenceNode simpleNameReferenceNode) { + checkConfigurableQualifier(simpleNameReferenceNode, context, + expressionStatementNode); + } + }); + } + }); + } else if (node instanceof NamedArgumentNode namedArgumentNode) { + checkConfigurableQualifier(namedArgumentNode.expression(), context, expressionStatementNode); + } + } + } + } + } + + private void checkConfigurableQualifier(ExpressionNode argumentNode, SyntaxNodeAnalysisContext context, + ExpressionStatementNode expressionStatementNode) { + semanticModels.forEach(semanticModel -> { + Symbol symbol = semanticModel.symbol(argumentNode).orElse(null); + if (symbol instanceof VariableSymbol variableSymbol) { + variableSymbol.qualifiers().stream().filter(qualifier -> qualifier + .toString().equals(CONFIGURABLE_QUALIFIER)).forEach(qualifier -> { + this.reporter.reportIssue(getDocument(context), + expressionStatementNode.location(), + AVOID_LOGGING_CONFIGURABLE_VARIABLES.getId()); + }); + } + }); + } + + private static Document getDocument(SyntaxNodeAnalysisContext context) { + return context.currentPackage().module(context.moduleId()).document(context.documentId()); + } +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleFactory.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleFactory.java new file mode 100644 index 00000000..7a7d8441 --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleFactory.java @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. + * + * 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.stdlib.log.compiler.staticcodeanalyzer; + +import io.ballerina.scan.Rule; +import io.ballerina.scan.RuleKind; + +/** + * {@code RuleFactory} contains the logic to create a {@link Rule}. + */ +public class RuleFactory { + public static Rule createRule(int id, String description, RuleKind kind) { + return new RuleImpl(id, description, kind); + } +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleImpl.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleImpl.java new file mode 100644 index 00000000..7fd2bdfb --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleImpl.java @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org). + * + * 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.stdlib.log.compiler.staticcodeanalyzer; + +import io.ballerina.scan.Rule; +import io.ballerina.scan.RuleKind; + +class RuleImpl implements Rule { + private final int id; + private final String description; + private final RuleKind kind; + + RuleImpl(int id, String description, RuleKind kind) { + this.id = id; + this.description = description; + this.kind = kind; + } + + @Override + public String id() { + return Integer.toString(this.id); + } + + @Override + public int numericId() { + return this.id; + } + + @Override + public String description() { + return this.description; + } + + @Override + public RuleKind kind() { + return this.kind; + } +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzer.java new file mode 100644 index 00000000..f3801d6f --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzer.java @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org). + * + * 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.stdlib.log.compiler.staticcodeanalyzer; + +import io.ballerina.projects.plugins.CodeAnalysisContext; +import io.ballerina.projects.plugins.CodeAnalyzer; +import io.ballerina.scan.Reporter; + +import java.util.List; + +import static io.ballerina.compiler.syntax.tree.SyntaxKind.CALL_STATEMENT; + +public class StaticCodeAnalyzer extends CodeAnalyzer { + private final Reporter reporter; + + public StaticCodeAnalyzer(Reporter reporter) { + this.reporter = reporter; + } + + @Override + public void init(CodeAnalysisContext analysisContext) { + analysisContext.addSyntaxNodeAnalysisTask(new LogStatementAnalyzer(reporter), + List.of(CALL_STATEMENT)); + } +} diff --git a/compiler-plugin/src/main/java/module-info.java b/compiler-plugin/src/main/java/module-info.java index bcfdf10e..74865172 100644 --- a/compiler-plugin/src/main/java/module-info.java +++ b/compiler-plugin/src/main/java/module-info.java @@ -20,4 +20,5 @@ requires io.ballerina.lang; requires io.ballerina.parser; requires io.ballerina.tools.api; + requires io.ballerina.scan; } diff --git a/compiler-plugin/src/main/resources/rules.json b/compiler-plugin/src/main/resources/rules.json new file mode 100644 index 00000000..858981ab --- /dev/null +++ b/compiler-plugin/src/main/resources/rules.json @@ -0,0 +1,7 @@ +[ + { + "id": 1, + "kind": "VULNERABILITY", + "description": "Avoid logging configurable variables" + } +] \ No newline at end of file diff --git a/gradle.properties b/gradle.properties index 42be9e95..3a9789a2 100644 --- a/gradle.properties +++ b/gradle.properties @@ -9,6 +9,7 @@ shadowJarPluginVersion=8.1.1 downloadPluginVersion=5.4.0 releasePluginVersion=2.6.0 ballerinaGradlePluginVersion=2.3.0 +balScanVersion=0.5.0 #stdlib dependencies stdlibIoVersion=1.7.0 diff --git a/integration-tests/Ballerina.toml b/integration-tests/Ballerina.toml index a4b2c935..43229161 100644 --- a/integration-tests/Ballerina.toml +++ b/integration-tests/Ballerina.toml @@ -1,20 +1,20 @@ [package] org = "ballerina" name = "integration_tests" -version = "@toml.version@" +version = "2.11.1" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "log-native" -path = "../native/build/libs/log-native-@project.version@.jar" +path = "../native/build/libs/log-native-2.11.1-SNAPSHOT.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "log-test-utils" scope = "testOnly" -path = "../test-utils/build/libs/log-test-utils-@project.version@.jar" +path = "../test-utils/build/libs/log-test-utils-2.11.1-SNAPSHOT.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "io-native" -path = "./lib/io-native-@io.native.version@.jar" \ No newline at end of file +path = "./lib/io-native-1.7.0.jar" \ No newline at end of file diff --git a/settings.gradle b/settings.gradle index eca70c49..3752449f 100644 --- a/settings.gradle +++ b/settings.gradle @@ -39,6 +39,7 @@ include ':log-native' include ':log-test-utils' include ':log-ballerina' include ':log-compiler-plugin' +include ':log-compiler-plugin-tests' include ':log-integration-tests' project(':checkstyle').projectDir = file("build-config${File.separator}checkstyle") @@ -46,6 +47,7 @@ project(':log-native').projectDir = file('native') project(':log-test-utils').projectDir = file('test-utils') project(':log-ballerina').projectDir = file('ballerina') project(':log-compiler-plugin').projectDir = file('compiler-plugin') +project(':log-compiler-plugin-tests').projectDir = file('compiler-plugin-tests') project(':log-integration-tests').projectDir = file('integration-tests') gradleEnterprise { From 9b1282cf9c9a896f9bf053e49c6daebd925c289d Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Sat, 22 Feb 2025 23:15:36 +0530 Subject: [PATCH 06/15] Apply suggestions from code review --- .../static_code_analyzer/ballerina_packages/rule1/Config.toml | 2 +- compiler-plugin-tests/src/test/resources/testng.xml | 2 +- compiler-plugin/src/main/resources/rules.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml index f04fc9a6..fb5911d2 100644 --- a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml @@ -1 +1 @@ -password = "xyz" \ No newline at end of file +password = "xyz" diff --git a/compiler-plugin-tests/src/test/resources/testng.xml b/compiler-plugin-tests/src/test/resources/testng.xml index 0e632ae7..15c183ce 100644 --- a/compiler-plugin-tests/src/test/resources/testng.xml +++ b/compiler-plugin-tests/src/test/resources/testng.xml @@ -26,4 +26,4 @@ - \ No newline at end of file + diff --git a/compiler-plugin/src/main/resources/rules.json b/compiler-plugin/src/main/resources/rules.json index 858981ab..5963a1f4 100644 --- a/compiler-plugin/src/main/resources/rules.json +++ b/compiler-plugin/src/main/resources/rules.json @@ -4,4 +4,4 @@ "kind": "VULNERABILITY", "description": "Avoid logging configurable variables" } -] \ No newline at end of file +] From 864827dbf26f6cbd5420e68b5ed943acb9b9d148 Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Sat, 22 Feb 2025 23:30:39 +0530 Subject: [PATCH 07/15] fix test failiure --- .../expected_output/rule1.json | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json b/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json index 7b7c9618..e748fece 100644 --- a/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json @@ -2,11 +2,11 @@ { "location": { "filePath": "main.bal", - "startLine": 5, - "endLine": 5, + "startLine": 21, + "endLine": 21, "startColumn": 4, "endColumn": 28, - "startOffset": 98, + "startOffset": 743, "length": 24 }, "rule": { @@ -22,11 +22,11 @@ { "location": { "filePath": "main.bal", - "startLine": 6, - "endLine": 6, + "startLine": 22, + "endLine": 22, "startColumn": 4, "endColumn": 48, - "startOffset": 127, + "startOffset": 772, "length": 44 }, "rule": { @@ -42,11 +42,11 @@ { "location": { "filePath": "main.bal", - "startLine": 7, - "endLine": 7, + "startLine": 23, + "endLine": 23, "startColumn": 4, "endColumn": 40, - "startOffset": 176, + "startOffset": 821, "length": 36 }, "rule": { @@ -62,11 +62,11 @@ { "location": { "filePath": "main.bal", - "startLine": 8, - "endLine": 8, + "startLine": 24, + "endLine": 24, "startColumn": 4, "endColumn": 50, - "startOffset": 217, + "startOffset": 862, "length": 46 }, "rule": { @@ -82,11 +82,11 @@ { "location": { "filePath": "main.bal", - "startLine": 17, - "endLine": 17, + "startLine": 33, + "endLine": 33, "startColumn": 8, "endColumn": 51, - "startOffset": 394, + "startOffset": 1039, "length": 43 }, "rule": { From 8862f36546726ed811354be33ab563962ae77e16 Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Sat, 22 Feb 2025 23:37:18 +0530 Subject: [PATCH 08/15] Apply suggestions from code review --- integration-tests/Ballerina.toml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/integration-tests/Ballerina.toml b/integration-tests/Ballerina.toml index 43229161..a4b2c935 100644 --- a/integration-tests/Ballerina.toml +++ b/integration-tests/Ballerina.toml @@ -1,20 +1,20 @@ [package] org = "ballerina" name = "integration_tests" -version = "2.11.1" +version = "@toml.version@" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "log-native" -path = "../native/build/libs/log-native-2.11.1-SNAPSHOT.jar" +path = "../native/build/libs/log-native-@project.version@.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "log-test-utils" scope = "testOnly" -path = "../test-utils/build/libs/log-test-utils-2.11.1-SNAPSHOT.jar" +path = "../test-utils/build/libs/log-test-utils-@project.version@.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "io-native" -path = "./lib/io-native-1.7.0.jar" \ No newline at end of file +path = "./lib/io-native-@io.native.version@.jar" \ No newline at end of file From 81d0c7d8d9fa4aa8a0814c2882a1a506e15dd47c Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Sun, 23 Feb 2025 20:28:38 +0530 Subject: [PATCH 09/15] Address review changes --- compiler-plugin-tests/build.gradle | 2 +- .../ballerina_packages/rule1/Config.toml | 1 + .../ballerina_packages/rule1/main.bal | 16 +--- .../expected_output/rule1.json | 90 +++++++++++-------- .../src/test/resources/testng.xml | 4 +- .../compiler/staticcodeanalyzer/LogRule.java | 3 +- .../LogStatementAnalyzer.java | 28 +++--- compiler-plugin/src/main/resources/rules.json | 2 +- 8 files changed, 78 insertions(+), 68 deletions(-) diff --git a/compiler-plugin-tests/build.gradle b/compiler-plugin-tests/build.gradle index afae323e..a0372cc2 100644 --- a/compiler-plugin-tests/build.gradle +++ b/compiler-plugin-tests/build.gradle @@ -22,7 +22,7 @@ plugins { id 'com.github.spotbugs' } -description = 'Ballerina - Cache Compiler Plugin Tests' +description = 'Ballerina - Log Compiler Plugin Tests' dependencies { checkstyle project(':checkstyle') diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml index fb5911d2..fc9c4cd5 100644 --- a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Config.toml @@ -1 +1,2 @@ password = "xyz" +user = "abc" diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal index c106f83c..db0a4ccc 100644 --- a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal @@ -15,30 +15,18 @@ // under the License. import ballerina/log; -import ballerina/http; -import ballerina/file; public function main() { log:printInfo(password); log:printError(string `Error: ${password}`); log:printError("Error " + password); log:printWarn("Warning", password = password); + log:printError("Error", password = password, user = user); } function log() { log:printInfo("Info"); } -service on new http:Listener(9090) { - resource function get test() { - log:printInfo("Info", password = password); - } -} - -service on new file:Listener({path: "/tmp", recursive: true}) { - remote function onCreate(file:FileEvent event) { - log:printInfo("File created: ", path = event.name); - } -} - configurable string password = ?; +configurable string user = ?; diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json b/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json index e748fece..49bd67b5 100644 --- a/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json @@ -2,17 +2,17 @@ { "location": { "filePath": "main.bal", - "startLine": 21, - "endLine": 21, - "startColumn": 4, - "endColumn": 28, - "startOffset": 743, - "length": 24 + "startLine": 19, + "endLine": 19, + "startColumn": 18, + "endColumn": 26, + "startOffset": 711, + "length": 8 }, "rule": { "id": "ballerina/log:1", "numericId": 1, - "description": "Avoid logging configurable variables", + "description": "Potentially-sensitive configurable variables are logged", "ruleKind": "VULNERABILITY" }, "source": "BUILT_IN", @@ -22,17 +22,17 @@ { "location": { "filePath": "main.bal", - "startLine": 22, - "endLine": 22, - "startColumn": 4, - "endColumn": 48, - "startOffset": 772, - "length": 44 + "startLine": 20, + "endLine": 20, + "startColumn": 36, + "endColumn": 44, + "startOffset": 758, + "length": 8 }, "rule": { "id": "ballerina/log:1", "numericId": 1, - "description": "Avoid logging configurable variables", + "description": "Potentially-sensitive configurable variables are logged", "ruleKind": "VULNERABILITY" }, "source": "BUILT_IN", @@ -42,17 +42,17 @@ { "location": { "filePath": "main.bal", - "startLine": 23, - "endLine": 23, - "startColumn": 4, - "endColumn": 40, - "startOffset": 821, - "length": 36 + "startLine": 21, + "endLine": 21, + "startColumn": 30, + "endColumn": 38, + "startOffset": 801, + "length": 8 }, "rule": { "id": "ballerina/log:1", "numericId": 1, - "description": "Avoid logging configurable variables", + "description": "Potentially-sensitive configurable variables are logged", "ruleKind": "VULNERABILITY" }, "source": "BUILT_IN", @@ -62,17 +62,17 @@ { "location": { "filePath": "main.bal", - "startLine": 24, - "endLine": 24, - "startColumn": 4, - "endColumn": 50, - "startOffset": 862, - "length": 46 + "startLine": 22, + "endLine": 22, + "startColumn": 40, + "endColumn": 48, + "startOffset": 852, + "length": 8 }, "rule": { "id": "ballerina/log:1", "numericId": 1, - "description": "Avoid logging configurable variables", + "description": "Potentially-sensitive configurable variables are logged", "ruleKind": "VULNERABILITY" }, "source": "BUILT_IN", @@ -82,17 +82,37 @@ { "location": { "filePath": "main.bal", - "startLine": 33, - "endLine": 33, - "startColumn": 8, - "endColumn": 51, - "startOffset": 1039, - "length": 43 + "startLine": 23, + "endLine": 23, + "startColumn": 39, + "endColumn": 47, + "startOffset": 902, + "length": 8 + }, + "rule": { + "id": "ballerina/log:1", + "numericId": 1, + "description": "Potentially-sensitive configurable variables are logged", + "ruleKind": "VULNERABILITY" + }, + "source": "BUILT_IN", + "fileName": "rule1/main.bal", + "filePath": "/Users/danesh/ballerina-platform/module-ballerina-log/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" + }, + { + "location": { + "filePath": "main.bal", + "startLine": 23, + "endLine": 23, + "startColumn": 56, + "endColumn": 60, + "startOffset": 919, + "length": 4 }, "rule": { "id": "ballerina/log:1", "numericId": 1, - "description": "Avoid logging configurable variables", + "description": "Potentially-sensitive configurable variables are logged", "ruleKind": "VULNERABILITY" }, "source": "BUILT_IN", diff --git a/compiler-plugin-tests/src/test/resources/testng.xml b/compiler-plugin-tests/src/test/resources/testng.xml index 15c183ce..85e46103 100644 --- a/compiler-plugin-tests/src/test/resources/testng.xml +++ b/compiler-plugin-tests/src/test/resources/testng.xml @@ -19,9 +19,9 @@ --> - + - + diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java index 6b9cc25a..e5fb813d 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java @@ -24,7 +24,8 @@ import static io.ballerina.stdlib.log.compiler.staticcodeanalyzer.RuleFactory.createRule; public enum LogRule { - AVOID_LOGGING_CONFIGURABLE_VARIABLES(createRule(1, "Avoid logging configurable variables", VULNERABILITY)); + AVOID_LOGGING_CONFIGURABLE_VARIABLES(createRule(1, + "Potentially-sensitive configurable variables are logged", VULNERABILITY)); private final Rule rule; diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java index b5513cd6..87eeb366 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java @@ -53,6 +53,8 @@ public class LogStatementAnalyzer implements AnalysisTask semanticModels = new ArrayList<>(); + Document document = null; + private final Reporter reporter; public LogStatementAnalyzer(Reporter reporter) { @@ -68,6 +70,11 @@ public void perform(SyntaxNodeAnalysisContext context) { }); } + // If the document is null, we get the document of the context + if (document == null) { + document = context.currentPackage().module(context.moduleId()).document(context.documentId()); + } + if (context.node() instanceof ExpressionStatementNode expressionStatementNode) { // Check if the log statement has a configurable qualifier List childlist = expressionStatementNode.expression().childEntries().stream() @@ -94,7 +101,7 @@ public void perform(SyntaxNodeAnalysisContext context) { positionalArgumentNode.childEntries().forEach(childNodeEntry -> { Node expression = childNodeEntry.node().orElse(null); if (expression instanceof SimpleNameReferenceNode simpleNameReferenceNode) { - checkConfigurableQualifier(simpleNameReferenceNode, context, expressionStatementNode); + checkConfigurableQualifier(simpleNameReferenceNode); } else if (expression instanceof TemplateExpressionNode templateExpressionNode) { templateExpressionNode.content().forEach(content -> { if (content instanceof InterpolationNode interpolationNode) { @@ -102,8 +109,7 @@ public void perform(SyntaxNodeAnalysisContext context) { Node interpolationExpression = interpolationChild.node().orElse(null); if (interpolationExpression instanceof SimpleNameReferenceNode simpleNameReferenceNode) { - checkConfigurableQualifier(simpleNameReferenceNode, context, - expressionStatementNode); + checkConfigurableQualifier(simpleNameReferenceNode); } }); } @@ -112,36 +118,30 @@ public void perform(SyntaxNodeAnalysisContext context) { binaryExpressionNode.childEntries().forEach(childEntry -> { Node childNode = childEntry.node().orElse(null); if (childNode instanceof SimpleNameReferenceNode simpleNameReferenceNode) { - checkConfigurableQualifier(simpleNameReferenceNode, context, - expressionStatementNode); + checkConfigurableQualifier(simpleNameReferenceNode); } }); } }); } else if (node instanceof NamedArgumentNode namedArgumentNode) { - checkConfigurableQualifier(namedArgumentNode.expression(), context, expressionStatementNode); + checkConfigurableQualifier(namedArgumentNode.expression()); } } } } } - private void checkConfigurableQualifier(ExpressionNode argumentNode, SyntaxNodeAnalysisContext context, - ExpressionStatementNode expressionStatementNode) { + private void checkConfigurableQualifier(ExpressionNode argumentNode) { semanticModels.forEach(semanticModel -> { Symbol symbol = semanticModel.symbol(argumentNode).orElse(null); if (symbol instanceof VariableSymbol variableSymbol) { variableSymbol.qualifiers().stream().filter(qualifier -> qualifier .toString().equals(CONFIGURABLE_QUALIFIER)).forEach(qualifier -> { - this.reporter.reportIssue(getDocument(context), - expressionStatementNode.location(), + this.reporter.reportIssue(document, + argumentNode.location(), AVOID_LOGGING_CONFIGURABLE_VARIABLES.getId()); }); } }); } - - private static Document getDocument(SyntaxNodeAnalysisContext context) { - return context.currentPackage().module(context.moduleId()).document(context.documentId()); - } } diff --git a/compiler-plugin/src/main/resources/rules.json b/compiler-plugin/src/main/resources/rules.json index 5963a1f4..d335f9f1 100644 --- a/compiler-plugin/src/main/resources/rules.json +++ b/compiler-plugin/src/main/resources/rules.json @@ -2,6 +2,6 @@ { "id": 1, "kind": "VULNERABILITY", - "description": "Avoid logging configurable variables" + "description": "Potentially-sensitive configurable variables are logged" } ] From 99d77addc09af075d6e558bab6efafcca47a0083 Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Sun, 23 Feb 2025 20:35:17 +0530 Subject: [PATCH 10/15] Add since annotation --- .../compiler/staticcodeanalyzer/ProcessOutputGobbler.java | 1 + .../compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java | 1 + .../stdlib/log/compiler/staticcodeanalyzer/LogRule.java | 4 ++++ .../compiler/staticcodeanalyzer/LogStatementAnalyzer.java | 5 +++++ .../stdlib/log/compiler/staticcodeanalyzer/RuleFactory.java | 2 ++ .../stdlib/log/compiler/staticcodeanalyzer/RuleImpl.java | 5 +++++ .../log/compiler/staticcodeanalyzer/StaticCodeAnalyzer.java | 5 +++++ 7 files changed, 23 insertions(+) diff --git a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/ProcessOutputGobbler.java b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/ProcessOutputGobbler.java index 9c62e6ab..9dc26dae 100644 --- a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/ProcessOutputGobbler.java +++ b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/ProcessOutputGobbler.java @@ -26,6 +26,7 @@ /** * Helper class to consume the process streams. + * @since 2.12.0 */ class ProcessOutputGobbler implements Runnable { private final InputStream inputStream; diff --git a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java index 91cff480..0d13908a 100644 --- a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java +++ b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java @@ -35,6 +35,7 @@ /** * This class includes tests for Ballerina Http static code analyzer. + * @since 2.12.0 */ class StaticCodeAnalyzerTest { diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java index e5fb813d..e32236e7 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java @@ -23,6 +23,10 @@ import static io.ballerina.scan.RuleKind.VULNERABILITY; import static io.ballerina.stdlib.log.compiler.staticcodeanalyzer.RuleFactory.createRule; +/** + * Enum to hold the log rules. + * @since 2.12.0 + */ public enum LogRule { AVOID_LOGGING_CONFIGURABLE_VARIABLES(createRule(1, "Potentially-sensitive configurable variables are logged", VULNERABILITY)); diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java index 87eeb366..20ec64a0 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java @@ -44,6 +44,11 @@ import static io.ballerina.stdlib.log.compiler.staticcodeanalyzer.LogRule.AVOID_LOGGING_CONFIGURABLE_VARIABLES; +/** + * This class analyzes the log statements and checks if the log statement is logging a configurable variable. + * + * @since 2.12.0 + */ public class LogStatementAnalyzer implements AnalysisTask { public static final String CONFIGURABLE_QUALIFIER = "CONFIGURABLE"; diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleFactory.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleFactory.java index 7a7d8441..7a68f2a8 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleFactory.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleFactory.java @@ -23,6 +23,8 @@ /** * {@code RuleFactory} contains the logic to create a {@link Rule}. + * + * @since 2.12.0 */ public class RuleFactory { public static Rule createRule(int id, String description, RuleKind kind) { diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleImpl.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleImpl.java index 7fd2bdfb..384690c2 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleImpl.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/RuleImpl.java @@ -21,6 +21,11 @@ import io.ballerina.scan.Rule; import io.ballerina.scan.RuleKind; +/** + * {@code RuleFactory} contains the logic to create a {@link Rule}. + * + * @since 2.12.0 + */ class RuleImpl implements Rule { private final int id; private final String description; diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzer.java index f3801d6f..2403eedb 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzer.java @@ -26,6 +26,11 @@ import static io.ballerina.compiler.syntax.tree.SyntaxKind.CALL_STATEMENT; +/** + * Static code analyzer for log module. + * + * @since 2.12.0 + */ public class StaticCodeAnalyzer extends CodeAnalyzer { private final Reporter reporter; From 3edb5309e84fdca1986162244a181483a3bf3b2e Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Mon, 24 Feb 2025 10:16:40 +0530 Subject: [PATCH 11/15] Fix review suggestions --- .../StaticCodeAnalyzerTest.java | 2 +- .../ballerina_packages/rule1/main.bal | 2 + .../expected_output/rule1.json | 80 ++++++++++++++++--- .../log/compiler/LogCompilerPlugin.java | 2 - .../compiler/staticcodeanalyzer/LogRule.java | 2 - .../LogStatementAnalyzer.java | 2 +- 6 files changed, 74 insertions(+), 16 deletions(-) diff --git a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java index 0d13908a..c5f155d2 100644 --- a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java +++ b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java @@ -123,7 +123,7 @@ private static String normalizeJson(String json) { .replaceAll("\\s*\\[\\s*", "[") .replaceAll("\\s*]\\s*", "]") .replaceAll("\n", "") - .replaceAll(":\".*module-ballerina-http", ":\"module-ballerina-http"); + .replaceAll(":\".*module-ballerina-log", ":\"module-ballerina-log"); return isWindows() ? normalizedJson.replaceAll("/", "\\\\\\\\") : normalizedJson; } diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal index db0a4ccc..18011f51 100644 --- a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal @@ -19,9 +19,11 @@ import ballerina/log; public function main() { log:printInfo(password); log:printError(string `Error: ${password}`); + log:printWarn(`Error: ${password}`); log:printError("Error " + password); log:printWarn("Warning", password = password); log:printError("Error", password = password, user = user); + log:printError(password, user = user); } function log() { diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json b/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json index 49bd67b5..29138eb9 100644 --- a/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/expected_output/rule1.json @@ -44,9 +44,9 @@ "filePath": "main.bal", "startLine": 21, "endLine": 21, - "startColumn": 30, - "endColumn": 38, - "startOffset": 801, + "startColumn": 28, + "endColumn": 36, + "startOffset": 799, "length": 8 }, "rule": { @@ -64,9 +64,9 @@ "filePath": "main.bal", "startLine": 22, "endLine": 22, - "startColumn": 40, - "endColumn": 48, - "startOffset": 852, + "startColumn": 30, + "endColumn": 38, + "startOffset": 842, "length": 8 }, "rule": { @@ -84,9 +84,29 @@ "filePath": "main.bal", "startLine": 23, "endLine": 23, + "startColumn": 40, + "endColumn": 48, + "startOffset": 893, + "length": 8 + }, + "rule": { + "id": "ballerina/log:1", + "numericId": 1, + "description": "Potentially-sensitive configurable variables are logged", + "ruleKind": "VULNERABILITY" + }, + "source": "BUILT_IN", + "fileName": "rule1/main.bal", + "filePath": "/Users/danesh/ballerina-platform/module-ballerina-log/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" + }, + { + "location": { + "filePath": "main.bal", + "startLine": 24, + "endLine": 24, "startColumn": 39, "endColumn": 47, - "startOffset": 902, + "startOffset": 943, "length": 8 }, "rule": { @@ -102,11 +122,51 @@ { "location": { "filePath": "main.bal", - "startLine": 23, - "endLine": 23, + "startLine": 24, + "endLine": 24, "startColumn": 56, "endColumn": 60, - "startOffset": 919, + "startOffset": 960, + "length": 4 + }, + "rule": { + "id": "ballerina/log:1", + "numericId": 1, + "description": "Potentially-sensitive configurable variables are logged", + "ruleKind": "VULNERABILITY" + }, + "source": "BUILT_IN", + "fileName": "rule1/main.bal", + "filePath": "/Users/danesh/ballerina-platform/module-ballerina-log/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" + }, + { + "location": { + "filePath": "main.bal", + "startLine": 25, + "endLine": 25, + "startColumn": 19, + "endColumn": 27, + "startOffset": 986, + "length": 8 + }, + "rule": { + "id": "ballerina/log:1", + "numericId": 1, + "description": "Potentially-sensitive configurable variables are logged", + "ruleKind": "VULNERABILITY" + }, + "source": "BUILT_IN", + "fileName": "rule1/main.bal", + "filePath": "/Users/danesh/ballerina-platform/module-ballerina-log/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" + }, + { + "location": { + "filePath": "main.bal", + "startLine": 25, + "endLine": 25, + "startColumn": 36, + "endColumn": 40, + "startOffset": 1003, "length": 4 }, "rule": { diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/LogCompilerPlugin.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/LogCompilerPlugin.java index 9676b93b..fc701eb7 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/LogCompilerPlugin.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/LogCompilerPlugin.java @@ -32,8 +32,6 @@ public class LogCompilerPlugin extends CompilerPlugin { @Override public void init(CompilerPluginContext context) { - - //context.addCodeModifier(new LogCodeModifier()); Object object = context.userData().get(SCANNER_CONTEXT); if (object instanceof ScannerContext scannerContext) { context.addCodeAnalyzer(new StaticCodeAnalyzer(scannerContext.getReporter())); diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java index e32236e7..54eb37c3 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogRule.java @@ -50,6 +50,4 @@ public String toString() { return "{\"id\":" + this.getId() + ", \"kind\":\"" + this.rule.kind() + "\"," + " \"description\" : \"" + this.rule.description() + "\"}"; } - - } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java index 20ec64a0..1f8d6618 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java @@ -54,7 +54,7 @@ public class LogStatementAnalyzer implements AnalysisTask logFunctions = Arrays.asList("printInfo", "printDebug", "printError", "printWarn"); + final List logFunctions = Arrays.asList("printInfo", "printError", "printWarn"); List semanticModels = new ArrayList<>(); From db88d73e6f32196d53e1e6c77cf372cf6651c43b Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Mon, 24 Feb 2025 15:33:19 +0530 Subject: [PATCH 12/15] Add import alias support --- .../ballerina_packages/rule1/main.bal | 3 +- .../LogStatementAnalyzer.java | 38 +++++++++++++------ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal index 18011f51..4a126265 100644 --- a/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal +++ b/compiler-plugin-tests/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal @@ -15,6 +15,7 @@ // under the License. import ballerina/log; +import ballerina/log as lg; public function main() { log:printInfo(password); @@ -23,7 +24,7 @@ public function main() { log:printError("Error " + password); log:printWarn("Warning", password = password); log:printError("Error", password = password, user = user); - log:printError(password, user = user); + lg:printError(password, user = user); } function log() { diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java index 1f8d6618..2cbc31e9 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/staticcodeanalyzer/LogStatementAnalyzer.java @@ -25,7 +25,10 @@ import io.ballerina.compiler.syntax.tree.ChildNodeEntry; import io.ballerina.compiler.syntax.tree.ExpressionNode; import io.ballerina.compiler.syntax.tree.ExpressionStatementNode; +import io.ballerina.compiler.syntax.tree.ImportOrgNameNode; +import io.ballerina.compiler.syntax.tree.ImportPrefixNode; import io.ballerina.compiler.syntax.tree.InterpolationNode; +import io.ballerina.compiler.syntax.tree.ModulePartNode; import io.ballerina.compiler.syntax.tree.NamedArgumentNode; import io.ballerina.compiler.syntax.tree.Node; import io.ballerina.compiler.syntax.tree.NodeList; @@ -52,14 +55,13 @@ public class LogStatementAnalyzer implements AnalysisTask { public static final String CONFIGURABLE_QUALIFIER = "CONFIGURABLE"; - public static final String LOG_MODULE_PREFIX = "log"; + public static final String LOG_MODULE = "log"; + public static final String BALLERINA_ORG = "ballerina"; final List logFunctions = Arrays.asList("printInfo", "printError", "printWarn"); List semanticModels = new ArrayList<>(); - Document document = null; - private final Reporter reporter; public LogStatementAnalyzer(Reporter reporter) { @@ -75,11 +77,23 @@ public void perform(SyntaxNodeAnalysisContext context) { }); } - // If the document is null, we get the document of the context - if (document == null) { - document = context.currentPackage().module(context.moduleId()).document(context.documentId()); + Document document = context.currentPackage().module(context.moduleId()).document(context.documentId()); + List importPrefix = new ArrayList<>(); + if (document.syntaxTree().rootNode() instanceof ModulePartNode modulePartNode) { + importPrefix = modulePartNode.imports().stream() + .filter(importDeclarationNode -> { + ImportOrgNameNode importOrgNameNode = importDeclarationNode.orgName().orElse(null); + return importOrgNameNode != null && BALLERINA_ORG.equals(importOrgNameNode.orgName().text()); + }) + .filter(importDeclarationNode -> importDeclarationNode.moduleName().stream().anyMatch( + moduleNameNode -> LOG_MODULE.equals(moduleNameNode.text()))) + .map(importDeclarationNode -> { + ImportPrefixNode importPrefixNode = importDeclarationNode.prefix().orElse(null); + return importPrefixNode != null ? importPrefixNode.prefix().text() : LOG_MODULE; + }).toList(); } + if (context.node() instanceof ExpressionStatementNode expressionStatementNode) { // Check if the log statement has a configurable qualifier List childlist = expressionStatementNode.expression().childEntries().stream() @@ -91,7 +105,7 @@ public void perform(SyntaxNodeAnalysisContext context) { Node firstChild = childlist.getFirst().node().orElse(null); if (firstChild instanceof QualifiedNameReferenceNode qualifiedNameReferenceNode - && qualifiedNameReferenceNode.modulePrefix().text().equals(LOG_MODULE_PREFIX) + && importPrefix.contains(qualifiedNameReferenceNode.modulePrefix().text()) && logFunctions.contains(qualifiedNameReferenceNode.identifier().text())) { // The argument of the log function is the third child. second and fourth child are the parentheses @@ -106,7 +120,7 @@ public void perform(SyntaxNodeAnalysisContext context) { positionalArgumentNode.childEntries().forEach(childNodeEntry -> { Node expression = childNodeEntry.node().orElse(null); if (expression instanceof SimpleNameReferenceNode simpleNameReferenceNode) { - checkConfigurableQualifier(simpleNameReferenceNode); + checkConfigurableQualifier(simpleNameReferenceNode, document); } else if (expression instanceof TemplateExpressionNode templateExpressionNode) { templateExpressionNode.content().forEach(content -> { if (content instanceof InterpolationNode interpolationNode) { @@ -114,7 +128,7 @@ public void perform(SyntaxNodeAnalysisContext context) { Node interpolationExpression = interpolationChild.node().orElse(null); if (interpolationExpression instanceof SimpleNameReferenceNode simpleNameReferenceNode) { - checkConfigurableQualifier(simpleNameReferenceNode); + checkConfigurableQualifier(simpleNameReferenceNode, document); } }); } @@ -123,20 +137,20 @@ public void perform(SyntaxNodeAnalysisContext context) { binaryExpressionNode.childEntries().forEach(childEntry -> { Node childNode = childEntry.node().orElse(null); if (childNode instanceof SimpleNameReferenceNode simpleNameReferenceNode) { - checkConfigurableQualifier(simpleNameReferenceNode); + checkConfigurableQualifier(simpleNameReferenceNode, document); } }); } }); } else if (node instanceof NamedArgumentNode namedArgumentNode) { - checkConfigurableQualifier(namedArgumentNode.expression()); + checkConfigurableQualifier(namedArgumentNode.expression(), document); } } } } } - private void checkConfigurableQualifier(ExpressionNode argumentNode) { + private void checkConfigurableQualifier(ExpressionNode argumentNode, Document document) { semanticModels.forEach(semanticModel -> { Symbol symbol = semanticModel.symbol(argumentNode).orElse(null); if (symbol instanceof VariableSymbol variableSymbol) { From c4d630c869901090fd416dd1e34e434545e742a2 Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Mon, 24 Feb 2025 20:35:42 +0530 Subject: [PATCH 13/15] add compiler plugin path --- codecov.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/codecov.yml b/codecov.yml index 0e744bb3..54805cfb 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,5 +1,6 @@ fixes: - "ballerina/log/*/::log-ballerina/" + - "io/ballerina/stdlib/log/compiler/::./compiler-plugin/src/main/java/io/ballerina/stdlib/log/compiler/" ignore: - "**/tests" From e7fa0cbfe3be223e1b79be0a3a26216f60ca4cdd Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Tue, 25 Feb 2025 12:40:38 +0530 Subject: [PATCH 14/15] enable jacoco in compiler plugin tests --- compiler-plugin-tests/build.gradle | 16 +++++++++++++++- gradle.properties | 1 + 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/compiler-plugin-tests/build.gradle b/compiler-plugin-tests/build.gradle index a0372cc2..51c24ee6 100644 --- a/compiler-plugin-tests/build.gradle +++ b/compiler-plugin-tests/build.gradle @@ -20,6 +20,12 @@ plugins { id 'java' id 'checkstyle' id 'com.github.spotbugs' + id 'jacoco' +} + +jacoco { + toolVersion = "${jacocoVersion}" + reportsDirectory = file("$project.buildDir/reports/jacoco") } description = 'Ballerina - Log Compiler Plugin Tests' @@ -44,7 +50,6 @@ tasks.withType(JavaCompile) { sourceCompatibility = JavaVersion.VERSION_21 test { - systemProperty "ballerina.offline.flag", "true" useTestNG() { suites 'src/test/resources/testng.xml' } @@ -60,6 +65,15 @@ test { } } } + finalizedBy jacocoTestReport +} + +jacocoTestReport { + dependsOn test + reports { + xml.required = true + } + sourceSets project(':log-compiler-plugin').sourceSets.main } spotbugsTest { diff --git a/gradle.properties b/gradle.properties index 3a9789a2..48525130 100644 --- a/gradle.properties +++ b/gradle.properties @@ -10,6 +10,7 @@ downloadPluginVersion=5.4.0 releasePluginVersion=2.6.0 ballerinaGradlePluginVersion=2.3.0 balScanVersion=0.5.0 +jacocoVersion=0.8.10 #stdlib dependencies stdlibIoVersion=1.7.0 From 71c8468cd9183d9b527474a8a8355c7142cb63a6 Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Tue, 25 Feb 2025 16:00:03 +0530 Subject: [PATCH 15/15] ignore the compiler-plugin until sortout the coverage issue --- codecov.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/codecov.yml b/codecov.yml index 54805cfb..ec993452 100644 --- a/codecov.yml +++ b/codecov.yml @@ -5,6 +5,7 @@ fixes: ignore: - "**/tests" - "test-utils" + - "compiler-plugin" coverage: precision: 2