From 87b3c7805b6a2ccdd5fbcc39f3a701259855d9fa Mon Sep 17 00:00:00 2001 From: A Tobey Date: Wed, 9 Sep 2026 09:13:29 -0400 Subject: [PATCH] fix: name the word to quote when a path command name meets `=` Two error paths from the literal-word stack answered with an internal token name instead of the fix. `./bin=1` reported "found './bin' expected 'NEWLINE', 'set', identifier, assignment, POSIX function, ...". The command parser hands an adjacent `=` to assignment parsing so `x=1` stays an assignment, but only an identifier can start an assignment lvalue. For a path name the deferral was to a parser that could never apply, so the glued-word diagnosis never ran and a generic alternative list stood in its place. The filter now fires only for an identifier name; `./bin=1`, `src/bin=1`, `2026/report=1`, `/usr/bin/x=1`, and `true=1` all reach the glued-word message again. The last three never reached it, on any release. Reporting the right message exposed the wrong span: it named `1`, not `./bin=1`. Two causes. A bare `=` was not a glue-candidate unit, so word/`=`/word never formed one run; and the run had to begin at or after the grammar's own error position, while `reject_glued_args` reports at the run's *last* fragment. An `=` between two adjacent words now joins the run, and a run qualifies when it reaches that position rather than starting after it. `p=~10-20` reported "found 'DASHNUM(10-20)'". The tilde-split suffix list carried every other word token but `DashNumWord`, so a dash-number path did not fuse. `p=~2024-01-02` parses like `p=~user` now. CHANGELOG entries for the four merged layers were in reverse dependency order; they read in the order the changes build. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 37 +++++++------- crates/kaish-kernel/src/lexer.rs | 4 +- crates/kaish-kernel/src/parser.rs | 49 +++++++++++++------ .../tests/glued_arg_span_tests.rs | 28 +++++++++++ .../tests/tilde_assignment_words_tests.rs | 2 + 5 files changed, 86 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89bda837..d2121d44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,28 +12,31 @@ breaking entries are marked **BREAKING**. ### Fixed -- Home-relative assignments such as `p=~/x` now keep the assignment delimiter - separate from the path. The `=~` regex operator remains intact inside - tests, including tests containing command substitutions with assignments. - -- Numeric filenames and versions such as `123.txt` and `1.2.3` now stay - literal words. Float-prefixed globs such as `1.0*` preserve their exact - spelling and match filenames without changing scalar number rules. -- Keywords inside colon words and globs (`true:foo`, `do*`) now remain - literal text. Embedded `+` and tilde paths containing `:` are accepted - consistently; standalone keywords and plus-prefixed flags keep their meaning. - -- For-loop items now require whitespace between them. Quote a whole word to - join text with interpolation; adjacent fragments are refused before the - substitution or loop body runs, instead of becoming separate iterations. - -- Literal paths such as `.git/HEAD`, `2026/report`, `./`, and `../` now - parse as one word. Relative, absolute, and tilde paths accept `@` and `+` +- Literal paths such as `.git/HEAD`, `2026/report`, `./`, and `../` now parse + as one word. Relative, absolute, and tilde paths accept `@` and `+` consistently; Git revision paths such as `HEAD:src/main.rs` stay one argument. Text joined with an expansion still requires quoting. - Relative executable paths such as `.git/hooks/pre-commit` and `../bin/check` now parse. A command name attached to its first argument is refused instead of silently running a different executable (`./bin$x` as `./bin $x`). +- For-loop items now require whitespace between them. Quote a whole word to + join text with interpolation; adjacent fragments are refused before the + substitution or loop body runs, instead of becoming separate iterations. +- Numeric filenames and versions such as `123.txt` and `1.2.3` now stay + literal words. Float-prefixed globs such as `1.0*` preserve their exact + spelling and match filenames without changing scalar number rules. +- Keywords inside colon words and globs (`true:foo`, `do*`) now remain literal + text. Embedded `+` and tilde paths containing `:` are accepted consistently; + standalone keywords and plus-prefixed flags keep their meaning. +- Home-relative assignments such as `p=~/x` now keep the assignment delimiter + separate from the path. The `=~` regex operator remains intact inside tests, + including tests containing command substitutions with assignments. +- A command name that cannot start an assignment now reports glued words and + names the word to quote. `./bin=1` and `2026/report=1` gave a generic + "expected" list; they now name `./bin=1` and `2026/report=1`. +- Home-relative assignments accept a dash-number path: `p=~10-20` and + `p=~2024-01-02` parse like `p=~user` instead of failing with an internal + token name. ## [0.17.1] - 2026-09-02 diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index a5da47bf..f9fbe458 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -3203,8 +3203,8 @@ fn split_tilde_assignments(tokens: Vec>, source: &str) -> Vec s }, - select! { Token::RelativePath(s) => s }, - just(Token::True).to("true".to_string()), - just(Token::False).to("false".to_string()), - just(Token::Colon).to(":".to_string()), - just(Token::Dot).to(".".to_string()), + ident_parser().map(|name| (name, true)), + path_parser().map(|name| (name, false)), + select! { Token::DotSlashPath(s) => (s, false) }, + select! { Token::RelativePath(s) => (s, false) }, + just(Token::True).to(("true".to_string(), false)), + just(Token::False).to(("false".to_string(), false)), + just(Token::Colon).to((":".to_string(), false)), + just(Token::Dot).to((".".to_string(), false)), )); // NB: the "at most one stdin source per command" rule is enforced by a @@ -2148,13 +2151,17 @@ where // structurally after parsing, where the message is fully under our control // (verified empirically 2026-06-07). command_name - .map_with(|name, extra| -> (String, Span) { (name, extra.span()) }) + .map_with(|(name, is_identifier), extra| -> (String, bool, Span) { + (name, is_identifier, extra.span()) + }) // An adjacent `=` belongs to assignment parsing, including its errors. + // Only an identifier can start an assignment, so a path name keeps the + // command alternative and the glued-word diagnosis that names the fix. .then(just(Token::Eq).map_with(|_, extra| -> Span { extra.span() }).or_not().rewind()) - .filter(|((_, name_span), equals)| { - !equals.is_some_and(|span| name_span.end == span.start) + .filter(|((_, is_identifier, name_span), equals)| { + !(*is_identifier && equals.is_some_and(|span| name_span.end == span.start)) }) - .map(|(name, _)| name) + .map(|((name, _, name_span), _)| (name, name_span)) .then(args_list_parser().map_with(|args, extra| -> (Vec, Span) { (args, extra.span()) })) .validate(|((name, name_span), (args, args_span)), _, emitter| { if !args.is_empty() && name_span.end == args_span.start { @@ -3749,6 +3756,18 @@ fn glue_candidate_units(tokens: &[(Token, Span)]) -> Vec { continue; } + // A bare `=` between two adjacent words is part of the run, not a + // break in it: a name that cannot start an assignment (`./bin=1`) + // reaches argv as word/`=`/word and must be reported as one word. + if matches!(tok, Token::Eq) + && units.last().is_some_and(|last: &Span| last.end == span.start) + && word_unit(tokens, i + 1).is_some_and(|(next, _)| next.start == span.end) + { + units.push(*span); + i += 1; + continue; + } + i += 1; } units @@ -3865,7 +3884,7 @@ fn validate_glued_args( // in regions the grammar parsed happily, such as `$X==1` inside // `[[ ]]`. Take the first run at or after the // grammar's own position so the earlier legal run cannot win. - if units[start_idx].start < from_offset { + if units[end_idx].end <= from_offset { continue; } let span: Span = (units[start_idx].start..units[end_idx].end).into(); diff --git a/crates/kaish-kernel/tests/glued_arg_span_tests.rs b/crates/kaish-kernel/tests/glued_arg_span_tests.rs index fe1c2746..812fffc3 100644 --- a/crates/kaish-kernel/tests/glued_arg_span_tests.rs +++ b/crates/kaish-kernel/tests/glued_arg_span_tests.rs @@ -22,6 +22,7 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] use kaish_kernel::parser::parse; +use rstest::rstest; /// The text the argv-glue error's own span covers, after checking it is /// that error and only that error. @@ -274,3 +275,30 @@ fn the_legal_test_condition_really_is_legal() { parse("if [[ $X==1 ]]; then echo hi; fi") .expect("a test condition may contain adjacent operands and operators"); } + +/// A command name that is not an identifier cannot start an assignment, so +/// `name=value` there is glued argv and must name the word to quote — not +/// fall through to the assignment alternative's generic "expected …" list. +#[rstest] +#[case("./bin=1", "./bin=1")] +#[case("./bin=$x", "./bin=$x")] +#[case("src/bin=1", "src/bin=1")] +#[case("2026/report=1", "2026/report=1")] +#[case("/usr/bin/x=1", "/usr/bin/x=1")] +#[case("true=1", "true=1")] +fn non_identifier_name_with_equals_names_the_whole_word( + #[case] source: &str, + #[case] expected: &str, +) { + assert_eq!(glued_span_text(source), expected); +} + +/// An identifier name still hands an adjacent `=` to assignment parsing. +#[rstest] +#[case("x=1")] +#[case("cat=1")] +#[case("x = 1")] +#[case("x[0]=1")] +fn identifier_name_with_equals_stays_an_assignment(#[case] source: &str) { + parse(source).expect("must parse as an assignment"); +} diff --git a/crates/kaish-kernel/tests/tilde_assignment_words_tests.rs b/crates/kaish-kernel/tests/tilde_assignment_words_tests.rs index d09d2ad8..f614749e 100644 --- a/crates/kaish-kernel/tests/tilde_assignment_words_tests.rs +++ b/crates/kaish-kernel/tests/tilde_assignment_words_tests.rs @@ -16,6 +16,8 @@ use rstest::rstest; #[case("~fixture-user/x")] #[case("~007")] #[case("~1.0")] +#[case("~10-20")] +#[case("~2024-01-02")] fn tilde_assignment_has_an_assignment_delimiter(#[case] path: &str) { let source = format!("p={path}"); let tokens = tokenize(&source).unwrap();