Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lint: fix lints of new Rust version #6102

Merged
merged 17 commits into from
Mar 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
21d6eab
lint: fix lint `clippy::suspicious_open_options`
Krysztal112233 Mar 21, 2024
63d92cd
lint: fix `clippy::useless_vec` in `unit_tests.rs`
Krysztal112233 Mar 21, 2024
0bb1793
lint: fix `clippy::bool_assert_comparison` in `perms.rs`
Krysztal112233 Mar 21, 2024
72b7266
lint: allow `clippy::needless_borrow` in `perms.rs`
Krysztal112233 Mar 21, 2024
b66d6df
lint: fix `clippy::needless_borrows_for_generic_args` in `perms.rs`
Krysztal112233 Mar 21, 2024
eb3fac3
lint: fix `clippy::needless_borrows_for_generic_args` in `perms.rs`
Krysztal112233 Mar 21, 2024
ee09f27
lint: fix `clippy::useless_format` in `test_cksum.rs`
Krysztal112233 Mar 21, 2024
fd36404
lint: fix `clippy::unnecessary_to_owned` in `test_cksum.rs`
Krysztal112233 Mar 21, 2024
2b5e7ca
lint: fix `clippy::manual_str_repeat` in `parse_glob.rs`
Krysztal112233 Mar 21, 2024
48e376e
lint: allow `clippy::suspicious_open_options` in `sort.rs`
Krysztal112233 Mar 21, 2024
35c39a6
Merge branch 'uutils:main' into main
Krysztal112233 Mar 22, 2024
ef8c379
lint: fix `clippy::redundant_clone` of `test_dd.rs` `test_cp.rs`
Krysztal112233 Mar 22, 2024
d21dc12
lint: fix `clippy::suspicious_open_options` of project.
Krysztal112233 Mar 23, 2024
1484d06
lint: fix `clippy::manual_main_separator_str` for `util.rs` on Window…
Krysztal112233 Mar 23, 2024
6f5dfa3
lint: fix `unused_imports` of `util.rs`
Krysztal112233 Mar 23, 2024
a61761f
util: fix compile failed on Windows.
Krysztal112233 Mar 23, 2024
29d14cb
lint: fix `unused_imports` on freebsd target.
Krysztal112233 Mar 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .clippy.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
msrv = "1.70.0"
cognitive-complexity-threshold = 10
cognitive-complexity-threshold = 24
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait this was super low all this time??? No wonder it was so annoying. Let's just remove this line; the default value is 25 according to the documentation

Copy link
Member

@tertsdiepraam tertsdiepraam Mar 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sylvestre I see that you introduced the lower value. I think 10 is a bit too low, but I'm happy to compromise on something inbetween as well. I think the problem with 10 is that it's a good hint that a function is too complex, but too restrictive to enforce.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, it was to identify too long/complex functions. I am happy where we are now with 25
thanks

2 changes: 2 additions & 0 deletions src/uu/dd/src/parseargs/unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::parseargs::Parser;
use crate::StatusLevel;

