Sync main with 1.6.x branch#711
Conversation
Fix worker name duplication not identified in diagnostics
Add lang lib related function support
📝 WalkthroughWalkthroughThis PR introduces type parameter replacement logic for Ballerina generic language functions. A new Changes
Sequence Diagram(s)sequenceDiagram
participant FC as FunctionCall/<br/>MethodCall
participant CB as CallBuilder
participant SM as Semantic Model
participant FS as FileSystem
FC->>CB: resolveLangLibReturnType(workspaceManager,<br/>filePath, flowNode)
CB->>CB: isLangLibFunction()
alt Is Lang Lib Function
CB->>SM: Find return type template<br/>and placeholders
CB->>CB: For each placeholder
CB->>SM: Lookup parameter from flowNode
CB->>FS: Resolve concrete type via<br/>semantic model
CB->>CB: Replace placeholder with<br/>concrete type
CB-->>FC: Return resolved return type
else Not Lang Lib
CB-->>FC: Return null
end
alt Resolved Type Available
FC->>FC: newVariableWithType(<br/>resolvedReturnType)
else Fallback
FC->>FC: newVariableWithInferredType()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/FormBuilder.java (1)
220-232:⚠️ Potential issue | 🟡 MinorGuard against null
typeNamein the hidden-flag check.
replaceTypeParameters(null)returns null, soreplacedTypeName.equals(typeName)can throw. UseObjects.equals(or default to"") to keep this method null-safe.🛠️ Suggested fix
+import java.util.Objects;- String replacedTypeName = TypeParameterReplacer.replaceTypeParameters(typeName); + String replacedTypeName = TypeParameterReplacer.replaceTypeParameters(typeName); @@ - .hidden(hidden || !replacedTypeName.equals(typeName)) + .hidden(hidden || !Objects.equals(replacedTypeName, typeName))🤖 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-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/FormBuilder.java` around lines 220 - 232, The hidden flag check in FormBuilder uses replacedTypeName.equals(typeName) which can NPE when typeName is null since TypeParameterReplacer.replaceTypeParameters(typeName) may return null; update the hidden() call to use a null-safe comparison such as Objects.equals(replacedTypeName, typeName) (or default typeName to "" before comparing) so the expression hidden(hidden || !Objects.equals(replacedTypeName, typeName)) is null-safe; reference the FormBuilder code path around TypeParameterReplacer.replaceTypeParameters(typeName) and the propertyBuilder.hidden(...) invocation to locate where to change.
🧹 Nitpick comments (3)
flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/to_source/source/function_call/Ballerina.toml (1)
1-5: Package name may not align with test purpose.The package name
new_connection1doesn't match the directory namefunction_call. If this is intentional for test isolation, it's fine; otherwise, consider renaming for clarity.🤖 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/resources/to_source/source/function_call/Ballerina.toml` around lines 1 - 5, The package name in Ballerina.toml is set to "new_connection1" which does not match the test directory "function_call"; change the package name value (org/name/version block) in Ballerina.toml so the name field matches the intended test/module name (e.g., "function_call") or, if the mismatch is intentional for isolation, add a short inline comment in the test manifest explaining why "new_connection1" is used to avoid confusion; update the name property only (leave org/version/distribution unchanged) and ensure any test imports or references use the new name.flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/to_source/source/function_call-langlib.bal (1)
1-1: Unused import:ballerina/lang.array.The
lang.arraymodule is imported but no array lang-lib functions are called in this file. If this is scaffolding for test modifications that will add lang-lib calls, this is fine; otherwise, consider removing the unused import.🤖 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/resources/to_source/source/function_call-langlib.bal` at line 1, The import statement "import ballerina/lang.array;" is unused in this test source; either remove the unused import from the top of the file or add a lang.array usage to justify it — edit the file to delete the "import ballerina/lang.array;" line (or add a call to an array lang-lib function where appropriate) so the import no longer triggers the unused-import warning.flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/to_source/config/function_call-lang-lib_5.json (1)
87-87: Consider a more descriptive variable name.The variable name
intArrayTypeis slightly misleading since the actual type is[int, CSV][](an array of tuples), not an integer array. A name likeenumeratedRecordsorindexedRecordswould better convey the enumerate result's semantics.🤖 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/resources/to_source/config/function_call-lang-lib_5.json` at line 87, The variable name intArrayType is misleading because it represents an array of tuples ([int, CSV][]) — rename intArrayType to a more descriptive identifier such as enumeratedRecords (or indexedRecords) and update all references; specifically change the "value" entry currently set to "intArrayType" to "enumeratedRecords" and adjust any tests, fixtures or code that reference intArrayType (e.g., places that parse or assert against that variable name) to the new identifier so names remain consistent across the test resource and related code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/node/CallBuilder.java`:
- Around line 281-288: The code assumes
flowNode.getProperty(Property.TYPE_KEY).get().value() and
flowNode.codedata().lineRange() are non-null which can cause NPEs; update
CallBuilder to defensively check both the property's value and the node's
lineRange before using them (e.g., verify typeProperty.isPresent() &&
typeProperty.get().value() != null, and that flowNode.codedata() != null &&
flowNode.codedata().lineRange() != null) and bail out (return null) when either
is missing; apply the same null-guard pattern to the other occurrence around the
code handling lineRange at the later block (the one referenced at 345-346) so
both typeName extraction and lineRange usage are safe.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/diagnostics/config/worker.json`:
- Around line 28-37: The codedata line range in the diagnostics config
references the wrong file: update the codedata.lineRange.fileName value
(currently "functions.bal") to match the test context ("worker.bal") so file
path resolution for diagnostics is correct; locate the
codedata.lineRange.fileName key in the JSON (in the diagnostics config for this
test) and change its string to "worker.bal".
---
Outside diff comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/FormBuilder.java`:
- Around line 220-232: The hidden flag check in FormBuilder uses
replacedTypeName.equals(typeName) which can NPE when typeName is null since
TypeParameterReplacer.replaceTypeParameters(typeName) may return null; update
the hidden() call to use a null-safe comparison such as
Objects.equals(replacedTypeName, typeName) (or default typeName to "" before
comparing) so the expression hidden(hidden || !Objects.equals(replacedTypeName,
typeName)) is null-safe; reference the FormBuilder code path around
TypeParameterReplacer.replaceTypeParameters(typeName) and the
propertyBuilder.hidden(...) invocation to locate where to change.
---
Nitpick comments:
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/to_source/config/function_call-lang-lib_5.json`:
- Line 87: The variable name intArrayType is misleading because it represents an
array of tuples ([int, CSV][]) — rename intArrayType to a more descriptive
identifier such as enumeratedRecords (or indexedRecords) and update all
references; specifically change the "value" entry currently set to
"intArrayType" to "enumeratedRecords" and adjust any tests, fixtures or code
that reference intArrayType (e.g., places that parse or assert against that
variable name) to the new identifier so names remain consistent across the test
resource and related code.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/to_source/source/function_call-langlib.bal`:
- Line 1: The import statement "import ballerina/lang.array;" is unused in this
test source; either remove the unused import from the top of the file or add a
lang.array usage to justify it — edit the file to delete the "import
ballerina/lang.array;" line (or add a call to an array lang-lib function where
appropriate) so the import no longer triggers the unused-import warning.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/to_source/source/function_call/Ballerina.toml`:
- Around line 1-5: The package name in Ballerina.toml is set to
"new_connection1" which does not match the test directory "function_call";
change the package name value (org/name/version block) in Ballerina.toml so the
name field matches the intended test/module name (e.g., "function_call") or, if
the mismatch is intentional for isolation, add a short inline comment in the
test manifest explaining why "new_connection1" is used to avoid confusion;
update the name property only (leave org/version/distribution unchanged) and
ensure any test imports or references use the new name.
...enerator-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/node/CallBuilder.java
Show resolved
Hide resolved
| "lineRange": { | ||
| "fileName": "functions.bal", | ||
| "startLine": { | ||
| "line": 4, | ||
| "offset": 15 | ||
| }, | ||
| "endLine": { | ||
| "line": 4, | ||
| "offset": 22 | ||
| } |
There was a problem hiding this comment.
Align codedata.lineRange.fileName with the test file.
The context targets worker.bal, but the codedata line range points at functions.bal, which can mis-resolve file paths during diagnostics. Consider keeping them consistent.
🔧 Proposed fix
- "fileName": "functions.bal",
+ "fileName": "worker.bal",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "lineRange": { | |
| "fileName": "functions.bal", | |
| "startLine": { | |
| "line": 4, | |
| "offset": 15 | |
| }, | |
| "endLine": { | |
| "line": 4, | |
| "offset": 22 | |
| } | |
| "lineRange": { | |
| "fileName": "worker.bal", | |
| "startLine": { | |
| "line": 4, | |
| "offset": 15 | |
| }, | |
| "endLine": { | |
| "line": 4, | |
| "offset": 22 | |
| } |
🤖 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/resources/diagnostics/config/worker.json`
around lines 28 - 37, The codedata line range in the diagnostics config
references the wrong file: update the codedata.lineRange.fileName value
(currently "functions.bal") to match the test context ("worker.bal") so file
path resolution for diagnostics is correct; locate the
codedata.lineRange.fileName key in the JSON (in the diagnostics config for this
test) and change its string to "worker.bal".
There was a problem hiding this comment.
Pull request overview
This PR syncs main with the 1.6.x branch by updating flow-model generation to better handle Ballerina langlib type parameters (and adding/adjusting related test resources).
Changes:
- Add
TypeParameterReplacerand apply it when building property type metadata (replacing langlib type parameters with broader supertypes). - Enhance function/method call source generation to optionally emit an explicit resolved return type for langlib calls.
- Update/extend LS-extension test resources (new langlib “to_source” fixtures, updated node templates, and a new worker diagnostics scenario).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/to_source/source/function_call/main.bal | Adds langlib “to_source” test source fixture. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/to_source/source/function_call/Ballerina.toml | Adds Ballerina package metadata for the new test fixture. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/to_source/source/function_call-langlib.bal | Adds a second langlib “to_source” source fixture. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/to_source/config/function_call-lang-lib_5.json | Adds expected edit output for a langlib enumerate call insertion. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/node_template/config/lang_lib-table-reduce.json | Updates templates to use replaced type-parameter supertypes and hides result type. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/node_template/config/lang_lib-stream-reduce.json | Same as above for stream reduce. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/node_template/config/lang_lib-map-reduce.json | Same as above for map reduce. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/node_template/config/lang_lib-error-detail.json | Same as above for error detail. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/node_template/config/lang_lib-array-reduce.json | Same as above for array reduce. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/node_template/config/lang_lib-array-map.json | Same as above for array map. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/node_template/config/lang_lib-array-lastIndexOf.json | Updates array lastIndexOf template types. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/diagnostics/source/worker.bal | Adds a worker-based diagnostics fixture source. |
| flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/diagnostics/config/worker.json | Adds expected diagnostics config for duplicate worker name scenario. |
| flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/node/MethodCall.java | Emits explicit resolved return type when available (langlib calls). |
| flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/node/FunctionCall.java | Emits explicit resolved return type when available (langlib calls). |
| flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/node/CallBuilder.java | Implements langlib return-type resolution via semantic model lookup. |
| flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/SourceBuilder.java | Adds newVariableWithType(...) helper for explicit type declarations. |
| flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/Property.java | Applies type-parameter replacement when setting ballerinaType metadata. |
| flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/model/FormBuilder.java | Replaces type-parameter placeholders in type/return-type property values and auto-hides them. |
| flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/expressioneditor/services/IdentifierDiagnosticsRequest.java | Extends redeclared-symbol detection to include worker symbols. |
| flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/TypeParameterReplacer.java | New utility for replacing langlib type parameter strings with supertypes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Purpose
$subject.
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary
This pull request synchronizes the main branch with the 1.6.x branch and introduces improvements to type handling and worker symbol detection in the flow model generator.
Key Changes
Type Parameter Resolution
TypeParameterReplacerutility class that centralizes replacements for generic type parameters across array, error, map, stream, xml, table, and value constructsLanguage Library Function Support
FunctionCallandMethodCallclasses to conditionally use resolved return types when available, falling back to inferred types otherwisenewVariableWithType(String resolvedType)method toSourceBuilderfor explicit type declarationsWorker Symbol Handling
Type Annotation Updates
(any|error)unions instead of specific placeholder typesTest Coverage
Files Modified
Approximately 24 files modified with 440+ lines added, including core logic changes, test configurations, and test resources.