Merge #1374

1374: Implement `cargo lint` and fix some clippy errors r=alanhdu a=alanhdu

This creates a `cargo lint` command that runs clippy with certain lints disabled. I've also gone ahead and fixed some of the lint errors, although there are many more still to go.

cc #848 

Co-authored-by: Alan Du <[email protected]>

Author: bors[bot]

.cargo/config

@@ -12,6 +12,9 @@ install-code = "run --package tools --bin tools -- install-code"
 # Formats the full repository or installs the git hook to do it automatically.
 format       = "run --package tools --bin tools -- format"
 format-hook  = "run --package tools --bin tools -- format-hook"
+# Run clippy
+lint         = "run --package tools --bin tools -- lint"
+
 # Runs the fuzzing test suite (currently only parser)
 fuzz-tests   = "run --package tools --bin tools -- fuzz-tests"
 

crates/ra_assists/src/ast_editor.rs

@@ -212,21 +212,17 @@ impl AstEditor<ast::FnDef> {
     }
 
     pub fn strip_attrs_and_docs(&mut self) {
-        loop {
-            if let Some(start) = self
-                .ast()
-                .syntax()
-                .children_with_tokens()
-                .find(|it| it.kind() == ATTR || it.kind() == COMMENT)
-            {
-                let end = match start.next_sibling_or_token() {
-                    Some(el) if el.kind() == WHITESPACE => el,
-                    Some(_) | None => start,
-                };
-                self.ast = self.replace_children(RangeInclusive::new(start, end), iter::empty());
-            } else {
-                break;
-            }
+        while let Some(start) = self
+            .ast()
+            .syntax()
+            .children_with_tokens()
+            .find(|it| it.kind() == ATTR || it.kind() == COMMENT)
+        {
+            let end = match start.next_sibling_or_token() {
+                Some(el) if el.kind() == WHITESPACE => el,
+                Some(_) | None => start,
+            };
+            self.ast = self.replace_children(RangeInclusive::new(start, end), iter::empty());
         }
     }
 }

crates/ra_assists/src/auto_import.rs

@@ -334,7 +334,7 @@ fn best_action_for_target<'b, 'a: 'b>(
         .filter_map(ast::UseItem::use_tree)
         .map(|u| walk_use_tree_for_best_action(&mut storage, None, u, target))
         .fold(None, |best, a| {
-            best.and_then(|best| Some(*ImportAction::better(&best, &a))).or(Some(a))
+            best.and_then(|best| Some(*ImportAction::better(&best, &a))).or_else(|| Some(a))
         });
 
     match best_action {
@@ -347,7 +347,7 @@ fn best_action_for_target<'b, 'a: 'b>(
             let anchor = container
                 .children()
                 .find(|n| n.range().start() < anchor.range().start())
-                .or(Some(anchor));
+                .or_else(|| Some(anchor));
 
             return ImportAction::add_new_use(anchor, false);
         }

crates/ra_assists/src/change_visibility.rs

@@ -59,7 +59,7 @@ fn vis_offset(node: &SyntaxNode) -> TextUnit {
         })
         .next()
         .map(|it| it.range().start())
