refactor(lexer): drop Result<Token> wrapper from operator readers#62
Merged
Conversation
The five operator-reader functions in `src/lexer/operators.rs` (`read_pipe_operator`, `read_ampersand_operator`, `read_semicolon_operator`, `read_less_operator`, `read_greater_operator`) returned `Result<Token>` with no error path — each gated by `#[allow(clippy::unnecessary_wraps)]`. They also repeated the `command_start = true; reserved_words_ok = true` block across nine list-separator branches. Change return type to `Token`, introduce a private `command_token` helper on `Lexer` that reuses the existing `set_command_start()` primitive, and route every list-separator branch through it. File-redirect branches keep plain `Token::new` since they must not re-arm reserved-word recognition. Drop all five `unnecessary_wraps` allows. Dispatcher in `lexer/mod.rs` now wraps the infallible operator calls in `Ok(...)`, matching the style already used for `(`, `)`, and `\n`. Add guard tests pinning the new invariant: - `list_separators_re_arm_reserved_words` — every list separator (`|&`, `&&`, `||`, `;;`, `;&`, `;;&`) re-arms reserved-word recognition after `foo=` has cleared it. - `file_redirects_do_not_re_arm_reserved_words` — `>`, `>>`, `&>`, `<`, `<<<` leave the flag clear so following words stay plain `Word`. No behavior change — the 241 existing tests still pass, plus 2 new guard tests. Downstream rippy (1332 tests) passes against a local patched build of this branch. Closes #51 Refs #61 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mpecan
pushed a commit
that referenced
this pull request
Apr 19, 2026
🤖 I have created a release *beep* *boop* --- ## [0.2.0](rable-v0.1.15...rable-v0.2.0) (2026-04-18) ### ⚠ BREAKING CHANGES * tighten lexer API surface and relocate WordSpan to ast ([#70](#70)) ### Bug Fixes * **format:** align cmdsub reformatter with bash canonical form ([#49](#49)) ([c7a4411](c7a4411)) * **lexer:** accept sloppy heredoc terminator in cmdsub mode ([#50](#50)) ([40f394f](40f394f)) * **lexer:** backticks opaque when content is invalid ([#71](#71)) ([e72166f](e72166f)), closes [#38](#38) * **lexer:** disable reserved-word recognition after assignment words ([#44](#44)) ([42e1fc0](42e1fc0)) * **lexer:** stop treating ]] and unbalanced [...] as special outside conditionals ([#45](#45)) ([4bf5a5c](4bf5a5c)) * **parser:** fall back from (( … )) arith to nested subshells ([#48](#48)) ([1437f00](1437f00)) ### Code Refactoring * **format:** introduce Formatter struct ([#65](#65)) ([d965a8f](d965a8f)) * **lexer:** drop Result<Token> wrapper from operator readers ([#62](#62)) ([d52a841](d52a841)) * **lexer:** split read_word_token into classify + advance + dispatch helpers ([#63](#63)) ([3ba09f5](3ba09f5)) * **parser:** extract fill_heredoc_contents visitor helpers ([#68](#68)) ([40e6165](40e6165)) * **parser:** extract helpers from three oversize parsers ([#69](#69)) ([25d0762](25d0762)) * **sexp:** dispatch NodeKind Display to per-category helpers ([#66](#66)) ([44b0330](44b0330)) * **sexp:** table-drive ANSI-C escape dispatch ([#67](#67)) ([91a5267](91a5267)) * tighten lexer API surface and relocate WordSpan to ast ([#70](#70)) ([5171d01](5171d01)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: repository-butler[bot] <166800726+repository-butler[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #51 · Part of #61 (v0.2.0 Refactoring Cycle, PR 1 of 10)
Summary
Pure signature refactor of
src/lexer/operators.rs:read_pipe_operator,read_ampersand_operator,read_semicolon_operator,read_less_operator,read_greater_operator) fromResult<Token>toToken. Drop the five#[allow(clippy::unnecessary_wraps)]attributes — the fallibility was fiction.Lexer::command_tokenhelper that centralises thecommand_start = true; reserved_words_ok = truetransition by delegating to the existingset_command_start()primitive. Nine list-separator branches now route through it; file-redirect branches keep plainToken::new.lexer/mod.rswraps the now-infallible calls inOk(...), matching the style already used for(,), and\n.Net diff: +98 / −61 across three files; −6
#[allow(clippy::*)]attributes.Test plan
cargo fmt && cargo clippy --all-targets -- -D warnings— clean.cargo test— 243 passed, 0 failed (was 241; +2 new guard tests).cargo check --all-targetsandcargo testagainst../rippy(1332 tests) with rable patched to this branch — all green.cargo check --all-targets -p tokfagainst../tokfwith rable patched to this branch — clean.grep -rn "unnecessary_wraps" src/lexer/— zero matches.New guard tests
Two tests added to
src/lexer/tests.rspin the invariant this refactor centralises:list_separators_re_arm_reserved_words—|&,&&,||,;;,;&,;;&all re-arm reserved-word recognition afterfoo=clears it.file_redirects_do_not_re_arm_reserved_words—>,>>,&>,<,<<<leave the flag clear, so a followingforstays a plainWord.Raised by Agent 4 of the multi-agent review: the newly centralised contract was only covered indirectly via the Parable corpus. These lock it in at the lexer level.
Stack
Fresh from
main; no stack.