feat(semantic tokens): highlight module names in declarations and imports

[16bit-ykiko]

Summary

• Highlight module name identifiers (e.g. foo, bar in export module foo.bar;) as SymbolKind::Module in semantic tokens
• Highlight import module names (e.g. foo in import foo;) using directives.imports name locations
• Module declarations use getCurrentNamedModule()->DefinitionLoc + lexer scan to find name tokens

Test plan

• SemanticTokens.ModuleDeclaration — export module foo;
• SemanticTokens.ModuleDeclarationDotted — export module foo.bar;
• SemanticTokens.ModuleImport — PCM build + import foo;
• All 16 SemanticTokens tests pass, no regressions

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Enhanced semantic token support for C++20 modules, including dotted module names, partitions, fragments, imports and re-exports for more accurate highlighting.

• Bug Fixes

  • Improved conflict resolution so directive tokens no longer mask other semantic kinds; ensures module/import used as identifiers are tokenized correctly.

• Tests

  • Added unit tests covering module declarations, imports, partitions and edge cases.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new module-highlighting pass into the semantic token collection pipeline that emits module/import tokens and changes directive-vs-other-kind merge rules; removes commented module-parsing in the visitor; and extends unit tests to validate C++20 module tokenization.

Changes

Cohort / File(s)	Summary
Module Token Emission src/feature/semantic_tokens.cpp	Adds highlight_modules() invoked between run() and merge_tokens() to emit SymbolKind::Keyword/SymbolKind::Module tokens for import directives and current module declarations; adjusts token-merge conflict resolution so Directive no longer always wins. Adds #include <clang/Basic/Module.h>.
Semantic Visitor Cleanup src/semantic/semantic_visitor.h	Removes the previously commented-out/manual token-buffer-based module parsing block from SemanticVisitor::run(), deferring module handling to the collector.
Module Tokenization Tests tests/unit/feature/semantic_tokens_tests.cpp	Adds multiple tests covering exported module declarations (including dotted names and partitions), global/private fragments, imports/re-exports via generated PCM module interfaces, and cases ensuring module/import used as identifiers are tokenized as identifiers. Temporary PCM artifacts cleaned up in tests.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Collector as SemanticTokensCollector
    participant Visitor as SemanticVisitor
    participant DirStore as DirectiveStore (module imports)
    participant Lexer as Lexer (from DefinitionLoc)
    participant Merger as TokenMerger

    Collector->>Visitor: run() (gathers tokens & directives)
    Visitor-->>Collector: tokens + directive records
    Collector->>DirStore: query import directives & locations
    DirStore-->>Collector: import name locations
    Collector->>Lexer: lex from module DefinitionLoc (skip 'module')
    Lexer-->>Collector: identifiers and separators
    Collector->>Collector: emit Keyword/Module tokens for imports & declaration
    Collector->>Merger: merge_tokens(incoming tokens, existing)
    Merger-->>Collector: merged token set (directives no longer always win)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat: classify dependent-name semantic token modifier #387: Related work that adjusted modifier handling and added scaffolding for module occurrence handling used by semantic token emission.

Poem

🐰 I hopped through tokens, bright and new,
I sniffed module names and keywords too,
Old scraps removed, a cleaner trail,
Now imports and modules tell their tale,
I dance on code — a tiny review! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: adding semantic token highlighting for module names in both module declarations and import statements.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/semantic-tokens-module

---

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

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/feature/semantic_tokens.cpp`:
- Around line 333-341: The loop in semantic_tokens.cpp currently tags every
identifier before the semicolon as SymbolKind::Module; change it to only mark
the first identifier (the actual module name) encountered after entering the
loop. Before the while(true) loop introduce a bool like seenModuleName = false,
and inside the loop replace the unconditional token.is_identifier() branch with
a check if(token.is_identifier() && !seenModuleName) then create the
LocalSourceRange and push the Module token and set seenModuleName = true; leave
other tokens unmarked so trailing attributes/identifiers are not highlighted.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 99cdf916-d2c2-4188-ba62-5ee210c58747

📥 Commits

Reviewing files that changed from the base of the PR and between 3dab2ea and 48f208c.

📒 Files selected for processing (3)

• src/feature/semantic_tokens.cpp
• src/semantic/semantic_visitor.h
• tests/unit/feature/semantic_tokens_tests.cpp

[coderabbitai]

🧹 Nitpick comments (1)

tests/unit/feature/semantic_tokens_tests.cpp (1)

451-488: Extract the temporary PCM setup into a helper/guard.

These two tests duplicate the same module build plumbing and manual fs::remove() cleanup. That makes the cases harder to maintain, and any ASSERT_* before the final remove() leaks the temp file.

Refactor sketch

+struct TempPcm {
+    std::string path;
+    ~TempPcm() { fs::remove(path); }
+};
+
+auto build_test_pcm(llvm::StringRef module_name, llvm::StringRef source) -> TempPcm {
+    auto pcm_path = fs::createTemporaryFile("test-mod", "pcm");
+    ASSERT_TRUE(pcm_path.has_value());
+
+    Tester mod;
+    mod.add_main((std::string(module_name) + ".cppm").c_str(), std::string(source));
+    mod.prepare("-std=c++20");
+    mod.params.kind = CompilationKind::ModuleInterface;
+    mod.params.output_file = *pcm_path;
+
+    auto built = clice::compile(mod.params);
+    ASSERT_TRUE(built.completed());
+    return TempPcm{*pcm_path};
+}

Also applies to: 502-540

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/unit/feature/semantic_tokens_tests.cpp` around lines 451 - 488, Extract
the temporary PCM creation, building and cleanup into a reusable helper/RAII
guard so tests no longer duplicate plumbing and can't leak files on early ASSERT
failures: move the logic that calls fs::createTemporaryFile("test-mod", "pcm"),
constructs the Tester mod, sets mod.params.kind/ output_file, calls
clice::compile(mod.params) and asserts completion into a helper like
MakeBuiltPCM or PCMGuard that returns the pcm path and removes the file in its
destructor; update TEST_CASE(ModuleImport) to call that helper to obtain the pcm
path, use it for -fmodule-file, and remove the explicit fs::remove(*pcm_path) so
cleanup is automatic and safe on failures.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/unit/feature/semantic_tokens_tests.cpp`:
- Around line 451-488: Extract the temporary PCM creation, building and cleanup
into a reusable helper/RAII guard so tests no longer duplicate plumbing and
can't leak files on early ASSERT failures: move the logic that calls
fs::createTemporaryFile("test-mod", "pcm"), constructs the Tester mod, sets
mod.params.kind/ output_file, calls clice::compile(mod.params) and asserts
completion into a helper like MakeBuiltPCM or PCMGuard that returns the pcm path
and removes the file in its destructor; update TEST_CASE(ModuleImport) to call
that helper to obtain the pcm path, use it for -fmodule-file, and remove the
explicit fs::remove(*pcm_path) so cleanup is automatic and safe on failures.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 301ea7e9-98d3-4cbe-a67e-242f42868453

📥 Commits

Reviewing files that changed from the base of the PR and between 48f208c and d28e8bc.

📒 Files selected for processing (3)

• src/feature/semantic_tokens.cpp
• src/semantic/semantic_visitor.h
• tests/unit/feature/semantic_tokens_tests.cpp

💤 Files with no reviewable changes (1)

• src/semantic/semantic_visitor.h

🚧 Files skipped from review as they are similar to previous changes (1)

• src/feature/semantic_tokens.cpp