Rework init_numbered_fields:

* Only check the name of a single field
* Don't lint in macros
* Check for side effects
* Don't use a binary heap
This commit is contained in:
Jason Newcomb 2024-06-12 13:21:09 -04:00
parent a4132817fb
commit 5b7ffa1a63
5 changed files with 77 additions and 41 deletions

View file

@ -1,13 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::SyntaxContext;
use std::borrow::Cow;
use std::cmp::Reverse;
use std::collections::BinaryHeap;
declare_clippy_lint! {
/// ### What it does
@ -44,38 +43,56 @@ declare_lint_pass!(NumberedFields => [INIT_NUMBERED_FIELDS]);
impl<'tcx> LateLintPass<'tcx> for NumberedFields {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
if let ExprKind::Struct(path, fields, None) = e.kind {
if !fields.is_empty()
&& !e.span.from_expansion()
&& fields
.iter()
.all(|f| f.ident.as_str().as_bytes().iter().all(u8::is_ascii_digit))
&& !matches!(cx.qpath_res(path, e.hir_id), Res::Def(DefKind::TyAlias, ..))
{
let expr_spans = fields
.iter()
.map(|f| (Reverse(f.ident.as_str().parse::<usize>().unwrap()), f.expr.span))
.collect::<BinaryHeap<_>>();
let mut appl = Applicability::MachineApplicable;
let snippet = format!(
"{}({})",
snippet_with_applicability(cx, path.span(), "..", &mut appl),
expr_spans
.into_iter_sorted()
.map(|(_, span)| snippet_with_context(cx, span, path.span().ctxt(), "..", &mut appl).0)
.intersperse(Cow::Borrowed(", "))
.collect::<String>()
);
span_lint_and_sugg(
cx,
INIT_NUMBERED_FIELDS,
e.span,
"used a field initializer for a tuple struct",
"try",
snippet,
appl,
);
}
if let ExprKind::Struct(path, fields @ [field, ..], None) = e.kind
// If the first character of any field is a digit it has to be a tuple.
&& field.ident.as_str().as_bytes().first().is_some_and(u8::is_ascii_digit)
// Type aliases can't be used as functions.
&& !matches!(
cx.qpath_res(path, e.hir_id),
Res::Def(DefKind::TyAlias | DefKind::AssocTy, _)
)
// This is the only syntax macros can use that works for all struct types.
&& !e.span.from_expansion()
&& let mut has_side_effects = false
&& let Ok(mut expr_spans) = fields
.iter()
.map(|f| {
has_side_effects |= f.expr.can_have_side_effects();
f.ident.as_str().parse::<usize>().map(|x| (x, f.expr.span))
})
.collect::<Result<Vec<_>, _>>()
// We can only reorder the expressions if there are no side effects.
&& (!has_side_effects || expr_spans.is_sorted_by_key(|&(idx, _)| idx))
{
span_lint_and_then(
cx,
INIT_NUMBERED_FIELDS,
e.span,
"used a field initializer for a tuple struct",
|diag| {
if !has_side_effects {
// We already checked the order if there are side effects.
expr_spans.sort_by_key(|&(idx, _)| idx);
}
let mut app = Applicability::MachineApplicable;
diag.span_suggestion(
e.span,
"use tuple initialization",
format!(
"{}({})",
snippet_with_applicability(cx, path.span(), "..", &mut app),
expr_spans
.into_iter()
.map(
|(_, span)| snippet_with_context(cx, span, SyntaxContext::root(), "..", &mut app).0
)
.intersperse(Cow::Borrowed(", "))
.collect::<String>()
),
app,
);
},
);
}
}
}

View file

@ -4,6 +4,7 @@
#![feature(f128)]
#![feature(f16)]
#![feature(if_let_guard)]
#![feature(is_sorted)]
#![feature(iter_intersperse)]
#![feature(let_chains)]
#![feature(never_type)]

View file

@ -39,4 +39,13 @@ fn main() {
struct TupleStructVec(Vec<usize>);
let _ = TupleStructVec(vec![0, 1, 2, 3]);
{
struct S(i32, i32);
let mut iter = [1i32, 1i32].into_iter();
let _ = S {
1: iter.next().unwrap(),
0: iter.next().unwrap(),
};
}
}

View file

@ -47,4 +47,13 @@ fn main() {
struct TupleStructVec(Vec<usize>);
let _ = TupleStructVec { 0: vec![0, 1, 2, 3] };
{
struct S(i32, i32);
let mut iter = [1i32, 1i32].into_iter();
let _ = S {
1: iter.next().unwrap(),
0: iter.next().unwrap(),
};
}
}

View file

@ -1,5 +1,5 @@
error: used a field initializer for a tuple struct
--> tests/ui/numbered_fields.rs:17:13
--> tests/ui/init_numbered_fields.rs:17:13
|
LL | let _ = TupleStruct {
| _____________^
@ -7,13 +7,13 @@ LL | | 0: 1u32,
LL | | 1: 42,
LL | | 2: 23u8,
LL | | };
| |_____^ help: try: `TupleStruct(1u32, 42, 23u8)`
| |_____^ help: use tuple initialization: `TupleStruct(1u32, 42, 23u8)`
|
= note: `-D clippy::init-numbered-fields` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::init_numbered_fields)]`
error: used a field initializer for a tuple struct
--> tests/ui/numbered_fields.rs:24:13
--> tests/ui/init_numbered_fields.rs:24:13
|
LL | let _ = TupleStruct {
| _____________^
@ -21,13 +21,13 @@ LL | | 0: 1u32,
LL | | 2: 2u8,
LL | | 1: 3u32,
LL | | };
| |_____^ help: try: `TupleStruct(1u32, 3u32, 2u8)`
| |_____^ help: use tuple initialization: `TupleStruct(1u32, 3u32, 2u8)`
error: used a field initializer for a tuple struct
--> tests/ui/numbered_fields.rs:49:13
--> tests/ui/init_numbered_fields.rs:49:13
|
LL | let _ = TupleStructVec { 0: vec![0, 1, 2, 3] };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `TupleStructVec(vec![0, 1, 2, 3])`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use tuple initialization: `TupleStructVec(vec![0, 1, 2, 3])`
error: aborting due to 3 previous errors