Major clippy overhaul.

[jhscheer]

There were severe problems with our clippy linting:

• clippy suggested newer features than our MSRV
• clippy did not check all targets, e.g. "tests" were completely unchecked

This PR:

• adds clippy.toml to be able to set MSRV for clippy. However, this only works if clippy is invoked like this: cargo +nightly clippy.
• adds --target=${{ matrix.job.target }} to github actions (I hope this is enough, locally I run cargo +nightly clippy --all-targets --all-features
• fixes all clippy warnings that were exposed with the previous steps

[miDeb]

Is this allow needed? Also, it seems like we're not handling errors in symlink_file for linux/macos, but always returning Ok(()). Maybe it should be the same as for windows below?

[jhscheer]

Haven't checked yet (it's so cumbersome to boot up a windows VM).
I also noticed errors are not handled in symlink_file, however there's an 'unwrap' which should be enough, i.e. print the error message and then panic.

[miDeb]

I'm not sure I understand. Which unwrap are you referring to? Any error from the call to std::os::unix::fs::symlink(source, dest).context(context) is not handled, or am I missing something?

Also, you changed this from unnecessary_wraps to unnecessary_unwrap, but... there's no unwrap in here. That's what confused me. Anyways,I'm booting up my windows VM to check :)

[miDeb]

Removing this allow doesn't make clippy complain on windows (checked in a VM).
However, there are two other warnings:

warning: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
   --> src\uu\du\src/du.rs:150:27
    |
150 | fn get_size_on_disk(path: &PathBuf) -> u64 {
    |                           ^^^^^^^^ help: change this to: `&Path`
    |
    = note: `#[warn(clippy::ptr_arg)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg

warning: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
   --> src\uu\du\src/du.rs:182:24
    |
182 | fn get_file_info(path: &PathBuf) -> Option<FileInfo> {
    |                        ^^^^^^^^ help: change this to: `&Path`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg

warning: 2 warnings emitted

With stable there is another one:

warning: name `DWORD` contains a capitalized acronym
   --> src\uu\rm\src/rm.rs:406:14
    |
406 |     pub type DWORD = c_ulong;
    |              ^^^^^ help: consider making the acronym lowercase, except the initial letter: `Dword`
    |
    = note: `#[warn(clippy::upper_case_acronyms)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

warning: 1 warning emitted

[jhscheer]

I'm not sure I understand. Which unwrap are you referring to? Any error from the call to std::os::unix::fs::symlink(source, dest).context(context) is not handled, or am I missing something?

Oh, I was referring to L539:

coreutils/tests/common/util.rs

Lines 533 to 540 in 3d42d41

	pub fn symlink_file(&self, src: &str, dst: &str) {
	log_info(
	"symlink",
	&format!("{},{}", self.plus_as_string(src), self.plus_as_string(dst)),
	);
	symlink_file(&self.plus(src), &self.plus(dst)).unwrap();
	}

Also, you changed this from unnecessary_wraps to unnecessary_unwrap, but... there's no unwrap in here. That's what confused me.

Clippy complained that there's no unnecessary_wraps, I thought it's a typo and followed clippy's suggestion to use unnecessary_unwrap. However, now that you mention it, I checked and unnecessary_wraps does indeed exit. Strange.

[miDeb]

There is: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps Jan Scheer ***@***.***> schrieb am Sa., 29. Mai 2021, 19:08:

…

***@***.**** commented on this pull request. ------------------------------ In src/uu/cp/src/cp.rs <#2298 (comment)>: > @@ -1089,7 +1089,7 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu } #[cfg(not(windows))] -#[allow(clippy::unnecessary_wraps)] // needed for windows version +#[allow(clippy::unnecessary_unwrap)] // needed for windows version I'm not sure I understand. Which unwrap are you referring to? Any error from the call to std::os::unix::fs::symlink(source, dest).context(context) is not handled, or am I missing something? Oh, I was referring to L539: https://github.com/uutils/coreutils/blob/3d42d41a72e7f43d6d5a88d73eed4dae181f7dc9/tests/common/util.rs#L533-L540 Also, you changed this from unnecessary_wraps to unnecessary_unwrap, but... there's no unwrap in here. That's what confused me. There is no unnecessary_wraps, probably a typo and clippy's suggestion was to use unnecessary_unwrap. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2298 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKQ77NSAOPTRA6A6MRBEY6TTQENRNANCNFSM45XTOCSA> .