Fix incorrect diagnostic message when reserved keyword is used in module import

[notdulain]

Purpose

When a reserved keyword (e.g., client) is used in a module import path without the ^ escape prefix, the compiler produces misleading diagnostics. For instance, import ballerinax/client.config; results in the error cannot resolve module 'ballerinax/.config as config'. The reserved keyword is dropped from the module path in the diagnostic message because BLangIdentifier.value is empty for unescaped reserved keywords.

Fixes #44509
Fixes #44519

Approach

In BLangImportPackage.getQualifiedPackageName(), updated the package name construction to fall back to originalValue when .value is empty. This ensures that the original source text of the reserved keyword is preserved when formatting the qualified package name for diagnostic messages.

Added a negative test case in ImportsNegativeTests.java to verify that the generated diagnostic includes the reserved keyword correctly stringified in the module path.

Samples

N/A

Remarks

N/A

Check List

• Read the Contributing Guide
• Updated Change Log
• Checked Tooling Support (#)
• Added necessary tests
  • Unit Tests
  • Spec Conformance Tests
  • Integration Tests
  • Ballerina By Example Tests
• Increased Test Coverage
• Added necessary documentation
  • API documentation
  • Module documentation in Module.md files
  • Ballerina By Examples

Overview

This pull request fixes misleading compiler diagnostics that omitted unescaped reserved keywords (for example client) from module import paths shown in error messages. The change ensures the original source text for such identifiers is preserved when the compiler formats qualified package names for diagnostics.

Changes Made

• Compiler: Preserve original identifier text when constructing qualified package names. BLangImportPackage.getQualifiedPackageName() now falls back to an identifier's originalValue when the parsed value is empty. BLangNodeBuilder was also updated to extract reserved-keyword text from parser-provided invalid-token minutiae when the parser synthesizes a missing identifier, ensuring the identifier's originalValue is set.
• Tests: Added a negative unit test (testReservedKeywordInModuleImportDiagnostic) and supporting test resources to verify diagnostics include the reserved keyword in the module path (e.g., ballerinax/client.config as config). Regression test assertions were adjusted to reflect parser-emitted diagnostic ordering and token positions.

Impact

Improves the accuracy and clarity of compiler diagnostics for import errors involving unescaped reserved keywords, making troubleshooting more straightforward for developers.

Related issues: #44509, #44519

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5fd22b4d-08f4-4651-b678-41dbcd06ebb4

📥 Commits

Reviewing files that changed from the base of the PR and between f346fd5 and 57c65d8.

📒 Files selected for processing (2)

• compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/parser/BLangNodeBuilder.java
• tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/imports/ImportsNegativeTests.java

---

📝 Walkthrough

Walkthrough

Updated import name construction and qualified-name rendering to preserve original token text for parser-synthesized/missing identifiers (reserved-keyword cases); added a negative test project and a unit test validating diagnostic ordering and messages for such imports.

Changes

Reserved-keyword import parsing and rendering

Layer / File(s)	Summary
Parser Recovery compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/parser/BLangNodeBuilder.java	transform(ImportDeclarationNode) now checks name.isMissing() and, if missing, uses the first name.leadingInvalidTokens() text to recover the original keyword text when building module name components.
Identifier Representation / Qualified-name compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/tree/BLangImportPackage.java	getQualifiedPackageName() now substitutes id.originalValue when id.value is empty so the computed qualified package string preserves original identifier text for recovered/escaped components.
Tests / Resources tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/imports/ImportsNegativeTests.java, tests/.../ReservedKeywordImportTestProject/*	Added testReservedKeywordInModuleImportDiagnostic() plus Ballerina.toml and reserved-keyword-import-negative.bal to compile a project with import ballerinax/client.config; and assert three ordered diagnostics and total error count.

Sequence Diagram(s)

sequenceDiagram
    participant Parser as Parser (Syntax)
    participant NodeBuilder as BLangNodeBuilder
    participant ImportPkg as BLangImportPackage
    participant Diagnostics as Diagnostics Engine
    participant TestRunner as Unit Test

    Parser-->>NodeBuilder: produce ImportDeclarationNode (with possible missing name token)
    NodeBuilder-->>ImportPkg: construct identifier components (use leadingInvalidTokens when missing)
    ImportPkg-->>Diagnostics: compute qualified package name (use originalValue if value empty)
    TestRunner->>Diagnostics: compile test project and collect diagnostics
    Diagnostics-->>TestRunner: return ordered diagnostics (module-resolve -> parser tokens -> missing identifier)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• [Bug]: Incorrect diagnostics when using reserved keyword in module import #44509: Addresses incorrect rendering/diagnostics for module imports containing reserved keywords by preserving original token text when building qualified names.

Poem

🐰 A dotted path once forced to hide,
I nibbled back tokens that parsers lied,
Originals hop home, diagnostics sing true,
Tests clap their paws — a clearer view! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main change: fixing incorrect diagnostic messages when reserved keywords appear in module imports.
Description check	✅ Passed	The description includes all critical sections: Purpose with issue references, Approach explaining the technical solution, and a checklist with unit tests completed.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.42.1)

compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/parser/BLangNodeBuilder.java

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[notdulain]

can I get a review for this 👀 @MaryamZi @gimantha @sameerajayasoma @hasithaa

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}