Auto merge of #15017 - Veykril:clippy, r=<try>

Add unstable cargo lint tables and make use of clippy

Figured we can allow all lints and then selectively disable the ones we don't want. The nice thing is this doesn't break stable builds, although it does spam a bit for each cargo toml ...

Author: bors

Cargo.toml

@@ -92,3 +92,25 @@ serde_json = "1.0.94"
 triomphe = { version = "0.1.8", default-features = false, features = ["std"] }
 
 rustc_lexer = { version = "0.1.0", package = "ra-ap-rustc_lexer" }
+
+[workspace.lints.rust]
+rust_2018_idioms = "warn"
+unused_lifetimes = "warn"
+semicolon_in_expressions_from_macros = "warn"
+
+[workspace.lints.clippy]
+all = { level = "warn", priority = -1 }
+# () makes a fine error in most cases
+result_unit_err = "allow"
+# We don't expose public APIs that matter like this
+len_without_is_empty = "allow"
+# Should be tackled at some point
+too_many_arguments = "allow"
+# Should be tackled at some point
+type_complexity = "allow"
+# we currently prefer explicit control flow return over `...?;` statements whose result is unused
+question_mark = "allow"
+# We have macros that rely on this currently
+enum_variant_names = "allow"
+# Builder pattern disagrees
+new_ret_no_self = "allow"

crates/base-db/Cargo.toml

@@ -27,3 +27,6 @@ syntax.workspace = true
 test-utils.workspace = true
 tt.workspace = true
 vfs.workspace = true
+
+[lints]
+workspace = true

crates/base-db/src/fixture.rs

@@ -116,7 +116,7 @@ impl ChangeFixture {
         let toolchain = toolchain
             .map(|it| {
                 ReleaseChannel::from_str(&it)
-                    .unwrap_or_else(|| panic!("unknown release channel found: {it}"))
+                    .unwrap_or_else(|()| panic!("unknown release channel found: {it}"))
             })
             .unwrap_or(ReleaseChannel::Stable);
         let mut change = Change::new();

crates/base-db/src/input.rs

@@ -282,13 +282,17 @@ impl ReleaseChannel {
             ReleaseChannel::Nightly => "nightly",
         }
     }
