tr: handle trailing-backslash warnings for both sets

tr checked only SET1 for an unescaped trailing backslash, so SET2 never
produced GNU's portability warning. Warning writes also went through
helpers that discard stderr errors, so redirecting a warning to /dev/full
still exited successfully.

Move terminal-backslash detection from the SET1-only pre-scan in tr.rs to
operand parse completion, so SET1 and SET2 are checked in parse order and
the warning follows any warning emitted while scanning the same operand.
Parse both operands before applying semantic validation, so SET2 warnings
and syntax errors surface ahead of a semantic rejection of a
syntactically valid SET1.

Add a tr-local checked diagnostic writer that records exit status 1 when
a warning cannot be written, without returning early, so stdin processing
and stdout output continue. Route tr's existing ambiguous-octal and
invalid-UTF-8 warnings through the same writer. The shared uucore
diagnostic macros are unchanged.

Author: EffortlessSteven

src/uu/tr/src/operation.rs

@@ -21,10 +21,33 @@ use std::{
     fmt::{Debug, Display},
     io::{BufRead, Write},
 };
-use uucore::error::{FromIo, UError, UResult};
+use uucore::error::{FromIo, UError, UResult, set_exit_code};
 use uucore::translate;
 
-use uucore::show_warning;
+/// Write a diagnostic to stderr, recording exit status 1 if the write fails.
+///
+/// Unlike `show_warning!`, this does not discard stderr write errors. The
+/// failure is recorded but not propagated, so translation of stdin continues
+/// and the translated stdout is preserved, as GNU does. A successful
+/// diagnostic leaves the stored status unchanged.
+fn emit_diagnostic(message: impl Display) {
+    let mut stderr = std::io::stderr().lock();
+
+    if writeln!(stderr, "{}: {message}", uucore::util_name()).is_err() {
+        set_exit_code(1);
+    }
+}
+
+/// As [`emit_diagnostic`], but prefixed like `show_warning!`.
+fn emit_warning(message: impl Display) {
+    emit_diagnostic(format_args!("warning: {message}"));
+}
+
+/// Whether the raw operand ends in an odd-length run of backslashes, i.e. a
+/// final backslash that is not itself escaped.
+fn has_unescaped_trailing_backslash(input: &[u8]) -> bool {
+    input.iter().rev().take_while(|&&b| b == b'\\').count() % 2 == 1
+}
 
 /// Common trait for operations that can process chunks of data
 pub trait ChunkProcessor {
@@ -217,12 +240,18 @@ impl Sequence {
     ) -> Result<(Vec<u8>, Vec<u8>), BadSequence> {
         let is_char_star = |s: &&Self| -> bool { matches!(s, Self::CharStar(_)) };
 
+        // Both operands are parsed before either is validated, so that warnings
+        // and syntax errors from set2 are reported ahead of a semantic
+        // rejection of a syntactically valid set1, as GNU does. Set1 is still
+        // parsed first, so a syntax error in it prevents set2 from being
+        // parsed at all.
         let set1 = Self::from_str(set1_str)?;
+        let mut set2 = Self::from_str(set2_str)?;
+
         if set1.iter().filter(is_char_star).count() != 0 {
             return Err(BadSequence::CharRepeatInSet1);
         }
 
-        let mut set2 = Self::from_str(set2_str)?;
         if set2.iter().filter(is_char_star).count() > 1 {
             return Err(BadSequence::MultipleCharRepeatInSet2);
         }
@@ -350,7 +379,7 @@ impl Sequence {
 
 impl Sequence {
     pub fn from_str(input: &[u8]) -> Result<Vec<Self>, BadSequence> {
-        many0(alt((
+        let parsed = many0(alt((
             Self::parse_char_range,
             Self::parse_char_star,
             Self::parse_char_repeat,
@@ -363,9 +392,17 @@ impl Sequence {
         )))
         .parse(input)
         .map(|(_, r)| r)
-        .unwrap()
-        .into_iter()
-        .collect::<Result<Vec<_>, _>>()
+        .unwrap();
+
+        // Warn once per operand, after the whole operand has been scanned, so
+        // that this warning follows any warning emitted while parsing it and
+        // precedes a syntax error found in the same operand.
+        if has_unescaped_trailing_backslash(input) {
+            // This message already carries its own localized "warning: " prefix.
+            emit_diagnostic(translate!("tr-warning-unescaped-backslash"));
+        }
+
+        parsed.into_iter().collect::<Result<Vec<_>, _>>()
     }
 
     fn parse_octal(input: &[u8]) -> IResult<&[u8], u8> {
@@ -409,12 +446,11 @@ impl Sequence {
                     if let Ok(origin_octal) = std::str::from_utf8(input) {
                         let actual_octal_tail: &str = std::str::from_utf8(&input[0..2]).unwrap();
                         let outstand_char: char = char::from_u32(input[2] as u32).unwrap();
-                        show_warning!(
-                            "{}",
+                        emit_warning(
                             translate!("tr-warning-ambiguous-octal-escape", "origin_octal" => origin_octal, "actual_octal_tail" => actual_octal_tail, "outstand_char" => outstand_char)
                         );
                     } else {
-                        show_warning!("{}", translate!("tr-warning-invalid-utf8"));
+                        emit_warning(translate!("tr-warning-invalid-utf8"));
                     }
                 }
                 result

src/uu/tr/src/tr.rs

@@ -19,7 +19,7 @@ use uucore::display::Quotable;
 use uucore::error::{UResult, USimpleError, UUsageError};
 use uucore::fs::is_stdin_directory;
 use uucore::translate;
-use uucore::{format_usage, os_str_as_bytes, show};
+use uucore::{format_usage, os_str_as_bytes};
 
 mod options {
     pub const COMPLEMENT: &str = "complement";
@@ -84,18 +84,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         }
     }
 
-    if let Some(first) = sets.first() {
-        let slice = os_str_as_bytes(first)?;
-        let trailing_backslashes = slice.iter().rev().take_while(|&&c| c == b'\\').count();
-        if trailing_backslashes % 2 == 1 {
-            // The trailing backslash has a non-backslash character before it.
-            show!(USimpleError::new(
-                0,
-                translate!("tr-warning-unescaped-backslash")
-            ));
-        }
-    }
-
     let stdin = stdin();
     let mut locked_stdin = stdin.lock();
     let mut locked_stdout = stdout().lock();

tests/by-util/test_tr.rs

@@ -1572,6 +1572,120 @@ fn test_trailing_backslash() {
         .stdout_is("abc");
 }
 
+#[test]
+fn test_trailing_backslash_warning_in_set2() {
+    new_ucmd!()
+        .args(&[".", r"\"])
+        .pipe_in(".")
+        .succeeds()
+        .stderr_is("tr: warning: an unescaped backslash at end of string is not portable\n")
+        .stdout_is("\\");
+}
+
+#[test]
+fn test_trailing_backslash_warning_in_both_sets() {
+    new_ucmd!()
+        .args(&[r"\", r"\"])
+        .pipe_in("\\")
+        .succeeds()
+        .stderr_is(concat!(
+            "tr: warning: an unescaped backslash at end of string is not portable\n",
+            "tr: warning: an unescaped backslash at end of string is not portable\n",
+        ))
+        .stdout_is("\\");
+}
+
+#[test]
+fn test_escaped_trailing_backslash_in_set2_does_not_warn() {
+    new_ucmd!()
+        .args(&[".", r"\\"])
+        .pipe_in(".")
+        .succeeds()
+        .no_stderr()
+        .stdout_is("\\");
+}
+
+#[test]
+fn test_set1_syntax_error_prevents_set2_warning() {
+    new_ucmd!()
+        .args(&["z-a", r"\"])
+        .pipe_in("")
+        .fails()
+        .stderr_only("tr: range-endpoints of 'z-a' are in reverse collating sequence order\n");
+}
+
+#[test]
+fn test_set2_warning_precedes_set1_semantic_error() {
+    new_ucmd!()
+        .args(&["[x*]", r"\"])
+        .pipe_in("")
+        .fails()
+        .stderr_is(concat!(
+            "tr: warning: an unescaped backslash at end of string is not portable\n",
+            "tr: the [c*] repeat construct may not appear in string1\n",
+        ));
+}
+
+#[test]
+fn test_set2_syntax_error_precedes_set1_semantic_error() {
+    new_ucmd!()
+        .args(&["[x*]", "z-a"])
+        .pipe_in("")
+        .fails()
+        .stderr_only("tr: range-endpoints of 'z-a' are in reverse collating sequence order\n");
+}
+
+#[test]
+fn test_set2_trailing_warning_precedes_its_syntax_error() {
+    new_ucmd!()
+        .args(&["x", r"z-a\"])
+        .pipe_in("")
+        .fails()
+        .stderr_is(concat!(
+            "tr: warning: an unescaped backslash at end of string is not portable\n",
+            "tr: range-endpoints of 'z-a' are in reverse collating sequence order\n",
+        ));
+}
+
+#[test]
+fn test_parser_warning_precedes_trailing_backslash_warning() {
+    // Only the relative warning order is relevant to this regression.
+    let result = new_ucmd!()
+        .args(&["-d", r"\501\"])
+        .pipe_in("(1Ł)")
+        .succeeds();
+    result.stdout_is("Ł)");
+
+    let stderr = result.stderr_str();
+    let ambiguous_octal = stderr.find("warning: the ambiguous octal escape").unwrap();
+    let trailing_backslash = stderr
+        .find("warning: an unescaped backslash at end of string is not portable")
+        .unwrap();
+    assert!(ambiguous_octal < trailing_backslash);
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_trailing_backslash_warning_write_failure() {
+    new_ucmd!()
+        .args(&[".", r"\"])
+        .pipe_in(".")
+        .set_stderr(std::fs::File::create("/dev/full").unwrap())
+        .fails_with_code(1)
+        .stdout_is("\\");
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_ambiguous_octal_warning_write_failure() {
+    new_ucmd!()
+        .args(&["-d", r"\501"])
+        .pipe_in("(1Ł)")
+        .set_stderr(std::fs::File::create("/dev/full").unwrap())
+        .fails_with_code(1)
+        .stdout_is("Ł)");
+}
+
 #[test]
 fn test_multibyte_octal_sequence() {
     new_ucmd!()