Cargo: Enable most pedantic Clippy lints + some from nursery & restriction

Cargo.toml

@@ -30,7 +30,57 @@ exclude = [
     "rust/ql/integration-tests",
 ]
 
+[workspace.lints.rust]
+redundant_imports = "warn"
+
 [workspace.lints.clippy]
 all = { level = "warn", priority = -1 }
-explicit_iter_loop = "warn"
-manual_let_else = "warn"
+pedantic = { level = "warn", priority = -1 }
+
+# Allowlist of pedantic rules that we've observed to be of questionable value
+missing_errors_doc = "allow"
+missing_panics_doc = "allow"
+must_use_candidate = "allow"
+implicit_hasher = "allow"
+if_not_else = "allow"
+# This lint has a lot of results in generated code.
+unnecessary_wraps = "allow"
+# Spelling out the full method name can often be tedious.
+# E.g., `|v| v.as_slice()` vs `vec::Vec::as_slice`
+redundant_closure_for_method_calls = "allow"
+# The next three are only false positives and questionable results
+cast_possible_truncation = "allow"
+cast_possible_wrap = "allow"
+cast_sign_loss = "allow"
+# False positives
+struct_excessive_bools = "allow"
+# False positives for things like "matches" and "matched"
+similar_names = "allow"
+match_same_arms = "allow"
+struct_field_names = "allow"
+
+# Additional lints from the `restriction` and `nursery` group
+as_pointer_underscore = "warn"
+clone_on_ref_ptr = "warn"
+collection_is_never_read = "warn"
+deref_by_slicing = "warn"
+derive_partial_eq_without_eq = "warn"
+disallowed_script_idents = "warn"
+empty_drop = "warn"
+empty_enum_variants_with_brackets = "warn"
+empty_structs_with_brackets = "warn"
+fn_to_numeric_cast_any = "warn"
+if_then_some_else_none = "warn"
+needless_pass_by_ref_mut = "warn"
+or_fun_call = "warn"
+redundant_clone = "warn"
+redundant_type_annotations = "warn"
+renamed_function_params = "warn"
+semicolon_inside_block = "warn"
+undocumented_unsafe_blocks = "warn"
+unnecessary_safety_comment = "warn"
+unnecessary_safety_doc = "warn"
+unnecessary_struct_initialization = "warn"
+unneeded_field_pattern = "warn"
+unused_result_ok = "warn"
+too_long_first_doc_paragraph = "warn"

MODULE.bazel

@@ -142,7 +142,6 @@ use_repo(
     "vendor_rs__globset-0.4.18",
     "vendor_rs__itertools-0.14.0",
     "vendor_rs__itertools-0.15.0",
-    "vendor_rs__lazy_static-1.5.1",
     "vendor_rs__mustache-0.9.0",
     "vendor_rs__num-traits-0.2.19",
     "vendor_rs__num_cpus-1.17.0",

clippy.toml

@@ -0,0 +1,2 @@
+# Names that do not need to be backtick quoted within documentation comments.
+doc-valid-idents = ["CodeQL", ".."]

cpp/extractor/bmn/src/compiler.rs

@@ -303,8 +303,8 @@ fn find_posix(
     let (command_path, command_version) = get_compiler_version(kind, name)?;
     let (commandpp_path, commandpp_version) = get_compiler_version(kind, namepp)?;
 
-    let mimic_c_flags = vec![OsString::from("--mimic"), command_path.clone()];
-    let mimic_cpp_flags = vec![OsString::from("--mimic"), commandpp_path.clone()];
+    let mimic_c_flags = vec![OsString::from("--mimic"), command_path];
+    let mimic_cpp_flags = vec![OsString::from("--mimic"), commandpp_path];
 
     let default_compiler_include_checker = CompilerDefaultIncludes::from(telemetry, name, namepp);
 
@@ -518,28 +518,38 @@ fn get_compiler_override(
             &clang_c_compiler,
             &clang_cpp_compiler,
         )
-        .ok_or(anyhow!(
-            "Could not find clang compilers: {clang_c_compiler:?}, {clang_cpp_compiler:?}"
-        ))
+        .ok_or_else(|| {
+            anyhow!(
+                "Could not find clang compilers: {}, {}",
+                clang_c_compiler.display(),
+                clang_cpp_compiler.display()
+            )
+        })
     } else if override_instruction == "gnu" {
         // These can be just a command name, e.g. "gcc-18", or a full path, e.g.
         // "/usr/bin/gcc-18"
         let gnu_c_compiler =
             environment::get_required_os(environment::CODEQL_EXTRACTOR_CPP_BMN_COMPILER_GNU_C)?;
         let gnu_cpp_compiler =
             environment::get_required_os(environment::CODEQL_EXTRACTOR_CPP_BMN_COMPILER_GNU_CPP)?;
-        find_posix(telemetry, Compiler::Gnu, &gnu_c_compiler, &gnu_cpp_compiler).ok_or(anyhow!(
-            "Could not find gnu compilers: {gnu_c_compiler:?}, {gnu_cpp_compiler:?}"
-        ))
+        find_posix(telemetry, Compiler::Gnu, &gnu_c_compiler, &gnu_cpp_compiler).ok_or_else(|| {
+            anyhow!(
+                "Could not find gnu compilers: {}, {}",
+                gnu_c_compiler.display(),
+                gnu_cpp_compiler.display()
+            )
+        })
     } else if override_instruction == "msvc" {
         // These can be just a command name, e.g. "cl", or a full path, e.g.
         // "c:\msvc\cl.exe"
         let msvc_compiler =
             environment::get_required_os(environment::CODEQL_EXTRACTOR_CPP_BMN_COMPILER_MSVC)?;
-        use_cl(&msvc_compiler).ok_or(anyhow!("Could not find msvc compiler: {msvc_compiler:?}"))
+        use_cl(&msvc_compiler)
+            .ok_or_else(|| anyhow!("Could not find msvc compiler: {}", msvc_compiler.display()))
     } else {
         Err(anyhow!(
-            "Unsupported compiler override instruction: {override_instruction:?}"
+            "Unsupported compiler override instruction: {}",
+            override_instruction.display()
         ))
     }
 }

cpp/extractor/bmn/src/dependency_installation.rs

@@ -45,7 +45,7 @@ fn run_command(command: &mut Command) -> anyhow::Result<()> {
             } else {
                 OsString::from("Failed to read output")
             };
-            Err(anyhow!("\"{command_name}\" failed: {error:?}"))
+            Err(anyhow!("\"{command_name}\" failed: {}", error.display()))
         }
         Err(e) => Err(anyhow!("Error waiting for process \"{command_name}\": {e}")),
     }
@@ -86,7 +86,7 @@ fn install_package(package_name: &str) -> anyhow::Result<()> {
 }
 
 /// Struct to manage package installation state
-struct PackageInstaller {}
+struct PackageInstaller;
 
 impl PackageInstaller {
     /// Create a new `PackageInstaller` and perform an initial `apt-get update`
@@ -95,11 +95,15 @@ impl PackageInstaller {
             bail!("'sudo apt-get' not available.");
         }
         apt_update()?;
-        Ok(Self {})
+        Ok(Self)
     }
 
     /// Ensure the given package is installed
-    #[allow(clippy::unused_self)]
+    #[allow(
+        clippy::unused_self,
+        clippy::needless_pass_by_ref_mut,
+        reason = "Package installation mutates system state, so require exclusive access."
+    )]
     fn install_package(&mut self, package_name: &str) -> anyhow::Result<()> {
         install_package(package_name)
     }

cpp/extractor/bmn/src/directory_tree.rs

@@ -171,8 +171,6 @@ mod tests {
     mod from_path {
         use super::*;
         use crate::test_utils::get_resource;
-        use std::collections::HashSet;
-        use std::ffi::OsString;
 
         fn check_files(tree: &DirectoryTree, file_name: &str, expected: &[PathBuf]) {
             let files = tree

cpp/extractor/bmn/src/extraction_command_runner.rs

@@ -383,9 +383,7 @@ mod tests {
     use super::*;
     use crate::project_definitions::Language;
     use all_asserts::{assert_false, assert_true};
-    use std::ffi::OsString;
     use std::path::PathBuf;
-    use std::time::Duration;
 
     fn make_command(
         args: &[&str],
@@ -407,7 +405,6 @@ mod tests {
 
     #[allow(clippy::unnecessary_wraps)]
     fn fake_status(exit_code: i32) -> io::Result<ExitStatus> {
-        use std::process::ExitStatus;
         #[cfg(unix)]
         {
             use std::os::unix::process::ExitStatusExt;

cpp/extractor/bmn/src/file_filter.rs

@@ -97,7 +97,8 @@ fn relativize_path(path: PathBuf) -> Result<PathBuf> {
                 Ok(PathBuf::from_iter(sliced))
             }
             _ => Err(anyhow!(
-                "Path cannot start with .. or the drive prefix: {path:?}"
+                "Path cannot start with .. or the drive prefix: {}",
+                path.display()
             )),
         }
     }
@@ -331,11 +332,11 @@ impl FileFilter {
         if include_filter_count == 0 {
             // No include, everything not excluded should be accepted, so add an include all filter
             // at the beginning to include everything else
-            index_filters.insert(0, FilterRule::Include(wildcard.clone()));
+            index_filters.insert(0, FilterRule::Include(wildcard));
         } else {
             // If only include filters are specified, add an exclude all filter at the beginning to
             // exclude everything else
-            index_filters.insert(0, FilterRule::Exclude(wildcard.clone()));
+            index_filters.insert(0, FilterRule::Exclude(wildcard));
         }
 
         Ok(FileFilter {
@@ -459,7 +460,6 @@ impl FileFilter {
 mod tests {
     use super::*;
     use all_asserts::{assert_false, assert_true};
-    use std::path::PathBuf;
 
     // Using the current directory as root for tests to avoid issues with absolute paths on Windows
     fn get_cwd() -> PathBuf {

cpp/extractor/bmn/src/gnu_compiler_default_include_finder.rs

@@ -95,7 +95,8 @@ fn execute_command_with_args_get_stderr(cmd: &OsStr, args: &[&str]) -> Result<Ve
     match Command::new(cmd).args(args).stdin(Stdio::piped()).output() {
         Ok(output) if output.status.success() => Ok(output.stderr),
         _ => Err(anyhow!(
-            "Error invoking the compiler {cmd:?} to get the default include folders."
+            "Error invoking the compiler {} to get the default include folders.",
+            cmd.display()
         )),
     }
 }
@@ -597,7 +598,7 @@ End of search list."#;
         assert_true!(output.is_err());
         assert_eq!(
             output.unwrap_err().to_string(),
-            format!("Error invoking the compiler {cmd:?} to get the default include folders.")
+            "Error invoking the compiler command-not-existing to get the default include folders."
         );
     }
 
@@ -608,7 +609,7 @@ End of search list."#;
         assert_true!(result.is_err());
         assert_eq!(
             result.unwrap_err().to_string(),
-            format!("Error invoking the compiler {cmd:?} to get the default include folders.")
+            "Error invoking the compiler command-not-existing to get the default include folders."
         );
     }
 }

cpp/extractor/bmn/src/include_finder.rs

@@ -291,7 +291,6 @@ mod tests {
     use crate::include_scanner;
     use crate::test_utils::{create_populate_tmp_dir, get_resource};
     use all_asserts::assert_true;
-    use std::path::Path;
 
     fn create_tree(files: &[&str]) -> DirectoryTree {
         DirectoryTree::from_files(files.iter().map(PathBuf::from).collect::<Vec<_>>())
@@ -655,7 +654,7 @@ mod tests {
 
             assert_eq!(
                 result.include_directories,
-                vec![path.clone(), path.clone().join("dir2")]
+                vec![path.clone(), path.join("dir2")]
             );
             assert!(!result.unresolvable.is_empty());
             assert_eq!(
@@ -678,7 +677,7 @@ mod tests {
 
             let result = find_includes(&tree, &empty_tree(), &no_default_includes, &includes);
 
-            assert_eq!(result.include_directories, vec![path.clone()]);
+            assert_eq!(result.include_directories, vec![path]);
             assert_eq!(
                 result.unresolvable,
                 vec![

cpp/extractor/bmn/src/include_scanner.rs

@@ -32,12 +32,12 @@ fn get_includes_cached(filename: PathBuf) -> Vec<IncludeDirective> {
                 .map(|global_include| {
                     IncludeDirective::system(replace_backslashes(&global_include[1]), &filename)
                 })
-                .or(
+                .or_else(|| {
                     // #include "(.*)"
                     LOCAL_INCLUDE_RE.captures(line).map(|local_include| {
                         IncludeDirective::local(replace_backslashes(&local_include[1]), &filename)
-                    }),
-                )
+                    })
+                })
         })
         .collect()
 }
@@ -65,7 +65,7 @@ mod tests {
             IncludeDirective::system("missing_system.hpp", &file_name),
         ];
 
-        let actual = get_includes(file_name.clone());
+        let actual = get_includes(file_name);
 
         assert_eq!(actual, expected);
     }

cpp/extractor/bmn/src/logger.rs

@@ -1,7 +1,7 @@
 use crate::environment;
 use std::sync::LazyLock;
 
-#[derive(PartialEq, PartialOrd)]
+#[derive(PartialEq, Eq, PartialOrd)]
 pub enum Verbosity {
     Silent,
     Error,

cpp/extractor/bmn/src/main.rs

@@ -52,8 +52,8 @@ fn get_extractor_executable() -> Result<PathBuf> {
     let extractor = PathBuf::from_iter([&root_dir, "tools", &platform, &binary_name]);
     if !extractor.exists() {
         Err(anyhow::anyhow!(
-            "Extractor executable not found at: {:?}",
-            extractor
+            "Extractor executable not found at: {}",
+            extractor.display()
         ))
     } else {
         Ok(extractor)

cpp/extractor/bmn/src/overlays.rs

@@ -54,21 +54,23 @@ pub fn get_overlay_settings() -> Result<OverlaySettings> {
         (Some(overlay_changes), Some(metadata_in), None) => {
             let changes_file = fs::File::open(overlay_changes)?;
             let json: Value = from_reader(changes_file)?;
-            let obj = json.as_object().ok_or(anyhow!(
-                "Overlay changes file does not contain a JSON object."
-            ))?;
+            let obj = json
+                .as_object()
+                .ok_or_else(|| anyhow!("Overlay changes file does not contain a JSON object."))?;
             // TODO: Log/warn if there are any fields other than "changes"
             let files = obj.get("changes").ok_or_else(|| {
                 anyhow!("Overlay changes file does not contain a 'changes' field.")
             })?;
-            let files_array = files.as_array().ok_or(anyhow!(
-                "Overlay changes file 'changes' field is not an array."
-            ))?;
+            let files_array = files
+                .as_array()
+                .ok_or_else(|| anyhow!("Overlay changes file 'changes' field is not an array."))?;
             let changed_files = files_array
                 .iter()
                 .map(|v| {
                     Ok(v.as_str()
-                        .ok_or(anyhow!("Overlay changes file contains non-string entries."))?
+                        .ok_or_else(|| {
+                            anyhow!("Overlay changes file contains non-string entries.")
+                        })?
                         .into())
                 })
                 .collect::<Result<_>>()?;