[WIP] Add diagnostic range to flow model diagnostics#713
[WIP] Add diagnostic range to flow model diagnostics#713kanushka wants to merge 2 commits intoballerina-platform:1.6.xfrom
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe changes extend the diagnostic system to include location information (LineRange) alongside severity and message. The DiagnosticHandler, Diagnostics model, and corresponding test files are updated to support diagnostic range context throughout the system. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
kanushka
changed the title
kanushka
force-pushed
the
codex/add-diagnostic-range-flow-model
branch
from
f7d0088 to
87735d1
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
flow-model-generator/modules/flow-model-generator-ls-extension/src/test/java/io/ballerina/flowmodelgenerator/extension/ModelGeneratorTest.java (1)
130-196: Duplicate helper methods - same asFlowModelDiagnosticsTest.As noted in the review of
FlowModelDiagnosticsTest.java, these helper methods are duplicated. Consider extracting them to a shared location to maintain DRY principles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/java/io/ballerina/flowmodelgenerator/extension/ModelGeneratorTest.java` around lines 130 - 196, The two helper methods stripDiagnosticRanges and assertDiagnosticsIncludeRange are duplicated from FlowModelDiagnosticsTest; extract them into a shared utility class (e.g., DiagnosticTestUtils) and update ModelGeneratorTest and FlowModelDiagnosticsTest to call DiagnosticTestUtils.stripDiagnosticRanges(...) and DiagnosticTestUtils.assertDiagnosticsIncludeRange(...). Ensure the new utility class is package-visible or public as needed, include the same method signatures and any required imports, and remove the duplicate method definitions from ModelGeneratorTest so both tests reuse the single implementation.flow-model-generator/modules/flow-model-generator-ls-extension/src/test/java/io/ballerina/flowmodelgenerator/extension/FlowModelDiagnosticsTest.java (1)
117-182: Consider extracting duplicate helper methods to the existingTestUtilsclass.Both
stripDiagnosticRanges(lines 117–148) andassertDiagnosticsIncludeRange(lines 150–182) are duplicated inModelGeneratorTest.java. SinceTestUtils.javaalready exists in this test package, these methods should be moved there to reduce duplication and centralize maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/java/io/ballerina/flowmodelgenerator/extension/FlowModelDiagnosticsTest.java` around lines 117 - 182, The two duplicated helper methods stripDiagnosticRanges(JsonElement) and assertDiagnosticsIncludeRange(JsonElement, String) should be moved into the existing TestUtils class and the duplicates removed from FlowModelDiagnosticsTest and ModelGeneratorTest; create corresponding public static methods in TestUtils with the same signatures, update both test classes to call TestUtils.stripDiagnosticRanges(...) and TestUtils.assertDiagnosticsIncludeRange(...)/(or static-import them) instead of their local copies, and run tests to ensure no references remain to the removed local methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/java/io/ballerina/flowmodelgenerator/extension/FlowModelDiagnosticsTest.java`:
- Around line 117-182: The two duplicated helper methods
stripDiagnosticRanges(JsonElement) and
assertDiagnosticsIncludeRange(JsonElement, String) should be moved into the
existing TestUtils class and the duplicates removed from
FlowModelDiagnosticsTest and ModelGeneratorTest; create corresponding public
static methods in TestUtils with the same signatures, update both test classes
to call TestUtils.stripDiagnosticRanges(...) and
TestUtils.assertDiagnosticsIncludeRange(...)/(or static-import them) instead of
their local copies, and run tests to ensure no references remain to the removed
local methods.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/java/io/ballerina/flowmodelgenerator/extension/ModelGeneratorTest.java`:
- Around line 130-196: The two helper methods stripDiagnosticRanges and
assertDiagnosticsIncludeRange are duplicated from FlowModelDiagnosticsTest;
extract them into a shared utility class (e.g., DiagnosticTestUtils) and update
ModelGeneratorTest and FlowModelDiagnosticsTest to call
DiagnosticTestUtils.stripDiagnosticRanges(...) and
DiagnosticTestUtils.assertDiagnosticsIncludeRange(...). Ensure the new utility
class is package-visible or public as needed, include the same method signatures
and any required imports, and remove the duplicate method definitions from
ModelGeneratorTest so both tests reuse the single implementation.
1 participant
Summary
This change adds source code range information to flow-model diagnostics returned by
flowDesignService/getFlowModeland related flow diagnostics responses.Related issue: wso2/product-ballerina-integrator#2447
Problem
Flow model diagnostics previously exposed severity and message, but not the diagnostic location range. Without the range, clients cannot reliably invoke diagnostic-based code actions against the exact compiler-reported location.
Root Cause
DiagnosticHandlerwas only copying severity/message intoDiagnostics.Info. The underlying compiler diagnostic line range was available but not propagated into the serialized flow model diagnostic object.Fix
Diagnostics.Infoto carryrange(LineRange).DiagnosticHandlerto populaterangefromcurrentDiagnostic.location().lineRange().rangeduring equality comparison.getFlowModelrange presence.Validation
Executed focused tests:
./gradlew :flow-model-generator:flow-model-generator-ls-extension:test --tests io.ballerina.flowmodelgenerator.extension.ModelGeneratorTest.testDiagnosticRangesInFlowModel --tests io.ballerina.flowmodelgenerator.extension.FlowModelDiagnosticsTest.testMultipleRequestsBoth passed in this environment.
Summary
This pull request adds source code range information to flow-model diagnostics returned by the flow design service, improving the precision of diagnostic locations provided to users.
Changes
Core Functionality:
Diagnostics.Inforecord to include aLineRangefield for capturing diagnostic location informationDiagnosticHandlerto propagate source code ranges from compiler diagnostics to the diagnostic information structureDiagnostics.Builderto support range parameters when creating diagnostic entriesTesting and Validation:
testDiagnosticRangesInFlowModel) to validate that the flow model generation API correctly returns diagnostics with range informationOutcome
Diagnostic information now includes precise source code location details, enhancing the flow model service's diagnostic feedback to users and enabling better error/warning localization in integrated development environments.