sed: accept Ng substitute flags and point carets at the culprit

src/sed/command.rs

@@ -137,6 +137,8 @@ pub enum RegexMode {
 pub struct ReplacementTemplate {
     pub parts: Vec<ReplacementPart>,
     pub max_group_number: usize, // Highest used group number (e.g. 8 for \8)
+    /// Where the first reference to `max_group_number` appears in the script.
+    pub max_group_location: Option<ScriptLocation>,
 }
 
 impl Default for ReplacementTemplate {
@@ -161,6 +163,7 @@ impl ReplacementTemplate {
         Self {
             parts,
             max_group_number: max_group_number.try_into().unwrap(),
+            max_group_location: None,
         }
     }
 
@@ -229,6 +232,7 @@ pub struct Substitution {
     pub regex: Option<Regex>,                         // Regular expression
     pub replacement: ReplacementTemplate,             // Specified broken-down replacement
     pub occurrence: usize,                            // Which occurrence to substitute
+    pub global: bool,                                 // True if 'g' flag: also every later one
     pub print_flag: bool,                             // True if 'p' flag
     pub p_before_e: bool,                             // True if 'p' appears before 'e'
     pub ignore_case: bool,                            // True if 'I' flag

src/sed/compiler.rs

@@ -729,6 +729,7 @@ pub fn compile_replacement(
 ) -> UResult<ReplacementTemplate> {
     let mut parts = Vec::new();
     let mut literal = Vec::new();
+    let mut max_group: Option<(u32, ScriptLocation)> = None;
 
     let delimiter = line.current();
     line.advance();
@@ -764,6 +765,12 @@ pub fn compile_replacement(
                             if ref_num == 0 {
                                 parts.push(ReplacementPart::WholeMatch);
                             } else {
+                                if max_group.as_ref().is_none_or(|(n, _)| ref_num > *n) {
+                                    // Point at the backslash, one byte back.
+                                    let mut location = ScriptLocation::at_position(lines, line);
+                                    location.column_number -= 1;
+                                    max_group = Some((ref_num, location));
+                                }
                                 parts.push(ReplacementPart::Group(ref_num));
                             }
                             line.advance();
@@ -815,7 +822,9 @@ pub fn compile_replacement(
                     if !literal.is_empty() {
                         parts.push(ReplacementPart::Literal(literal));
                     }
-                    return Ok(ReplacementTemplate::new(parts));
+                    let mut template = ReplacementTemplate::new(parts);
+                    template.max_group_location = max_group.map(|(_, location)| location);
+                    return Ok(template);
                 }
 
                 _ => {
@@ -889,14 +898,14 @@ fn compile_subst_command(
     if let Some(regex) = &subst.regex
         && subst.replacement.max_group_number > regex.captures_len() - 1
     {
-        return compilation_error(
-            lines,
-            line,
-            format!(
-                "invalid reference \\{} on `s' command's RHS",
-                subst.replacement.max_group_number
-            ),
+        let msg = format!(
+            "invalid reference \\{} on `s' command's RHS",
+            subst.replacement.max_group_number
         );
+        return match &subst.replacement.max_group_location {
+            Some(location) => semantic_error(location, msg),
+            None => compilation_error(lines, line, msg),
+        };
     }
     cmd.data = CommandData::Substitution(subst);
 
@@ -958,9 +967,10 @@ pub fn compile_subst_flags(
     posix: bool,
     sandbox: bool,
 ) -> UResult<()> {
-    let mut seen_g_or_n = false;
+    let mut seen_number = false;
 
     subst.occurrence = 1; // default
+    subst.global = false;
     subst.print_flag = false;
     subst.p_before_e = false;
     subst.ignore_case = false;
@@ -976,15 +986,10 @@ pub fn compile_subst_flags(
 
         match line.current() {
             'g' => {
-                if seen_g_or_n {
-                    return compilation_error(
-                        lines,
-                        line,
-                        "multiple 'g' or numeric flags in substitute command",
-                    );
+                if subst.global {
+                    return compilation_error(lines, line, "multiple `g' options to `s' command");
                 }
-                seen_g_or_n = true;
-                subst.occurrence = 0;
+                subst.global = true;
                 line.advance();
             }
 
@@ -1024,11 +1029,11 @@ pub fn compile_subst_flags(
             }
 
             _c @ '1'..='9' => {
-                if seen_g_or_n {
+                if seen_number {
                     return compilation_error(
                         lines,
                         line,
-                        "multiple 'g' or numeric flags in substitute command",
+                        "multiple number options to `s' command",
                     );
                 }
 
@@ -1049,7 +1054,7 @@ pub fn compile_subst_flags(
                 }
 
                 subst.occurrence = number;
-                seen_g_or_n = true;
+                seen_number = true;
             }
 
             'w' => {
@@ -2601,7 +2606,8 @@ mod tests {
         let mut subst = Substitution::default();
 
         compile_subst_flags(&lines, &mut chars, &mut subst, false, false).unwrap();
-        assert_eq!(subst.occurrence, 0); // 'g' means all occurrences
+        assert_eq!(subst.occurrence, 1);
+        assert!(subst.global);
     }
 
     #[test]
@@ -2659,26 +2665,38 @@ mod tests {
     }
 
     #[test]
-    fn test_compile_subst_flag_g_and_number_should_fail() {
-        let (lines, mut chars) = make_providers("g3");
+    fn test_compile_subst_flag_g_and_number() {
+        for flags in ["g3", "3g"] {
+            let (lines, mut chars) = make_providers(flags);
+            let mut subst = Substitution::default();
+
+            compile_subst_flags(&lines, &mut chars, &mut subst, false, false).unwrap();
+            assert_eq!(subst.occurrence, 3);
+            assert!(subst.global);
+        }
+    }
+
+    #[test]
+    fn test_compile_subst_flag_multiple_g_should_fail() {
+        let (lines, mut chars) = make_providers("gpg");
         let mut subst = Substitution::default();
 
         let err = compile_subst_flags(&lines, &mut chars, &mut subst, false, false).unwrap_err();
         assert!(
             err.to_string()
-                .contains("multiple 'g' or numeric flags in substitute command")
+                .contains(":3: error: multiple `g' options to `s' command")
         );
     }
 
     #[test]
-    fn test_compile_subst_flag_number_and_g_should_fail() {
-        let (lines, mut chars) = make_providers("2g");
+    fn test_compile_subst_flag_multiple_numbers_should_fail() {
+        let (lines, mut chars) = make_providers("2g3");
         let mut subst = Substitution::default();
 
         let err = compile_subst_flags(&lines, &mut chars, &mut subst, false, false).unwrap_err();
         assert!(
             err.to_string()
-                .contains("multiple 'g' or numeric flags in substitute command")
+                .contains(":3: error: multiple number options to `s' command")
         );
     }
 

src/sed/processor.rs

@@ -305,8 +305,8 @@ fn substitute(
     // The following let block allows a common input_runtime_error to be
     // called once in all cases, and most importantly, to finish the regex
     // mutable borrowing of context, so as to reuse context in the error call.
-    let subst_result = match (sub.occurrence, sub.replacement.max_group_number) {
-        (1, 0) => {
+    let subst_result = match (sub.occurrence, sub.global, sub.replacement.max_group_number) {
+        (1, false, 0) => {
             // Example: s/foo/bar/: find() is enough.
             match regex.find(pattern) {
                 Err(e) => Err(e),
@@ -323,7 +323,7 @@ fn substitute(
             }
         }
 
-        (1, _) => {
+        (1, false, _) => {
             // Example: s/\(.\)\(.\)/\2\1/: captures() is enough.
             match regex.captures(pattern) {
                 Err(e) => Err(e),
@@ -341,7 +341,7 @@ fn substitute(
             }
         }
 
-        (_, _) => {
+        (_, _, _) => {
             // Example: s/(.)(.)/\2\1/3: captures_iter() is needed.
             // Iterate over multiple captures of the RE in the pattern.
             'captures: {
@@ -357,7 +357,7 @@ fn substitute(
                     // Always write the unmatched text before this match.
                     result.extend_from_slice(&text[last_end..m.start()]);
 
-                    if sub.occurrence == 0 || count == sub.occurrence {
+                    if count == sub.occurrence || (sub.global && count > sub.occurrence) {
                         let replacement = sub.replacement.apply_captures(command, &caps)?;
                         result.extend_from_slice(&replacement);
                         replaced = true;
@@ -370,7 +370,7 @@ fn substitute(
 
                     // Early exit if only a specific occurrence,
                     // (likely 1) needed replacing.
-                    if count == sub.occurrence {
+                    if !sub.global && count == sub.occurrence {
                         break 'captures Ok(());
                     }
                 }

tests/by-util/test_sed.rs

@@ -579,6 +579,22 @@ fn subst_multiline_flag_matches_embedded_line_end() {
         .stdout_is("foX\nbaX\n");
 }
 
+#[test]
+fn subst_number_and_g_replace_from_that_match_on() {
+    for flags in ["2g", "g2"] {
+        new_ucmd!()
+            .arg(format!("s/a/b/{flags}"))
+            .pipe_in("aaaa\n")
+            .succeeds()
+            .stdout_is("abbb\n");
+    }
+    new_ucmd!()
+        .arg(r"s/\(a\)/<\1>/3g")
+        .pipe_in("aaaa\n")
+        .succeeds()
+        .stdout_is("aa<a><a>\n");
+}
+
 // Check appropriate selection and behavior of fast_Regex matcher
 // Literal matcher
 check_output!(subst_literal_start, ["-e", r"s/^l1/L1/", LINES1]);
@@ -1271,6 +1287,43 @@ fn diagnostics_reach_errors_raised_after_the_line_was_read() {
         ));
 }
 
+#[test]
+fn diagnostics_underline_the_invalid_back_reference() {
+    // The message is raised after the flags are read; the caret still
+    // points at the first `\3`, not past the end of the command.
+    new_ucmd!()
+        .env("UUTILS_DIAG", "always")
+        .args(&[r"s/\(a\)/x\3\1\3/"])
+        .fails()
+        .code_is(1)
+        .stderr_is(concat!(
+            "sed: <script argument 1>:1:10: error: invalid reference \\3 on `s' command's RHS\n",
+            "   ╭─[ <script argument 1>:1:10 ]\n",
+            "   │\n",
+            r" 1 │ s/\(a\)/x\3\1\3/",
+            "\n",
+            "   │          ─\n",
+            "───╯\n",
+        ));
+}
+
+#[test]
+fn diagnostics_underline_the_second_number_flag() {
+    new_ucmd!()
+        .env("UUTILS_DIAG", "always")
+        .args(&["s/a/b/2g3"])
+        .fails()
+        .code_is(1)
+        .stderr_is(concat!(
+            "sed: <script argument 1>:1:9: error: multiple number options to `s' command\n",
+            "   ╭─[ <script argument 1>:1:9 ]\n",
+            "   │\n",
+            " 1 │ s/a/b/2g3\n",
+            "   │         ─\n",
+            "───╯\n",
+        ));
+}
+
 #[test]
 fn diagnostics_line_up_under_wide_characters() {
     // `日本` takes four columns: the underline must sit under `q`.