Skip to content

Commit

Permalink
Rework init_numbered_fields:
Browse files Browse the repository at this point in the history
* Only check the name of a single field
* Don't lint in macros
* Check for side effects
* Don't use a binary heap
  • Loading branch information
Jarcho committed Jul 7, 2024
1 parent 18a3384 commit a398354
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 41 deletions.
87 changes: 52 additions & 35 deletions clippy_lints/src/init_numbered_fields.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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,
);
},
);
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#![feature(f128)]
#![feature(f16)]
#![feature(if_let_guard)]
#![feature(is_sorted)]
#![feature(iter_intersperse)]
#![feature(let_chains)]
#![feature(never_type)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
}
}
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
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 {
| _____________^
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 {
| _____________^
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

0 comments on commit a398354

Please sign in to comment.