From 3300d80e3ffacf279748cc077b0bebb2688f48b6 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Sat, 25 Sep 2021 11:02:49 +0200 Subject: [PATCH 1/7] tests: silence clippy warnings for unused_imports --- tests/test_util_name.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_util_name.rs b/tests/test_util_name.rs index b0a78a2e8..76b6f4728 100644 --- a/tests/test_util_name.rs +++ b/tests/test_util_name.rs @@ -1,3 +1,4 @@ +#![allow(unused_imports)] mod common; use common::util::TestScenario; @@ -25,6 +26,7 @@ fn execution_phrase_double() { #[test] #[cfg(feature = "ls")] +#[cfg(any(unix, windows))] fn execution_phrase_single() { use std::process::Command; @@ -63,6 +65,7 @@ fn util_name_double() { #[test] #[cfg(feature = "sort")] +#[cfg(any(unix, windows))] fn util_name_single() { use std::{ io::Write, From 7c5395a27a0d9fafef98bad2ae4ad3e0876bab72 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Mon, 18 Oct 2021 23:47:09 +0200 Subject: [PATCH 2/7] build.rs: silence clippy warnings --- build.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/build.rs b/build.rs index 4fbb27cce..261eb5d9a 100644 --- a/build.rs +++ b/build.rs @@ -46,6 +46,8 @@ pub fn main() { "type UtilityMap = HashMap<&'static str, (fn(T) -> i32, fn() -> App<'static, 'static>)>;\n\ \n\ fn util_map() -> UtilityMap {\n\ + \t#[allow(unused_mut)]\n\ + \t#[allow(clippy::let_and_return)]\n\ \tlet mut map = UtilityMap::new();\n\ " .as_bytes(), From 5e5bb91043eb598b39adf46410f97b9ae5dbcf03 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 23 Oct 2021 23:21:45 -0300 Subject: [PATCH 3/7] sort: remove unecessary implementation --- src/uu/sort/src/sort.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index bd79a6811..84ded8ba9 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -171,10 +171,6 @@ impl UError for SortError { _ => 2, } } - - fn usage(&self) -> bool { - false - } } impl Display for SortError { From 21a2d0ce408166eac3850892e7ce2ed54488a20d Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 23 Oct 2021 23:22:21 -0300 Subject: [PATCH 4/7] sort: use Range.is_empty --- src/uu/sort/src/sort.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 84ded8ba9..ae1bcfa4c 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -659,18 +659,14 @@ impl<'a> Line<'a> { " ".repeat(UnicodeWidthStr::width(&line[..selection.start])) )?; - // TODO: Once our minimum supported rust version is at least 1.47, use selection.is_empty() instead. - #[allow(clippy::len_zero)] - { - if selection.len() == 0 { - writeln!(writer, "^ no match for key")?; - } else { - writeln!( - writer, - "{}", - "_".repeat(UnicodeWidthStr::width(&line[selection])) - )?; - } + if selection.is_empty() { + writeln!(writer, "^ no match for key")?; + } else { + writeln!( + writer, + "{}", + "_".repeat(UnicodeWidthStr::width(&line[selection])) + )?; } } if settings.mode != SortMode::Random From 1f15b8fce4b30c805aec5ea46360134fda82bc04 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 23 Oct 2021 23:25:22 -0300 Subject: [PATCH 5/7] cksum: use while loops instead of unroll! --- src/uu/cksum/src/cksum.rs | 73 +++++---------------------------------- 1 file changed, 9 insertions(+), 64 deletions(-) diff --git a/src/uu/cksum/src/cksum.rs b/src/uu/cksum/src/cksum.rs index 55c4f980f..92853a3e8 100644 --- a/src/uu/cksum/src/cksum.rs +++ b/src/uu/cksum/src/cksum.rs @@ -25,60 +25,15 @@ const NAME: &str = "cksum"; const SYNTAX: &str = "[OPTIONS] [FILE]..."; const SUMMARY: &str = "Print CRC and size for each file"; -// this is basically a hack to get "loops" to work on Rust 1.33. Once we update to Rust 1.46 or -// greater, we can just use while loops -macro_rules! unroll { - (256, |$i:ident| $s:expr) => {{ - unroll!(@ 32, 0 * 32, $i, $s); - unroll!(@ 32, 1 * 32, $i, $s); - unroll!(@ 32, 2 * 32, $i, $s); - unroll!(@ 32, 3 * 32, $i, $s); - unroll!(@ 32, 4 * 32, $i, $s); - unroll!(@ 32, 5 * 32, $i, $s); - unroll!(@ 32, 6 * 32, $i, $s); - unroll!(@ 32, 7 * 32, $i, $s); - }}; - (8, |$i:ident| $s:expr) => {{ - unroll!(@ 8, 0, $i, $s); - }}; - - (@ 32, $start:expr, $i:ident, $s:expr) => {{ - unroll!(@ 8, $start + 0 * 8, $i, $s); - unroll!(@ 8, $start + 1 * 8, $i, $s); - unroll!(@ 8, $start + 2 * 8, $i, $s); - unroll!(@ 8, $start + 3 * 8, $i, $s); - }}; - (@ 8, $start:expr, $i:ident, $s:expr) => {{ - unroll!(@ 4, $start, $i, $s); - unroll!(@ 4, $start + 4, $i, $s); - }}; - (@ 4, $start:expr, $i:ident, $s:expr) => {{ - unroll!(@ 2, $start, $i, $s); - unroll!(@ 2, $start + 2, $i, $s); - }}; - (@ 2, $start:expr, $i:ident, $s:expr) => {{ - unroll!(@ 1, $start, $i, $s); - unroll!(@ 1, $start + 1, $i, $s); - }}; - (@ 1, $start:expr, $i:ident, $s:expr) => {{ - let $i = $start; - let _ = $s; - }}; -} - const fn generate_crc_table() -> [u32; CRC_TABLE_LEN] { let mut table = [0; CRC_TABLE_LEN]; - // NOTE: works on Rust 1.46 - //let mut i = 0; - //while i < CRC_TABLE_LEN { - // table[i] = crc_entry(i as u8) as u32; - // - // i += 1; - //} - unroll!(256, |i| { + let mut i = 0; + while i < CRC_TABLE_LEN { table[i] = crc_entry(i as u8) as u32; - }); + + i += 1; + } table } @@ -86,19 +41,8 @@ const fn generate_crc_table() -> [u32; CRC_TABLE_LEN] { const fn crc_entry(input: u8) -> u32 { let mut crc = (input as u32) << 24; - // NOTE: this does not work on Rust 1.33, but *does* on 1.46 - //let mut i = 0; - //while i < 8 { - // if crc & 0x8000_0000 != 0 { - // crc <<= 1; - // crc ^= 0x04c1_1db7; - // } else { - // crc <<= 1; - // } - // - // i += 1; - //} - unroll!(8, |_i| { + let mut i = 0; + while i < 8 { let if_condition = crc & 0x8000_0000; let if_body = (crc << 1) ^ 0x04c1_1db7; let else_body = crc << 1; @@ -108,7 +52,8 @@ const fn crc_entry(input: u8) -> u32 { let condition_table = [else_body, if_body]; crc = condition_table[(if_condition != 0) as usize]; - }); + i += 1; + } crc } From ea9b23984125cb5bf82afda05eb62bd460a811bc Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sun, 24 Oct 2021 00:03:33 -0300 Subject: [PATCH 6/7] uniq: use UResult --- src/uu/uniq/src/uniq.rs | 118 +++++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 56 deletions(-) diff --git a/src/uu/uniq/src/uniq.rs b/src/uu/uniq/src/uniq.rs index 382118bdb..c6c6a444d 100644 --- a/src/uu/uniq/src/uniq.rs +++ b/src/uu/uniq/src/uniq.rs @@ -5,17 +5,14 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -#[macro_use] -extern crate uucore; - use clap::{crate_version, App, Arg, ArgMatches}; use std::fs::File; -use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Result, Write}; +use std::io::{self, stdin, stdout, BufRead, BufReader, BufWriter, Read, Write}; use std::path::Path; use std::str::FromStr; use strum_macros::{AsRefStr, EnumString}; use uucore::display::Quotable; -use uucore::error::{UResult, USimpleError}; +use uucore::error::{FromIo, UResult, USimpleError}; static ABOUT: &str = "Report or omit repeated lines."; pub mod options { @@ -56,36 +53,45 @@ struct Uniq { zero_terminated: bool, } +macro_rules! write_line_terminator { + ($writer:expr, $line_terminator:expr) => { + $writer + .write_all(&[$line_terminator]) + .map_err_context(|| "Could not write line terminator".to_string()) + }; +} + impl Uniq { pub fn print_uniq( &self, reader: &mut BufReader, writer: &mut BufWriter, - ) { + ) -> UResult<()> { let mut first_line_printed = false; let mut group_count = 1; let line_terminator = self.get_line_terminator(); let mut lines = reader.split(line_terminator).map(get_line_string); let mut line = match lines.next() { - Some(l) => l, - None => return, + Some(l) => l?, + None => return Ok(()), }; // compare current `line` with consecutive lines (`next_line`) of the input // and if needed, print `line` based on the command line options provided for next_line in lines { + let next_line = next_line?; if self.cmp_keys(&line, &next_line) { if (group_count == 1 && !self.repeats_only) || (group_count > 1 && !self.uniques_only) { - self.print_line(writer, &line, group_count, first_line_printed); + self.print_line(writer, &line, group_count, first_line_printed)?; first_line_printed = true; } line = next_line; group_count = 1; } else { if self.all_repeated { - self.print_line(writer, &line, group_count, first_line_printed); + self.print_line(writer, &line, group_count, first_line_printed)?; first_line_printed = true; line = next_line; } @@ -93,14 +99,15 @@ impl Uniq { } } if (group_count == 1 && !self.repeats_only) || (group_count > 1 && !self.uniques_only) { - self.print_line(writer, &line, group_count, first_line_printed); + self.print_line(writer, &line, group_count, first_line_printed)?; first_line_printed = true; } if (self.delimiters == Delimiters::Append || self.delimiters == Delimiters::Both) && first_line_printed { - crash_if_err!(1, writer.write_all(&[line_terminator])); + write_line_terminator!(writer, line_terminator)?; } + Ok(()) } fn skip_fields<'a>(&self, line: &'a str) -> &'a str { @@ -192,41 +199,43 @@ impl Uniq { line: &str, count: usize, first_line_printed: bool, - ) { + ) -> UResult<()> { let line_terminator = self.get_line_terminator(); if self.should_print_delimiter(count, first_line_printed) { - crash_if_err!(1, writer.write_all(&[line_terminator])); + write_line_terminator!(writer, line_terminator)?; } - crash_if_err!( - 1, - if self.show_counts { - writer.write_all(format!("{:7} {}", count, line).as_bytes()) - } else { - writer.write_all(line.as_bytes()) - } - ); - crash_if_err!(1, writer.write_all(&[line_terminator])); + if self.show_counts { + writer.write_all(format!("{:7} {}", count, line).as_bytes()) + } else { + writer.write_all(line.as_bytes()) + } + .map_err_context(|| "Failed to write line".to_string())?; + + write_line_terminator!(writer, line_terminator) } } -fn get_line_string(io_line: Result>) -> String { - let line_bytes = crash_if_err!(1, io_line); - crash_if_err!(1, String::from_utf8(line_bytes)) +fn get_line_string(io_line: io::Result>) -> UResult { + let line_bytes = io_line.map_err_context(|| "failed to split lines".to_string())?; + String::from_utf8(line_bytes) + .map_err(|e| USimpleError::new(1, format!("failed to convert line to utf8: {}", e))) } -fn opt_parsed(opt_name: &str, matches: &ArgMatches) -> Option { - matches.value_of(opt_name).map(|arg_str| { - let opt_val: Option = arg_str.parse().ok(); - opt_val.unwrap_or_else(|| { - crash!( +fn opt_parsed(opt_name: &str, matches: &ArgMatches) -> UResult> { + Ok(match matches.value_of(opt_name) { + Some(arg_str) => Some(arg_str.parse().map_err(|_| { + USimpleError::new( 1, - "Invalid argument for {}: {}", - opt_name, - arg_str.maybe_quote() + format!( + "Invalid argument for {}: {}", + opt_name, + arg_str.maybe_quote() + ), ) - }) + })?), + None => None, }) } @@ -266,8 +275,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { 1 => (files[0].clone(), "-".to_owned()), 2 => (files[0].clone(), files[1].clone()), _ => { - // Cannot happen as clap will fail earlier - return Err(USimpleError::new(1, format!("Extra operand: {}", files[2]))); + unreachable!() // Cannot happen as clap will fail earlier } }; @@ -279,18 +287,16 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { || matches.is_present(options::GROUP), delimiters: get_delimiter(&matches), show_counts: matches.is_present(options::COUNT), - skip_fields: opt_parsed(options::SKIP_FIELDS, &matches), - slice_start: opt_parsed(options::SKIP_CHARS, &matches), - slice_stop: opt_parsed(options::CHECK_CHARS, &matches), + skip_fields: opt_parsed(options::SKIP_FIELDS, &matches)?, + slice_start: opt_parsed(options::SKIP_CHARS, &matches)?, + slice_stop: opt_parsed(options::CHECK_CHARS, &matches)?, ignore_case: matches.is_present(options::IGNORE_CASE), zero_terminated: matches.is_present(options::ZERO_TERMINATED), }; uniq.print_uniq( - &mut open_input_file(in_file_name), - &mut open_output_file(out_file_name), - ); - - Ok(()) + &mut open_input_file(in_file_name)?, + &mut open_output_file(out_file_name)?, + ) } pub fn uu_app() -> App<'static, 'static> { @@ -390,7 +396,7 @@ fn get_delimiter(matches: &ArgMatches) -> Delimiters { .value_of(options::ALL_REPEATED) .or_else(|| matches.value_of(options::GROUP)); if let Some(delimiter_arg) = value { - crash_if_err!(1, Delimiters::from_str(delimiter_arg)) + Delimiters::from_str(delimiter_arg).unwrap() // All possible values for ALL_REPEATED are &str's of Delimiters } else if matches.is_present(options::GROUP) { Delimiters::Separate } else { @@ -398,26 +404,26 @@ fn get_delimiter(matches: &ArgMatches) -> Delimiters { } } -fn open_input_file(in_file_name: String) -> BufReader> { +fn open_input_file(in_file_name: String) -> UResult>> { let in_file = if in_file_name == "-" { Box::new(stdin()) as Box } else { let path = Path::new(&in_file_name[..]); - let in_file = File::open(&path); - let r = crash_if_err!(1, in_file); - Box::new(r) as Box + let in_file = File::open(&path) + .map_err_context(|| format!("Could not open {}", in_file_name.maybe_quote()))?; + Box::new(in_file) as Box }; - BufReader::new(in_file) + Ok(BufReader::new(in_file)) } -fn open_output_file(out_file_name: String) -> BufWriter> { +fn open_output_file(out_file_name: String) -> UResult>> { let out_file = if out_file_name == "-" { Box::new(stdout()) as Box } else { let path = Path::new(&out_file_name[..]); - let in_file = File::create(&path); - let w = crash_if_err!(1, in_file); - Box::new(w) as Box + let out_file = File::create(&path) + .map_err_context(|| format!("Could not create {}", out_file_name.maybe_quote()))?; + Box::new(out_file) as Box }; - BufWriter::new(out_file) + Ok(BufWriter::new(out_file)) } From 1d8381064a69c3a14f5023b730b6fecd4673199a Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sun, 24 Oct 2021 02:16:22 -0300 Subject: [PATCH 7/7] tests/uniq: update test --- tests/by-util/test_uniq.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/by-util/test_uniq.rs b/tests/by-util/test_uniq.rs index c191ffcaf..cd013891b 100644 --- a/tests/by-util/test_uniq.rs +++ b/tests/by-util/test_uniq.rs @@ -145,7 +145,9 @@ fn test_invalid_utf8() { .arg("not-utf8-sequence.txt") .run() .failure() - .stderr_only("uniq: invalid utf-8 sequence of 1 bytes from index 0"); + .stderr_only( + "uniq: failed to convert line to utf8: invalid utf-8 sequence of 1 bytes from index 0", + ); } #[test]