Skip to content

Make collapsible_if recognize the let_chains feature #14481

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.

Sign up for GitHub

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 3 commits into from
Mar 28, 2025
Merged

Make collapsible_if recognize the let_chains feature #14481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9df5177
Convert the `collapsible_if` early lint to a late one
samueltardieu Mar 26, 2025
cd70152
Make `collapsible_if` recognize the `let_chains` feature
samueltardieu Mar 26, 2025
79c6911
Apply `collapsible_if` to Clippy itself
samueltardieu Mar 26, 2025
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
76 changes: 38 additions & 38 deletions clippy_dev/src/update_lints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,53 +402,53 @@ fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec<Lint>) -> io
}
}

if path.exists() {
if let Some(lint) = lints.iter().find(|l| l.name == name) {
if lint.module == name {
// The lint name is the same as the file, we can just delete the entire file
fs::remove_file(path)?;
} else {
// We can't delete the entire file, just remove the declaration

if let Some(Some("mod.rs")) = path.file_name().map(OsStr::to_str) {
// Remove clippy_lints/src/some_mod/some_lint.rs
let mut lint_mod_path = path.to_path_buf();
lint_mod_path.set_file_name(name);
lint_mod_path.set_extension("rs");
if path.exists()
&& let Some(lint) = lints.iter().find(|l| l.name == name)
{
if lint.module == name {
// The lint name is the same as the file, we can just delete the entire file
fs::remove_file(path)?;
} else {
// We can't delete the entire file, just remove the declaration

let _ = fs::remove_file(lint_mod_path);
}
if let Some(Some("mod.rs")) = path.file_name().map(OsStr::to_str) {
// Remove clippy_lints/src/some_mod/some_lint.rs
let mut lint_mod_path = path.to_path_buf();
lint_mod_path.set_file_name(name);
lint_mod_path.set_extension("rs");

let mut content =
fs::read_to_string(path).unwrap_or_else(|_| panic!("failed to read `{}`", path.to_string_lossy()));
let _ = fs::remove_file(lint_mod_path);
}

eprintln!(
"warn: you will have to manually remove any code related to `{name}` from `{}`",
path.display()
);
let mut content =
fs::read_to_string(path).unwrap_or_else(|_| panic!("failed to read `{}`", path.to_string_lossy()));

assert!(
content[lint.declaration_range.clone()].contains(&name.to_uppercase()),
"error: `{}` does not contain lint `{}`'s declaration",
path.display(),
lint.name
);
eprintln!(
"warn: you will have to manually remove any code related to `{name}` from `{}`",
path.display()
);

// Remove lint declaration (declare_clippy_lint!)
content.replace_range(lint.declaration_range.clone(), "");
assert!(
content[lint.declaration_range.clone()].contains(&name.to_uppercase()),
"error: `{}` does not contain lint `{}`'s declaration",
path.display(),
lint.name
);

// Remove the module declaration (mod xyz;)
let mod_decl = format!("\nmod {name};");
content = content.replacen(&mod_decl, "", 1);
// Remove lint declaration (declare_clippy_lint!)
content.replace_range(lint.declaration_range.clone(), "");

remove_impl_lint_pass(&lint.name.to_uppercase(), &mut content);
fs::write(path, content).unwrap_or_else(|_| panic!("failed to write to `{}`", path.to_string_lossy()));
}
// Remove the module declaration (mod xyz;)
let mod_decl = format!("\nmod {name};");
content = content.replacen(&mod_decl, "", 1);

remove_test_assets(name);
remove_lint(name, lints);
return Ok(true);
remove_impl_lint_pass(&lint.name.to_uppercase(), &mut content);
fs::write(path, content).unwrap_or_else(|_| panic!("failed to write to `{}`", path.to_string_lossy()));
}

remove_test_assets(name);
remove_lint(name, lints);
return Ok(true);
}

Ok(false)
Expand Down
8 changes: 4 additions & 4 deletions clippy_dev/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ pub fn clippy_project_root() -> PathBuf {
let current_dir = std::env::current_dir().unwrap();
for path in current_dir.ancestors() {
let result = fs::read_to_string(path.join("Cargo.toml"));
if let Err(err) = &result {
if err.kind() == io::ErrorKind::NotFound {
continue;
}
if let Err(err) = &result
&& err.kind() == io::ErrorKind::NotFound
{
continue;
}

let content = result.unwrap();
Expand Down
18 changes: 10 additions & 8 deletions clippy_lints/src/arbitrary_source_item_ordering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,11 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering {
continue;
}

if let Some(cur_v) = cur_v {
if cur_v.ident.name.as_str() > variant.ident.name.as_str() && cur_v.span != variant.span {
Self::lint_member_name(cx, &variant.ident, &cur_v.ident);
}
if let Some(cur_v) = cur_v
&& cur_v.ident.name.as_str() > variant.ident.name.as_str()
&& cur_v.span != variant.span
{
Self::lint_member_name(cx, &variant.ident, &cur_v.ident);
}
cur_v = Some(variant);
}
Expand All @@ -278,10 +279,11 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering {
continue;
}

if let Some(cur_f) = cur_f {
if cur_f.ident.name.as_str() > field.ident.name.as_str() && cur_f.span != field.span {
Self::lint_member_name(cx, &field.ident, &cur_f.ident);
}
if let Some(cur_f) = cur_f
&& cur_f.ident.name.as_str() > field.ident.name.as_str()
&& cur_f.span != field.span
{
Self::lint_member_name(cx, &field.ident, &cur_f.ident);
}
cur_f = Some(field);
}
Expand Down
23 changes: 12 additions & 11 deletions clippy_lints/src/attrs/blanket_clippy_restriction_lints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,18 @@ use rustc_span::{DUMMY_SP, sym};

pub(super) fn check(cx: &EarlyContext<'_>, name: Symbol, items: &[MetaItemInner]) {
for lint in items {
if let Some(lint_name) = extract_clippy_lint(lint) {
if lint_name.as_str() == "restriction" && name != sym::allow {
span_lint_and_help(
cx,
BLANKET_CLIPPY_RESTRICTION_LINTS,
lint.span(),
"`clippy::restriction` is not meant to be enabled as a group",
None,
"enable the restriction lints you need individually",
);
}
if let Some(lint_name) = extract_clippy_lint(lint)
&& lint_name.as_str() == "restriction"
&& name != sym::allow
{
span_lint_and_help(
cx,
BLANKET_CLIPPY_RESTRICTION_LINTS,
lint.span(),
"`clippy::restriction` is not meant to be enabled as a group",
None,
"enable the restriction lints you need individually",
);
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/attrs/deprecated_semver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ use rustc_span::Span;
use semver::Version;

pub(super) fn check(cx: &EarlyContext<'_>, span: Span, lit: &MetaItemLit) {
if let LitKind::Str(is, _) = lit.kind {
if is.as_str() == "TBD" || Version::parse(is.as_str()).is_ok() {
return;
}
if let LitKind::Str(is, _) = lit.kind
&& (is.as_str() == "TBD" || Version::parse(is.as_str()).is_ok())
{
return;
}
span_lint(
cx,
Expand Down
41 changes: 20 additions & 21 deletions clippy_lints/src/attrs/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,28 +573,27 @@ impl EarlyLintPass for PostExpansionEarlyAttributes {
}

fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
if let Some(items) = &attr.meta_item_list() {
if let Some(ident) = attr.ident() {
if matches!(ident.name, sym::allow) && self.msrv.meets(msrvs::LINT_REASONS_STABILIZATION) {
allow_attributes::check(cx, attr);
}
if matches!(ident.name, sym::allow | sym::expect) && self.msrv.meets(msrvs::LINT_REASONS_STABILIZATION)
if let Some(items) = &attr.meta_item_list()
&& let Some(ident) = attr.ident()
{
if matches!(ident.name, sym::allow) && self.msrv.meets(msrvs::LINT_REASONS_STABILIZATION) {
allow_attributes::check(cx, attr);
}
if matches!(ident.name, sym::allow | sym::expect) && self.msrv.meets(msrvs::LINT_REASONS_STABILIZATION) {
allow_attributes_without_reason::check(cx, ident.name, items, attr);
}
if is_lint_level(ident.name, attr.id) {
blanket_clippy_restriction_lints::check(cx, ident.name, items);
}
if items.is_empty() || !attr.has_name(sym::deprecated) {
return;
}
for item in items {
if let MetaItemInner::MetaItem(mi) = &item
&& let MetaItemKind::NameValue(lit) = &mi.kind
&& mi.has_name(sym::since)
{
allow_attributes_without_reason::check(cx, ident.name, items, attr);
}
if is_lint_level(ident.name, attr.id) {
blanket_clippy_restriction_lints::check(cx, ident.name, items);
}
if items.is_empty() || !attr.has_name(sym::deprecated) {
return;
}
for item in items {
if let MetaItemInner::MetaItem(mi) = &item
&& let MetaItemKind::NameValue(lit) = &mi.kind
&& mi.has_name(sym::since)
{
deprecated_semver::check(cx, item.span(), lit);
}
deprecated_semver::check(cx, item.span(), lit);
}
}
}
Expand Down
130 changes: 65 additions & 65 deletions clippy_lints/src/attrs/useless_attribute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,75 +14,75 @@ pub(super) fn check(cx: &EarlyContext<'_>, item: &Item, attrs: &[Attribute]) {
if attr.span.in_external_macro(cx.sess().source_map()) {
return;
}
if let Some(lint_list) = &attr.meta_item_list() {
if attr.ident().is_some_and(|ident| is_lint_level(ident.name, attr.id)) {
for lint in lint_list {
match item.kind {
ItemKind::Use(..) => {
let (namespace @ (Some(sym::clippy) | None), Some(name)) = namespace_and_lint(lint) else {
return;
};
if let Some(lint_list) = &attr.meta_item_list()
&& attr.ident().is_some_and(|ident| is_lint_level(ident.name, attr.id))
{
for lint in lint_list {
match item.kind {
ItemKind::Use(..) => {
let (namespace @ (Some(sym::clippy) | None), Some(name)) = namespace_and_lint(lint) else {
return;
};

if namespace.is_none()
&& matches!(
name.as_str(),
"ambiguous_glob_reexports"
| "dead_code"
| "deprecated"
| "hidden_glob_reexports"
| "unreachable_pub"
| "unused"
| "unused_braces"
| "unused_import_braces"
| "unused_imports"
)
{
return;
}
if namespace.is_none()
&& matches!(
name.as_str(),
"ambiguous_glob_reexports"
| "dead_code"
| "deprecated"
| "hidden_glob_reexports"
| "unreachable_pub"
| "unused"
| "unused_braces"
| "unused_import_braces"
| "unused_imports"
)
{
return;
}

if namespace == Some(sym::clippy)
&& matches!(
name.as_str(),
"wildcard_imports"
| "enum_glob_use"
| "redundant_pub_crate"
| "macro_use_imports"
| "unsafe_removed_from_name"
| "module_name_repetitions"
| "single_component_path_imports"
| "disallowed_types"
| "unused_trait_names"
)
{
return;
}
},
ItemKind::ExternCrate(..) => {
if is_word(lint, sym::unused_imports) && skip_unused_imports {
return;
}
if is_word(lint, sym!(unused_extern_crates)) {
return;
}
},
_ => {},
}
if namespace == Some(sym::clippy)
&& matches!(
name.as_str(),
"wildcard_imports"
| "enum_glob_use"
| "redundant_pub_crate"
| "macro_use_imports"
| "unsafe_removed_from_name"
| "module_name_repetitions"
| "single_component_path_imports"
| "disallowed_types"
| "unused_trait_names"
)
{
return;
}
},
ItemKind::ExternCrate(..) => {
if is_word(lint, sym::unused_imports) && skip_unused_imports {
return;
}
if is_word(lint, sym!(unused_extern_crates)) {
return;
}
},
_ => {},
}
let line_span = first_line_of_span(cx, attr.span);
}
let line_span = first_line_of_span(cx, attr.span);

if let Some(src) = line_span.get_source_text(cx) {
if src.contains("#[") {
#[expect(clippy::collapsible_span_lint_calls)]
span_lint_and_then(cx, USELESS_ATTRIBUTE, line_span, "useless lint attribute", |diag| {
diag.span_suggestion(
line_span,
"if you just forgot a `!`, use",
src.replacen("#[", "#![", 1),
Applicability::MaybeIncorrect,
);
});
}
}
if let Some(src) = line_span.get_source_text(cx)
&& src.contains("#[")
{
#[expect(clippy::collapsible_span_lint_calls)]
span_lint_and_then(cx, USELESS_ATTRIBUTE, line_span, "useless lint attribute", |diag| {
diag.span_suggestion(
line_span,
"if you just forgot a `!`, use",
src.replacen("#[", "#![", 1),
Applicability::MaybeIncorrect,
);
});
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/await_holding_invalid.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,9 @@ impl<'tcx> LateLintPass<'tcx> for AwaitHolding {
def_id,
..
}) = expr.kind
&& let Some(coroutine_layout) = cx.tcx.mir_coroutine_witnesses(*def_id)
{
if let Some(coroutine_layout) = cx.tcx.mir_coroutine_witnesses(*def_id) {
self.check_interior_types(cx, coroutine_layout);
}
self.check_interior_types(cx, coroutine_layout);
}
}
}
Expand Down
Loading