Auto merge of #16470 - Veykril:clippy-disallow, r=lnicola

internal: Lint debug prints and disallowed types with clippy

.github/workflows/ci.yaml

@@ -105,7 +105,7 @@ jobs:
 
       - name: clippy
         if: matrix.os == 'ubuntu-latest'
-        run: cargo clippy --all-targets
+        run: cargo clippy --all-targets -- -D clippy::disallowed_macros -D clippy::dbg_macro -D clippy::todo -D clippy::print_stdout -D clippy::print_stderr
 
   # Weird targets to catch non-portable code
   rust-cross:

Cargo.toml

@@ -164,6 +164,8 @@ len_without_is_empty = "allow"
 enum_variant_names = "allow"
 # Builder pattern disagrees
 new_ret_no_self = "allow"
+# Has a bunch of false positives
+useless_asref = "allow"
 
 ## Following lints should be tackled at some point
 borrowed_box = "allow"
@@ -178,9 +180,12 @@ type_complexity = "allow"
 wrong_self_convention = "allow"
 
 ## warn at following lints
+# CI raises these to deny
 dbg_macro = "warn"
 todo = "warn"
-unimplemented = "allow"
+print_stdout = "warn"
+print_stderr = "warn"
+
 rc_buffer = "warn"
 # FIXME enable this, we use this pattern a lot so its annoying work ...
 # str_to_string = "warn"

clippy.toml

@@ -0,0 +1,5 @@
+disallowed-types = [
+    { path = "std::collections::HashMap", reason = "use FxHashMap" },
+    { path = "std::collections::HashSet", reason = "use FxHashSet" },
+    { path = "std::collections::hash_map::RandomState", reason = "use BuildHasherDefault<FxHasher>"}
+]

crates/flycheck/src/lib.rs

@@ -493,9 +493,7 @@ impl CargoActor {
                     // Skip certain kinds of messages to only spend time on what's useful
                     JsonMessage::Cargo(message) => match message {
                         cargo_metadata::Message::CompilerArtifact(artifact) if !artifact.fresh => {
-                            self.sender
-                                .send(CargoMessage::CompilerArtifact(Box::new(artifact)))
-                                .unwrap();
+                            self.sender.send(CargoMessage::CompilerArtifact(artifact)).unwrap();
                         }
                         cargo_metadata::Message::CompilerMessage(msg) => {
                             self.sender.send(CargoMessage::Diagnostic(msg.message)).unwrap();
@@ -539,8 +537,9 @@ impl CargoActor {
     }
 }
 
+#[allow(clippy::large_enum_variant)]
 enum CargoMessage {
-    CompilerArtifact(Box<cargo_metadata::Artifact>),
+    CompilerArtifact(cargo_metadata::Artifact),
     Diagnostic(Diagnostic),
 }
 

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

@@ -1980,10 +1980,7 @@ fn pat_literal_to_hir(lit: &ast::LiteralPat) -> Option<(Literal, ast::Literal)>
     let ast_lit = lit.literal()?;
     let mut hir_lit: Literal = ast_lit.kind().into();
     if lit.minus_token().is_some() {
-        let Some(h) = hir_lit.negate() else {
-            return None;
-        };
-        hir_lit = h;
+        hir_lit = hir_lit.negate()?;
     }
     Some((hir_lit, ast_lit))
 }

crates/hir-def/src/item_scope.rs

@@ -222,17 +222,15 @@ impl ItemScope {
         self.declarations.iter().copied()
     }
 
-    pub fn extern_crate_decls(
-        &self,
-    ) -> impl Iterator<Item = ExternCrateId> + ExactSizeIterator + '_ {
+    pub fn extern_crate_decls(&self) -> impl ExactSizeIterator<Item = ExternCrateId> + '_ {
         self.extern_crate_decls.iter().copied()
     }
 
-    pub fn use_decls(&self) -> impl Iterator<Item = UseId> + ExactSizeIterator + '_ {
+    pub fn use_decls(&self) -> impl ExactSizeIterator<Item = UseId> + '_ {
         self.use_decls.iter().copied()
     }
 
-    pub fn impls(&self) -> impl Iterator<Item = ImplId> + ExactSizeIterator + '_ {
+    pub fn impls(&self) -> impl ExactSizeIterator<Item = ImplId> + '_ {
         self.impls.iter().copied()
     }
 

crates/hir-ty/src/layout/tests.rs

@@ -1,8 +1,7 @@
-use std::collections::HashMap;
-
 use chalk_ir::{AdtId, TyKind};
 use either::Either;
 use hir_def::db::DefDatabase;
+use rustc_hash::FxHashMap;
 use test_fixture::WithFixture;
 use triomphe::Arc;
 
@@ -16,7 +15,7 @@ use crate::{
 mod closure;
 
 fn current_machine_data_layout() -> String {
-    project_model::target_data_layout::get(None, None, &HashMap::default()).unwrap()
+    project_model::target_data_layout::get(None, None, &FxHashMap::default()).unwrap()
 }
 
 fn eval_goal(ra_fixture: &str, minicore: &str) -> Result<Arc<Layout>, LayoutError> {

crates/hir-ty/src/tests.rs

@@ -10,7 +10,7 @@ mod regression;
 mod simple;
 mod traits;
 
-use std::{collections::HashMap, env};
+use std::env;
 
 use base_db::{FileRange, SourceDatabaseExt};
 use expect_test::Expect;
@@ -25,6 +25,7 @@ use hir_def::{
 };
 use hir_expand::{db::ExpandDatabase, InFile};
 use once_cell::race::OnceBool;
+use rustc_hash::FxHashMap;
 use stdx::format_to;
 use syntax::{
     ast::{self, AstNode, HasName},
@@ -90,9 +91,9 @@ fn check_impl(ra_fixture: &str, allow_none: bool, only_types: bool, display_sour
     let (db, files) = TestDB::with_many_files(ra_fixture);
 
     let mut had_annotations = false;
-    let mut mismatches = HashMap::new();
-    let mut types = HashMap::new();
-    let mut adjustments = HashMap::<_, Vec<_>>::new();
+    let mut mismatches = FxHashMap::default();
+    let mut types = FxHashMap::default();
+    let mut adjustments = FxHashMap::<_, Vec<_>>::default();
     for (file_id, annotations) in db.extract_annotations() {
         for (range, expected) in annotations {
             let file_range = FileRange { file_id, range };

crates/hir-ty/src/traits.rs

@@ -187,7 +187,7 @@ struct LoggingRustIrDatabaseLoggingOnDrop<'a>(LoggingRustIrDatabase<Interner, Ch
 
 impl Drop for LoggingRustIrDatabaseLoggingOnDrop<'_> {
     fn drop(&mut self) {
-        eprintln!("chalk program:\n{}", self.0);
+        tracing::info!("chalk program:\n{}", self.0);
     }
 }
 

crates/hir/src/diagnostics.rs

@@ -546,9 +546,7 @@ impl AnyDiagnostic {
                             source_map.pat_syntax(pat).expect("unexpected synthetic");
 
                         // cast from Either<Pat, SelfParam> -> Either<_, Pat>
-                        let Some(ptr) = AstPtr::try_from_raw(value.syntax_node_ptr()) else {
-                            return None;
-                        };
+                        let ptr = AstPtr::try_from_raw(value.syntax_node_ptr())?;
                         InFile { file_id, value: ptr }
                     }
                 };

crates/ide-assists/src/handlers/desugar_doc_comment.rs

@@ -27,9 +27,7 @@ use crate::{
 pub(crate) fn desugar_doc_comment(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
     let comment = ctx.find_token_at_offset::<ast::Comment>()?;
     // Only allow doc comments
-    let Some(placement) = comment.kind().doc else {
-        return None;
-    };
+    let placement = comment.kind().doc?;
 
     // Only allow comments which are alone on their line
     if let Some(prev) = comment.syntax().prev_token() {

crates/ide-assists/src/handlers/extract_module.rs

@@ -1,14 +1,12 @@
-use std::{
-    collections::{HashMap, HashSet},
-    iter,
-};
+use std::iter;
 
 use hir::{HasSource, HirFileIdExt, ModuleSource};
 use ide_db::{
     assists::{AssistId, AssistKind},
     base_db::FileId,
     defs::{Definition, NameClass, NameRefClass},
     search::{FileReference, SearchScope},
+    FxHashMap, FxHashSet,
 };
 use itertools::Itertools;
 use smallvec::SmallVec;
@@ -235,9 +233,9 @@ impl Module {
     fn get_usages_and_record_fields(
         &self,
         ctx: &AssistContext<'_>,
-    ) -> (HashMap<FileId, Vec<(TextRange, String)>>, Vec<SyntaxNode>) {
+    ) -> (FxHashMap<FileId, Vec<(TextRange, String)>>, Vec<SyntaxNode>) {
         let mut adt_fields = Vec::new();
-        let mut refs: HashMap<FileId, Vec<(TextRange, String)>> = HashMap::new();
+        let mut refs: FxHashMap<FileId, Vec<(TextRange, String)>> = FxHashMap::default();
 
         //Here impl is not included as each item inside impl will be tied to the parent of
         //implementing block(a struct, enum, etc), if the parent is in selected module, it will
@@ -320,7 +318,7 @@ impl Module {
         &self,
         ctx: &AssistContext<'_>,
         node_def: Definition,
-        refs_in_files: &mut HashMap<FileId, Vec<(TextRange, String)>>,
+        refs_in_files: &mut FxHashMap<FileId, Vec<(TextRange, String)>>,
     ) {
         for (file_id, references) in node_def.usages(&ctx.sema).all() {
             let source_file = ctx.sema.parse(file_id);
@@ -400,7 +398,7 @@ impl Module {
         ctx: &AssistContext<'_>,
     ) -> Vec<TextRange> {
         let mut import_paths_to_be_removed: Vec<TextRange> = vec![];
-        let mut node_set: HashSet<String> = HashSet::new();
+        let mut node_set: FxHashSet<String> = FxHashSet::default();
 
         for item in self.body_items.clone() {
             for x in item.syntax().descendants() {

crates/ide-assists/src/handlers/generate_delegate_methods.rs

@@ -1,7 +1,5 @@
-use std::collections::HashSet;
-
 use hir::{self, HasCrate, HasVisibility};
-use ide_db::path_transform::PathTransform;
+use ide_db::{path_transform::PathTransform, FxHashSet};
 use syntax::{
     ast::{
         self, edit_in_place::Indent, make, AstNode, HasGenericParams, HasName, HasVisibility as _,
@@ -71,7 +69,7 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<'
 
     let sema_field_ty = ctx.sema.resolve_type(&field_ty)?;
     let mut methods = vec![];
-    let mut seen_names = HashSet::new();
+    let mut seen_names = FxHashSet::default();
 
     for ty in sema_field_ty.autoderef(ctx.db()) {
         let krate = ty.krate(ctx.db());

crates/ide-assists/src/handlers/generate_delegate_trait.rs

@@ -502,9 +502,7 @@ fn generate_args_for_impl(
     trait_params: &Option<GenericParamList>,
     old_trait_args: &FxHashSet<String>,
 ) -> Option<ast::GenericArgList> {
-    let Some(old_impl_args) = old_impl_gpl.map(|gpl| gpl.to_generic_args().generic_args()) else {
-        return None;
-    };
+    let old_impl_args = old_impl_gpl.map(|gpl| gpl.to_generic_args().generic_args())?;
     // Create pairs of the args of `self_ty` and corresponding `field_ty` to
     // form the substitution list
     let mut arg_substs = FxHashMap::default();

crates/ide-assists/src/handlers/inline_type_alias.rs

@@ -3,12 +3,12 @@
 // - Remove unused aliases if there are no longer any users, see inline_call.rs.
 
 use hir::{HasSource, PathResolution};
+use ide_db::FxHashMap;
 use ide_db::{
     defs::Definition, imports::insert_use::ast_to_remove_for_path_in_use_stmt,
     search::FileReference,
 };
 use itertools::Itertools;
-use std::collections::HashMap;
 use syntax::{
     ast::{self, make, HasGenericParams, HasName},
     ted, AstNode, NodeOrToken, SyntaxNode,
@@ -189,14 +189,14 @@ fn inline(alias_def: &ast::TypeAlias, alias_instance: &ast::PathType) -> Option<
     Some(repl)
 }
 
-struct LifetimeMap(HashMap<String, ast::Lifetime>);
+struct LifetimeMap(FxHashMap<String, ast::Lifetime>);
 
 impl LifetimeMap {
     fn new(
         instance_args: &Option<ast::GenericArgList>,
         alias_generics: &ast::GenericParamList,
     ) -> Option<Self> {
-        let mut inner = HashMap::new();
+        let mut inner = FxHashMap::default();
 
         let wildcard_lifetime = make::lifetime("'_");
         let lifetimes = alias_generics
@@ -231,14 +231,14 @@ impl LifetimeMap {
     }
 }
 
-struct ConstAndTypeMap(HashMap<String, SyntaxNode>);
+struct ConstAndTypeMap(FxHashMap<String, SyntaxNode>);
 
 impl ConstAndTypeMap {
     fn new(
         instance_args: &Option<ast::GenericArgList>,
         alias_generics: &ast::GenericParamList,
     ) -> Option<Self> {
-        let mut inner = HashMap::new();
+        let mut inner = FxHashMap::default();
         let instance_generics = generic_args_to_const_and_type_generics(instance_args);
         let alias_generics = generic_param_list_to_const_and_type_generics(alias_generics);
 

crates/ide-assists/src/handlers/merge_match_arms.rs

@@ -1,5 +1,6 @@
 use hir::Type;
-use std::{collections::HashMap, iter::successors};
+use ide_db::FxHashMap;
+use std::iter::successors;
 use syntax::{
     algo::neighbor,
     ast::{self, AstNode, HasName},
@@ -95,7 +96,7 @@ fn contains_placeholder(a: &ast::MatchArm) -> bool {
 }
 
 fn are_same_types(
-    current_arm_types: &HashMap<String, Option<Type>>,
+    current_arm_types: &FxHashMap<String, Option<Type>>,
     arm: &ast::MatchArm,
     ctx: &AssistContext<'_>,
 ) -> bool {
@@ -114,11 +115,11 @@ fn are_same_types(
 fn get_arm_types(
     context: &AssistContext<'_>,
     arm: &ast::MatchArm,
-) -> HashMap<String, Option<Type>> {
-    let mut mapping: HashMap<String, Option<Type>> = HashMap::new();
+) -> FxHashMap<String, Option<Type>> {
+    let mut mapping: FxHashMap<String, Option<Type>> = FxHashMap::default();
 
     fn recurse(
-        map: &mut HashMap<String, Option<Type>>,
+        map: &mut FxHashMap<String, Option<Type>>,
         ctx: &AssistContext<'_>,
         pat: &Option<ast::Pat>,
     ) {

crates/ide-assists/src/handlers/remove_unused_imports.rs

@@ -1,11 +1,11 @@
-use std::collections::{hash_map::Entry, HashMap};
+use std::collections::hash_map::Entry;
 
 use hir::{HirFileIdExt, InFile, InRealFile, Module, ModuleSource};
 use ide_db::{
     base_db::FileRange,
     defs::Definition,
     search::{FileReference, ReferenceCategory, SearchScope},
-    RootDatabase,
+    FxHashMap, RootDatabase,
 };
 use syntax::{ast, AstNode};
 use text_edit::TextRange;
@@ -44,7 +44,7 @@ pub(crate) fn remove_unused_imports(acc: &mut Assists, ctx: &AssistContext<'_>)
     let uses = uses_up.chain(uses_down).collect::<Vec<_>>();
 
     // Maps use nodes to the scope that we should search through to find
-    let mut search_scopes = HashMap::<Module, Vec<SearchScope>>::new();
+    let mut search_scopes = FxHashMap::<Module, Vec<SearchScope>>::default();
 
     // iterator over all unused use trees
     let mut unused = uses

crates/ide-assists/src/handlers/unwrap_result_return_type.rs

@@ -47,9 +47,7 @@ pub(crate) fn unwrap_result_return_type(acc: &mut Assists, ctx: &AssistContext<'
         return None;
     }
 
-    let Some(ok_type) = unwrap_result_type(type_ref) else {
-        return None;
-    };
+    let ok_type = unwrap_result_type(type_ref)?;
 
     acc.add(
         AssistId("unwrap_result_return_type", AssistKind::RefactorRewrite),

crates/ide-assists/src/utils/suggest_name.rs

@@ -1,9 +1,7 @@
 //! This module contains functions to suggest names for expressions, functions and other items
 
-use std::collections::HashSet;
-
 use hir::Semantics;
-use ide_db::RootDatabase;
+use ide_db::{FxHashSet, RootDatabase};
 use itertools::Itertools;
 use stdx::to_lower_snake_case;
 use syntax::{
@@ -78,7 +76,7 @@ pub(crate) fn for_unique_generic_name(
             ast::GenericParam::TypeParam(t) => t.name().unwrap().to_string(),
             p => p.to_string(),
         })
-        .collect::<HashSet<_>>();
+        .collect::<FxHashSet<_>>();
     let mut name = name.to_string();
     let base_len = name.len();
     let mut count = 0;

crates/ide-db/src/tests/sourcegen_lints.rs

@@ -241,6 +241,7 @@ fn unescape(s: &str) -> String {
     s.replace(r#"\""#, "").replace(r#"\n"#, "\n").replace(r#"\r"#, "")
 }
 
+#[allow(clippy::print_stderr)]
 fn generate_descriptor_clippy(buf: &mut String, path: &Path) {
     let file_content = std::fs::read_to_string(path).unwrap();
     let mut clippy_lints: Vec<ClippyLint> = Vec::new();