From 058f618e04fb3e701ccc6392d3d2e831a7e3927d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 10:24:38 +0000 Subject: [PATCH 1/6] Start fix for #1476 Assisted-by: Claude Code:claude-opus-5-5 From c3d338b9d9f47b2156f988fd87d375ef2b46471f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 10:40:36 +0000 Subject: [PATCH 2/6] =?UTF-8?q?Test=20gem=20unwind=20after=20x=20=3D=20if?= =?UTF-8?q?=20=E2=80=A6=20end=20in=20source=20block?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A gem declared in the user's own source "…" do block loses its DEPENDENCIES `!` on hosted rollback/remove when an assignment-form keyword construct (v = if/case/begin … end) precedes it, so frozen installs fail on Bundler 4.0.19+ (#1476). Add unit cases for the block scanner and the restore round trip, and an e2e Bundler case. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_gem_build.rs | 34 ++++++++- .../src/patch/redirect/upstream/gem.rs | 73 +++++++++++++++++++ 2 files changed, 103 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs index 93cebb4f4..1487fb9ab 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -444,6 +444,10 @@ enum Driver { /// `source "" do` block (#1056): bundler locks it with a `!` /// source pin, which the unwind must keep. ScanVexSourceBlock, + /// [`Driver::ScanVexSourceBlock`] with assignment-form keyword + /// constructs (`v = if … end`, `w = case … end`) before the gem + /// (#1476): their `end`s must not close the user's source block. + ScanVexSourceBlockAssignment, /// A heredoc option continues beyond the declaration's physical line. ScanVexHeredocDeclaration, /// A double-quoted interpolation can itself contain a heredoc opener. @@ -526,6 +530,9 @@ impl Driver { Driver::ScanVexTwin => "scan --mode hosted (Gemfile + gems.rb twin)", Driver::ScanVexGroupBlock => "scan --mode hosted (gem in a group block)", Driver::ScanVexSourceBlock => "scan --mode hosted (gem in a source block)", + Driver::ScanVexSourceBlockAssignment => { + "scan --mode hosted (gem in a source block after `v = if … end`)" + } Driver::ScanVexSemicolonJoinedDeclaration => { "scan --mode hosted (two `;`-joined gem declarations)" } @@ -823,6 +830,12 @@ async fn redirect_scanned_project( "source \"{up}/upstream\"\n\nsource \"{up}/upstream\" do\n gem \"{DEP}\", \"{DEP_VERSION}\"\nend\n", up = server.uri() ), + Driver::ScanVexSourceBlockAssignment => format!( + "source \"{up}/upstream\"\n\nsource \"{up}/upstream\" do\n v = if ENV[\"ZZ\"]\n \"a\"\n \ + else\n \"b\"\n end\n w = case ENV[\"ZZ\"]\n when \"x\" then \"a\"\n \ + else \"b\"\n end\n gem \"{DEP}\", \"{DEP_VERSION}\"\nend\n", + up = server.uri() + ), Driver::ScanVexEvalGemfile => { std::fs::write(proj.join("Gemfile.common"), format!("gem \"{DEP}\"\n")).unwrap(); format!( @@ -1071,6 +1084,7 @@ async fn redirect_scanned_project( | Driver::ScanVexEvalGemfile | Driver::ScanVexGroupBlock | Driver::ScanVexSourceBlock + | Driver::ScanVexSourceBlockAssignment | Driver::ScanVexCustomGitSource | Driver::ScanVexMultiLineDeclaration | Driver::ScanVexConditionalDeclaration @@ -1275,6 +1289,7 @@ async fn redirect_scanned_project( Driver::ScanVex | Driver::ScanVexGroupBlock | Driver::ScanVexSourceBlock + | Driver::ScanVexSourceBlockAssignment | Driver::ScanVexTrailingSemicolonDeclaration => { assert_eq!(env["vex"]["statements"], 1, "vex block: {env}"); assert!( @@ -2342,18 +2357,29 @@ async fn gem_hosted_group_block_pin_survives_a_refused_vendored_takeover() { /// pre-scan pair, `!` included, so a FROZEN install of the restored pair /// still succeeds (it exited 16 when the unwind dropped the `!`). /// Converged without CHECKSUMS so the restore needs no rubygems.org sha. +/// #1476: the same holds when assignment-form `v = if … end` / +/// `w = case … end` constructs precede the gem inside the block. #[tokio::test(flavor = "multi_thread")] #[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 2.2); \ the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"] async fn gem_hosted_unwind_keeps_a_source_block_bang() { - for command in ["rollback", "remove"] { + for (driver, command) in [ + (Driver::ScanVexSourceBlock, "rollback"), + (Driver::ScanVexSourceBlock, "remove"), + (Driver::ScanVexSourceBlockAssignment, "rollback"), + (Driver::ScanVexSourceBlockAssignment, "remove"), + ] { + let label = match driver { + Driver::ScanVexSourceBlockAssignment => "source-block-assignment", + _ => "source-block", + }; let Some(fx) = redirect_scanned_project( - &format!("source-block {command}"), + &format!("{label} {command}"), Spelling::Gemfile, false, true, None, - Driver::ScanVexSourceBlock, + driver, ) .await else { @@ -2416,7 +2442,7 @@ async fn gem_hosted_unwind_keeps_a_source_block_bang() { fx.pristine_gemfile, "{command}: the Gemfile comes back byte for byte" ); - let fresh = stage_fresh_checkout(&fx, &format!("source-block-{command}")); + let fresh = stage_fresh_checkout(&fx, &format!("{label}-{command}")); let install = bundle_env(&fresh, &["install"], &[("BUNDLE_FROZEN", "true")]); assert!( install.status.success(), diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs index c44c6f24b..6ad307727 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs @@ -1361,6 +1361,17 @@ mod tests { // Nested in a group inside the source block, parenthesized opener. "source \"https://rubygems.org\"\n\ngem \"puma\"\nsource(\"https://rubygems.org\") do\n \ group :web do\n gem \"rails\", \"7.0.0\"\n end\nend\n", + // Assignment-form keyword constructs before the declaration + // (#1476): their `end`s close them, not the source block, and + // the declaration comes back at the block's indent. + "source \"https://rubygems.org\"\n\ngem \"puma\"\nsource \"https://rubygems.org\" do\n \ + v = if ENV[\"ZZ\"]\n \"a\"\n else\n \"b\"\n end\n gem \"rails\", \"7.0.0\"\nend\n", + "source \"https://rubygems.org\"\n\ngem \"puma\"\nsource \"https://rubygems.org\" do\n \ + v = case ENV[\"ZZ\"]\n when \"x\" then \"a\"\n else \"b\"\n end\n \ + w = begin\n 1\n rescue\n 2\n end\n gem \"rails\", \"7.0.0\"\nend\n", + "source \"https://rubygems.org\"\n\ngem \"puma\"\nsource \"https://rubygems.org\" do\n \ + x ||= unless ENV[\"ZZ\"]\n 1\n end\n y = (while false\n end)\n \ + gem \"rails\", \"7.0.0\"\nend\n", ] { let lock = format!( "GEM\n remote: https://rubygems.org/\n specs:\n puma (6.0.0)\n \ @@ -1442,6 +1453,68 @@ mod tests { assert!(!at( "source \"https://x\" do\n gem \"a\" if true\nend\ngem \"rails\"\n" )); + // An assignment-form construct (#1476) opens one too, whichever + // keyword, operator or expression position introduces it. + for opener in [ + "v = if ENV[\"A\"]", + "v = unless ENV[\"A\"]", + "v = case ENV[\"A\"]\n when \"x\" then 1", + "v = begin\n 1\n rescue\n 2", + "v ||= while false", + "v += until true", + "v = (if true", + "foo(1, if true", + "x = 1; if true", + "v = [case 1\n when 1 then 2", + "v = for x in [] do", + "if a then 1 else\n 2 end; v = (if true", + ] { + let t = format!( + "source \"https://x\" do\n {opener}\n 1\n end\n gem \"rails\"\nend\n" + ); + assert!(at(&t), "{t}"); + } + // Closed on its own line, it nets out. + assert!(at( + "source \"https://x\" do\n v = if a then 1 else 2 end\n gem \"rails\"\nend\n" + )); + assert!(at( + "source \"https://x\" do\n v = (case a when 1 then 2 end).to_s\n gem \"rails\"\nend\n" + )); + assert!(!at( + "v = if a then 1 else 2 end\nsource \"https://x\" do\n gem \"a\"\nend\ngem \"rails\"\n" + )); + // Modifiers after a value, names that only look like keywords + // (`x.class`, `:if`, `if:`, `begin?`, `"if"`), and an endless + // `def` open nothing, so the source block's `end` still closes it. + for line in [ + "v = 1 if true", + "v = x unless y", + "x.save! if y", + "x.empty? while y", + "return if y", + "v = x =~ /re/ if y", + "v = x.class", + "v = range.begin", + "v = [:if, :case]", + "foo(if: 1, case: 2)", + "v = x.begin?", + "v = \"if case begin\"", + "def x = 1", + "def x(a) = a", + "v = loop { break }", + ] { + let t = format!("source \"https://x\" do\n {line}\nend\ngem \"rails\"\n"); + assert!(!at(&t), "{t}"); + } + // A `while … do` loop takes one `end`, not two. + assert!(!at( + "source \"https://x\" do\n while false do\n end\nend\ngem \"rails\"\n" + )); + // A setter `def x=(v)` still needs its `end`. + assert!(at( + "source \"https://x\" do\n def x=(v)\n end\n gem \"rails\"\nend\n" + )); assert!(!at("if true\n gem \"a\"\nend\ngem \"rails\"\n")); // A `#` inside a string (`#{}` interpolation) is no comment: the // trailing `do` still opens the source block. From 807e52d42221a63a84ac78ba4d75e30a0ce040b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 10:40:36 +0000 Subject: [PATCH 3/6] =?UTF-8?q?Keep=20gem=20source-block=20bang=20after=20?= =?UTF-8?q?x=20=3D=20if=20=E2=80=A6=20end?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hosted gem unwind only counted if/case/begin/… as opening a block when the keyword started its line. In `v = if …` nothing was opened, but the matching `end` still closed the user's source block, so the restored lock dropped its `!` pin and Bundler 4.0.19+ frozen installs failed after rollback or remove. The restored declaration also came back at column 0 after a blank line. Count keywords word by word wherever they start an expression (after =, (, ',', ;, …), treat `end` as closing one, net out one-line constructs, skip modifiers, names like x.class or if:, and endless defs. Derive the restored declaration's indent from the same block stack. Fixes #1476 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/upstream/gem.rs | 202 ++++++++++++++---- 1 file changed, 161 insertions(+), 41 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs index 6ad307727..b7e7aa3af 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs @@ -502,10 +502,12 @@ fn provably_appended(text: &str, block: &Block) -> bool { /// The blank line + indent to put before a declaration restored in place of /// `block`. The rewriter's `^\s*gem` match swallowed whatever whitespace /// preceded the original line, which no file records, so this re-derives the -/// conventional layout from the neighbors: inside a block (`group … do`) -/// or right after another declaration or a comment, the neighbor's indent -/// and no blank line; after anything else (`source`, `ruby`, `end`, …) one -/// blank line and the indent of the declaration that follows, if any. +/// conventional layout from the neighbors: right after the line that opens +/// the enclosing block (`group … do`), that line's indent + 2; anywhere +/// else inside a block (after an `end`, a statement, #1476), or right after +/// another declaration or a comment, the neighbor's indent and no blank +/// line; after anything else at the top level (`source`, `ruby`, `end`, …) +/// one blank line and the indent of the declaration that follows, if any. fn declaration_prefix(text: &str, block: &Block, eol: &str) -> String { let Some(prev) = line_before(text, block.start) else { return String::new(); @@ -514,11 +516,11 @@ fn declaration_prefix(text: &str, block: &Block, eol: &str) -> String { if trimmed.is_empty() { return String::new(); } - let code = ruby_code(trimmed).trim_end(); - if code == "do" || code.ends_with(" do") || (code.ends_with('|') && code.contains(" do |")) { + let depth = block_stack(text, block.start).len(); + if depth > block_stack(text, line_before_start(text, block.start)).len() { return format!("{} ", indent_of(prev)); } - if trimmed.starts_with('#') || gem_declaration_any(trimmed).is_some() { + if depth > 0 || trimmed.starts_with('#') || gem_declaration_any(trimmed).is_some() { return indent_of(prev).to_string(); } let next = text[block.end..].split('\n').next().unwrap_or_default(); @@ -533,31 +535,163 @@ fn declaration_prefix(text: &str, block: &Block, eol: &str) -> String { /// Whether byte `at` of the manifest sits inside a `source "" do` /// block (at any depth: a `group … do` nested in it keeps the source). /// Bundler records a dependency declared there with a `!` source pin in the -/// lock's `DEPENDENCIES`, whichever URL the block names (#1056). A line-level -/// `do` / `end` count, like [`declaration_prefix`]'s: a block opened and -/// closed on one line nets out. Keyword constructs (`if`, `case`, `def`, …) -/// that start a line are counted too, so their `end` does not close an -/// enclosing `source … do` block. +/// lock's `DEPENDENCIES`, whichever URL the block names (#1056). fn in_source_block(text: &str, at: usize) -> bool { + block_stack(text, at).contains(&true) +} + +/// The Ruby constructs still open at byte `at` of the manifest, outermost +/// first: `true` for a `source … do` block, `false` for anything else that +/// Ruby closes with `end` (`group … do`, `if`, `case`, `def`, …). A +/// word-level count over each line's code: a construct opened and closed +/// on one line nets out, and a keyword opens one wherever it starts an +/// expression (`v = if …`, `(case …`, #1476), not only at the start of a +/// line, so its `end` never closes an enclosing block. +fn block_stack(text: &str, at: usize) -> Vec { let mut open: Vec = Vec::new(); for line in text[..at].lines() { - let code = ruby_code(line).trim(); - let opens = code == "do" - || code.ends_with(" do") - || code.ends_with(")do") - || (code.ends_with('|') && code.contains(" do |")); - if opens { - let source = code - .strip_prefix("source") - .is_some_and(|rest| rest.starts_with([' ', '\t', '('])); - open.push(source); - } else if opens_keyword_block(code) { - open.push(false); - } else if code == "end" || code.starts_with("end ") || code.starts_with("end.") { - open.pop(); + let code = masked_ruby_code(line); + let code = code.trim(); + let source_line = code + .strip_prefix("source") + .is_some_and(|rest| rest.starts_with([' ', '\t', '('])); + let mut source_do = source_line; + // A `while`/`until`/`for` opened on this line takes its optional + // `do` as part of the loop, not as a block of its own. + let mut loop_do = false; + for (word, start, end) in ruby_words(code) { + let opens = match word { + "do" if loop_do => { + loop_do = false; + false + } + "do" => true, + "if" | "unless" => starts_expression(&code[..start]), + "while" | "until" => { + let opens = starts_expression(&code[..start]); + loop_do |= opens; + opens + } + "for" => { + loop_do = true; + true + } + "def" => !endless_def(&code[end..]), + "case" | "begin" | "class" | "module" => true, + "end" => { + open.pop(); + false + } + _ => false, + }; + if opens { + open.push(word == "do" && std::mem::take(&mut source_do)); + } } } - open.contains(&true) + open +} + +/// The keyword-like words of a masked code line, with their byte range: a +/// symbol (`:if`), method call (`.class`) or variable sigil is a name, and +/// so are a predicate or bang (`begin?`) and a hash label (`if:`). +fn ruby_words(code: &str) -> impl Iterator { + let bytes = code.as_bytes(); + let is_ident = |b: u8| b.is_ascii_alphanumeric() || b == b'_'; + let mut i = 0; + std::iter::from_fn(move || { + while i < bytes.len() { + if !is_ident(bytes[i]) { + i += 1; + continue; + } + let start = i; + while i < bytes.len() && is_ident(bytes[i]) { + i += 1; + } + let prev_ok = start == 0 || !matches!(bytes[start - 1], b':' | b'.' | b'@' | b'$'); + let next_ok = match bytes.get(i) { + Some(b'?' | b'!') => false, + Some(b':') => bytes.get(i + 1) == Some(&b':'), + _ => true, + }; + if prev_ok && next_ok { + return Some((&code[start..i], start, i)); + } + } + None + }) +} + +/// Whether a keyword after `before` (the code before it on its line) +/// starts an expression, as opposed to modifying the statement before it +/// (`gem "x" if cond`, `x.save! if y`). +fn starts_expression(before: &str) -> bool { + let before = before.trim_end(); + match before.as_bytes().last() { + None => true, + Some(b'=' | b'(' | b'[' | b'{' | b',' | b';' | b'|' | b'&') => true, + Some(_) => before + .rsplit(|c: char| !(c.is_ascii_alphanumeric() || c == '_')) + .next() + .is_some_and(|w| matches!(w, "then" | "else" | "do" | "begin" | "ensure")), + } +} + +/// Whether `rest` (what follows `def`) is an endless method (`def x = 1`, +/// `def x(a) = a`), which Ruby closes without an `end`. A setter +/// (`def x=(v)`) is not. +fn endless_def(rest: &str) -> bool { + let rest = rest.trim_start(); + let name_len = rest + .find(|c: char| !(c.is_ascii_alphanumeric() || matches!(c, '_' | '.' | '?' | '!'))) + .unwrap_or(rest.len()); + let (name, mut tail) = rest.split_at(name_len); + if name.is_empty() { + return false; + } + if let Some(params) = tail.strip_prefix('(') { + let Some(close) = params.find(')') else { + return false; + }; + tail = ¶ms[close + 1..]; + } else if !tail.starts_with([' ', '\t']) { + return false; + } + tail.trim_start() + .strip_prefix('=') + .is_some_and(|t| !t.starts_with(['=', '~', '>'])) +} + +/// [`ruby_code`] with every quoted string's contents masked (`x`), so a +/// keyword inside a string literal never counts as code. +fn masked_ruby_code(line: &str) -> String { + let mut quote: Option = None; + let mut escaped = false; + ruby_code(line) + .chars() + .map(|c| match quote { + Some(_) if escaped => { + escaped = false; + 'x' + } + Some(_) if c == '\\' => { + escaped = true; + 'x' + } + Some(q) if c == q => { + quote = None; + c + } + Some(_) => 'x', + None => { + if c == '"' || c == '\'' { + quote = Some(c); + } + c + } + }) + .collect() } /// `line` without its trailing `#` comment. A `#` inside a quoted string @@ -580,20 +714,6 @@ fn ruby_code(line: &str) -> &str { line } -/// Whether the (comment-stripped, trimmed) line opens a keyword construct -/// that Ruby closes with `end`: one that starts the line, not a modifier -/// (`gem "x" if cond`), and not closed on the same line. -fn opens_keyword_block(code: &str) -> bool { - let first = code - .split(|c: char| c.is_whitespace() || c == '(' || c == ';') - .next() - .unwrap_or_default(); - matches!( - first, - "if" | "unless" | "while" | "until" | "case" | "begin" | "def" | "class" | "module" - ) && !(code.ends_with(" end") || code.ends_with(";end")) -} - /// How the gem comes back into the manifest. enum Decl { /// Gone: the block was the rewriter's append for a transitive gem. From b6dfe852811f3b874ad16af0b277b7ae91b59b7e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 10:47:19 +0000 Subject: [PATCH 4/6] Indent restored gem from its enclosing block After an `end` indented deeper than the block's body (a case/when laid out under its assignment), the restored declaration copied that `end`'s indent, so the Gemfile did not come back byte for byte. Take the indent of the line that opened the enclosing block + 2 instead. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/upstream/gem.rs | 49 ++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs index b7e7aa3af..8b0791880 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs @@ -502,12 +502,13 @@ fn provably_appended(text: &str, block: &Block) -> bool { /// The blank line + indent to put before a declaration restored in place of /// `block`. The rewriter's `^\s*gem` match swallowed whatever whitespace /// preceded the original line, which no file records, so this re-derives the -/// conventional layout from the neighbors: right after the line that opens -/// the enclosing block (`group … do`), that line's indent + 2; anywhere -/// else inside a block (after an `end`, a statement, #1476), or right after -/// another declaration or a comment, the neighbor's indent and no blank -/// line; after anything else at the top level (`source`, `ruby`, `end`, …) -/// one blank line and the indent of the declaration that follows, if any. +/// conventional layout from the neighbors: right after another declaration +/// or a comment, the neighbor's indent and no blank line; anywhere else +/// inside a block (right after its `group … do`, after an `end` or a +/// statement, #1476), the indent of the line that opened the enclosing +/// block + 2 and no blank line; after anything else at the top level +/// (`source`, `ruby`, `end`, …) one blank line and the indent of the +/// declaration that follows, if any. fn declaration_prefix(text: &str, block: &Block, eol: &str) -> String { let Some(prev) = line_before(text, block.start) else { return String::new(); @@ -516,13 +517,19 @@ fn declaration_prefix(text: &str, block: &Block, eol: &str) -> String { if trimmed.is_empty() { return String::new(); } - let depth = block_stack(text, block.start).len(); - if depth > block_stack(text, line_before_start(text, block.start)).len() { - return format!("{} ", indent_of(prev)); - } - if depth > 0 || trimmed.starts_with('#') || gem_declaration_any(trimmed).is_some() { + let enclosing = block_stack(text, block.start) + .last() + .map(|&(_, indent)| indent); + let opened_by_prev = || { + block_stack(text, block.start).len() + > block_stack(text, line_before_start(text, block.start)).len() + }; + if (trimmed.starts_with('#') || gem_declaration_any(trimmed).is_some()) && !opened_by_prev() { return indent_of(prev).to_string(); } + if let Some(indent) = enclosing { + return format!("{indent} "); + } let next = text[block.end..].split('\n').next().unwrap_or_default(); let indent = if gem_declaration_any(next.trim()).is_some() { indent_of(next) @@ -537,18 +544,19 @@ fn declaration_prefix(text: &str, block: &Block, eol: &str) -> String { /// Bundler records a dependency declared there with a `!` source pin in the /// lock's `DEPENDENCIES`, whichever URL the block names (#1056). fn in_source_block(text: &str, at: usize) -> bool { - block_stack(text, at).contains(&true) + block_stack(text, at).iter().any(|&(source, _)| source) } /// The Ruby constructs still open at byte `at` of the manifest, outermost -/// first: `true` for a `source … do` block, `false` for anything else that -/// Ruby closes with `end` (`group … do`, `if`, `case`, `def`, …). A +/// first, each with the indent of the line that opened it: `true` for a +/// `source … do` block, `false` for anything else that Ruby closes with +/// `end` (`group … do`, `if`, `case`, `def`, …). A /// word-level count over each line's code: a construct opened and closed /// on one line nets out, and a keyword opens one wherever it starts an /// expression (`v = if …`, `(case …`, #1476), not only at the start of a /// line, so its `end` never closes an enclosing block. -fn block_stack(text: &str, at: usize) -> Vec { - let mut open: Vec = Vec::new(); +fn block_stack(text: &str, at: usize) -> Vec<(bool, &str)> { + let mut open: Vec<(bool, &str)> = Vec::new(); for line in text[..at].lines() { let code = masked_ruby_code(line); let code = code.trim(); @@ -585,7 +593,10 @@ fn block_stack(text: &str, at: usize) -> Vec { _ => false, }; if opens { - open.push(word == "do" && std::mem::take(&mut source_do)); + open.push(( + word == "do" && std::mem::take(&mut source_do), + indent_of(line), + )); } } } @@ -1492,6 +1503,10 @@ mod tests { "source \"https://rubygems.org\"\n\ngem \"puma\"\nsource \"https://rubygems.org\" do\n \ x ||= unless ENV[\"ZZ\"]\n 1\n end\n y = (while false\n end)\n \ gem \"rails\", \"7.0.0\"\nend\n", + // An `end` indented deeper than the block's body: the + // declaration takes the block's body indent, not the `end`'s. + "source \"https://rubygems.org\"\n\ngem \"puma\"\nsource \"https://rubygems.org\" do\n \ + w = case ENV[\"ZZ\"]\n when \"x\" then \"a\"\n end\n gem \"rails\", \"7.0.0\"\nend\n", ] { let lock = format!( "GEM\n remote: https://rubygems.org/\n specs:\n puma (6.0.0)\n \ From c9f3945e0a1c71d56aa83d9b9114cedd96433b3f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 11:08:30 +0000 Subject: [PATCH 5/6] Skip literals, comments, heredocs in gem scan Review follow-ups for the source-block scanner (#1476): - a keyword after a label (require: if), =>, a ternary, and/or/not or an operator starts an expression, so its end is counted; - %w[]/%i[] arrays, regexps, backticks, =begin/=end comments and heredoc bodies are no longer read as code; - a ; ends a loop's statement, so a later do opens its own block; - def begin / x.then if treat the keyword-like word as a name; - the restored declaration keeps a blank line before a following declaration after an inner end, as before; - the stack is computed once per restore, with opener line offsets. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/upstream/gem.rs | 423 +++++++++++++----- 1 file changed, 314 insertions(+), 109 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs index 8b0791880..1cb5fcd4e 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs @@ -502,13 +502,13 @@ fn provably_appended(text: &str, block: &Block) -> bool { /// The blank line + indent to put before a declaration restored in place of /// `block`. The rewriter's `^\s*gem` match swallowed whatever whitespace /// preceded the original line, which no file records, so this re-derives the -/// conventional layout from the neighbors: right after another declaration -/// or a comment, the neighbor's indent and no blank line; anywhere else -/// inside a block (right after its `group … do`, after an `end` or a -/// statement, #1476), the indent of the line that opened the enclosing -/// block + 2 and no blank line; after anything else at the top level -/// (`source`, `ruby`, `end`, …) one blank line and the indent of the -/// declaration that follows, if any. +/// conventional layout from the neighbors: right after the line that opens +/// the enclosing block (`group … do`), that line's indent + 2; right after +/// another declaration or a comment, the neighbor's indent and no blank +/// line; before another declaration, one blank line and that declaration's +/// indent; anywhere else inside a block (after an `end` or a statement, +/// #1476), the enclosing block's indent + 2 and no blank line; anywhere +/// else at the top level (after `source`, `ruby`, `end`, …) one blank line. fn declaration_prefix(text: &str, block: &Block, eol: &str) -> String { let Some(prev) = line_before(text, block.start) else { return String::new(); @@ -517,26 +517,25 @@ fn declaration_prefix(text: &str, block: &Block, eol: &str) -> String { if trimmed.is_empty() { return String::new(); } - let enclosing = block_stack(text, block.start) + let stack = block_stack(text, block.start); + let enclosing = stack.last().map(|f| indent_of(&text[f.line..])); + if stack .last() - .map(|&(_, indent)| indent); - let opened_by_prev = || { - block_stack(text, block.start).len() - > block_stack(text, line_before_start(text, block.start)).len() - }; - if (trimmed.starts_with('#') || gem_declaration_any(trimmed).is_some()) && !opened_by_prev() { - return indent_of(prev).to_string(); + .is_some_and(|f| f.line == line_before_start(text, block.start)) + { + return format!("{} ", indent_of(prev)); } - if let Some(indent) = enclosing { - return format!("{indent} "); + if trimmed.starts_with('#') || gem_declaration_any(trimmed).is_some() { + return indent_of(prev).to_string(); } let next = text[block.end..].split('\n').next().unwrap_or_default(); - let indent = if gem_declaration_any(next.trim()).is_some() { - indent_of(next) - } else { - "" - }; - format!("{eol}{indent}") + if gem_declaration_any(next.trim()).is_some() { + return format!("{eol}{}", indent_of(next)); + } + match enclosing { + Some(indent) => format!("{indent} "), + None => eol.to_string(), + } } /// Whether byte `at` of the manifest sits inside a `source "" do` @@ -544,59 +543,102 @@ fn declaration_prefix(text: &str, block: &Block, eol: &str) -> String { /// Bundler records a dependency declared there with a `!` source pin in the /// lock's `DEPENDENCIES`, whichever URL the block names (#1056). fn in_source_block(text: &str, at: usize) -> bool { - block_stack(text, at).iter().any(|&(source, _)| source) + block_stack(text, at).iter().any(|f| f.source) } -/// The Ruby constructs still open at byte `at` of the manifest, outermost -/// first, each with the indent of the line that opened it: `true` for a -/// `source … do` block, `false` for anything else that Ruby closes with -/// `end` (`group … do`, `if`, `case`, `def`, …). A -/// word-level count over each line's code: a construct opened and closed -/// on one line nets out, and a keyword opens one wherever it starts an -/// expression (`v = if …`, `(case …`, #1476), not only at the start of a -/// line, so its `end` never closes an enclosing block. -fn block_stack(text: &str, at: usize) -> Vec<(bool, &str)> { - let mut open: Vec<(bool, &str)> = Vec::new(); - for line in text[..at].lines() { - let code = masked_ruby_code(line); - let code = code.trim(); - let source_line = code +/// A Ruby construct still open at some point of the manifest. +struct Frame { + /// A `source … do` block (anything else Ruby closes with `end`: + /// `group … do`, `if`, `case`, `def`, … is not). + source: bool, + /// Byte offset of the start of the line that opened it. + line: usize, +} + +/// The Ruby constructs still open at byte `at` (a line start) of the +/// manifest, outermost first. A word-level count over each line's code: +/// a construct opened and closed on one line nets out, and a keyword opens +/// one wherever it starts an expression (`v = if …`, `require: case …`, +/// #1476), not only at the start of a line, so its `end` never closes an +/// enclosing block. String, `%w[]`, regexp and backtick literals, +/// comments, `=begin` … `=end` blocks and heredoc bodies are not code. +fn block_stack(text: &str, at: usize) -> Vec { + let mut open: Vec = Vec::new(); + let mut block_comment = false; + // Pending heredoc terminators: (identifier, may be indented). + let mut heredocs: std::collections::VecDeque<(String, bool)> = Default::default(); + let mut start = 0; + for raw in text[..at].split_inclusive('\n') { + let line_start = start; + start += raw.len(); + let line = raw.trim_end_matches(['\n', '\r']); + // `=begin` / `=end` start at column 0 and are followed by + // whitespace or the end of the line (Ruby's own rule). + let marker = |word: &str| { + line.strip_prefix(word) + .is_some_and(|rest| rest.is_empty() || rest.starts_with(char::is_whitespace)) + }; + if block_comment { + block_comment = !marker("=end"); + continue; + } + if let Some((id, indented)) = heredocs.front() { + let body = if *indented { line.trim_start() } else { line }; + if body == id { + heredocs.pop_front(); + } + continue; + } + if marker("=begin") { + block_comment = true; + continue; + } + let code = ruby_code(line); + let masked = masked_ruby_code(code); + heredocs.extend(heredoc_openers(code, &masked)); + let masked = masked.trim(); + let mut source_do = masked .strip_prefix("source") .is_some_and(|rest| rest.starts_with([' ', '\t', '('])); - let mut source_do = source_line; - // A `while`/`until`/`for` opened on this line takes its optional - // `do` as part of the loop, not as a block of its own. - let mut loop_do = false; - for (word, start, end) in ruby_words(code) { - let opens = match word { - "do" if loop_do => { - loop_do = false; - false - } - "do" => true, - "if" | "unless" => starts_expression(&code[..start]), - "while" | "until" => { - let opens = starts_expression(&code[..start]); - loop_do |= opens; - opens - } - "for" => { - loop_do = true; - true + for statement in masked.split(';') { + // A `while`/`until`/`for` takes its optional `do` as part of + // the loop, not as a block of its own. + let mut loop_do = false; + // The word after `def` is the method's name (`def begin`). + let mut method_name = false; + for (word, start, end) in ruby_words(statement) { + if std::mem::take(&mut method_name) { + continue; } - "def" => !endless_def(&code[end..]), - "case" | "begin" | "class" | "module" => true, - "end" => { - open.pop(); - false + let opens = match word { + "do" => !std::mem::take(&mut loop_do), + "if" | "unless" => starts_expression(&statement[..start]), + "while" | "until" => { + let opens = starts_expression(&statement[..start]); + loop_do |= opens; + opens + } + "for" => { + loop_do = true; + true + } + "def" => { + method_name = true; + !endless_def(&statement[end..]) + } + "case" | "begin" | "class" | "module" => true, + "end" => { + open.pop(); + false + } + _ => false, + }; + if opens { + open.push(Frame { + source: word == "do" && std::mem::take(&mut source_do), + line: line_start, + }); } - _ => false, - }; - if opens { - open.push(( - word == "do" && std::mem::take(&mut source_do), - indent_of(line), - )); } } } @@ -608,16 +650,15 @@ fn block_stack(text: &str, at: usize) -> Vec<(bool, &str)> { /// so are a predicate or bang (`begin?`) and a hash label (`if:`). fn ruby_words(code: &str) -> impl Iterator { let bytes = code.as_bytes(); - let is_ident = |b: u8| b.is_ascii_alphanumeric() || b == b'_'; let mut i = 0; std::iter::from_fn(move || { while i < bytes.len() { - if !is_ident(bytes[i]) { + if !is_ident_byte(bytes[i]) { i += 1; continue; } let start = i; - while i < bytes.len() && is_ident(bytes[i]) { + while i < bytes.len() && is_ident_byte(bytes[i]) { i += 1; } let prev_ok = start == 0 || !matches!(bytes[start - 1], b':' | b'.' | b'@' | b'$'); @@ -634,18 +675,56 @@ fn ruby_words(code: &str) -> impl Iterator { }) } -/// Whether a keyword after `before` (the code before it on its line) -/// starts an expression, as opposed to modifying the statement before it -/// (`gem "x" if cond`, `x.save! if y`). +fn is_ident_byte(b: u8) -> bool { + b.is_ascii_alphanumeric() || b == b'_' || !b.is_ascii() +} + +/// Whether a keyword or literal after `before` (the masked code before it +/// in its statement) starts an expression, as opposed to modifying the +/// statement before it (`gem "x" if cond`, `x.save! if y`) or dividing. fn starts_expression(before: &str) -> bool { let before = before.trim_end(); - match before.as_bytes().last() { - None => true, - Some(b'=' | b'(' | b'[' | b'{' | b',' | b';' | b'|' | b'&') => true, - Some(_) => before - .rsplit(|c: char| !(c.is_ascii_alphanumeric() || c == '_')) - .next() - .is_some_and(|w| matches!(w, "then" | "else" | "do" | "begin" | "ensure")), + let Some(last) = before.bytes().next_back() else { + return true; + }; + match last { + b'=' | b'(' | b'[' | b'{' | b',' | b'|' | b'&' | b'>' | b'<' | b'+' | b'-' | b'*' + | b'~' => true, + // A unary `!`, not a bang method (`x.save! if y`). + b'!' => before.len() == 1 || !is_ident_byte(before.as_bytes()[before.len() - 2]), + // A label (`require: if …`) or a ternary's `:`, not `Foo::`. + b':' => !before.ends_with("::"), + // A ternary's `?`, not a predicate (`x.empty? if y`). + b'?' => before[..before.len() - 1].ends_with(char::is_whitespace), + b if is_ident_byte(b) => { + let word_start = before + .bytes() + .rposition(|b| !is_ident_byte(b)) + .map_or(0, |i| i + 1); + let called = word_start > 0 + && matches!(before.as_bytes()[word_start - 1], b'.' | b':' | b'@' | b'$'); + !called + && matches!( + &before[word_start..], + "then" + | "else" + | "elsif" + | "do" + | "begin" + | "ensure" + | "and" + | "or" + | "not" + | "when" + | "in" + | "if" + | "unless" + | "while" + | "until" + | "case" + ) + } + _ => false, } } @@ -655,7 +734,7 @@ fn starts_expression(before: &str) -> bool { fn endless_def(rest: &str) -> bool { let rest = rest.trim_start(); let name_len = rest - .find(|c: char| !(c.is_ascii_alphanumeric() || matches!(c, '_' | '.' | '?' | '!'))) + .find(|c: char| !(c.is_alphanumeric() || matches!(c, '_' | '.' | '?' | '!'))) .unwrap_or(rest.len()); let (name, mut tail) = rest.split_at(name_len); if name.is_empty() { @@ -674,35 +753,115 @@ fn endless_def(rest: &str) -> bool { .is_some_and(|t| !t.starts_with(['=', '~', '>'])) } -/// [`ruby_code`] with every quoted string's contents masked (`x`), so a -/// keyword inside a string literal never counts as code. -fn masked_ruby_code(line: &str) -> String { - let mut quote: Option = None; - let mut escaped = false; - ruby_code(line) - .chars() - .map(|c| match quote { - Some(_) if escaped => { - escaped = false; - 'x' - } - Some(_) if c == '\\' => { - escaped = true; - 'x' +/// `code` (a comment-stripped line) with the contents of every string, +/// backtick, `%`-literal (`%w[…]`, `%i(…)`, `%q{…}`, …) and regexp literal +/// masked as `x`, byte for byte, so a keyword inside one never counts as +/// code. The delimiters stay. +fn masked_ruby_code(code: &str) -> String { + let bytes = code.as_bytes(); + let mut out = bytes.to_vec(); + let mut i = 0; + while i < bytes.len() { + let b = bytes[i]; + let (open, close, body) = match b { + b'"' | b'\'' | b'`' => (b, b, i + 1), + b'/' if starts_expression(&code[..i]) => (b, b, i + 1), + b'%' if starts_expression(&code[..i]) => { + let mut j = i + 1; + if matches!( + bytes.get(j), + Some(b'w' | b'W' | b'i' | b'I' | b'q' | b'Q' | b'r') + ) { + j += 1; + } + let Some(&open) = bytes.get(j) else { + break; + }; + if open.is_ascii_alphanumeric() || open.is_ascii_whitespace() || !open.is_ascii() { + i += 1; + continue; + } + let close = match open { + b'(' => b')', + b'[' => b']', + b'{' => b'}', + b'<' => b'>', + _ => open, + }; + (open, close, j + 1) } - Some(q) if c == q => { - quote = None; - c + _ => { + i += 1; + continue; } - Some(_) => 'x', - None => { - if c == '"' || c == '\'' { - quote = Some(c); + }; + let mut depth = 0usize; + let mut j = body; + while j < bytes.len() { + let c = bytes[j]; + if c == b'\\' { + out[j] = b'x'; + if j + 1 < bytes.len() { + out[j + 1] = b'x'; } - c + j += 2; + continue; } - }) - .collect() + if c == close && depth == 0 { + break; + } + if open != close && c == open { + depth += 1; + } else if open != close && c == close { + depth -= 1; + } + out[j] = b'x'; + j += 1; + } + i = j + 1; + } + // The delimiters are ASCII, so a masked run never cuts a multi-byte + // character: the result is still UTF-8. + String::from_utf8(out).unwrap_or_else(|_| code.to_string()) +} + +/// The heredocs a line opens (`<<~EOS`, `<<-"EOS"`, `< Vec<(String, bool)> { + let bytes = masked.as_bytes(); + let mut out = Vec::new(); + let mut i = 0; + while let Some(at) = masked[i..].find("<<").map(|p| p + i) { + i = at + 2; + if at > 0 && (is_ident_byte(bytes[at - 1]) || matches!(bytes[at - 1], b')' | b']' | b'}')) { + continue; + } + let mut j = at + 2; + let indented = matches!(bytes.get(j), Some(b'~' | b'-')); + if indented { + j += 1; + } + let id = match bytes.get(j) { + Some(&q @ (b'"' | b'\'' | b'`')) => code[j + 1..] + .find(q as char) + .map(|end| &code[j + 1..j + 1 + end]), + Some(&b) if b.is_ascii_alphabetic() || b == b'_' => { + let len = masked[j..] + .bytes() + .position(|b| !is_ident_byte(b)) + .unwrap_or(masked.len() - j); + Some(&code[j..j + len]) + } + _ => None, + }; + if let Some(id) = id.filter(|id| !id.is_empty()) { + out.push((id.to_string(), indented)); + i = j + 1; + } + } + out } /// `line` without its trailing `#` comment. A `#` inside a quoted string @@ -1239,6 +1398,19 @@ mod tests { // only the block goes. let t = format!("gem \"puma\"\n{block}\n"); assert_eq!(run(&t, &Decl::Transitive), "gem \"puma\"\n"); + // After an inner `end` inside a block (#1476): the block's indent + + // 2, a blank line only when another declaration follows. + let t = format!("group :test do\n if x\n gem \"a\"\n end\n{block}\nend\n"); + assert_eq!( + run(&t, &direct), + "group :test do\n if x\n gem \"a\"\n end\n gem \"rails\", \"7.0.0\"\nend\n" + ); + let t = + format!("group :test do\n if x\n gem \"a\"\n end\n{block}\n gem \"b\"\nend\n"); + assert_eq!( + run(&t, &direct), + "group :test do\n if x\n gem \"a\"\n end\n\n gem \"rails\", \"7.0.0\"\n gem \"b\"\nend\n" + ); let t = format!("gem \"puma\"\n{block}\n"); assert_eq!( run(&t, &Decl::Direct(constraint_args("rails (>= 6, ~> 7.0)"))), @@ -1601,6 +1773,10 @@ mod tests { "foo(1, if true", "x = 1; if true", "v = [case 1\n when 1 then 2", + "gem \"foo\", require: if ENV[\"A\"]", + "gem \"foo\", :require => case ENV[\"A\"]\n when \"x\" then 1", + "v = c ? case x\n when 1 then 2", + "v = a and if b", "v = for x in [] do", "if a then 1 else\n 2 end; v = (if true", ] { @@ -1638,6 +1814,13 @@ mod tests { "def x = 1", "def x(a) = a", "v = loop { break }", + "v = compute.then if ENV[\"X\"]", + "gem \"a\", require: %w[active_support/core_ext/class]", + "gem \"a\", platforms: %i[mri case]", + "gem \"a\" if RUBY_PLATFORM =~ /linux|case/", + "v = `echo if`", + "v = a / b if c", + "v = a % b if c", ] { let t = format!("source \"https://x\" do\n {line}\nend\ngem \"rails\"\n"); assert!(!at(&t), "{t}"); @@ -1646,6 +1829,28 @@ mod tests { assert!(!at( "source \"https://x\" do\n while false do\n end\nend\ngem \"rails\"\n" )); + // A `;` ends the loop's statement: a later `do` opens a block. + assert!(at( + "source \"https://x\" do\n while q.any?; q.each do |x|\n end\n end\n gem \"rails\"\nend\n" + )); + // A method named like a keyword is a name (`def begin`). + assert!(!at( + "source \"https://x\" do\n def begin\n 1\n end\nend\ngem \"rails\"\n" + )); + // `=begin` … `=end` comments and heredoc bodies are not code. + assert!(!at( + "source \"https://x\" do\n=begin\nGems for the CI box\n=end\nend\ngem \"rails\"\n" + )); + assert!(at( + "source \"https://x\" do\n NOTE = <<~EOS\n the end\n end\n EOS\n gem \"rails\"\nend\n" + )); + assert!(at( + "source \"https://x\" do\n NOTE = <<-'EOS' + < Date: Tue, 13 Oct 2026 11:28:04 +0000 Subject: [PATCH 6/6] Carry literals across lines in gem block scan Second review pass on the source-block scanner (#1476): - mask strings and %-literals that span lines, and find the comment with the same literal rules (a ' in a regexp, # in %q()); - add %s() / %x() literals; - class < Vec { let mut block_comment = false; // Pending heredoc terminators: (identifier, may be indented). let mut heredocs: std::collections::VecDeque<(String, bool)> = Default::default(); + // A string or `%`-literal still open at the end of the last line. + let mut literal: Option = None; + // The code of the statement the last line left unfinished (a trailing + // `,`, `\`, `=`, …), and the start of the line it began on. + let mut carried = String::new(); + let mut statement_line = 0; let mut start = 0; for raw in text[..at].split_inclusive('\n') { let line_start = start; @@ -589,18 +595,23 @@ fn block_stack(text: &str, at: usize) -> Vec { } continue; } - if marker("=begin") { + if literal.is_none() && marker("=begin") { block_comment = true; continue; } - let code = ruby_code(line); - let masked = masked_ruby_code(code); - heredocs.extend(heredoc_openers(code, &masked)); + let continues = literal.is_some() || !carried.is_empty(); + if !continues { + statement_line = line_start; + } + let masked = masked_ruby_code(line, &mut literal); + heredocs.extend(heredoc_openers(line, &masked)); let masked = masked.trim(); - let mut source_do = masked - .strip_prefix("source") - .is_some_and(|rest| rest.starts_with([' ', '\t', '('])); - for statement in masked.split(';') { + let mut source_do = !continues + && masked + .strip_prefix("source") + .is_some_and(|rest| rest.starts_with([' ', '\t', '('])); + for (n, statement) in masked.split(';').enumerate() { + let lead = if n == 0 { carried.as_str() } else { "" }; // A `while`/`until`/`for` takes its optional `do` as part of // the loop, not as a block of its own. let mut loop_do = false; @@ -610,11 +621,12 @@ fn block_stack(text: &str, at: usize) -> Vec { if std::mem::take(&mut method_name) { continue; } + let expression = || starts_expression(&format!("{lead} {}", &statement[..start])); let opens = match word { "do" => !std::mem::take(&mut loop_do), - "if" | "unless" => starts_expression(&statement[..start]), + "if" | "unless" => expression(), "while" | "until" => { - let opens = starts_expression(&statement[..start]); + let opens = expression(); loop_do |= opens; opens } @@ -636,11 +648,23 @@ fn block_stack(text: &str, at: usize) -> Vec { if opens { open.push(Frame { source: word == "do" && std::mem::take(&mut source_do), - line: line_start, + line: statement_line, }); } } } + let unfinished = literal.is_some() + || masked.ends_with(['\\', ',', '(', '[', '+', '-', '*', '=']) + || masked.ends_with("||") + || masked.ends_with("&&"); + carried = if unfinished { + let tail = masked.rsplit(';').next().unwrap_or_default(); + format!("{carried} {}", tail.trim_end_matches('\\')) + .trim() + .to_string() + } else { + String::new() + }; } open } @@ -688,8 +712,8 @@ fn starts_expression(before: &str) -> bool { return true; }; match last { - b'=' | b'(' | b'[' | b'{' | b',' | b'|' | b'&' | b'>' | b'<' | b'+' | b'-' | b'*' - | b'~' => true, + b'=' | b'(' | b'[' | b'{' | b',' | b';' | b'|' | b'&' | b'>' | b'<' | b'+' | b'-' + | b'*' | b'~' => true, // A unary `!`, not a bang method (`x.save! if y`). b'!' => before.len() == 1 || !is_ident_byte(before.as_bytes()[before.len() - 2]), // A label (`require: if …`) or a ternary's `:`, not `Foo::`. @@ -729,22 +753,39 @@ fn starts_expression(before: &str) -> bool { } /// Whether `rest` (what follows `def`) is an endless method (`def x = 1`, -/// `def x(a) = a`), which Ruby closes without an `end`. A setter -/// (`def x=(v)`) is not. +/// `def x(a = f()) = a`, `def ==(o) = …`), which Ruby closes without an +/// `end`. A setter (`def x=(v)`) is not. fn endless_def(rest: &str) -> bool { let rest = rest.trim_start(); - let name_len = rest - .find(|c: char| !(c.is_alphanumeric() || matches!(c, '_' | '.' | '?' | '!'))) - .unwrap_or(rest.len()); - let (name, mut tail) = rest.split_at(name_len); - if name.is_empty() { + let ident = |c: char| c.is_alphanumeric() || c == '_'; + let name_len = if rest.starts_with(ident) { + // `x`, `self.x`, `x?`, `x!`. + let len = rest + .find(|c: char| !(ident(c) || c == '.')) + .unwrap_or(rest.len()); + len + usize::from(rest[len..].starts_with(['?', '!'])) + } else { + // An operator method: `==`, `<=>`, `[]=`, `+@`, …. + rest.find(|c: char| !"[]=!<>+-*/%~^&|@".contains(c)) + .unwrap_or(rest.len()) + }; + if name_len == 0 { return false; } - if let Some(params) = tail.strip_prefix('(') { - let Some(close) = params.find(')') else { + let mut tail = &rest[name_len..]; + if tail.starts_with('(') { + let mut depth = 0usize; + let Some(close) = tail.find(|c: char| { + match c { + '(' => depth += 1, + ')' => depth -= 1, + _ => {} + } + depth == 0 + }) else { return false; }; - tail = ¶ms[close + 1..]; + tail = &tail[close + 1..]; } else if !tail.starts_with([' ', '\t']) { return false; } @@ -753,83 +794,150 @@ fn endless_def(rest: &str) -> bool { .is_some_and(|t| !t.starts_with(['=', '~', '>'])) } -/// `code` (a comment-stripped line) with the contents of every string, +/// A string or `%`-literal left open at the end of a line. +#[derive(Clone, Copy)] +struct Literal { + open: u8, + close: u8, + /// Nesting of `open` inside a bracketed literal (`%w[a [b] c]`). + depth: usize, +} + +/// `line` up to its `#` comment, with the contents of every string, /// backtick, `%`-literal (`%w[…]`, `%i(…)`, `%q{…}`, …) and regexp literal -/// masked as `x`, byte for byte, so a keyword inside one never counts as -/// code. The delimiters stay. -fn masked_ruby_code(code: &str) -> String { - let bytes = code.as_bytes(); +/// masked as `x`, byte for byte, so a keyword or `#` inside one never counts +/// as code. The delimiters stay. A string or `%`-literal still open at the +/// end of the line is carried in `literal` to mask the next line from its +/// start. +fn masked_ruby_code(line: &str, literal: &mut Option) -> String { + let bytes = line.as_bytes(); let mut out = bytes.to_vec(); let mut i = 0; + if let Some(lit) = literal.take() { + match mask_literal(bytes, &mut out, 0, lit) { + Ok(end) => i = end + 1, + Err(lit) => { + *literal = Some(lit); + i = bytes.len(); + } + } + } while i < bytes.len() { let b = bytes[i]; - let (open, close, body) = match b { - b'"' | b'\'' | b'`' => (b, b, i + 1), - b'/' if starts_expression(&code[..i]) => (b, b, i + 1), - b'%' if starts_expression(&code[..i]) => { + let before = std::str::from_utf8(&out[..i]).unwrap_or_default(); + let (lit, body) = match b { + b'#' => { + out.truncate(i); + break; + } + b'"' | b'\'' | b'`' => ( + Literal { + open: b, + close: b, + depth: 0, + }, + i + 1, + ), + b'/' if starts_expression(before) => ( + Literal { + open: b, + close: b, + depth: 0, + }, + i + 1, + ), + b'%' if starts_expression(before) => { let mut j = i + 1; - if matches!( - bytes.get(j), - Some(b'w' | b'W' | b'i' | b'I' | b'q' | b'Q' | b'r') - ) { + if bytes.get(j).is_some_and(|c| b"wWiIqQrsx".contains(c)) { j += 1; } - let Some(&open) = bytes.get(j) else { - break; - }; - if open.is_ascii_alphanumeric() || open.is_ascii_whitespace() || !open.is_ascii() { - i += 1; - continue; + match bytes.get(j) { + Some(&open) if open.is_ascii_punctuation() => { + let close = match open { + b'(' => b')', + b'[' => b']', + b'{' => b'}', + b'<' => b'>', + _ => open, + }; + ( + Literal { + open, + close, + depth: 0, + }, + j + 1, + ) + } + _ => { + i += 1; + continue; + } } - let close = match open { - b'(' => b')', - b'[' => b']', - b'{' => b'}', - b'<' => b'>', - _ => open, - }; - (open, close, j + 1) } _ => { i += 1; continue; } }; - let mut depth = 0usize; - let mut j = body; - while j < bytes.len() { - let c = bytes[j]; - if c == b'\\' { - out[j] = b'x'; - if j + 1 < bytes.len() { - out[j + 1] = b'x'; - } - j += 2; - continue; + match mask_literal(bytes, &mut out, body, lit) { + Ok(end) => i = end + 1, + // An unclosed `/` was a division after all. + Err(_) if b == b'/' => { + out[body..].copy_from_slice(&bytes[body..]); + i += 1; } - if c == close && depth == 0 { + Err(lit) => { + *literal = Some(lit); break; } - if open != close && c == open { - depth += 1; - } else if open != close && c == close { - depth -= 1; - } - out[j] = b'x'; - j += 1; } - i = j + 1; } // The delimiters are ASCII, so a masked run never cuts a multi-byte // character: the result is still UTF-8. - String::from_utf8(out).unwrap_or_else(|_| code.to_string()) + String::from_utf8(out).unwrap_or_default() } -/// The heredocs a line opens (`<<~EOS`, `<<-"EOS"`, `< Vec<(String, bool)> { +/// Masks `lit`'s body in `out` from byte `from` of `bytes`: `Ok` with the +/// closing delimiter's index, or `Err` with the literal still open at the +/// end of the line. +fn mask_literal( + bytes: &[u8], + out: &mut [u8], + from: usize, + mut lit: Literal, +) -> Result { + let mut j = from; + while j < bytes.len() { + let c = bytes[j]; + if c == b'\\' { + out[j] = b'x'; + if let Some(next) = out.get_mut(j + 1) { + *next = b'x'; + } + j += 2; + continue; + } + if c == lit.close && lit.depth == 0 { + return Ok(j); + } + if lit.open != lit.close && c == lit.open { + lit.depth += 1; + } else if lit.open != lit.close && c == lit.close { + lit.depth -= 1; + } + out[j] = b'x'; + j += 1; + } + Err(lit) +} + +/// The heredocs a line opens (`<<~EOS`, `<<-"EOS"`, `x = < Vec<(String, bool)> { let bytes = masked.as_bytes(); let mut out = Vec::new(); let mut i = 0; @@ -844,15 +952,18 @@ fn heredoc_openers(code: &str, masked: &str) -> Vec<(String, bool)> { j += 1; } let id = match bytes.get(j) { - Some(&q @ (b'"' | b'\'' | b'`')) => code[j + 1..] + Some(&q @ (b'"' | b'\'' | b'`')) => masked[j + 1..] .find(q as char) - .map(|end| &code[j + 1..j + 1 + end]), - Some(&b) if b.is_ascii_alphabetic() || b == b'_' => { + .map(|end| &line[j + 1..j + 1 + end]), + Some(&b) + if (b.is_ascii_alphabetic() || b == b'_') + && (indented || starts_expression(&masked[..at])) => + { let len = masked[j..] .bytes() .position(|b| !is_ident_byte(b)) .unwrap_or(masked.len() - j); - Some(&code[j..j + len]) + Some(&line[j..j + len]) } _ => None, }; @@ -864,26 +975,6 @@ fn heredoc_openers(code: &str, masked: &str) -> Vec<(String, bool)> { out } -/// `line` without its trailing `#` comment. A `#` inside a quoted string -/// (`"https://#{host}/"` interpolation, a URL fragment) is code, not a -/// comment. -fn ruby_code(line: &str) -> &str { - let mut quote: Option = None; - let mut escaped = false; - for (i, c) in line.char_indices() { - match quote { - Some(_) if escaped => escaped = false, - Some(_) if c == '\\' => escaped = true, - Some(q) if c == q => quote = None, - Some(_) => {} - None if c == '"' || c == '\'' => quote = Some(c), - None if c == '#' => return &line[..i], - None => {} - } - } - line -} - /// How the gem comes back into the manifest. enum Decl { /// Gone: the block was the rewriter's append for a transitive gem. @@ -1411,6 +1502,12 @@ mod tests { run(&t, &direct), "group :test do\n if x\n gem \"a\"\n end\n\n gem \"rails\", \"7.0.0\"\n gem \"b\"\nend\n" ); + // A multi-line opener: the indent of the statement's first line. + let t = format!("group :development,\n :test do\n if x\n end\n{block}\nend\n"); + assert_eq!( + run(&t, &direct), + "group :development,\n :test do\n if x\n end\n gem \"rails\", \"7.0.0\"\nend\n" + ); let t = format!("gem \"puma\"\n{block}\n"); assert_eq!( run(&t, &Decl::Direct(constraint_args("rails (>= 6, ~> 7.0)"))), @@ -1851,6 +1948,43 @@ mod tests { assert!(at( "source \"https://x\" do\n list << FOO\n end_marker = 1\n gem \"rails\"\nend\n" )); + // A multi-line `%w[` array or string is masked across lines. + assert!(!at( + "source \"https://x\" do\n gem \"a\", require: %w[\n active_support/core_ext/module\n ]\nend\ngem \"rails\"\n" + )); + assert!(!at( + "source \"https://x\" do\n NOTE = \"if\n case\"\nend\ngem \"rails\"\n" + )); + // `class <