fix(parser)!: track byte offsets instead of char counts in source locations

[lu-zero]

Summary

• SourcePosition.index now tracks byte offsets rather than UTF-8 character counts, fixing off-by-one drift per multi-byte character that broke any downstream use of the index for source slicing (diagnostics, syntax highlighting, LSP, etc.)
• Two fixes in the tokenizer: next_char() increments by ch.len_utf8() instead of 1, and the whitespace-skip branch does the same
• Updated doc comments on SourcePosition, SourcePositionOffset, and SourceSpan::length() to reflect byte semantics
• Added two tests verifying correct byte indices for tokens following multi-byte UTF-8 characters

Closes #1127

Breaking change

SourcePosition.index previously documented as a "character index" now returns a byte offset. Any downstream consumer relying on character-count semantics will need to adjust.

[lu-zero]

@reubeno not sure if it this alone warrants bumping to 0.5 but staying API-compatible would be more churn ...

[github-actions]

Public API changes for crate: brush-parser

Removed items

-pub brush_parser::SourcePosition::index: usize
-pub brush_parser::SourcePositionOffset::index: usize

Added items

+pub brush_parser::SourcePosition::offset: usize
+pub brush_parser::SourcePositionOffset::offset: usize

Performance Benchmark Report

Benchmark name	Baseline (μs)	Test/PR (μs)	Delta (μs)	Delta %
clone_shell_object	18.20 μs	18.40 μs	0.20 μs	🟠 +1.09%
eval_arithmetic	0.14 μs	0.13 μs	-0.01 μs	⚪ Unchanged
expand_one_string	1.70 μs	1.65 μs	-0.05 μs	⚪ Unchanged
for_loop	29.75 μs	30.07 μs	0.32 μs	🟠 +1.08%
full_peg_complex	54.22 μs	54.89 μs	0.66 μs	🟠 +1.22%
full_peg_for_loop	6.21 μs	6.30 μs	0.10 μs	🟠 +1.55%
full_peg_nested_expansions	15.86 μs	16.19 μs	0.33 μs	🟠 +2.08%
full_peg_pipeline	4.12 μs	4.23 μs	0.11 μs	🟠 +2.77%
full_peg_simple	1.71 μs	1.75 μs	0.03 μs	🟠 +1.98%
function_call	3.65 μs	3.53 μs	-0.12 μs	⚪ Unchanged
instantiate_shell	53.29 μs	54.31 μs	1.02 μs	⚪ Unchanged
instantiate_shell_with_init_scripts	24520.47 μs	25023.46 μs	502.99 μs	⚪ Unchanged
parse_peg_bash_completion	2237.03 μs	2222.75 μs	-14.29 μs	⚪ Unchanged
parse_peg_complex	19.48 μs	19.31 μs	-0.17 μs	⚪ Unchanged
parse_peg_for_loop	1.96 μs	1.95 μs	-0.01 μs	⚪ Unchanged
parse_peg_pipeline	2.01 μs	2.02 μs	0.01 μs	⚪ Unchanged
parse_peg_simple	1.07 μs	1.09 μs	0.02 μs	🟠 +1.68%
run_echo_builtin_command	13.26 μs	12.87 μs	-0.39 μs	⚪ Unchanged
tokenize_sample_script	3.47 μs	3.46 μs	-0.01 μs	⚪ Unchanged

Code Coverage Report: Only Changed Files listed

Package	Base Coverage	New Coverage	Difference
brush-parser/src/ast.rs	🔴 48.31%	🔴 49.53%	🟢 1.22%
Overall Coverage	🟢 75.73%	🟢 75.74%	🟢 0.01%

Minimum allowed coverage is 70%, this run produced 75.74%
Maximum allowed coverage difference is -5%, this run produced 0.01%

Test Summary: bash-completion test suite

Outcome	Count	Percentage
✅ Pass	1582	75.01
❗️ Error	18	0.85
❌ Fail	155	7.35
⏩ Skip	339	16.07
❎ Expected Fail	13	0.62
✔️ Unexpected Pass	2	0.09
📊 Total	2109	100.00

[reubeno]

@lu-zero What do you think about changing the field name to reflect the meaning change? That seems like something we ought to do so that all downstream consumers are forced to reevaluate whether their logic is still correct.

[lu-zero]

if you prefer we can, but the new release is already breaking enough that...

[lu-zero]

using offset is probably also more descriptive of what is it.

[reubeno]

using offset is probably also more descriptive of what is it.

Sure.

My main concern is avoiding a situation where the meaning of a field has changed but its name hasn't. Even if there are other breaking API changes, I'd want to lean on a compile break to at least give callers a chance to consider whether they need to change their logic.