Skip to content

fix: Eliminate unsafe manual token position updates in CSS parser#147

Merged
bartveneman merged 2 commits intomainfrom
fix/unsafe-os-tracking
Feb 26, 2026
Merged

fix: Eliminate unsafe manual token position updates in CSS parser#147
bartveneman merged 2 commits intomainfrom
fix/unsafe-os-tracking

Conversation

@bartveneman
Copy link
Member

Fixes position state desynchronization bugs and consolidates position
management into safe, consistent APIs.

Changes:

  1. Add Lexer.skip_whitespace_in_range() method

    • Properly maintains pos/line/column state during whitespace
      skipping
    • Centralizes whitespace/comment skipping logic in lexer
    • Eliminates code duplication across parsers
  2. Replace manual backtracking with save_position/restore_position API

    • parse-selector.ts: Fix try_parse_combinator() - eliminate 9 lines
      of
      manual pos/line/column restoration
    • parse-selector.ts: Fix namespace parsing fallback paths in
      parse_type_or_namespace_selector() and
      parse_universal_or_namespace_selector()
    • parse-atrule-prelude.ts: Fix parse_single_media_query() modifier
      backtracking
  3. Fix unsafe whitespace skipping methods

    • parse-atrule-prelude.ts: Replace skip_whitespace() to use
      lexer.skip_whitespace_in_range() instead of utility that only
      updates pos
    • parse-anplusb.ts: Same fix for skip_whitespace()
  4. Fix unsafe position-only assignments in sub-range parsing

    • parse-atrule-prelude.ts: Fix parse_feature_value() to use
      temporary
      lexer instead of corrupting main lexer position state
    • parse-selector.ts: Fix parse_lang_identifiers() same issue

Impact:

  • Fixes 4 bugs where line/column could become desynchronized from pos
  • Eliminates ~30 lines of duplicated manual state management
  • Makes backtracking impossible to implement incorrectly
  • All 1162 tests passing

@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2026

Bundle Report

Changes will increase total bundle size by 751 bytes (0.51%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
@projectwallace/css-parser-esm 146.7kB 751 bytes (0.51%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: @projectwallace/css-parser-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
parse-*.js 137 bytes 22.98kB 0.6%
parse-*.js -124 bytes 26.07kB -0.47%
parse-*.js -120 bytes 8.5kB -1.39%
tokenize.js 858 bytes 18.5kB 4.86%

Files in parse-*.js:

  • ./src/parse-atrule-prelude.ts → Total Size: 22.17kB

Files in parse-*.js:

  • ./src/parse-selector.ts → Total Size: 24.68kB

Files in parse-*.js:

  • ./src/parse-anplusb.ts → Total Size: 8.13kB

Files in tokenize.js:

  • ./src/tokenize.ts → Total Size: 17.89kB

@bartveneman bartveneman merged commit 493ae84 into main Feb 26, 2026
5 checks passed
@bartveneman bartveneman deleted the fix/unsafe-os-tracking branch February 26, 2026 18:48
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 95.55556% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.96%. Comparing base (7c8b17e) to head (4968022).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/parse-selector.ts 87.50% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #147      +/-   ##
==========================================
+ Coverage   94.85%   94.96%   +0.10%     
==========================================
  Files          16       16              
  Lines        2704     2721      +17     
  Branches      709      712       +3     
==========================================
+ Hits         2565     2584      +19     
+ Misses        139      137       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants