Skip to content

internal: Lint debug prints and disallowed types with clippy #16470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
5 changes: 5 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -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>"}
]
7 changes: 3 additions & 4 deletions crates/flycheck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -539,8 +537,9 @@ impl CargoActor {
}
}

#[allow(clippy::large_enum_variant)]
enum CargoMessage {
CompilerArtifact(Box<cargo_metadata::Artifact>),
CompilerArtifact(cargo_metadata::Artifact),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha

Diagnostic(Diagnostic),
}

Expand Down
5 changes: 1 addition & 4 deletions crates/hir-def/src/body/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
8 changes: 3 additions & 5 deletions crates/hir-def/src/item_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
5 changes: 2 additions & 3 deletions crates/hir-ty/src/layout/tests.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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> {
Expand Down
9 changes: 5 additions & 4 deletions crates/hir-ty/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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},
Expand Down Expand Up @@ -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 };
Expand Down
2 changes: 1 addition & 1 deletion crates/hir-ty/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
4 changes: 1 addition & 3 deletions crates/hir/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
};
Expand Down
4 changes: 1 addition & 3 deletions crates/ide-assists/src/handlers/desugar_doc_comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
14 changes: 6 additions & 8 deletions crates/ide-assists/src/handlers/extract_module.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down
6 changes: 2 additions & 4 deletions crates/ide-assists/src/handlers/generate_delegate_methods.rs
Original file line number Diff line number Diff line change
@@ -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 _,
Expand Down Expand Up @@ -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());
Expand Down
4 changes: 1 addition & 3 deletions crates/ide-assists/src/handlers/generate_delegate_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 5 additions & 5 deletions crates/ide-assists/src/handlers/inline_type_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down
11 changes: 6 additions & 5 deletions crates/ide-assists/src/handlers/merge_match_arms.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -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 {
Expand All @@ -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>,
) {
Expand Down
6 changes: 3 additions & 3 deletions crates/ide-assists/src/handlers/remove_unused_imports.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions crates/ide-assists/src/handlers/unwrap_result_return_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 2 additions & 4 deletions crates/ide-assists/src/utils/suggest_name.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions crates/ide-db/src/tests/sourcegen_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading