refactor(document links): use Lexer for unified directive argument scanning

[16bit-ykiko]

Summary

• Replace hand-written character scanning in document_links.cpp with the project's Lexer class for finding filename arguments in preprocessor directives
• Extend Lexer to activate header_name mode for #embed/#include_next, and expose set_header_name_mode() for __has_include/__has_embed contexts
• Remove unused Include::filename_range field (had a latent assert crash on macro-expanded includes)
• Add MacroInclude unit test covering #include MACRO scenario

Test plan

• 498 unit tests pass (including new MacroInclude test)
• 119 integration tests pass
• 2/2 smoke tests pass

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Document links now resolve includes written via macros; directive parsing recognizes include, include_next, embed and _has* patterns more reliably using lexer-driven argument detection.

• Refactor

  • Removed an internal filename-range field previously stored for include directives.

• Tests

  • Added unit tests covering directive argument extraction and macro-based include linking.

[coderabbitai]

📝 Walkthrough

Walkthrough

Removed storing include filename source ranges from internal Include records; added lexer-driven directive-argument discovery and a lexer header-name mode; document link discovery now uses the lexer; updated directive handling; added tests for macro/includes/__/embed argument detection.

Changes

Cohort / File(s)	Summary
Directive struct & collector src/compile/directive.h, src/compile/directive.cpp	Removed Include::filename_range and stopped populating it; clarified Include::location docs; adjusted DirectiveCollector::InclusionDirective signature/usage accordingly.
Lexer API & logic src/syntax/lexer.h, src/syntax/lexer.cpp	Added Lexer::set_header_name_mode() and find_directive_argument(...); expanded directive keyword handling to include include_next, embed, and __has_* forms; implemented scanning for the first filename-like argument token.
Document links feature src/feature/document_links.cpp	Replaced ad-hoc substring scanning with lexer-based find_directive_argument, threaded LangOptions into detection, renamed/refactored helpers to use lexer-derived ranges and early-return when range detection fails.
Tests tests/unit/feature/document_link_tests.cpp, tests/unit/syntax/lexer_tests.cpp	Added TEST_CASE(MacroInclude) and DirectiveArgument tests to cover quoted/angled/macro include detection, __has_include/__has_embed, #embed, and edge cases (empty directive, multi-line offsets).

Sequence Diagram

sequenceDiagram
    participant DL as DocumentLink Handler
    participant KW as Directive Classifier
    participant LXR as Lexer
    participant FS as File Resolver

    DL->>KW: examine preprocessor token
    KW-->>DL: classify (include / include_next / embed / __has_*)
    alt recognized directive
        DL->>LXR: call find_directive_argument(content, offset, LangOptions)
        alt requires header-name mode
            DL->>LXR: set_header_name_mode()
        end
        LXR-->>DL: return LocalSourceRange (or none)
        alt range found
            DL->>FS: resolve filename -> target path
            DL->>DL: create DocumentLink(range, target)
        else no range
            DL->>DL: skip link creation
        end
    else not a directive
        DL->>DL: skip
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat(document links): preserve PCH document links and add #embed support #413: Overlaps document-link/lexer changes — both adjust how filename ranges for includes/embeds are discovered and move toward lexer-based extraction.

Poem

🐇 I hopped through tokens, twitched my nose,
Searched for quotes where the header shows.
Lexer lit the hidden trail,
Macro crumbs became a trail.
Joyful hop — link found, off I goes! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.45% 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 accurately and concisely summarizes the main change: refactoring document links to use the Lexer for directive argument scanning instead of custom character scanning.