+}
+
+impl FromStr for ReleaseChannel {
+    type Err = ();
 
-    pub fn from_str(str: &str) -> Option<Self> {
-        Some(match str {
+    fn from_str(str: &str) -> Result<Self, Self::Err> {
+        Ok(match str {
             "" => ReleaseChannel::Stable,
             "nightly" => ReleaseChannel::Nightly,
             _ if str.starts_with("beta") => ReleaseChannel::Beta,
-            _ => return None,
+            _ => return Err(()),
         })
     }
 }

crates/cfg/Cargo.toml

@@ -29,3 +29,6 @@ derive_arbitrary = "1.2.2"
 # local deps
 mbe.workspace = true
 syntax.workspace = true
+
+[lints]
+workspace = true

crates/flycheck/Cargo.toml

@@ -24,3 +24,6 @@ command-group = "2.0.1"
 paths.workspace = true
 stdx.workspace = true
 toolchain.workspace = true
+
+[lints]
+workspace = true

crates/flycheck/src/lib.rs

@@ -513,6 +513,7 @@ impl CargoActor {
     }
 }
 
+#[allow(clippy::large_enum_variant)]
 enum CargoMessage {
     CompilerArtifact(cargo_metadata::Artifact),
     Diagnostic(Diagnostic),

crates/hir-def/Cargo.toml

@@ -51,3 +51,6 @@ expect-test = "1.4.0"
 
 # local deps
 test-utils.workspace = true
+
+[lints]
+workspace = true

crates/hir-def/src/attr.rs

@@ -379,7 +379,7 @@ fn parse_comma_sep<S>(subtree: &tt::Subtree<S>) -> Vec<SmolStr> {
 }
 
 impl AttrsWithOwner {
-    pub(crate) fn attrs_with_owner(db: &dyn DefDatabase, owner: AttrDefId) -> Self {
+    pub(crate) fn attrs_with_owner_query(db: &dyn DefDatabase, owner: AttrDefId) -> Self {
         Self { attrs: db.attrs(owner), owner }
     }
 

crates/hir-def/src/body.rs

@@ -242,12 +242,12 @@ impl Body {
                 }
             }
             Pat::Or(args) | Pat::Tuple { args, .. } | Pat::TupleStruct { args, .. } => {
-                args.iter().copied().for_each(|p| f(p));
+                args.iter().copied().for_each(&mut f);
             }
             Pat::Ref { pat, .. } => f(*pat),
             Pat::Slice { prefix, slice, suffix } => {
                 let total_iter = prefix.iter().chain(slice.iter()).chain(suffix.iter());
-                total_iter.copied().for_each(|p| f(p));
+                total_iter.copied().for_each(&mut f);
             }
             Pat::Record { args, .. } => {
                 args.iter().for_each(|RecordFieldPat { pat, .. }| f(*pat));

crates/hir-def/src/body/lower.rs

@@ -811,7 +811,7 @@ impl ExprCollector<'_> {
                 expr: iterator,
                 arms: Box::new([MatchArm { pat: iter_pat, guard: None, expr: loop_outer }]),
             },
-            syntax_ptr.clone(),
+            syntax_ptr,
         )
     }
 

crates/hir-def/src/data.rs

@@ -739,7 +739,7 @@ impl<'a> AssocItemCollector<'a> {
             self.diagnostics.push(DefDiagnostic::macro_expansion_parse_error(
                 self.module_id.local_id,
                 error_call_kind(),
-                errors.into(),
+                errors,
             ));
         }
 

crates/hir-def/src/data/adt.rs

@@ -473,7 +473,7 @@ fn lower_struct(
     trace: &mut Trace<FieldData, Either<ast::TupleField, ast::RecordField>>,
     ast: &InFile<ast::StructKind>,
 ) -> StructKind {
-    let ctx = LowerCtx::new(db, &expander.hygiene(), ast.file_id);
+    let ctx = LowerCtx::new(db, expander.hygiene(), ast.file_id);
 
     match &ast.value {
         ast::StructKind::Tuple(fl) => {

crates/hir-def/src/db.rs

@@ -193,7 +193,7 @@ pub trait DefDatabase: InternDatabase + ExpandDatabase + Upcast<dyn ExpandDataba
     fn attrs(&self, def: AttrDefId) -> Attrs;
 
     #[salsa::transparent]
-    #[salsa::invoke(AttrsWithOwner::attrs_with_owner)]
+    #[salsa::invoke(AttrsWithOwner::attrs_with_owner_query)]
     fn attrs_with_owner(&self, def: AttrDefId) -> AttrsWithOwner;
 
     // endregion:attrs
@@ -271,10 +271,8 @@ fn crate_supports_no_std(db: &dyn DefDatabase, crate_id: CrateId) -> bool {
             None => continue,
         };
 
-        let segments = tt.split(|tt| match tt {
-            tt::TokenTree::Leaf(tt::Leaf::Punct(p)) if p.char == ',' => true,
-            _ => false,
-        });
+        let segments =
+            tt.split(|tt| matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Punct(p)) if p.char == ','));
         for output in segments.skip(1) {
             match output {
                 [tt::TokenTree::Leaf(tt::Leaf::Ident(ident))] if ident.text == "no_std" => {

crates/hir-def/src/find_path.rs

@@ -206,7 +206,7 @@ fn find_path_for_module(
     }
 
     if let value @ Some(_) =
-        find_in_prelude(db, &root_def_map, &def_map, ItemInNs::Types(module_id.into()), from)
+        find_in_prelude(db, &root_def_map, def_map, ItemInNs::Types(module_id.into()), from)
     {
         return value;
     }

crates/hir-def/src/generics.rs

@@ -300,6 +300,7 @@ impl GenericParams {
         }
     }
 
+    #[allow(clippy::borrowed_box)]
     fn add_where_predicate_from_bound(
         &mut self,
         lower_ctx: &LowerCtx<'_>,

crates/hir-def/src/lang_item.rs

@@ -219,6 +219,7 @@ macro_rules! language_item_table {
                 }
             }
 
+            #[allow(clippy::should_implement_trait)]
             /// Opposite of [`LangItem::name`]
             pub fn from_str(name: &str) -> Option<Self> {
                 match name {

crates/hir-def/src/nameres/collector.rs

@@ -1388,14 +1388,14 @@ impl DefCollector<'_> {
                     always!(krate == loc.def.krate);
                     DefDiagnostic::unresolved_proc_macro(module_id, loc.kind.clone(), loc.def.krate)
                 }
-                _ => DefDiagnostic::macro_error(module_id, loc.kind.clone(), err.to_string()),
+                _ => DefDiagnostic::macro_error(module_id, loc.kind, err.to_string()),
             };
 
             self.def_map.diagnostics.push(diag);
         }
         if let errors @ [_, ..] = &*value {
             let loc: MacroCallLoc = self.db.lookup_intern_macro_call(macro_call_id);
-            let diag = DefDiagnostic::macro_expansion_parse_error(module_id, loc.kind, &errors);
+            let diag = DefDiagnostic::macro_expansion_parse_error(module_id, loc.kind, errors);
             self.def_map.diagnostics.push(diag);
         }
 
@@ -1847,7 +1847,7 @@ impl ModCollector<'_, '_> {
                         item_tree: self.item_tree,
                         mod_dir,
                     }
-                    .collect_in_top_module(&*items);
+                    .collect_in_top_module(items);
                     if is_macro_use {
                         self.import_all_legacy_macros(module_id);
                     }
@@ -2238,9 +2238,9 @@ impl ModCollector<'_, '_> {
     fn import_all_legacy_macros(&mut self, module_id: LocalModuleId) {
         let macros = self.def_collector.def_map[module_id].scope.collect_legacy_macros();
         for (name, macs) in macros {
-            macs.last().map(|&mac| {
-                self.def_collector.define_legacy_macro(self.module_id, name.clone(), mac)
-            });
+            if let Some(&mac) = macs.last() {
+                self.def_collector.define_legacy_macro(self.module_id, name.clone(), mac);
+            }
         }
     }
 

crates/hir-def/src/nameres/path_resolution.rs

@@ -454,7 +454,7 @@ impl DefMap {
         let macro_use_prelude = || {
             self.macro_use_prelude
                 .get(name)
-                .map_or(PerNs::none(), |&it| PerNs::macros(it.into(), Visibility::Public))
+                .map_or(PerNs::none(), |&it| PerNs::macros(it, Visibility::Public))
         };
         let prelude = || self.resolve_in_prelude(db, name);
 

crates/hir-def/src/nameres/tests/macros.rs

@@ -1294,8 +1294,8 @@ fn proc_attr(a: TokenStream, b: TokenStream) -> TokenStream { a }
 
     let actual = def_map
         .macro_use_prelude
-        .iter()
-        .map(|(name, _)| name.display(&db).to_string())
+        .keys()
+        .map(|name| name.display(&db).to_string())
         .sorted()
         .join("\n");
 

crates/hir-def/src/path.rs

@@ -150,7 +150,7 @@ impl Path {
 
     pub fn mod_path(&self) -> Option<&ModPath> {
         match self {
-            Path::Normal { mod_path, .. } => Some(&mod_path),
+            Path::Normal { mod_path, .. } => Some(mod_path),
             Path::LangItem(_) => None,
         }
     }
@@ -215,13 +215,13 @@ impl<'a> PathSegments<'a> {
     }
     pub fn skip(&self, len: usize) -> PathSegments<'a> {
         PathSegments {
-            segments: &self.segments.get(len..).unwrap_or(&[]),
+            segments: self.segments.get(len..).unwrap_or(&[]),
             generic_args: self.generic_args.and_then(|it| it.get(len..)),
         }
     }
     pub fn take(&self, len: usize) -> PathSegments<'a> {
         PathSegments {
-            segments: &self.segments.get(..len).unwrap_or(&self.segments),
+            segments: self.segments.get(..len).unwrap_or(self.segments),
             generic_args: self.generic_args.map(|it| it.get(..len).unwrap_or(it)),
         }
     }

crates/hir-def/src/path/lower.rs

@@ -45,7 +45,7 @@ pub(super) fn lower_path(mut path: ast::Path, ctx: &LowerCtx<'_>) -> Option<Path
                                 )
                             })
                             .map(Interned::new);
-                        if let Some(_) = args {
+                        if args.is_some() {
                             generic_args.resize(segments.len(), None);
                             generic_args.push(args);
                         }

crates/hir-def/src/per_ns.rs

@@ -5,19 +5,13 @@
 
 use crate::{item_scope::ItemInNs, visibility::Visibility, MacroId, ModuleDefId};
 
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub struct PerNs {
     pub types: Option<(ModuleDefId, Visibility)>,
     pub values: Option<(ModuleDefId, Visibility)>,
     pub macros: Option<(MacroId, Visibility)>,
 }
 
-impl Default for PerNs {
-    fn default() -> Self {
-        PerNs { types: None, values: None, macros: None }
-    }
-}
-
 impl PerNs {
     pub fn none() -> PerNs {
         PerNs { types: None, values: None, macros: None }