Skip to content

Fix final-word lookup without source offsets - #2178

Open
Don-Yin wants to merge 1 commit into
postcss:mainfrom
Don-Yin:fix-final-word-without-offsets
Open

Don-Yin wants to merge 1 commit into
postcss:mainfrom
Don-Yin:fix-final-word-without-offsets

Conversation

@Don-Yin

@Don-Yin Don-Yin commented Oct 23, 2026 •

Copy link
Copy Markdown

When a custom syntax provides only line/column positions, a word ending at the node's last character is not found. positionBy() falls back to the node start, and rangeBy() makes errors and warnings point there too.

For example, with postcss-sass@0.5.0:

let sass = require('postcss-sass')
let decl = sass.parse('a\n  color: red').first.first

decl.positionBy({ word: 'red' }).column // 3, expected 10
decl.rangeBy({ word: 'red' }).start.column // 3, expected 10
decl.error('Bad color', { word: 'red' }).column // 3, expected 10

The end line/column is inclusive, but slice() needs an exclusive end. This reuses the end offset already normalized by rangeBy() for both word searches, without adding another conversion rule.

The existing missing-offset tests now also check a word at the node end. Both fail before the change and pass afterward. pnpm test passes all 721 tests, coverage, lint, types, version, integration, and size checks.

The measured columns for the example above:

Reported start columns before and after the fix

Reuse the exclusive end offset already calculated by rangeBy() when
searching for a word in nodes from custom syntaxes. A line/column end
position is inclusive and otherwise leaves out the last character.

Extend the existing missing-offset tests with a word at the node end.

Signed-off-by: Don Yin <Don_Yin@outlook.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cfe990b2-726e-460a-80ec-28ef926944ec
📥 Commits

Reviewing files that changed from the base of the PR and between d190857 and 7a4ca82.

📒 Files selected for processing: 2
  • lib/node.js
  • test/node.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Node.positionBy({ word }) and Node.rangeBy({ word }) now search through the computed exclusive end offset. Tests cover lookup of X when source start and end offsets are missing.

Changes: Word lookup offsets

Layer / File(s) Summary
Use exclusive end offsets for word lookup
lib/node.js, test/node.test.ts
Both word lookup methods now use the computed exclusive end offset. Tests check X lookups when source boundary offsets are missing.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested reviewers: romainmenke, mahirhir

Merge Risk: ⚪ Minimal · up to 7a4ca

No actionable merge-blocking risk is identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: fixing final-word lookup when source offsets are missing.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
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.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant