diff --git a/src/uu/tr/locales/en-US.ftl b/src/uu/tr/locales/en-US.ftl index f91f2de3033..e4ef235efba 100644 --- a/src/uu/tr/locales/en-US.ftl +++ b/src/uu/tr/locales/en-US.ftl @@ -22,8 +22,8 @@ tr-error-write-error = write error # Warning messages tr-warning-unescaped-backslash = warning: an unescaped backslash at end of string is not portable -tr-warning-ambiguous-octal-escape = the ambiguous octal escape \{ $origin_octal } is being interpreted as the 2-byte sequence \0{ $actual_octal_tail }, { $outstand_char } -tr-warning-invalid-utf8 = invalid utf8 sequence +tr-warning-ambiguous-octal-escape = warning: the ambiguous octal escape \{ $origin_octal } is being interpreted as the 2-byte sequence \0{ $actual_octal_tail }, { $outstand_char } +tr-warning-invalid-utf8 = warning: invalid utf8 sequence # Sequence parsing error messages tr-error-missing-char-class-name = missing character class name '[::]' diff --git a/src/uu/tr/locales/fr-FR.ftl b/src/uu/tr/locales/fr-FR.ftl index 0b59ea133b0..9a293ad8b8d 100644 --- a/src/uu/tr/locales/fr-FR.ftl +++ b/src/uu/tr/locales/fr-FR.ftl @@ -22,9 +22,9 @@ tr-error-write-error = erreur d'écriture # Messages d'avertissement tr-warning-unescaped-backslash = avertissement : une barre oblique inverse non échappée à la fin de la chaîne n'est pas portable -tr-warning-ambiguous-octal-escape = l'échappement octal ambigu \{ $origin_octal } est en cours +tr-warning-ambiguous-octal-escape = avertissement : l'échappement octal ambigu \{ $origin_octal } est en cours d'interprétation comme la séquence de 2 octets \0{ $actual_octal_tail }, { $outstand_char } -tr-warning-invalid-utf8 = séquence UTF-8 non valide +tr-warning-invalid-utf8 = avertissement : séquence UTF-8 non valide # Messages d'erreur d'analyse de séquence tr-error-missing-char-class-name = nom de classe de caractères manquant '[::]' diff --git a/src/uu/tr/src/operation.rs b/src/uu/tr/src/operation.rs index 44c28c160c8..11204bbe1a0 100644 --- a/src/uu/tr/src/operation.rs +++ b/src/uu/tr/src/operation.rs @@ -21,10 +21,28 @@ 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); + } +} + +/// 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 +235,18 @@ impl Sequence { ) -> Result<(Vec, Vec), 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 +374,7 @@ impl Sequence { impl Sequence { pub fn from_str(input: &[u8]) -> Result, BadSequence> { - many0(alt(( + let parsed = many0(alt(( Self::parse_char_range, Self::parse_char_star, Self::parse_char_repeat, @@ -363,9 +387,16 @@ impl Sequence { ))) .parse(input) .map(|(_, r)| r) - .unwrap() - .into_iter() - .collect::, _>>() + .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) { + emit_diagnostic(translate!("tr-warning-unescaped-backslash")); + } + + parsed.into_iter().collect::, _>>() } fn parse_octal(input: &[u8]) -> IResult<&[u8], u8> { @@ -409,12 +440,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_diagnostic( 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_diagnostic(translate!("tr-warning-invalid-utf8")); } } result diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index 58976d3ac06..857d0748ee0 100644 --- a/src/uu/tr/src/tr.rs +++ b/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(); diff --git a/tests/by-util/test_tr.rs b/tests/by-util/test_tr.rs index 066474fc985..b4bcd6a128f 100644 --- a/tests/by-util/test_tr.rs +++ b/tests/by-util/test_tr.rs @@ -1572,6 +1572,116 @@ 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_warning_write_failures() { + // A warning that cannot be written sets the exit status without discarding + // the translated output, for both the trailing-backslash and the parser + // warning paths. + for (args, input, expected_stdout) in + [([".", r"\"], ".", "\\"), (["-d", r"\501"], "(1Ł)", "Ł)")] + { + new_ucmd!() + .args(&args) + .pipe_in(input) + .set_stderr(std::fs::File::create("/dev/full").unwrap()) + .fails_with_code(1) + .stdout_is(expected_stdout); + } +} + #[test] fn test_multibyte_octal_sequence() { new_ucmd!()