_{✏️ 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 refactor/document-links-lexer

---

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/document_links.cpp`:
- Around line 44-45: The Lexer is being created without the translation unit's
language options, causing dialect-sensitive tokenization mismatches; update the
Lexer construction in document_links.cpp from using the default opts to pass the
TU's options by calling Lexer with the second parameter &unit.lang_options()
(i.e., replace the current Lexer lexer(line) with a constructor that forwards
unit.lang_options()), ensuring the lambda's capture of unit is used so re-lexing
respects the original clang::LangOptions.

🪄 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: 98378ec5-3b87-4273-a8ed-9d771acf2d09

📥 Commits

Reviewing files that changed from the base of the PR and between 8bafaa8 and 46d37a8.

📒 Files selected for processing (6)

• src/compile/directive.cpp
• src/compile/directive.h
• src/feature/document_links.cpp
• src/syntax/lexer.cpp
• src/syntax/lexer.h
• tests/unit/feature/document_link_tests.cpp

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/unit/syntax/lexer_tests.cpp (1)

87-151: Add regressions for the new include_next branches.

src/syntax/lexer.cpp now has dedicated handling for both include_next and __has_include_next, but this suite never exercises either path. A small quoted/angled case for each would keep the newly added branches from drifting.

🤖 Prompt for AI Agents

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

In `@tests/unit/syntax/lexer_tests.cpp` around lines 87 - 151, Add unit tests
exercising the newly added include_next branches by adding four cases analogous
to the existing ones: an IncludeNextQuoted and IncludeNextAngled that call
EXPECT_RANGE(src, 0, R"("...")") / "<...>" on "#include_next \"foo.h\"" and
"#include_next <iostream>", and HasIncludeNextQuoted and HasIncludeNextAngled
that locate "__has_include_next" in "#if __has_include_next(\"foo.h\")" and "#if
__has_include_next(<vector>)" and call EXPECT_RANGE with the found offset; use
the same helper EXPECT_RANGE/find_directive_argument pattern as the other tests
so the new branches in find_directive_argument are exercised.

🤖 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/syntax/lexer.cpp`:
- Around line 125-155: The loop currently recognizes __has_* identifiers and the
following '(' before checking abs_begin < offset, causing argument matches even
when offset is at the directive start; to fix, move the "if(abs_begin < offset)
continue;" check to immediately after abs_begin/abs_end are computed and before
any logic that sets or tests after_has_keyword (i.e. before the
tok.is_identifier() block and before the tok.kind == clang::tok::l_paren check),
so that you only set after_has_keyword, call lexer.set_header_name_mode(), or
return a LocalSourceRange for header/identifier tokens when the token lies at or
after the provided offset; keep the rest of the conditions
(tok.is_header_name(), tok.kind == string_literal, is_directive_keyword check)
unchanged.

---

Nitpick comments:
In `@tests/unit/syntax/lexer_tests.cpp`:
- Around line 87-151: Add unit tests exercising the newly added include_next
branches by adding four cases analogous to the existing ones: an
IncludeNextQuoted and IncludeNextAngled that call EXPECT_RANGE(src, 0,
R"("...")") / "<...>" on "#include_next \"foo.h\"" and "#include_next
<iostream>", and HasIncludeNextQuoted and HasIncludeNextAngled that locate
"__has_include_next" in "#if __has_include_next(\"foo.h\")" and "#if
__has_include_next(<vector>)" and call EXPECT_RANGE with the found offset; use
the same helper EXPECT_RANGE/find_directive_argument pattern as the other tests
so the new branches in find_directive_argument are exercised.

🪄 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: 1a7fec79-5f0b-4df9-a9d3-f70e20c5545e

📥 Commits

Reviewing files that changed from the base of the PR and between e487d1d and 31e8c26.

📒 Files selected for processing (7)

• src/compile/directive.cpp
• src/compile/directive.h
• src/feature/document_links.cpp
• src/syntax/lexer.cpp
• src/syntax/lexer.h
• tests/unit/feature/document_link_tests.cpp
• tests/unit/syntax/lexer_tests.cpp

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

• tests/unit/feature/document_link_tests.cpp
• src/feature/document_links.cpp

[coderabbitai]

🧹 Nitpick comments (1)

src/syntax/lexer.cpp (1)

134-143: Consider centralizing the directive keyword checks.

The include/embed keyword list is now hard-coded here and also in Lexer::lex(). Pulling that into a shared helper would reduce drift the next time another directive variant is added.

🤖 Prompt for AI Agents

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

In `@src/syntax/lexer.cpp` around lines 134 - 143, This code duplicates directive
keyword checks (the hard-coded lists in lexer.cpp and in Lexer::lex()); extract
the include/embed keyword set into a shared helper (e.g., IsDirectiveKeyword or
a static const container) and use it here instead of comparing text against
"__has_include", "__has_include_next", "__has_embed", "include", "include_next",
"embed"; update the logic that sets after_has_keyword and ready to call that
helper (referencing tok.is_identifier(), tok.text(line), after_has_keyword,
ready, and Lexer::lex() so both places use the same helper) to avoid future
drift.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@src/syntax/lexer.cpp`:
- Around line 134-143: This code duplicates directive keyword checks (the
hard-coded lists in lexer.cpp and in Lexer::lex()); extract the include/embed
keyword set into a shared helper (e.g., IsDirectiveKeyword or a static const
container) and use it here instead of comparing text against "__has_include",
"__has_include_next", "__has_embed", "include", "include_next", "embed"; update
the logic that sets after_has_keyword and ready to call that helper (referencing
tok.is_identifier(), tok.text(line), after_has_keyword, ready, and Lexer::lex()
so both places use the same helper) to avoid future drift.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0627da46-bec5-42d2-a9d6-b9e65d416eca

📥 Commits

Reviewing files that changed from the base of the PR and between 31e8c26 and 9583294.

📒 Files selected for processing (7)

• src/compile/directive.cpp
• src/compile/directive.h
• src/feature/document_links.cpp
• src/syntax/lexer.cpp
• src/syntax/lexer.h
• tests/unit/feature/document_link_tests.cpp
• tests/unit/syntax/lexer_tests.cpp

✅ Files skipped from review due to trivial changes (2)

• src/compile/directive.h
• tests/unit/syntax/lexer_tests.cpp

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

• tests/unit/feature/document_link_tests.cpp
• src/syntax/lexer.h