fix(collapsible{,_else}_if): respect #[expect] on inner if

This commit is contained in:
Ada Alakbarova 2025-09-09 22:45:29 +02:00
parent 49e2f37b26
commit 593a9c98ae
No known key found for this signature in database
14 changed files with 256 additions and 27 deletions

View file

@ -1,16 +1,16 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::{IntoSpan as _, SpanRangeExt, snippet, snippet_block_with_applicability};
use clippy_utils::{span_contains_non_whitespace, tokenize_with_text};
use rustc_ast::BinOpKind;
use clippy_utils::{span_contains_non_whitespace, sym, tokenize_with_text};
use rustc_ast::{BinOpKind, MetaItemInner};
use rustc_errors::Applicability;
use rustc_hir::{Block, Expr, ExprKind, StmtKind};
use rustc_lexer::TokenKind;
use rustc_lint::{LateContext, LateLintPass};
use rustc_lint::{LateContext, LateLintPass, Level};
use rustc_session::impl_lint_pass;
use rustc_span::source_map::SourceMap;
use rustc_span::{BytePos, Span};
use rustc_span::{BytePos, Span, Symbol};
declare_clippy_lint! {
/// ### What it does
@ -95,14 +95,14 @@ impl CollapsibleIf {
fn check_collapsible_else_if(&self, cx: &LateContext<'_>, then_span: Span, else_block: &Block<'_>) {
if let Some(else_) = expr_block(else_block)
&& cx.tcx.hir_attrs(else_.hir_id).is_empty()
&& !else_.span.from_expansion()
&& let ExprKind::If(else_if_cond, ..) = else_.kind
&& !block_starts_with_significant_tokens(cx, else_block, else_, self.lint_commented_code)
&& self.check_significant_tokens_and_expect_attrs(cx, else_block, else_, sym::collapsible_else_if)
{
span_lint_and_then(
span_lint_hir_and_then(
cx,
COLLAPSIBLE_ELSE_IF,
else_.hir_id,
else_block.span,
"this `else { if .. }` block can be collapsed",
|diag| {
@ -166,15 +166,15 @@ impl CollapsibleIf {
fn check_collapsible_if_if(&self, cx: &LateContext<'_>, expr: &Expr<'_>, check: &Expr<'_>, then: &Block<'_>) {
if let Some(inner) = expr_block(then)
&& cx.tcx.hir_attrs(inner.hir_id).is_empty()
&& let ExprKind::If(check_inner, _, None) = &inner.kind
&& self.eligible_condition(cx, check_inner)
&& expr.span.eq_ctxt(inner.span)
&& !block_starts_with_significant_tokens(cx, then, inner, self.lint_commented_code)
&& self.check_significant_tokens_and_expect_attrs(cx, then, inner, sym::collapsible_if)
{
span_lint_and_then(
span_lint_hir_and_then(
cx,
COLLAPSIBLE_IF,
inner.hir_id,
expr.span,
"this `if` statement can be collapsed",
|diag| {
@ -219,6 +219,45 @@ impl CollapsibleIf {
!matches!(cond.kind, ExprKind::Let(..))
|| (cx.tcx.sess.edition().at_least_rust_2024() && self.msrv.meets(cx, msrvs::LET_CHAINS))
}
// Check that nothing significant can be found between the initial `{` of `inner_if` and
// the beginning of `inner_if_expr`...
//
// Unless it's only an `#[expect(clippy::collapsible{,_else}_if)]` attribute, in which case we
// _do_ need to lint, in order to actually fulfill its expectation (#13365)
fn check_significant_tokens_and_expect_attrs(
&self,
cx: &LateContext<'_>,
inner_if: &Block<'_>,
inner_if_expr: &Expr<'_>,
expected_lint_name: Symbol,
) -> bool {
match cx.tcx.hir_attrs(inner_if_expr.hir_id) {
[] => {
// There aren't any attributes, so just check for significant tokens
let span = inner_if.span.split_at(1).1.until(inner_if_expr.span);
!span_contains_non_whitespace(cx, span, self.lint_commented_code)
},
[attr]
if matches!(Level::from_attr(attr), Some((Level::Expect, _)))
&& let Some(metas) = attr.meta_item_list()
&& let Some(MetaItemInner::MetaItem(meta_item)) = metas.first()
&& let [tool, lint_name] = meta_item.path.segments.as_slice()
&& tool.ident.name == sym::clippy
&& [expected_lint_name, sym::style, sym::all].contains(&lint_name.ident.name) =>
{
// There is an `expect` attribute -- check that there is no _other_ significant text
let span_before_attr = inner_if.span.split_at(1).1.until(attr.span());
let span_after_attr = attr.span().between(inner_if_expr.span);
!span_contains_non_whitespace(cx, span_before_attr, self.lint_commented_code)
&& !span_contains_non_whitespace(cx, span_after_attr, self.lint_commented_code)
},
// There are other attributes, which are significant tokens -- check failed
_ => false,
}
}
}
impl_lint_pass!(CollapsibleIf => [COLLAPSIBLE_IF, COLLAPSIBLE_ELSE_IF]);
@ -242,18 +281,6 @@ impl LateLintPass<'_> for CollapsibleIf {
}
}
// Check that nothing significant can be found but whitespaces between the initial `{` of `block`
// and the beginning of `stop_at`.
fn block_starts_with_significant_tokens(
cx: &LateContext<'_>,
block: &Block<'_>,
stop_at: &Expr<'_>,
lint_commented_code: bool,
) -> bool {
let span = block.span.split_at(1).1.until(stop_at.span);
span_contains_non_whitespace(cx, span, lint_commented_code)
}
/// If `block` is a block with either one expression or a statement containing an expression,
/// return the expression. We don't peel blocks recursively, as extra blocks might be intentional.
fn expr_block<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> {

View file

@ -111,6 +111,8 @@ generate! {
clone_into,
cloned,
cognitive_complexity,
collapsible_else_if,
collapsible_if,
collect,
const_ptr,
contains,

View file

@ -1,7 +1,7 @@
#![allow(clippy::eq_op, clippy::nonminimal_bool)]
#![warn(clippy::collapsible_if)]
#[rustfmt::skip]
#[warn(clippy::collapsible_if)]
fn main() {
let (x, y) = ("hello", "world");
@ -48,3 +48,20 @@ fn main() {
}
//~^^^^^^ collapsible_else_if
}
fn issue_13365() {
// the comments don't stop us from linting, so the the `expect` *will* be fulfilled
if true {
} else {
// some other text before
#[expect(clippy::collapsible_else_if)]
if false {}
}
if true {
} else {
#[expect(clippy::collapsible_else_if)]
// some other text after
if false {}
}
}

View file

@ -1,7 +1,7 @@
#![allow(clippy::eq_op, clippy::nonminimal_bool)]
#![warn(clippy::collapsible_if)]
#[rustfmt::skip]
#[warn(clippy::collapsible_if)]
fn main() {
let (x, y) = ("hello", "world");
@ -53,3 +53,20 @@ fn main() {
}
//~^^^^^^ collapsible_else_if
}
fn issue_13365() {
// the comments don't stop us from linting, so the the `expect` *will* be fulfilled
if true {
} else {
// some other text before
#[expect(clippy::collapsible_else_if)]
if false {}
}
if true {
} else {
#[expect(clippy::collapsible_else_if)]
// some other text after
if false {}
}
}

View file

@ -87,6 +87,33 @@ fn issue_7318() {
//~^^^ collapsible_else_if
}
fn issue_13365() {
// all the `expect`s that we should fulfill
if true {
} else {
#[expect(clippy::collapsible_else_if)]
if false {}
}
if true {
} else {
#[expect(clippy::style)]
if false {}
}
if true {
} else {
#[expect(clippy::all)]
if false {}
}
if true {
} else {
#[expect(warnings)]
if false {}
}
}
fn issue14799() {
use std::ops::ControlFlow;

View file

@ -103,6 +103,33 @@ fn issue_7318() {
//~^^^ collapsible_else_if
}
fn issue_13365() {
// all the `expect`s that we should fulfill
if true {
} else {
#[expect(clippy::collapsible_else_if)]
if false {}
}
if true {
} else {
#[expect(clippy::style)]
if false {}
}
if true {
} else {
#[expect(clippy::all)]
if false {}
}
if true {
} else {
#[expect(warnings)]
if false {}
}
}
fn issue14799() {
use std::ops::ControlFlow;

View file

@ -151,7 +151,7 @@ LL | | }
| |_____^ help: collapse nested if block: `if false {}`
error: this `else { if .. }` block can be collapsed
--> tests/ui/collapsible_else_if.rs:130:12
--> tests/ui/collapsible_else_if.rs:157:12
|
LL | } else {
| ____________^

View file

@ -0,0 +1,22 @@
//@no-rustfix
#![warn(clippy::collapsible_else_if)]
fn issue_13365() {
// in the following examples, we won't lint because of the comments,
// so the the `expect` will be unfulfilled
if true {
} else {
// some other text before
#[expect(clippy::collapsible_else_if)]
if false {}
}
//~^^^ ERROR: this lint expectation is unfulfilled
if true {
} else {
#[expect(clippy::collapsible_else_if)]
// some other text after
if false {}
}
//~^^^^ ERROR: this lint expectation is unfulfilled
}

View file

@ -0,0 +1,17 @@
error: this lint expectation is unfulfilled
--> tests/ui/collapsible_else_if_unfixable.rs:10:18
|
LL | #[expect(clippy::collapsible_else_if)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D unfulfilled-lint-expectations` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(unfulfilled_lint_expectations)]`
error: this lint expectation is unfulfilled
--> tests/ui/collapsible_else_if_unfixable.rs:17:18
|
LL | #[expect(clippy::collapsible_else_if)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 2 previous errors

View file

@ -144,6 +144,24 @@ fn layout_check() -> u32 {
//~^^^^^ collapsible_if
}
fn issue13365() {
// all the `expect`s that we should fulfill
if true {
#[expect(clippy::collapsible_if)]
if true {}
}
if true {
#[expect(clippy::style)]
if true {}
}
if true {
#[expect(clippy::all)]
if true {}
}
}
fn issue14722() {
let x = if true {
Some(1)

View file

@ -154,6 +154,24 @@ fn layout_check() -> u32 {
//~^^^^^ collapsible_if
}
fn issue13365() {
// all the `expect`s that we should fulfill
if true {
#[expect(clippy::collapsible_if)]
if true {}
}
if true {
#[expect(clippy::style)]
if true {}
}
if true {
#[expect(clippy::all)]
if true {}
}
}
fn issue14722() {
let x = if true {
Some(1)

View file

@ -191,7 +191,7 @@ LL ~ ; 3
|
error: this `if` statement can be collapsed
--> tests/ui/collapsible_if.rs:178:5
--> tests/ui/collapsible_if.rs:196:5
|
LL | / if true {
LL | | (if true {

View file

@ -0,0 +1,20 @@
//@ no-rustfix
#![warn(clippy::collapsible_if)]
fn issue13365() {
// in the following examples, we won't lint because of the comments,
// so the the `expect` will be unfulfilled
if true {
// don't collapsible because of this comment
#[expect(clippy::collapsible_if)]
if true {}
}
//~^^^ ERROR: this lint expectation is unfulfilled
if true {
#[expect(clippy::collapsible_if)]
// don't collapsible because of this comment
if true {}
}
//~^^^^ ERROR: this lint expectation is unfulfilled
}

View file

@ -0,0 +1,17 @@
error: this lint expectation is unfulfilled
--> tests/ui/collapsible_if_unfixable.rs:9:18
|
LL | #[expect(clippy::collapsible_if)]
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D unfulfilled-lint-expectations` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(unfulfilled_lint_expectations)]`
error: this lint expectation is unfulfilled
--> tests/ui/collapsible_if_unfixable.rs:15:18
|
LL | #[expect(clippy::collapsible_if)]
| ^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 2 previous errors