Stricter clippy

Summary:
Enable `clippy::nursery`, `clippy::pedantic` and
`clippy::allow_attributes`. And disable noisy rules or rules that can't
easily be fixed now.

Reviewed By: bigfootjon

Differential Revision: D64553143

fbshipit-source-id: 6e1cf5c2dfe51b5a94b8917e4db4a95d1af5f119

Cargo.toml

@@ -53,4 +53,5 @@ codegen-units = 1
 strip = true
 
 [lints]
-rust = { unexpected_cfgs = { check-cfg = ["cfg(fbcode_build)"], level = "warn" } }
+clippy = { allow_attributes = { level = "warn", priority = -1 }, bool_assert_comparison = { level = "allow", priority = -1 }, cast_lossless = { level = "allow", priority = -1 }, cast_possible_truncation = { level = "allow", priority = -1 }, cast_precision_loss = { level = "allow", priority = -1 }, cast_sign_loss = { level = "allow", priority = -1 }, cognitive_complexity = { level = "allow", priority = -1 }, derive_partial_eq_without_eq = { level = "allow", priority = -1 }, doc_markdown = { level = "allow", priority = -1 }, manual_string_new = { level = "allow", priority = -1 }, missing_const_for_fn = { level = "allow", priority = -1 }, missing_errors_doc = { level = "allow", priority = -1 }, missing_panics_doc = { level = "allow", priority = -1 }, module_name_repetitions = { level = "allow", priority = -1 }, no_effect_underscore_binding = { level = "allow", priority = -1 }, nursery = { level = "warn", priority = -2 }, option_if_let_else = { level = "allow", priority = -1 }, pedantic = { level = "warn", priority = -2 }, struct_excessive_bools = { level = "allow", priority = -1 }, struct_field_names = { level = "allow", priority = -1 }, too_many_lines = { level = "allow", priority = -1 }, uninlined_format_args = { level = "allow", priority = -1 }, unreadable_literal = { level = "allow", priority = -1 }, use_self = { level = "allow", priority = -1 } }
+rust = { rust_2018_idioms = { level = "warn", priority = -2 }, unexpected_cfgs = { check-cfg = ["cfg(fbcode_build)", "cfg(dotslash_internal)"], level = "warn", priority = 0 } }

src/artifact_location.rs

@@ -78,7 +78,7 @@ pub fn determine_location(
         .join(key_prefix)
         .join(key_rest);
 
-    let mut executable = artifact_directory.to_owned();
+    let mut executable = artifact_directory.clone();
     executable.extend(Path::new(artifact_entry.path.as_str()));
     let lock_path = dotslash_cache.locks_dir(key_prefix).join(key_rest);
 

src/config.rs

@@ -51,6 +51,7 @@ fn readonly_default_as_true() -> bool {
     true
 }
 
+#[expect(clippy::trivially_copy_pass_by_ref)]
 fn is_true(b: &bool) -> bool {
     *b
 }

src/curl.rs

@@ -169,14 +169,14 @@ impl CurlError {
 }
 
 impl CurlCommand<'_> {
-    pub fn new(url: &OsStr) -> CurlCommand {
+    pub fn new(url: &OsStr) -> CurlCommand<'_> {
         CurlCommand {
             url,
             retry: NUM_TRANSIENT_ERROR_CURL_MAX_ATTEMPTS,
         }
     }
 
-    pub fn get_request(&self, target: &Path, context: &FetchContext) -> Result<(), CurlError> {
+    pub fn get_request(&self, target: &Path, context: &FetchContext<'_>) -> Result<(), CurlError> {
         // Because `target` is ultimately used with Command.args(), we should make
         // it possible to use a non-utf8 value, as unlikely as it is, in practice.
         let output_arg = target.to_str().unwrap();
@@ -211,6 +211,7 @@ impl CurlCommand<'_> {
         Ok(())
     }
 
+    #[expect(clippy::unused_self)]
     fn make_request(&self, curl_command: &mut Command) -> Result<Vec<u8>, CurlError> {
         let mut retries = 1..=NUM_RETRYABLE_CURL_MAX_ATTEMPTS;
         loop {
@@ -248,7 +249,7 @@ impl CurlCommand<'_> {
         }
     }
 
-    fn curl_command(&self, url: &OsStr, request_type: &CurlRequestType) -> Command {
+    fn curl_command(&self, url: &OsStr, request_type: &CurlRequestType<'_>) -> Command {
         let mut curl_command = Command::new("curl");
 
         // https://cygwin.com/cygwin-ug-net/using-cygwinenv.html

src/default_provider_factory.rs

@@ -14,7 +14,7 @@ use crate::http_provider::HttpProvider;
 use crate::provider::Provider;
 use crate::provider::ProviderFactory;
 
-pub(crate) struct DefaultProviderFactory;
+pub struct DefaultProviderFactory;
 
 impl ProviderFactory for DefaultProviderFactory {
     fn get_provider(&self, provider_type: &str) -> anyhow::Result<Box<dyn Provider>> {

src/dotslash_cache.rs

@@ -76,7 +76,6 @@ fn get_dotslash_cache() -> PathBuf {
 
     // `dirs` returns the preferred cache directory for the user and the
     // platform based on these rules: https://docs.rs/dirs/*/dirs/fn.cache_dir.html
-    #[cfg_attr(windows, allow(clippy::let_and_return))]
     let cache_dir = match dirs::cache_dir() {
         Some(cache_dir) => cache_dir.join("dotslash"),
         None => panic!("could not find DotSlash root - specify $DOTSLASH_CACHE"),
@@ -109,7 +108,7 @@ fn get_dotslash_cache() -> PathBuf {
     cache_dir
 }
 
-#[cfg_attr(windows, allow(dead_code))]
+#[cfg_attr(windows, expect(dead_code))]
 fn named_cache_dir_at<P: Into<PathBuf>>(dir: P) -> PathBuf {
     let mut name = OsString::from("dotslash-");
 

src/download.rs

@@ -92,12 +92,12 @@ pub fn download_artifact<P: ProviderFactory>(
             &file_lock,
             artifact_entry,
         ) {
-            Ok(_) => match verify_artifact(&fetch_destination, artifact_entry) {
-                Ok(_) => {
+            Ok(()) => match verify_artifact(&fetch_destination, artifact_entry) {
+                Ok(()) => {
                     unpack_verified_artifact(
                         &fetch_destination,
                         temp_dir_to_mv.path(),
-                        &artifact_entry.format,
+                        artifact_entry.format,
                         artifact_entry.path.as_str(),
                     )?;
                     if artifact_entry.readonly {
@@ -212,7 +212,7 @@ fn verify_artifact(
 fn unpack_verified_artifact(
     fetched_artifact: &Path,
     temp_dir_to_mv: &Path,
-    format: &ArtifactFormat,
+    format: ArtifactFormat,
     artifact_entry_path: &str,
 ) -> anyhow::Result<()> {
     match &format.extraction_policy() {

src/fetch_method.rs

@@ -10,7 +10,7 @@
 use serde::Deserialize;
 use serde::Serialize;
 
-#[derive(Default, Deserialize, Serialize, Debug, PartialEq, Eq)]
+#[derive(Copy, Clone, Default, Deserialize, Serialize, Debug, PartialEq, Eq)]
 pub enum ArtifactFormat {
     /// Artifact is a single file with no compression applied.
     #[default]
@@ -55,7 +55,7 @@ pub enum ArchiveFormat {
 }
 
 impl ArtifactFormat {
-    pub fn extraction_policy(&self) -> (Option<DecompressStep>, Option<ArchiveFormat>) {
+    pub fn extraction_policy(self) -> (Option<DecompressStep>, Option<ArchiveFormat>) {
         match self {
             Self::Plain => (None, None),
             Self::Gz => (Some(DecompressStep::Gzip), None),

src/locate.rs

@@ -20,7 +20,7 @@ use crate::dotslash_cache::DotslashCache;
 use crate::platform::SUPPORTED_PLATFORM;
 use crate::util::display::ListOf;
 
-pub(crate) fn locate_artifact(
+pub fn locate_artifact(
     dotslash_data: &str,
     dotslash_cache: &DotslashCache,
 ) -> anyhow::Result<(ArtifactEntry, ArtifactLocation)> {
@@ -58,8 +58,8 @@ pub(crate) fn locate_artifact(
 /// tmpwatch or tmpreaper. Those tools work better using the mtime rather than
 /// atime which is why we update the mtime. But this doesn't work on
 /// Windows sometimes.
-#[cfg_attr(windows, allow(dead_code))]
-pub(crate) fn update_artifact_mtime(executable: &Path) {
+#[cfg_attr(windows, expect(dead_code))]
+pub fn update_artifact_mtime(executable: &Path) {
     drop(filetime::set_file_mtime(
         executable,
         filetime::FileTime::now(),

src/print_entry_for_url.rs

@@ -28,7 +28,7 @@ type LooseArtifactEntry = ArtifactEntry<String>;
 
 /// This function creates an approximate ArtifactEntry for the specified URL
 /// and writes it to stdout as pretty-printed JSON.
-pub(crate) fn print_entry_for_url(url: &OsStr) -> anyhow::Result<()> {
+pub fn print_entry_for_url(url: &OsStr) -> anyhow::Result<()> {
     let curl_cmd = CurlCommand::new(url);
     let url = url
         .to_str()

src/progress.rs

@@ -88,7 +88,7 @@ pub fn display_progress(
 
 fn should_end_progress(recv: &mpsc::Receiver<()>) -> bool {
     match recv.try_recv() {
-        Ok(_) | Err(mpsc::TryRecvError::Disconnected) => true,
+        Ok(()) | Err(mpsc::TryRecvError::Disconnected) => true,
         Err(mpsc::TryRecvError::Empty) => false,
     }
 }

src/subcommand.rs

@@ -145,7 +145,7 @@ fn _run_subcommand(subcommand: &Subcommand, args: &mut ArgsOs) -> anyhow::Result
 
         Subcommand::CreateUrlEntry => {
             let url = take_exactly_one_arg(args)?;
-            print_entry_for_url(&url)?
+            print_entry_for_url(&url)?;
         }
 
         Subcommand::CacheDir => {

src/util/display.rs

@@ -15,6 +15,7 @@ use std::process::Output;
 
 /// TODO
 #[derive(Debug)]
+#[must_use]
 pub struct CommandDisplay<'a>(&'a Command);
 
 impl<'a> CommandDisplay<'a> {
@@ -37,6 +38,7 @@ impl fmt::Display for CommandDisplay<'_> {
 
 /// TODO
 #[derive(Debug)]
+#[must_use]
 pub struct CommandStderrDisplay<'a>(&'a Output);
 
 impl<'a> CommandStderrDisplay<'a> {

src/util/fs_ctx.rs

@@ -78,7 +78,7 @@ pub fn canonicalize<P: AsRef<Path>>(path: P) -> io::Result<PathBuf> {
     fs::canonicalize(&path).map_err(|source| wrap1(source, "canonicalize", path))
 }
 
-#[allow(dead_code)]
+#[cfg_attr(not(dotslash_internal), expect(dead_code))]
 pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> io::Result<u64> {
     fs::copy(&from, &to).map_err(|source| wrap2(source, "copy from", from, "to", to))
 }
@@ -87,7 +87,6 @@ pub fn create_dir_all<P: AsRef<Path>>(path: P) -> io::Result<()> {
     fs::create_dir_all(&path).map_err(|source| wrap1(source, "create directory", path))
 }
 
-#[allow(dead_code)]
 pub fn file_create<P: AsRef<Path>>(path: P) -> io::Result<fs::File> {
     fs::File::create(&path).map_err(|source| wrap1(source, "create file", path))
 }
@@ -96,7 +95,7 @@ pub fn file_open<P: AsRef<Path>>(path: P) -> io::Result<fs::File> {
     fs::File::open(&path).map_err(|source| wrap1(source, "open file", path))
 }
 
-#[cfg_attr(windows, allow(dead_code))]
+#[cfg_attr(all(windows, not(dotslash_internal), not(test)), expect(dead_code))]
 pub fn metadata<P: AsRef<Path>>(path: P) -> io::Result<fs::Metadata> {
     fs::metadata(&path).map_err(|source| wrap1(source, "get metadata for", path))
 }
@@ -110,7 +109,7 @@ pub fn namedtempfile_new_in<P: AsRef<Path>>(path: P) -> io::Result<tempfile::Nam
         .map_err(|source| wrap1(source, "create temp file in", path))
 }
 
-#[allow(dead_code)]
+#[cfg_attr(not(dotslash_internal), expect(dead_code))]
 pub fn read<P: AsRef<Path>>(path: P) -> io::Result<Vec<u8>> {
     fs::read(&path).map_err(|source| wrap1(source, "read file", path))
 }
@@ -131,7 +130,6 @@ pub fn remove_dir_all<P: AsRef<Path>>(path: P) -> io::Result<()> {
     fs::remove_dir_all(&path).map_err(|source| wrap1(source, "remove dir all", path))
 }
 
-#[cfg_attr(windows, allow(dead_code))]
 pub fn set_permissions<P: AsRef<Path>>(path: P, perm: fs::Permissions) -> io::Result<()> {
     fs::set_permissions(&path, perm).map_err(|source| wrap1(source, "set permissions for", path))
 }

src/util/make_tree_read_only.rs

@@ -26,7 +26,8 @@ pub fn make_tree_entries_read_only(folder: &Path) -> io::Result<()> {
         let metadata = symlink_metadata(entry.path())?;
         if metadata.is_symlink() {
             continue;
-        } else if metadata.is_dir() {
+        }
+        if metadata.is_dir() {
             make_tree_entries_read_only(&entry.path())?;
         }
 

tests/common/mod.rs

@@ -19,7 +19,7 @@ use snapbox::Substitutions;
 pub mod ci;
 
 #[path = "../../src/platform.rs"]
-#[allow(dead_code)]
+#[expect(dead_code)]
 mod platform;
 
 const PACK_TGZ_HTTP_ARCHIVE_CACHE_DIR: &str = "cf/df86e55cbbd2455fd1e36468c2b1ff7f8998d4";
@@ -175,7 +175,7 @@ impl DotSlashTestEnv {
         Ok(self)
     }
 
-    #[allow(dead_code)]
+    #[expect(dead_code)]
     pub fn dotslash_cache(&self) -> &Path {
         &self.tempdir_path
     }

windows_shim/dotslash_windows_shim.rs

@@ -32,7 +32,7 @@ fn main() {
         Ok(exit_code) => std::process::exit(exit_code.code().unwrap_or(1)),
         Err(err) => {
             if err.kind() == io::ErrorKind::NotFound {
-                eprintln!("dotslash-windows-shim: dotslash executable not found.")
+                eprintln!("dotslash-windows-shim: dotslash executable not found.");
             } else {
                 eprintln!("dotslash-windows-shim: `{}`.", err);
             };