-        .unwrap_or(node.range().start())
+        .unwrap_or_else(|| node.range().start())
 }
 
 fn change_vis(mut ctx: AssistCtx<impl HirDatabase>, vis: &ast::Visibility) -> Option<Assist> {

crates/ra_assists/src/introduce_variable.rs

@@ -57,9 +57,9 @@ pub(crate) fn introduce_variable(mut ctx: AssistCtx<impl HirDatabase>) -> Option
             if text.starts_with("\r\n") {
                 buf.push_str("\r\n");
                 buf.push_str(text.trim_start_matches("\r\n"));
-            } else if text.starts_with("\n") {
+            } else if text.starts_with('\n') {
                 buf.push_str("\n");
-                buf.push_str(text.trim_start_matches("\n"));
+                buf.push_str(text.trim_start_matches('\n'));
             } else {
                 buf.push_str(text);
             }

crates/ra_batch/src/lib.rs

@@ -34,10 +34,10 @@ impl salsa::Database for BatchDatabase {
 }
 
 fn vfs_file_to_id(f: ra_vfs::VfsFile) -> FileId {
-    FileId(f.0.into())
+    FileId(f.0)
 }
 fn vfs_root_to_id(r: ra_vfs::VfsRoot) -> SourceRootId {
-    SourceRootId(r.0.into())
+    SourceRootId(r.0)
 }
 
 impl BatchDatabase {

crates/ra_cli/src/analysis_stats.rs

@@ -31,18 +31,16 @@ pub fn run(verbose: bool, path: &str, only: Option<&str>) -> Result<()> {
 
             for decl in module.declarations(&db) {
                 num_decls += 1;
-                match decl {
-                    ModuleDef::Function(f) => funcs.push(f),
-                    _ => {}
+                if let ModuleDef::Function(f) = decl {
+                    funcs.push(f);
                 }
             }
 
             for impl_block in module.impl_blocks(&db) {
                 for item in impl_block.items(&db) {
                     num_decls += 1;
-                    match item {
-                        ImplItem::Method(f) => funcs.push(f),
-                        _ => {}
+                    if let ImplItem::Method(f) = item {
+                        funcs.push(f);
                     }
                 }
             }

crates/ra_hir/src/adt.rs

@@ -36,7 +36,7 @@ impl AdtDef {
 
 impl Struct {
     pub(crate) fn variant_data(&self, db: &impl DefDatabase) -> Arc<VariantData> {
-        db.struct_data((*self).into()).variant_data.clone()
+        db.struct_data(*self).variant_data.clone()
     }
 }
 

crates/ra_hir/src/code_model.rs

@@ -281,9 +281,8 @@ impl Module {
 
         for impl_block in self.impl_blocks(db) {
             for item in impl_block.items(db) {
-                match item {
-                    crate::ImplItem::Method(f) => f.diagnostics(db, sink),
-                    _ => (),
+                if let crate::ImplItem::Method(f) = item {
+                    f.diagnostics(db, sink);
                 }
             }
         }

crates/ra_hir/src/expr/validation.rs

@@ -31,11 +31,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
     pub(crate) fn validate_body(&mut self, db: &impl HirDatabase) {
         let body = self.func.body(db);
         for e in body.exprs() {
-            match e {
-                (id, Expr::StructLit { path, fields, spread }) => {
-                    self.validate_struct_literal(id, path, fields, spread, db)
-                }
-                _ => (),
+            if let (id, Expr::StructLit { path, fields, spread }) = e {
+                self.validate_struct_literal(id, path, fields, spread, db);
             }
         }
     }
@@ -44,7 +41,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         &mut self,
         id: ExprId,
         _path: &Option<Path>,
-        fields: &Vec<StructLitField>,
+        fields: &[StructLitField],
         spread: &Option<ExprId>,
         db: &impl HirDatabase,
     ) {
@@ -57,7 +54,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
             _ => return,
         };
 
-        let lit_fields: FxHashSet<_> = fields.into_iter().map(|f| &f.name).collect();
+        let lit_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect();
         let missed_fields: Vec<Name> = struct_def
             .fields(db)
             .iter()

crates/ra_hir/src/impl_block.rs

@@ -202,7 +202,6 @@ impl ModuleImplBlocks {
         };
 
         let (file_id, module_source) = m.module.definition_source(db);
-        let file_id: HirFileId = file_id.into();
         let node = match &module_source {
             ModuleSource::SourceFile(node) => node.syntax(),
             ModuleSource::Module(node) => {

crates/ra_hir/src/lang_item.rs

@@ -95,7 +95,7 @@ impl LangItems {
                 .nth(0);
             if let Some(lang_item_name) = lang_item_name {
                 let imp = ImplBlock::from_id(*module, impl_id);
-                self.items.entry(lang_item_name).or_insert(LangItemTarget::ImplBlock(imp));
+                self.items.entry(lang_item_name).or_insert_with(|| LangItemTarget::ImplBlock(imp));
             }
         }
 

crates/ra_hir/src/nameres.rs

@@ -332,7 +332,8 @@ impl CrateDefMap {
         let name = path.expand_macro_expr()?;
         // search local first
         // FIXME: Remove public_macros check when we have a correct local_macors implementation
-        let local = self.public_macros.get(&name).or(self.local_macros.get(&name)).map(|it| *it);
+        let local =
+            self.public_macros.get(&name).or_else(|| self.local_macros.get(&name)).map(|it| *it);
         if local.is_some() {
             return local;
         }
@@ -405,7 +406,7 @@ impl CrateDefMap {
         };
 
         for (i, segment) in segments {
-            let curr = match curr_per_ns.as_ref().left().map_or(None, |m| m.as_ref().take_types()) {
+            let curr = match curr_per_ns.as_ref().left().and_then(|m| m.as_ref().take_types()) {
                 Some(r) => r,
                 None => {
                     // we still have path segments left, but the path so far
@@ -421,10 +422,8 @@ impl CrateDefMap {
             curr_per_ns = match curr {
                 ModuleDef::Module(module) => {
                     if module.krate != self.krate {
-                        let path = Path {
-                            segments: path.segments[i..].iter().cloned().collect(),
-                            kind: PathKind::Self_,
-                        };
+                        let path =
+                            Path { segments: path.segments[i..].to_vec(), kind: PathKind::Self_ };
                         log::debug!("resolving {:?} in other crate", path);
                         let defp_map = db.crate_def_map(module.krate);
                         let (def, s) =
@@ -468,7 +467,7 @@ impl CrateDefMap {
                     );
 
                     return ResolvePathResult::with(
-                        Either::Left(PerNs::types((*s).into())),
+                        Either::Left(PerNs::types(*s)),
                         ReachedFixedPoint::Yes,
                         Some(i),
                     );
@@ -479,8 +478,10 @@ impl CrateDefMap {
     }
 
     fn resolve_name_in_crate_root_or_extern_prelude(&self, name: &Name) -> ItemOrMacro {
-        let from_crate_root =
-            self[self.root].scope.get_item_or_macro(name).unwrap_or(Either::Left(PerNs::none()));
+        let from_crate_root = self[self.root]
+            .scope
+            .get_item_or_macro(name)
+            .unwrap_or_else(|| Either::Left(PerNs::none()));
         let from_extern_prelude = self.resolve_name_in_extern_prelude(name);
 
         or(from_crate_root, Either::Left(from_extern_prelude))
@@ -505,8 +506,10 @@ impl CrateDefMap {
         //  - current module / scope
         //  - extern prelude
         //  - std prelude
-        let from_scope =
-            self[module].scope.get_item_or_macro(name).unwrap_or(Either::Left(PerNs::none()));;
+        let from_scope = self[module]
+            .scope
+            .get_item_or_macro(name)
+            .unwrap_or_else(|| Either::Left(PerNs::none()));;
         let from_extern_prelude =
             self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it));
         let from_prelude = self.resolve_in_prelude(db, name);
@@ -525,7 +528,7 @@ impl CrateDefMap {
             } else {
                 db.crate_def_map(prelude.krate)[prelude.module_id].scope.get_item_or_macro(name)
             };
-            resolution.unwrap_or(Either::Left(PerNs::none()))
+            resolution.unwrap_or_else(|| Either::Left(PerNs::none()))
         } else {
             Either::Left(PerNs::none())
         }

crates/ra_hir/src/nameres/collector.rs

@@ -556,7 +556,7 @@ where
 
     fn define_def(&mut self, def: &raw::DefData) {
         let module = Module { krate: self.def_collector.def_map.krate, module_id: self.module_id };
-        let ctx = LocationCtx::new(self.def_collector.db, module, self.file_id.into());
+        let ctx = LocationCtx::new(self.def_collector.db, module, self.file_id);
 
         macro_rules! def {
             ($kind:ident, $ast_id:ident) => {

crates/ra_hir/src/nameres/raw.rs

@@ -69,7 +69,7 @@ impl RawItems {
     ) -> (Arc<RawItems>, Arc<ImportSourceMap>) {
         let mut collector = RawItemsCollector {
             raw_items: RawItems::default(),
-            source_ast_id_map: db.ast_id_map(file_id.into()),
+            source_ast_id_map: db.ast_id_map(file_id),
             source_map: ImportSourceMap::default(),
         };
         if let Some(node) = db.parse_or_expand(file_id) {

crates/ra_hir/src/path.rs

@@ -116,7 +116,7 @@ impl Path {
 
     /// `true` if this path is just a standalone `self`
     pub fn is_self(&self) -> bool {
-        self.kind == PathKind::Self_ && self.segments.len() == 0
+        self.kind == PathKind::Self_ && self.segments.is_empty()
     }
 
     /// If this path is a single identifier, like `foo`, return its name.
@@ -140,7 +140,7 @@ impl GenericArgs {
             args.push(GenericArg::Type(type_ref));
         }
         // lifetimes and assoc type args ignored for now
-        if args.len() > 0 {
+        if !args.is_empty() {
             Some(GenericArgs { args })
         } else {
             None

crates/ra_hir/src/source_binder.rs

@@ -48,8 +48,8 @@ pub fn module_from_declaration(
 pub fn module_from_position(db: &impl HirDatabase, position: FilePosition) -> Option<Module> {
     let file = db.parse(position.file_id).tree;
     match find_node_at_offset::<ast::Module>(file.syntax(), position.offset) {
-        Some(m) if !m.has_semi() => module_from_inline(db, position.file_id.into(), m),
-        _ => module_from_file_id(db, position.file_id.into()),
+        Some(m) if !m.has_semi() => module_from_inline(db, position.file_id, m),
+        _ => module_from_file_id(db, position.file_id),
     }
 }
 
@@ -72,9 +72,9 @@ pub fn module_from_child_node(
     child: &SyntaxNode,
 ) -> Option<Module> {
     if let Some(m) = child.ancestors().filter_map(ast::Module::cast).find(|it| !it.has_semi()) {
-        module_from_inline(db, file_id.into(), m)
+        module_from_inline(db, file_id, m)
     } else {
-        module_from_file_id(db, file_id.into())
+        module_from_file_id(db, file_id)
     }
 }
 
@@ -99,14 +99,12 @@ pub fn struct_from_module(
     struct_def: &ast::StructDef,
 ) -> Struct {
     let (file_id, _) = module.definition_source(db);
-    let file_id = file_id.into();
     let ctx = LocationCtx::new(db, module, file_id);
     Struct { id: ctx.to_def(struct_def) }
 }
 
 pub fn enum_from_module(db: &impl HirDatabase, module: Module, enum_def: &ast::EnumDef) -> Enum {
     let (file_id, _) = module.definition_source(db);
-    let file_id = file_id.into();
     let ctx = LocationCtx::new(db, module, file_id);
     Enum { id: ctx.to_def(enum_def) }
 }
@@ -117,7 +115,6 @@ pub fn trait_from_module(
     trait_def: &ast::TraitDef,
 ) -> Trait {
     let (file_id, _) = module.definition_source(db);
-    let file_id = file_id.into();
     let ctx = LocationCtx::new(db, module, file_id);
     Trait { id: ctx.to_def(trait_def) }
 }

crates/ra_hir/src/traits.rs

@@ -77,13 +77,10 @@ impl TraitItemsIndex {
     pub(crate) fn trait_items_index(db: &impl DefDatabase, module: Module) -> TraitItemsIndex {
         let mut index = TraitItemsIndex { traits_by_def: FxHashMap::default() };
         for decl in module.declarations(db) {
-            match decl {
-                crate::ModuleDef::Trait(tr) => {
-                    for item in tr.trait_data(db).items() {
-                        index.traits_by_def.insert(*item, tr);
-                    }
+            if let crate::ModuleDef::Trait(tr) = decl {
+                for item in tr.trait_data(db).items() {
+                    index.traits_by_def.insert(*item, tr);
                 }
-                _ => {}
             }
         }
         index

crates/ra_hir/src/ty.rs

@@ -451,7 +451,7 @@ impl Ty {
     /// Substitutes `Ty::Bound` vars (as opposed to type parameters).
     pub fn subst_bound_vars(self, substs: &Substs) -> Ty {
         self.fold(&mut |ty| match ty {
-            Ty::Bound(idx) => substs.get(idx as usize).cloned().unwrap_or(Ty::Bound(idx)),
+            Ty::Bound(idx) => substs.get(idx as usize).cloned().unwrap_or_else(|| Ty::Bound(idx)),
             ty => ty,
         })
     }