Skip to content

Switch buffer from bytes to characters for proper multibyte handling#100

Open
tk0miya wants to merge 2 commits into
mainfrom
claude/fix-multibyte-linter-position-N4TsJ
Open

Switch buffer from bytes to characters for proper multibyte handling#100
tk0miya wants to merge 2 commits into
mainfrom
claude/fix-multibyte-linter-position-N4TsJ

Conversation

@tk0miya

@tk0miya tk0miya commented Jan 25, 2026

Copy link
Copy Markdown
Owner

Summary

This PR refactors the code rendering system to work with character positions instead of byte positions. This fixes incorrect handling of multibyte characters (e.g., emoji, CJK characters) in ERB templates.

Key Changes

  • Buffer representation: Changed from Array[Integer] (bytes) to Array[String] (characters)

    • Simplifies position calculations and eliminates byte/character conversion complexity
    • Final code generation now uses buffer.join instead of pack("C*")
  • Character constants: Added string equivalents for all byte constants in Characters module

    • CHAR_LF, CHAR_CR, CHAR_SPACE, CHAR_HASH, CHAR_SEMICOLON, CHAR_EQUALS, CHAR_UNDERSCORE, CHAR_LEFT_BRACE, CHAR_RIGHT_BRACE
    • Enables direct character manipulation without byte conversion
  • Position conversion: Implemented byte_to_char_pos method in Source class

    • Converts Herb's byte-based positions to character positions for RuboCop compatibility
    • Delegated through ParseResult and RubyRenderer
  • Converter refactoring: Updated generate_hybrid_code to work with character arrays

    • Uses byte_to_char_pos to convert positions before buffer manipulation
    • Properly handles multibyte character restoration
  • RubyRenderer updates: All position calculations now use character positions

    • render_code_node, render_output_marker, render_open_tag_node, render_close_tag_node, render_text_node, render_erb_comment_node, render_html_comment_node, render_tag_marker all updated
    • bleach_code now returns Array[String] and uses character-based logic
  • Test updates: Fixed test expectations to use character counts instead of byte counts

    • Multibyte character tests now show correct spacing (e.g., "日本語" is 3 characters, not 9 bytes)

Implementation Details

The core insight is that RuboCop operates on character positions, while Herb's parser provides byte positions. By converting the buffer to character-based operations, we eliminate the mismatch that caused incorrect positioning with multibyte characters. The byte_to_char_pos conversion is applied at the boundary where Herb positions are used in RuboCop's buffer.

The RubyRenderer was using byte-based positions for buffer operations,
but RuboCop uses character-based positions. This caused linter offenses
to be reported at incorrect line/column positions when the source file
contained multibyte characters before the offense location.

Changes:
- Add byte_to_char_pos method to Source class for converting Herb's
  byte positions to character positions
- Change RubyRenderer buffer from Array[Integer] (bytes) to Array[String]
  (characters)
- Update all buffer operations to use character positions instead of
  byte positions
- Add string constants (CHAR_*) to Characters module for character-based
  buffer operations
- Update Converter.generate_hybrid_code to work with character positions
- Update tests to check character length instead of byte length
- Add slice() and byte_range_to_char_range() methods to Source
- Change Tag to store char_from/char_to instead of byte-based range
- Update NodeLocationCollector to create Tags with character positions
- Update Converter to use slice() for character-based extraction
- Update RuboCopASTTransformer to use character-based tag lookup

This follows up on the multibyte position fix by ensuring all position
data is stored in character units rather than mixing byte and character
positions, making the codebase easier to work with.
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