#[cfg(not(any(target_os = "linux", target_os = "android")))]
#[allow(clippy::useless_vec)]
sylvestre marked this conversation as resolved.
Show resolved Hide resolved
#[test]
fn unimplemented_flags_should_error_non_linux() {
let mut succeeded = Vec::new();
Expand Down Expand Up @@ -55,6 +56,7 @@ fn unimplemented_flags_should_error_non_linux() {
}

#[test]
#[allow(clippy::useless_vec)]
fn unimplemented_flags_should_error() {
let mut succeeded = Vec::new();

Expand Down
1 change: 1 addition & 0 deletions src/uu/sort/src/sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ impl Output {
let file = if let Some(name) = name {
// This is different from `File::create()` because we don't truncate the output yet.
// This allows using the output file as an input file.
#[allow(clippy::suspicious_open_options)]
let file = OpenOptions::new()
.write(true)
.create(true)
Expand Down
21 changes: 11 additions & 10 deletions src/uucore/src/lib/features/perms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,10 +660,11 @@ mod tests {
assert_eq!(path.to_str(), Some(""));
// The main point to test here is that we don't crash.
// The result should be 'false', to avoid unnecessary and confusing warnings.
assert_eq!(false, is_root(&path, false));
assert_eq!(false, is_root(&path, true));
assert!(!is_root(&path, false));
assert!(!is_root(&path, true));
}

#[allow(clippy::needless_borrow)]
#[cfg(unix)]
#[test]
fn test_literal_root() {
Expand All @@ -675,16 +676,16 @@ mod tests {
"cfg(unix) but using non-unix path delimiters?!"
);
// Must return true, this is the main scenario that --preserve-root shall prevent.
assert_eq!(true, is_root(&path, false));
assert_eq!(true, is_root(&path, true));
assert!(is_root(&path, false));
assert!(is_root(&path, true));
}

#[cfg(unix)]
#[test]
fn test_symlink_slash() {
let temp_dir = tempdir().unwrap();
let symlink_path = temp_dir.path().join("symlink");
unix::fs::symlink(&PathBuf::from("/"), &symlink_path).unwrap();
unix::fs::symlink(PathBuf::from("/"), symlink_path).unwrap();
let symlink_path_slash = temp_dir.path().join("symlink/");
// Must return true, we're about to "accidentally" recurse on "/",
// since "symlink/" always counts as an already-entered directory
Expand All @@ -697,8 +698,8 @@ mod tests {
// chown: it is dangerous to operate recursively on 'slink-to-root/' (same as '/')
// chown: use --no-preserve-root to override this failsafe
// [$? = 1]
assert_eq!(true, is_root(&symlink_path_slash, false));
assert_eq!(true, is_root(&symlink_path_slash, true));
assert!(is_root(&symlink_path_slash, false));
assert!(is_root(&symlink_path_slash, true));
}

#[cfg(unix)]
Expand All @@ -707,9 +708,9 @@ mod tests {
// This covers both the commandline-argument case and the recursion case.
let temp_dir = tempdir().unwrap();
let symlink_path = temp_dir.path().join("symlink");
unix::fs::symlink(&PathBuf::from("/"), &symlink_path).unwrap();
unix::fs::symlink(PathBuf::from("/"), &symlink_path).unwrap();
// Only return true we're about to "accidentally" recurse on "/".
assert_eq!(false, is_root(&symlink_path, false));
assert_eq!(true, is_root(&symlink_path, true));
assert!(!is_root(&symlink_path, false));
assert!(is_root(&symlink_path, true));
}
}
2 changes: 1 addition & 1 deletion src/uucore/src/lib/parser/parse_glob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ mod tests {

// test that we don't look for closing square brackets unnecessarily
// Verifies issue #5584
let chars = std::iter::repeat("^[").take(174571).collect::<String>();
let chars = "^[".repeat(174571);
assert_eq!(fix_negation(chars.as_str()), chars);
}

Expand Down
2 changes: 1 addition & 1 deletion tests/by-util/test_cksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ fn test_base64_multiple_files() {
.arg("alice_in_wonderland.txt")
.succeeds()
.no_stderr()
.stdout_is_fixture_bytes(format!("base64/md5_multiple_files.expected"));
.stdout_is_fixture_bytes("base64/md5_multiple_files.expected");
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion tests/by-util/test_cp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3306,7 +3306,7 @@ fn test_symbolic_link_file() {
#[test]
fn test_src_base_dot() {
let ts = TestScenario::new(util_name!());
let at = ts.fixtures.clone();
let at = &ts.fixtures;
at.mkdir("x");
at.mkdir("y");
ts.ucmd()
Expand Down
4 changes: 2 additions & 2 deletions tests/by-util/test_dd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,7 @@ fn test_seek_past_dev() {
fn test_reading_partial_blocks_from_fifo() {
// Create the FIFO.
let ts = TestScenario::new(util_name!());
let at = ts.fixtures.clone();
let at = &ts.fixtures;
at.mkfifo("fifo");
let fifoname = at.plus_as_string("fifo");

Expand Down Expand Up @@ -1682,7 +1682,7 @@ fn test_reading_partial_blocks_from_fifo() {
fn test_reading_partial_blocks_from_fifo_unbuffered() {
// Create the FIFO.
let ts = TestScenario::new(util_name!());
let at = ts.fixtures.clone();
let at = ts.fixtures;
at.mkfifo("fifo");
let fifoname = at.plus_as_string("fifo");

Expand Down
1 change: 1 addition & 0 deletions tests/by-util/test_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ macro_rules! compare_with_gnu {
}

#[test]
#[allow(clippy::cognitive_complexity)] // Ignore clippy lint of too long function sign
tertsdiepraam marked this conversation as resolved.
Show resolved Hide resolved
fn test_env_with_gnu_reference_parsing_errors() {
let ts = TestScenario::new(util_name!());

Expand Down
4 changes: 3 additions & 1 deletion tests/by-util/test_touch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
// spell-checker:ignore (formats) cymdhm cymdhms mdhm mdhms ymdhm ymdhms datetime mktime

use crate::common::util::{AtPath, TestScenario};
use filetime::{set_symlink_file_times, FileTime};
#[cfg(not(target_os = "freebsd"))]
use filetime::set_symlink_file_times;
use filetime::FileTime;
use std::fs::remove_file;
use std::path::PathBuf;

Expand Down
2 changes: 2 additions & 0 deletions tests/common/random.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ mod tests {
}

#[test]
#[allow(clippy::cognitive_complexity)] // Ignore clippy lint of too long function sign
fn test_random_string_generate_with_delimiter_should_end_with_delimiter() {
let random_string = RandomString::generate_with_delimiter(Alphanumeric, 0, 1, true, 1);
assert_eq!(1, random_string.len());
Expand Down Expand Up @@ -251,6 +252,7 @@ mod tests {
}

#[test]
#[allow(clippy::cognitive_complexity)] // Ignore clippy lint of too long function sign
fn test_random_string_generate_with_delimiter_should_not_end_with_delimiter() {
let random_string = RandomString::generate_with_delimiter(Alphanumeric, 0, 0, false, 1);
assert_eq!(1, random_string.len());
Expand Down
6 changes: 3 additions & 3 deletions tests/common/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use std::os::unix::process::ExitStatusExt;
#[cfg(windows)]
use std::os::windows::fs::{symlink_dir, symlink_file};
#[cfg(windows)]
use std::path::MAIN_SEPARATOR;
use std::path::MAIN_SEPARATOR_STR;
use std::path::{Path, PathBuf};
use std::process::{Child, Command, ExitStatus, Output, Stdio};
use std::rc::Rc;
Expand Down Expand Up @@ -1016,7 +1016,7 @@ impl AtPath {

pub fn relative_symlink_file(&self, original: &str, link: &str) {
#[cfg(windows)]
let original = original.replace('/', &MAIN_SEPARATOR.to_string());
let original = original.replace('/', MAIN_SEPARATOR_STR);
log_info(
"symlink",
format!("{},{}", &original, &self.plus_as_string(link)),
Expand All @@ -1038,7 +1038,7 @@ impl AtPath {

pub fn relative_symlink_dir(&self, original: &str, link: &str) {
#[cfg(windows)]
let original = original.replace('/', &MAIN_SEPARATOR.to_string());
let original = original.replace('/', MAIN_SEPARATOR_STR);
log_info(
"symlink",
format!("{},{}", &original, &self.plus_as_string(link)),
Expand Down
Loading