Support scanning workspaces#79
Merged
azinneera merged 6 commits intoballerina-platform:mainfrom Apr 10, 2026
Merged
Conversation
azinneera
force-pushed
the
workspaces
branch
9 times, most recently
from
ac536e5 to
b284707
Compare
azinneera
force-pushed
the
workspaces
branch
3 times, most recently
from
737fa53 to
9b2626a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #79 +/- ##
============================================
- Coverage 81.21% 79.44% -1.77%
- Complexity 320 328 +8
============================================
Files 20 20
Lines 1123 1197 +74
Branches 159 173 +14
============================================
+ Hits 912 951 +39
- Misses 148 176 +28
- Partials 63 70 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
azinneera
force-pushed
the
workspaces
branch
10 times, most recently
from
9d38a43 to
81139ea
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds workspace-aware scanning with aggregated workspace reports, centralizes project loading via Changes
Sequence DiagramsequenceDiagram
participant CLI as "Scan CLI"
participant Loader as "ProjectLoader"
participant WSMgr as "Workspace Resolver"
participant Analyzer as "Static Analyzer"
participant Reporter as "Report Generator"
participant Disk as "Disk/Output"
CLI->>Loader: load(projectPath) / validate via ProjectPaths
Loader-->>CLI: LoadResult (Project or diagnostics)
alt WORKSPACE_PROJECT
CLI->>WSMgr: resolve workspace dependency graph
WSMgr-->>CLI: topologically sorted BuildProjects
loop per project
CLI->>Analyzer: executeProject(project)
Analyzer-->>CLI: projectIssues
CLI->>CLI: allIssues.addAll(projectIssues)
end
CLI->>Reporter: generateScanReport(allIssues, workspaceProject)
Reporter-->>CLI: reportContent (JSON / SARIF)
CLI->>Disk: Files.writeString(reportPath, reportContent)
else non-workspace
CLI->>Analyzer: executeProject(singleProject)
Analyzer-->>CLI: issues
CLI->>Reporter: generateScanReport(issues, project)
CLI->>Disk: Files.writeString(...)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
scan-command/src/test/java/io/ballerina/scan/internal/ReporterImplTest.java (1)
55-57: Consider adding a null-check or assertion for the loaded project.While test failures are acceptable for propagating load errors, a defensive assertion after
ProjectLoader.load()could provide clearer failure messages when the test resource is missing or invalid:Project project = ProjectLoader.load(validBalProject).project(); Assert.assertNotNull(project, "Failed to load project from: " + validBalProject);This would make test failures more debuggable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scan-command/src/test/java/io/ballerina/scan/internal/ReporterImplTest.java` around lines 55 - 57, Add a defensive null-check/assertion after calling ProjectLoader.load(validBalProject).project() in ReporterImplTest: verify that the returned Project (variable project) is not null and fail with a clear message including validBalProject when it is, so test failures show a readable error if the test resource is missing or loading failed; update the code around ProjectLoader.load(...) / project variable to include this assertion or an explicit null-check and message..github/workflows/central-publish.yml (1)
30-33: Consider aligning thesetup-ballerinaaction version with other workflows.This workflow uses
ballerina-platform/setup-ballerina@v1.1.0whilefull_build.ymluses@v1.1.1. For consistency and to ensure all workflows benefit from the same fixes/features, consider updating tov1.1.1.Additionally,
full_build.ymlnow clearsBALLERINA_HOMEbefore running the build. If this is needed to avoid environment conflicts, the same approach may be warranted here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/central-publish.yml around lines 30 - 33, Update the Ballerina setup action to use the same release as other workflows by changing the action reference from ballerina-platform/setup-ballerina@v1.1.0 to `@v1.1.1` (the same used in full_build.yml), and if full_build.yml clears BALLERINA_HOME to avoid environment conflicts, add a step before the build in this workflow to clear or unset BALLERINA_HOME (or explicitly set it) so both workflows behave consistently; look for the uses: ballerina-platform/setup-ballerina entry and add/update an environment-clearing step similar to the one in full_build.yml.scan-command/src/test/resources/command-outputs/windows/workspace-issues.txt (1)
1-42: Consider adding a trailing newline.The file lacks a trailing newline, which is a POSIX convention and can cause issues with some text processing tools or cause "No newline at end of file" warnings in diffs. This is a minor concern for a test fixture.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scan-command/src/test/resources/command-outputs/windows/workspace-issues.txt` around lines 1 - 42, Add a POSIX trailing newline to the end of the test fixture file named workspace-issues.txt so the file ends with a single newline character (ensure no extra blank lines), update the test resource content accordingly, and re-run tests/snapshots that consume this fixture to confirm diffs no longer report "No newline at end of file".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scan-command/build.gradle`:
- Around line 281-299: The try/catch around the exec that captures the bal home
output (in tasks.test using balHomeOutput and setting systemProperty
'ballerina.home') swallows failures and leaves tests with unclear errors; update
the catch to first attempt a fallback by reading System.getenv("BALLERINA_HOME")
and calling systemProperty 'ballerina.home' with that value if present
(trimmed), and if the env var is not set then fail the build explicitly by
throwing a GradleException with a clear message that 'bal home' failed and
BALLERINA_HOME is not set so tests cannot run; keep references to balHomeOutput
in the log message for debugging.
In `@scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java`:
- Around line 189-200: The problem is that execute() reuses the same ScanCmd
instance across workspace members so executeProject() accumulates Scan.toml
include/exclude filters across packages, causing cross-package conflicts; fix it
by making filters local to each member — either construct a fresh ScanCmd (or a
dedicated filter-holding object) per BuildProject before calling
executeProject(), or ensure executeProject() copies/clears the ScanCmd's
include/exclude fields (and any related state) at the start and restores/clears
them after execution so each package’s Scan.toml filters (include/exclude) are
applied only to that package; update references in execute(), executeProject(),
and any code that reads Scan.toml to use the per-project local filter instance.
- Around line 203-210: The code calls
accumulateWorkspaceReports(workspaceProject) and may generate a scan HTML report
even when the command was invoked with --list-rules (executeProject() returns
early and allIssues is empty); add a guard to skip workspace report accumulation
and ScanUtils.generateScanReport when the run is a rules-listing invocation:
detect the list-rules mode (the flag that triggers executeProject() to print
rules) and wrap the calls to accumulateWorkspaceReports(workspaceProject) and
ScanUtils.generateScanReport(allIssues, project.get(), targetDir) (as well as
the scanReport printing block controlled by scanReport) so they are not executed
for --list-rules. Ensure you reference the same flag/variable used to decide the
early return in executeProject().
- Around line 314-333: The code currently calls ScanUtils.saveToDirectory /
ScanUtils.saveSarifToDirectory for every Project member before checking
project.workspaceProject().isEmpty(), causing per-member report files; change
the flow in ScanCmd so you only invoke ScanUtils.saveToDirectory or
ScanUtils.saveSarifToDirectory when project.workspaceProject().isEmpty() (i.e.,
not a workspace member) — either move the save* calls inside the existing if
(project.workspaceProject().isEmpty()) block or add an early guard (if
(project.workspaceProject().isPresent()) continue) before calling
ScanUtils.saveToDirectory / ScanUtils.saveSarifToDirectory, keeping the
subsequent printing and ScanUtils.generateScanReport logic unchanged.
---
Nitpick comments:
In @.github/workflows/central-publish.yml:
- Around line 30-33: Update the Ballerina setup action to use the same release
as other workflows by changing the action reference from
ballerina-platform/setup-ballerina@v1.1.0 to `@v1.1.1` (the same used in
full_build.yml), and if full_build.yml clears BALLERINA_HOME to avoid
environment conflicts, add a step before the build in this workflow to clear or
unset BALLERINA_HOME (or explicitly set it) so both workflows behave
consistently; look for the uses: ballerina-platform/setup-ballerina entry and
add/update an environment-clearing step similar to the one in full_build.yml.
In `@scan-command/src/test/java/io/ballerina/scan/internal/ReporterImplTest.java`:
- Around line 55-57: Add a defensive null-check/assertion after calling
ProjectLoader.load(validBalProject).project() in ReporterImplTest: verify that
the returned Project (variable project) is not null and fail with a clear
message including validBalProject when it is, so test failures show a readable
error if the test resource is missing or loading failed; update the code around
ProjectLoader.load(...) / project variable to include this assertion or an
explicit null-check and message.
In
`@scan-command/src/test/resources/command-outputs/windows/workspace-issues.txt`:
- Around line 1-42: Add a POSIX trailing newline to the end of the test fixture
file named workspace-issues.txt so the file ends with a single newline character
(ensure no extra blank lines), update the test resource content accordingly, and
re-run tests/snapshots that consume this fixture to confirm diffs no longer
report "No newline at end of file".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2862ea3-4415-401d-9c1d-4996b7c02fb7
📒 Files selected for processing (51)
.github/workflows/central-publish.yml.github/workflows/full_build.yml.github/workflows/publish-release.ymlgradle.propertiesscan-command-test-utils/src/main/java/io/ballerina/scan/test/TestScanCmd.javascan-command/build.gradlescan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.javascan-command/src/main/java/io/ballerina/scan/utils/Constants.javascan-command/src/main/java/io/ballerina/scan/utils/ScanUtils.javascan-command/src/test/java/io/ballerina/scan/internal/ProjectAnalyzerTest.javascan-command/src/test/java/io/ballerina/scan/internal/ReporterImplTest.javascan-command/src/test/java/io/ballerina/scan/internal/ScanCmdTest.javascan-command/src/test/java/io/ballerina/scan/internal/StaticCodeAnalyzerTest.javascan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.javascan-command/src/test/resources/command-outputs/unix/exclude-rules-issues-report.txtscan-command/src/test/resources/command-outputs/unix/include-exclude-rules.txtscan-command/src/test/resources/command-outputs/unix/include-rules-issues-report.txtscan-command/src/test/resources/command-outputs/unix/invalid-platform-plugin-configurations.txtscan-command/src/test/resources/command-outputs/unix/list-rules-output.txtscan-command/src/test/resources/command-outputs/unix/platform-plugin-issue-output.txtscan-command/src/test/resources/command-outputs/unix/print-rules-to-console.txtscan-command/src/test/resources/command-outputs/unix/scan-toml-invalid-platform-config.txtscan-command/src/test/resources/command-outputs/unix/scan-toml-invalid-platform-jar.txtscan-command/src/test/resources/command-outputs/unix/toml-exclude-rules-issues-report.txtscan-command/src/test/resources/command-outputs/unix/toml-include-exclude-rules.txtscan-command/src/test/resources/command-outputs/unix/toml-include-rules-issues-report.txtscan-command/src/test/resources/command-outputs/unix/workspace-issues.txtscan-command/src/test/resources/command-outputs/windows/exclude-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/include-exclude-rules.txtscan-command/src/test/resources/command-outputs/windows/include-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/invalid-platform-plugin-configurations.txtscan-command/src/test/resources/command-outputs/windows/list-rules-output.txtscan-command/src/test/resources/command-outputs/windows/platform-plugin-issue-output.txtscan-command/src/test/resources/command-outputs/windows/print-rules-to-console.txtscan-command/src/test/resources/command-outputs/windows/scan-toml-invalid-platform-config.txtscan-command/src/test/resources/command-outputs/windows/scan-toml-invalid-platform-jar.txtscan-command/src/test/resources/command-outputs/windows/toml-exclude-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/toml-include-exclude-rules.txtscan-command/src/test/resources/command-outputs/windows/toml-include-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/workspace-issues.txtscan-command/src/test/resources/test-resources/workspace-project/Ballerina.tomlscan-command/src/test/resources/test-resources/workspace-project/bal-project-with-analyzer-configurations/.gitignorescan-command/src/test/resources/test-resources/workspace-project/bal-project-with-analyzer-configurations/Ballerina.tomlscan-command/src/test/resources/test-resources/workspace-project/bal-project-with-analyzer-configurations/Scan.tomlscan-command/src/test/resources/test-resources/workspace-project/bal-project-with-analyzer-configurations/main.balscan-command/src/test/resources/test-resources/workspace-project/bal-project-with-include-rule-configurations/.gitignorescan-command/src/test/resources/test-resources/workspace-project/bal-project-with-include-rule-configurations/Ballerina.tomlscan-command/src/test/resources/test-resources/workspace-project/bal-project-with-include-rule-configurations/Scan.tomlscan-command/src/test/resources/test-resources/workspace-project/bal-project-with-include-rule-configurations/main.balscan-command/tool-scan/Ballerina.tomlscan-command/tool-scan/Dependencies.toml
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.java (1)
118-122:⚠️ Potential issue | 🟠 MajorProtect global
user.dirrestoration withtry/finally.Line 119 mutates global JVM state, and restoration currently depends on normal flow. If
loadScanTomlConfigurations(...)throws, later tests can fail unpredictably due to leakeduser.dir(same pattern repeats in other methods).🔧 Suggested fix pattern
- System.setProperty("user.dir", ballerinaProject.toString()); - ScanTomlFile scanTomlFile = ScanUtils.loadScanTomlConfigurations(project, printStream).orElse(null); - System.setProperty("user.dir", userDir); + String previousUserDir = System.getProperty("user.dir"); + System.setProperty("user.dir", ballerinaProject.toString()); + ScanTomlFile scanTomlFile; + try { + scanTomlFile = ScanUtils.loadScanTomlConfigurations(project, printStream).orElse(null); + } finally { + System.setProperty("user.dir", previousUserDir); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.java` around lines 118 - 122, The test mutates the global System property "user.dir" before calling ProjectLoader.load(...) and ScanUtils.loadScanTomlConfigurations(...); wrap the mutation and call in a try/finally so the original userDir is always restored (use System.setProperty("user.dir", userDir) in the finally). Locate the block around Project project = ProjectLoader.load(...), the System.setProperty("user.dir", ...), and the call to ScanUtils.loadScanTomlConfigurations(...) and ensure the restoration happens in finally to avoid leaking global state across tests.
🧹 Nitpick comments (1)
scan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.java (1)
259-261: Avoid leaving remote Scan.toml coverage permanently disabled.Disabling this test removes validation for the remote-config success path. Consider replacing the external dependency with a deterministic local fixture/stub so the test can stay enabled in CI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.java` around lines 259 - 261, The test testLoadRemoteScanTomlConfigurations in ScanUtilsTest is permanently disabled; instead of leaving it skipped, replace the external dependency with a deterministic fixture or stub and re-enable the test. Update the test to point at a local resource or an embedded HTTP stub (e.g., WireMock or a simple local HTTP server) that serves the expected Scan.toml payload, or mock the HTTP client used by the loader so the remote fetch returns a known fixture; then remove the enabled=false flag so the test runs in CI and verifies the remote-config success path deterministically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@scan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.java`:
- Around line 118-122: The test mutates the global System property "user.dir"
before calling ProjectLoader.load(...) and
ScanUtils.loadScanTomlConfigurations(...); wrap the mutation and call in a
try/finally so the original userDir is always restored (use
System.setProperty("user.dir", userDir) in the finally). Locate the block around
Project project = ProjectLoader.load(...), the System.setProperty("user.dir",
...), and the call to ScanUtils.loadScanTomlConfigurations(...) and ensure the
restoration happens in finally to avoid leaking global state across tests.
---
Nitpick comments:
In `@scan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.java`:
- Around line 259-261: The test testLoadRemoteScanTomlConfigurations in
ScanUtilsTest is permanently disabled; instead of leaving it skipped, replace
the external dependency with a deterministic fixture or stub and re-enable the
test. Update the test to point at a local resource or an embedded HTTP stub
(e.g., WireMock or a simple local HTTP server) that serves the expected
Scan.toml payload, or mock the HTTP client used by the loader so the remote
fetch returns a known fixture; then remove the enabled=false flag so the test
runs in CI and verifies the remote-config success path deterministically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6bb1e0a7-3729-4bec-9063-d1a161ce2361
📒 Files selected for processing (1)
scan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.java
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java (3)
189-213:⚠️ Potential issue | 🟠 MajorInclude/exclude filters accumulate across workspace members causing cross-contamination.
The same
ScanCmdinstance is reused for all workspace members, butexecuteProject()(lines 293-294) appends each member'sScan.tomlfilters to the instance fieldsincludeRulesandexcludeRules. This causes member A's filters to affect member B's scan results, and can trigger spurious "include and exclude" conflicts even when each member's config is valid in isolation.Consider creating local copies of the filter lists at the start of
executeProject():List<String> effectiveIncludeRules = new ArrayList<>(includeRules); List<String> effectiveExcludeRules = new ArrayList<>(excludeRules); scanTomlFile.get().getRulesToInclude().stream().map(ScanTomlFile.RuleToFilter::id) .forEach(effectiveIncludeRules::add); // ... use effectiveIncludeRules/effectiveExcludeRules for filtering🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java` around lines 189 - 213, The ScanCmd instance reuses instance fields includeRules and excludeRules across workspace members causing filters from one member to leak into subsequent members; inside executeProject(BuildProject) create local copies (e.g., effectiveIncludeRules and effectiveExcludeRules) initialized from the instance fields, then append the current project's Scan.toml rules (from scanTomlFile.get().getRulesToInclude()/getRulesToExclude()) to those local lists and use the locals for all filtering and conflict checks instead of mutating includeRules/excludeRules; alternatively reset the instance fields at start/end of executeProject to prevent cross-contamination.
203-210:⚠️ Potential issue | 🟡 Minor
--list-rulesstill triggers workspace report generation with empty results.When
--list-rulesis used,executeProject()returns early at line 266, leavingallIssuesempty. However,accumulateWorkspaceReports()and the optional HTML report generation still execute, creating empty report artifacts.Suggested fix
executeProject(buildProject); } - outputStream.println(); - accumulateWorkspaceReports(workspaceProject); - if (scanReport) { + if (!listRules) { + outputStream.println(); + accumulateWorkspaceReports(workspaceProject); + } + if (scanReport && !listRules) { Path scanReportPath = ScanUtils.generateScanReport(allIssues, project.get(), targetDir);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java` around lines 203 - 210, The code always calls accumulateWorkspaceReports(workspaceProject) and may generate an HTML report even when executeProject() returned early for --list-rules leaving allIssues empty; modify the block in ScanCmd.java so that you first check whether allIssues is empty (or the list-rules flag is set) and skip calling accumulateWorkspaceReports(workspaceProject) and Skip ScanUtils.generateScanReport(...) / the scanReport printing when allIssues.isEmpty() (or listRules() is true); update the condition around the existing scanReport / accumulateWorkspaceReports calls to only run when there are issues to report.
315-334:⚠️ Potential issue | 🟠 MajorWorkspace members still get individual report files saved.
The code calls
saveSarifToDirectory()/saveToDirectory()for allBuildProjects (line 315-321) before checkingproject.workspaceProject().isEmpty()(line 323). This means each workspace member gets its ownscan_results.*file in its target directory, contradicting the PR objective of producing a single aggregated workspace report.Suggested fix
if (platforms.isEmpty() && !platformTriggered) { boolean isSarifFormat = ReportFormat.SARIF.equals(format); ScanUtils.printToConsole(issues, outputStream, isSarifFormat, project); - if (project.kind().equals(ProjectKind.BUILD_PROJECT)) { + boolean standaloneBuildProject = + project.kind().equals(ProjectKind.BUILD_PROJECT) && project.workspaceProject().isEmpty(); + if (standaloneBuildProject) { Path reportPath; if (isSarifFormat) { reportPath = ScanUtils.saveSarifToDirectory(issues, project, targetDir); } else { reportPath = ScanUtils.saveToDirectory(issues, project, targetDir); } - - if (project.workspaceProject().isEmpty()) { - outputStream.println(); - outputStream.println("View scan results at:"); - outputStream.println("\t" + reportPath.toUri()); - outputStream.println(); - if (scanReport) { - Path scanReportPath = ScanUtils.generateScanReport(issues, project, targetDir); - outputStream.println(); - outputStream.println("View scan report at:"); - outputStream.println("\t" + scanReportPath.toUri() + System.lineSeparator()); - } - } + outputStream.println(); + outputStream.println("View scan results at:"); + outputStream.println("\t" + reportPath.toUri()); + outputStream.println(); + if (scanReport) { + Path scanReportPath = ScanUtils.generateScanReport(issues, project, targetDir); + outputStream.println(); + outputStream.println("View scan report at:"); + outputStream.println("\t" + scanReportPath.toUri() + System.lineSeparator()); + } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java` around lines 315 - 334, The current logic always calls ScanUtils.saveSarifToDirectory()/ScanUtils.saveToDirectory() for every BuildProject (in ScanCmd) before checking project.workspaceProject().isEmpty(), which causes per-member report files; change it so saving individual report files only happens for workspace roots: move the calls to saveSarifToDirectory/saveToDirectory inside the block guarded by project.workspaceProject().isEmpty() (the same block that prints "View scan results at:"), or add an explicit check skip when project.workspaceProject().isPresent(); similarly only call ScanUtils.generateScanReport() for the workspace root (or let the workspace aggregation path handle report creation), ensuring workspace members do not write their own scan_results files.
🧹 Nitpick comments (1)
scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java (1)
381-401: LGTM! Consider mentioning workspace in error message.The refactored project loading using
ProjectLoader.load()is cleaner and correctly handles all project types including workspaces.Minor suggestion: The error message at line 386-387 could explicitly mention workspace projects to help users understand the supported project types:
"The specified path is not a valid Ballerina project or workspace: " + projectPath + "..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java` around lines 381 - 401, Update the user-facing error string in getProject() to explicitly mention workspaces: where ProjectPaths checks fail (the block that prints using outputStream and references projectPath), change the message to something like "The specified path is not a valid Ballerina project or workspace: " + projectPath + "..." so it clearly communicates that workspace roots are supported; keep the same outputStream usage and method logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java`:
- Around line 238-240: The catch block in ScanCmd currently throws a new
RuntimeException with only e.getMessage(), which loses the original stack trace;
update the catch for IOException in ScanCmd to rethrow while preserving the
cause (e.g., create the RuntimeException or ScanToolException with a descriptive
message and pass the caught IOException as the cause) so the original stack
trace is retained when the exception is handled.
---
Duplicate comments:
In `@scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java`:
- Around line 189-213: The ScanCmd instance reuses instance fields includeRules
and excludeRules across workspace members causing filters from one member to
leak into subsequent members; inside executeProject(BuildProject) create local
copies (e.g., effectiveIncludeRules and effectiveExcludeRules) initialized from
the instance fields, then append the current project's Scan.toml rules (from
scanTomlFile.get().getRulesToInclude()/getRulesToExclude()) to those local lists
and use the locals for all filtering and conflict checks instead of mutating
includeRules/excludeRules; alternatively reset the instance fields at start/end
of executeProject to prevent cross-contamination.
- Around line 203-210: The code always calls
accumulateWorkspaceReports(workspaceProject) and may generate an HTML report
even when executeProject() returned early for --list-rules leaving allIssues
empty; modify the block in ScanCmd.java so that you first check whether
allIssues is empty (or the list-rules flag is set) and skip calling
accumulateWorkspaceReports(workspaceProject) and Skip
ScanUtils.generateScanReport(...) / the scanReport printing when
allIssues.isEmpty() (or listRules() is true); update the condition around the
existing scanReport / accumulateWorkspaceReports calls to only run when there
are issues to report.
- Around line 315-334: The current logic always calls
ScanUtils.saveSarifToDirectory()/ScanUtils.saveToDirectory() for every
BuildProject (in ScanCmd) before checking project.workspaceProject().isEmpty(),
which causes per-member report files; change it so saving individual report
files only happens for workspace roots: move the calls to
saveSarifToDirectory/saveToDirectory inside the block guarded by
project.workspaceProject().isEmpty() (the same block that prints "View scan
results at:"), or add an explicit check skip when
project.workspaceProject().isPresent(); similarly only call
ScanUtils.generateScanReport() for the workspace root (or let the workspace
aggregation path handle report creation), ensuring workspace members do not
write their own scan_results files.
---
Nitpick comments:
In `@scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java`:
- Around line 381-401: Update the user-facing error string in getProject() to
explicitly mention workspaces: where ProjectPaths checks fail (the block that
prints using outputStream and references projectPath), change the message to
something like "The specified path is not a valid Ballerina project or
workspace: " + projectPath + "..." so it clearly communicates that workspace
roots are supported; keep the same outputStream usage and method logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e11979e-9f1d-4be0-bbe6-2ba3012ab87a
📒 Files selected for processing (28)
scan-command/build.gradlescan-command/src/main/java/io/ballerina/scan/internal/ReporterImpl.javascan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.javascan-command/src/main/java/io/ballerina/scan/utils/ScanUtils.javascan-command/src/test/java/io/ballerina/scan/internal/ScanCmdTest.javascan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.javascan-command/src/test/resources/command-outputs/unix/include-exclude-rules.txtscan-command/src/test/resources/command-outputs/unix/invalid-platform-plugin-configurations.txtscan-command/src/test/resources/command-outputs/unix/list-rules-output.txtscan-command/src/test/resources/command-outputs/unix/print-rules-to-console.txtscan-command/src/test/resources/command-outputs/unix/scan-toml-invalid-platform-config.txtscan-command/src/test/resources/command-outputs/unix/scan-toml-invalid-platform-jar.txtscan-command/src/test/resources/command-outputs/unix/toml-include-exclude-rules.txtscan-command/src/test/resources/command-outputs/unix/workspace-issues.txtscan-command/src/test/resources/command-outputs/windows/exclude-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/include-exclude-rules.txtscan-command/src/test/resources/command-outputs/windows/include-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/invalid-platform-plugin-configurations.txtscan-command/src/test/resources/command-outputs/windows/list-rules-output.txtscan-command/src/test/resources/command-outputs/windows/platform-plugin-issue-output.txtscan-command/src/test/resources/command-outputs/windows/print-rules-to-console.txtscan-command/src/test/resources/command-outputs/windows/scan-toml-invalid-platform-config.txtscan-command/src/test/resources/command-outputs/windows/scan-toml-invalid-platform-jar.txtscan-command/src/test/resources/command-outputs/windows/toml-exclude-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/toml-include-exclude-rules.txtscan-command/src/test/resources/command-outputs/windows/toml-include-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/workspace-issues.txtscan-command/tool-scan/Dependencies.toml
✅ Files skipped from review due to trivial changes (21)
- scan-command/src/test/resources/command-outputs/unix/invalid-platform-plugin-configurations.txt
- scan-command/src/test/resources/command-outputs/windows/toml-include-exclude-rules.txt
- scan-command/src/test/resources/command-outputs/unix/list-rules-output.txt
- scan-command/src/test/resources/command-outputs/windows/include-exclude-rules.txt
- scan-command/src/test/resources/command-outputs/windows/print-rules-to-console.txt
- scan-command/src/test/resources/command-outputs/unix/scan-toml-invalid-platform-config.txt
- scan-command/src/test/resources/command-outputs/unix/toml-include-exclude-rules.txt
- scan-command/src/test/resources/command-outputs/unix/print-rules-to-console.txt
- scan-command/src/test/resources/command-outputs/unix/scan-toml-invalid-platform-jar.txt
- scan-command/src/test/resources/command-outputs/windows/invalid-platform-plugin-configurations.txt
- scan-command/tool-scan/Dependencies.toml
- scan-command/src/test/resources/command-outputs/windows/scan-toml-invalid-platform-jar.txt
- scan-command/src/test/resources/command-outputs/windows/toml-include-rules-issues-report.txt
- scan-command/src/test/resources/command-outputs/windows/scan-toml-invalid-platform-config.txt
- scan-command/src/test/resources/command-outputs/windows/include-rules-issues-report.txt
- scan-command/src/test/resources/command-outputs/windows/platform-plugin-issue-output.txt
- scan-command/src/test/resources/command-outputs/windows/workspace-issues.txt
- scan-command/src/test/resources/command-outputs/windows/list-rules-output.txt
- scan-command/src/test/resources/command-outputs/windows/exclude-rules-issues-report.txt
- scan-command/src/test/resources/command-outputs/unix/workspace-issues.txt
- scan-command/src/test/java/io/ballerina/scan/internal/ScanCmdTest.java
🚧 Files skipped from review as they are similar to previous changes (5)
- scan-command/src/test/resources/command-outputs/unix/include-exclude-rules.txt
- scan-command/build.gradle
- scan-command/src/test/resources/command-outputs/windows/toml-exclude-rules-issues-report.txt
- scan-command/src/main/java/io/ballerina/scan/utils/ScanUtils.java
- scan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.java
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java (1)
315-334:⚠️ Potential issue | 🟠 MajorPer-member report files are still created during workspace scans.
Lines 317-321 call
saveSarifToDirectory/saveToDirectoryfor every project, including workspace members, before checkingproject.workspaceProject().isEmpty(). This createstarget/report/scan_results.*files under individual packages in addition to the aggregated workspace report, which contradicts the goal of generating a "single report for the entire workspace."The guard at line 323 only suppresses console output, not the file writes.
🛡️ Suggested fix - guard file writes for standalone projects only
- if (project.kind().equals(ProjectKind.BUILD_PROJECT)) { + boolean standaloneBuildProject = + project.kind().equals(ProjectKind.BUILD_PROJECT) && project.workspaceProject().isEmpty(); + if (standaloneBuildProject) { Path reportPath; if (isSarifFormat) { reportPath = ScanUtils.saveSarifToDirectory(issues, project, targetDir); } else { reportPath = ScanUtils.saveToDirectory(issues, project, targetDir); } - - if (project.workspaceProject().isEmpty()) { - outputStream.println(); - outputStream.println("View scan results at:"); - outputStream.println("\t" + reportPath.toUri()); - outputStream.println(); - if (scanReport) { - Path scanReportPath = ScanUtils.generateScanReport(issues, project, targetDir); - outputStream.println(); - outputStream.println("View scan report at:"); - outputStream.println("\t" + scanReportPath.toUri() + System.lineSeparator()); - } - } + outputStream.println(); + outputStream.println("View scan results at:"); + outputStream.println("\t" + reportPath.toUri()); + outputStream.println(); + if (scanReport) { + Path scanReportPath = ScanUtils.generateScanReport(issues, project, targetDir); + outputStream.println(); + outputStream.println("View scan report at:"); + outputStream.println("\t" + scanReportPath.toUri() + System.lineSeparator()); + } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java` around lines 315 - 334, The code currently calls ScanUtils.saveSarifToDirectory / ScanUtils.saveToDirectory for every BUILD_PROJECT (using project.kind() and isSarifFormat) before checking project.workspaceProject().isEmpty(), causing per-member files; modify the logic so that the file-write calls to saveSarifToDirectory/saveToDirectory only run when project.workspaceProject().isEmpty() (i.e., standalone package) and keep the existing console output and scan report generation (ScanUtils.generateScanReport) behavior unchanged for standalone projects; ensure workspace members skip those save* calls so only the aggregated workspace report is produced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scan-command/src/test/java/io/ballerina/scan/internal/ScanCmdTest.java`:
- Around line 364-380: Update the `@Test` description on the
testScanCommandInWorkspace method to accurately reflect that it tests scanning a
workspace (e.g., change text to "test scan command in workspace") and remove the
stray double semicolon after the replace call; locate the method named
testScanCommandInWorkspace and edit the annotation value and delete the extra
';' following the Files.readString(...).replace(...) call.
---
Duplicate comments:
In `@scan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.java`:
- Around line 315-334: The code currently calls ScanUtils.saveSarifToDirectory /
ScanUtils.saveToDirectory for every BUILD_PROJECT (using project.kind() and
isSarifFormat) before checking project.workspaceProject().isEmpty(), causing
per-member files; modify the logic so that the file-write calls to
saveSarifToDirectory/saveToDirectory only run when
project.workspaceProject().isEmpty() (i.e., standalone package) and keep the
existing console output and scan report generation
(ScanUtils.generateScanReport) behavior unchanged for standalone projects;
ensure workspace members skip those save* calls so only the aggregated workspace
report is produced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a922531-adc2-4fc4-81b7-1d4630778ea4
📒 Files selected for processing (33)
scan-command/build.gradlescan-command/src/main/java/io/ballerina/scan/internal/ReporterImpl.javascan-command/src/main/java/io/ballerina/scan/internal/ScanCmd.javascan-command/src/main/java/io/ballerina/scan/utils/ScanUtils.javascan-command/src/test/java/io/ballerina/scan/internal/ScanCmdTest.javascan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.javascan-command/src/test/resources/command-outputs/unix/exclude-rules-issues-report.txtscan-command/src/test/resources/command-outputs/unix/include-exclude-rules.txtscan-command/src/test/resources/command-outputs/unix/include-rules-issues-report.txtscan-command/src/test/resources/command-outputs/unix/invalid-platform-plugin-configurations.txtscan-command/src/test/resources/command-outputs/unix/list-rules-output.txtscan-command/src/test/resources/command-outputs/unix/platform-plugin-issue-output.txtscan-command/src/test/resources/command-outputs/unix/print-rules-to-console.txtscan-command/src/test/resources/command-outputs/unix/scan-toml-invalid-platform-config.txtscan-command/src/test/resources/command-outputs/unix/scan-toml-invalid-platform-jar.txtscan-command/src/test/resources/command-outputs/unix/toml-exclude-rules-issues-report.txtscan-command/src/test/resources/command-outputs/unix/toml-include-exclude-rules.txtscan-command/src/test/resources/command-outputs/unix/toml-include-rules-issues-report.txtscan-command/src/test/resources/command-outputs/unix/workspace-issues.txtscan-command/src/test/resources/command-outputs/windows/exclude-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/include-exclude-rules.txtscan-command/src/test/resources/command-outputs/windows/include-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/invalid-platform-plugin-configurations.txtscan-command/src/test/resources/command-outputs/windows/list-rules-output.txtscan-command/src/test/resources/command-outputs/windows/platform-plugin-issue-output.txtscan-command/src/test/resources/command-outputs/windows/print-rules-to-console.txtscan-command/src/test/resources/command-outputs/windows/scan-toml-invalid-platform-config.txtscan-command/src/test/resources/command-outputs/windows/scan-toml-invalid-platform-jar.txtscan-command/src/test/resources/command-outputs/windows/toml-exclude-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/toml-include-exclude-rules.txtscan-command/src/test/resources/command-outputs/windows/toml-include-rules-issues-report.txtscan-command/src/test/resources/command-outputs/windows/workspace-issues.txtscan-command/tool-scan/Dependencies.toml
✅ Files skipped from review due to trivial changes (26)
- scan-command/src/test/resources/command-outputs/windows/include-exclude-rules.txt
- scan-command/src/test/resources/command-outputs/windows/toml-include-exclude-rules.txt
- scan-command/src/test/resources/command-outputs/unix/invalid-platform-plugin-configurations.txt
- scan-command/src/test/resources/command-outputs/unix/scan-toml-invalid-platform-config.txt
- scan-command/src/test/resources/command-outputs/unix/print-rules-to-console.txt
- scan-command/src/test/resources/command-outputs/windows/print-rules-to-console.txt
- scan-command/src/test/resources/command-outputs/windows/invalid-platform-plugin-configurations.txt
- scan-command/src/test/resources/command-outputs/windows/list-rules-output.txt
- scan-command/src/test/resources/command-outputs/unix/scan-toml-invalid-platform-jar.txt
- scan-command/tool-scan/Dependencies.toml
- scan-command/src/test/resources/command-outputs/windows/toml-include-rules-issues-report.txt
- scan-command/src/test/resources/command-outputs/unix/platform-plugin-issue-output.txt
- scan-command/src/test/resources/command-outputs/unix/list-rules-output.txt
- scan-command/src/main/java/io/ballerina/scan/internal/ReporterImpl.java
- scan-command/src/test/resources/command-outputs/unix/include-rules-issues-report.txt
- scan-command/src/test/resources/command-outputs/unix/toml-include-rules-issues-report.txt
- scan-command/src/test/resources/command-outputs/unix/toml-exclude-rules-issues-report.txt
- scan-command/src/test/resources/command-outputs/windows/platform-plugin-issue-output.txt
- scan-command/src/test/resources/command-outputs/windows/include-rules-issues-report.txt
- scan-command/src/test/resources/command-outputs/unix/exclude-rules-issues-report.txt
- scan-command/src/test/resources/command-outputs/windows/scan-toml-invalid-platform-jar.txt
- scan-command/src/test/resources/command-outputs/windows/workspace-issues.txt
- scan-command/src/test/resources/command-outputs/windows/toml-exclude-rules-issues-report.txt
- scan-command/src/test/resources/command-outputs/unix/workspace-issues.txt
- scan-command/src/test/resources/command-outputs/unix/toml-include-exclude-rules.txt
- scan-command/src/test/resources/command-outputs/windows/exclude-rules-issues-report.txt
🚧 Files skipped from review as they are similar to previous changes (4)
- scan-command/src/test/resources/command-outputs/windows/scan-toml-invalid-platform-config.txt
- scan-command/src/test/resources/command-outputs/unix/include-exclude-rules.txt
- scan-command/src/main/java/io/ballerina/scan/utils/ScanUtils.java
- scan-command/src/test/java/io/ballerina/scan/utils/ScanUtilsTest.java
scan-command/src/test/java/io/ballerina/scan/internal/ScanCmdTest.java
Outdated
Show resolved
Hide resolved
keizer619
previously approved these changes
Apr 9, 2026
...urces/test-resources/workspace-project/bal-project-with-include-rule-configurations/main.bal
Outdated
Show resolved
Hide resolved
...resources/test-resources/workspace-project/bal-project-with-analyzer-configurations/main.bal
Outdated
Show resolved
Hide resolved
Co-authored-by: Tharik Kanaka <tharik.kanaka@gmail.com>
|
keizer619
approved these changes
Apr 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Check List
This PR implements workspace-aware scanning so the scan command can be run from a Ballerina workspace root and produce a single aggregated report for the entire workspace, resolving issue #78 and aligning with workspace support introduced in Ballerina Update 13.
Key changes
Workspace scanning
Project loading and diagnostics
Reporting and utilities
Tests and test resources
Build, CI, and toolchain updates
Reviewer notes