Skip to content

Commit b2ff678

Browse files
rustbotrustbot
authored
add new lints: rest_pattern_accessible_field and unnecessary_rest_pattern (#15000)
*[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/15000)* Add a new lint that warns when using a rest pattern when destructuring a struct. This lint was requested in this comment #10666 (comment) by @xxchan. changelog: [`rest_pattern_accessible_field`] new lint changelog: [`unnecessary_rest_pattern`] new lint
2 parents 732e6a3 + edce117 commit b2ff678

12 files changed

Lines changed: 838 additions & 3 deletions

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7394,6 +7394,7 @@ Released 2018-09-13
73947394
[`repr_packed_without_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#repr_packed_without_abi
73957395
[`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization
73967396
[`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
7397+
[`rest_pattern_accessible_field`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pattern_accessible_field
73977398
[`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
73987399
[`result_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_filter_map
73997400
[`result_large_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err
@@ -7559,6 +7560,7 @@ Released 2018-09-13
75597560
[`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
75607561
[`unnecessary_option_map_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_option_map_or_else
75617562
[`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings
7563+
[`unnecessary_rest_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_rest_pattern
75627564
[`unnecessary_result_map_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_result_map_or_else
75637565
[`unnecessary_safety_comment`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_comment
75647566
[`unnecessary_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_doc

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
684684
crate::repeat_vec_with_capacity::REPEAT_VEC_WITH_CAPACITY_INFO,
685685
crate::replace_box::REPLACE_BOX_INFO,
686686
crate::reserve_after_initialization::RESERVE_AFTER_INITIALIZATION_INFO,
687+
crate::rest_when_destructuring_struct::REST_PATTERN_ACCESSIBLE_FIELD_INFO,
688+
crate::rest_when_destructuring_struct::UNNECESSARY_REST_PATTERN_INFO,
687689
crate::return_self_not_must_use::RETURN_SELF_NOT_MUST_USE_INFO,
688690
crate::returns::LET_AND_RETURN_INFO,
689691
crate::returns::NEEDLESS_RETURN_INFO,

‎clippy_lints/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ mod regex;
326326
mod repeat_vec_with_capacity;
327327
mod replace_box;
328328
mod reserve_after_initialization;
329+
mod rest_when_destructuring_struct;
329330
mod return_self_not_must_use;
330331
mod returns;
331332
mod same_length_and_capacity;
@@ -862,6 +863,7 @@ rustc_lint::late_lint_methods!(
862863
WithCapacityZero: with_capacity_zero::WithCapacityZero = with_capacity_zero::WithCapacityZero,
863864
RefPatterns: ref_patterns::RefPatterns = ref_patterns::RefPatterns,
864865
RedundantElse: redundant_else::RedundantElse = redundant_else::RedundantElse,
866+
RestWhenDestructuringStruct: rest_when_destructuring_struct::RestWhenDestructuringStruct = rest_when_destructuring_struct::RestWhenDestructuringStruct,
865867
// add late passes here, used by `cargo dev new_lint`
866868
]]
867869
);

‎clippy_lints/src/rest_when_destructuring_struct.rs‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::is_from_proc_macro;
3+
use rustc_errors::Applicability;
4+
use rustc_lint::LateLintPass;
5+
use rustc_middle::ty;
6+
use rustc_session::declare_lint_pass;
7+
8+
use std::fmt::Write as _;
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
/// Disallow the use of rest patterns for accesible fields
13+
///
14+
/// ### Why restrict this?
15+
/// It might lead to unhandled fields when the struct changes.
16+
///
17+
/// ### Example
18+
/// ```no_run
19+
/// struct S {
20+
/// a: u8,
21+
/// b: u8,
22+
/// c: u8,
23+
/// }
24+
///
25+
/// let s = S { a: 1, b: 2, c: 3 };
26+
///
27+
/// let S { a, b, .. } = s;
28+
/// ```
29+
/// Use instead:
30+
/// ```no_run
31+
/// struct S {
32+
/// a: u8,
33+
/// b: u8,
34+
/// c: u8,
35+
/// }
36+
///
37+
/// let s = S { a: 1, b: 2, c: 3 };
38+
///
39+
/// let S { a, b, c: _ } = s;
40+
/// ```
41+
#[clippy::version = "1.99.0"]
42+
pub REST_PATTERN_ACCESSIBLE_FIELD,
43+
restriction,
44+
"rest pattern (`..`) used for accessible field"
45+
}
46+
47+
declare_clippy_lint! {
48+
/// ### What it does
49+
/// Disallow the use of rest patterns that don't capture any fields.
50+
///
51+
/// ### Why restrict this?
52+
/// It might lead to unhandled fields when the struct changes.
53+
///
54+
/// ### Example
55+
/// ```no_run
56+
/// struct S {
57+
/// a: u8,
58+
/// b: u8,
59+
/// c: u8,
60+
/// }
61+
///
62+
/// let s = S { a: 1, b: 2, c: 3 };
63+
///
64+
/// let S { a, b, c, .. } = s;
65+
/// ```
66+
/// Use instead:
67+
/// ```no_run
68+
/// struct S {
69+
/// a: u8,
70+
/// b: u8,
71+
/// c: u8,
72+
/// }
73+
///
74+
/// let s = S { a: 1, b: 2, c: 3 };
75+
///
76+
/// let S { a, b, c } = s;
77+
/// ```
78+
#[clippy::version = "1.99.0"]
79+
pub UNNECESSARY_REST_PATTERN,
80+
restriction,
81+
"unnecessary rest pattern (`..`) in destructuring expression"
82+
}
83+
84+
declare_lint_pass!(RestWhenDestructuringStruct => [
85+
REST_PATTERN_ACCESSIBLE_FIELD,
86+
UNNECESSARY_REST_PATTERN,
87+
]);
88+
89+
impl<'tcx> LateLintPass<'tcx> for RestWhenDestructuringStruct {
90+
fn check_pat(&mut self, cx: &rustc_lint::LateContext<'tcx>, pat: &'tcx rustc_hir::Pat<'tcx>) {
91+
if let rustc_hir::PatKind::Struct(path, fields, Some(dotdot)) = pat.kind
92+
&& let qty = cx.typeck_results().qpath_res(&path, pat.hir_id)
93+
&& let ty = cx.typeck_results().pat_ty(pat)
94+
&& let ty::Adt(a, _) = ty.kind()
95+
&& let Some(vid) = qty.opt_def_id().map(|x| a.variant_index_with_id(x))
96+
&& let Some(variant) = a.variants().get(vid)
97+
{
98+
let mut missing_suggestions = String::new();
99+
let mut needs_dotdot = variant.field_list_has_applicable_non_exhaustive();
100+
101+
for field in &variant.fields {
102+
if field.vis.is_accessible_from(cx.tcx.parent_module(pat.hir_id), cx.tcx) {
103+
if !fields.iter().any(|x| x.ident.name == field.name) {
104+
if !missing_suggestions.is_empty() {
105+
missing_suggestions.push_str(", ");
106+
}
107+
let _ = write!(missing_suggestions, "{}: _", field.name.as_str());
108+
}
109+
} else {
110+
needs_dotdot = true;
111+
}
112+
}
113+
114+
// Filter out results from macros
115+
if (missing_suggestions.is_empty() && needs_dotdot)
116+
|| pat.span.in_external_macro(cx.tcx.sess.source_map())
117+
|| is_from_proc_macro(cx, pat)
118+
{
119+
return;
120+
}
121+
122+
if !missing_suggestions.is_empty() {
123+
let suggestion_span = if needs_dotdot {
124+
missing_suggestions.push_str(", ");
125+
dotdot.shrink_to_lo()
126+
} else {
127+
dotdot
128+
};
129+
130+
let message = if fields.is_empty() {
131+
"consider explicitly ignoring fields with wildcard patterns (`x: _`)"
132+
} else {
133+
"consider explicitly ignoring remaining fields with wildcard patterns (`x: _`)"
134+
};
135+
136+
span_lint_and_then(
137+
cx,
138+
REST_PATTERN_ACCESSIBLE_FIELD,
139+
pat.span,
140+
"struct destructuring with rest (`..`)",
141+
|diag| {
142+
diag.span_suggestion_verbose(
143+
suggestion_span,
144+
message,
145+
&missing_suggestions,
146+
Applicability::MachineApplicable,
147+
);
148+
},
149+
);
150+
} else if !needs_dotdot {
151+
let message = "consider removing the unnecessary rest pattern (`..`)";
152+
span_lint_and_then(
153+
cx,
154+
UNNECESSARY_REST_PATTERN,
155+
pat.span,
156+
"unnecessary rest pattern (`..`)",
157+
|diag| {
158+
diag.span_suggestion_verbose(dotdot, message, "", Applicability::MachineApplicable);
159+
},
160+
);
161+
}
162+
}
163+
}
164+
}

‎clippy_utils/src/check_proc_macro.rs‎

Lines changed: 98 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,16 @@ use rustc_abi::ExternAbi;
1616
use rustc_ast as ast;
1717
use rustc_ast::AttrStyle;
1818
use rustc_ast::ast::{
19-
AttrKind, Attribute, GenericArgs, IntTy, LitIntType, LitKind, StrStyle, TraitObjectSyntax, UintTy,
19+
AttrKind, Attribute, BindingMode, GenericArgs, IntTy, LitIntType, LitKind, StrStyle, TraitObjectSyntax, UintTy,
2020
};
2121
use rustc_ast::token::CommentKind;
2222
use rustc_hir::intravisit::FnKind;
2323
use rustc_hir::{
2424
Block, BlockCheckMode, Body, BoundConstness, BoundPolarity, Closure, Destination, Expr, ExprKind, FieldDef,
2525
FnHeader, FnRetTy, HirId, Impl, ImplItem, ImplItemImplKind, ImplItemKind, IsAuto, Item, ItemKind, Lit, LoopSource,
26-
MatchSource, MutTy, Node, Path, PolyTraitRef, QPath, Safety, TraitBoundModifiers, TraitImplHeader, TraitItem,
27-
TraitItemKind, TraitRef, Ty, TyKind, UnOp, UnsafeSource, Variant, VariantData, YieldSource,
26+
MatchSource, MutTy, Node, PatExpr, PatExprKind, PatKind, Path, PolyTraitRef, QPath, Safety, TraitBoundModifiers,
27+
TraitImplHeader, TraitItem, TraitItemKind, TraitRef, Ty, TyKind, UnOp, UnsafeSource, Variant, VariantData,
28+
YieldSource,
2829
};
2930
use rustc_lint::{EarlyContext, LateContext, LintContext};
3031
use rustc_middle::ty::TyCtxt;
@@ -589,6 +590,99 @@ fn ident_search_pat(ident: Ident) -> (Pat, Pat) {
589590
(Pat::Sym(ident.name), Pat::Sym(ident.name))
590591
}
591592

593+
fn pat_search_pat(tcx: TyCtxt<'_>, pat: &rustc_hir::Pat<'_>) -> (Pat, Pat) {
594+
match pat.kind {
595+
// Tuple patterns cannot show up in proc-macro checks
596+
PatKind::Missing | PatKind::Err(_) | PatKind::Tuple(_, _) => (Pat::Str(""), Pat::Str("")),
597+
PatKind::Wild => (Pat::Sym(kw::Underscore), Pat::Sym(kw::Underscore)),
598+
PatKind::Binding(binding_mode, _, ident, Some(end_pat)) => {
599+
let start = if binding_mode == BindingMode::NONE {
600+
ident_search_pat(ident).0
601+
} else {
602+
Pat::Str(binding_mode.prefix_str())
603+
};
604+
605+
let (_, end) = pat_search_pat(tcx, end_pat);
606+
(start, end)
607+
},
608+
PatKind::Binding(binding_mode, _, ident, None) => {
609+
let (s, end) = ident_search_pat(ident);
610+
let start = if binding_mode == BindingMode::NONE {
611+
s
612+
} else {
613+
Pat::Str(binding_mode.prefix_str())
614+
};
615+
616+
(start, end)
617+
},
618+
PatKind::Struct(path, _, _) => {
619+
let (start, _) = qpath_search_pat(&path);
620+
(start, Pat::Str("}"))
621+
},
622+
PatKind::TupleStruct(path, _, _) => {
623+
let (start, _) = qpath_search_pat(&path);
624+
// This pattern cannot show up in proc-macro checks
625+
(start, Pat::Str(""))
626+
},
627+
PatKind::Or(plist) => {
628+
// documented invariant
629+
debug_assert!(plist.len() >= 2);
630+
let (start, _) = pat_search_pat(tcx, plist.first().unwrap());
631+
let (_, end) = pat_search_pat(tcx, plist.last().unwrap());
632+
(start, end)
633+
},
634+
PatKind::Never => (Pat::Str("!"), Pat::Str("")),
635+
PatKind::Box(p) => {
636+
let (_, end) = pat_search_pat(tcx, p);
637+
(Pat::Str("box"), end)
638+
},
639+
PatKind::Deref(_) => (Pat::Str("deref!"), Pat::Str("")),
640+
PatKind::Ref(p, _, _) => {
641+
let (_, end) = pat_search_pat(tcx, p);
642+
(Pat::Str("&"), end)
643+
},
644+
PatKind::Expr(expr) => pat_expr_search_pat(expr),
645+
PatKind::Guard(pat, guard) => {
646+
let (start, _) = pat_search_pat(tcx, pat);
647+
let (_, end) = expr_search_pat(tcx, guard);
648+
(start, end)
649+
},
650+
PatKind::Range(None, None, range) => match range {
651+
rustc_hir::RangeEnd::Included => (Pat::Str("..="), Pat::Str("")),
652+
rustc_hir::RangeEnd::Excluded => (Pat::Str(".."), Pat::Str("")),
653+
},
654+
PatKind::Range(r_start, r_end, range) => {
655+
let start = match r_start {
656+
Some(e) => pat_expr_search_pat(e).0,
657+
None => match range {
658+
rustc_hir::RangeEnd::Included => Pat::Str("..="),
659+
rustc_hir::RangeEnd::Excluded => Pat::Str(".."),
660+
},
661+
};
662+
663+
let end = match r_end {
664+
Some(e) => pat_expr_search_pat(e).1,
665+
None => match range {
666+
rustc_hir::RangeEnd::Included => Pat::Str("..="),
667+
rustc_hir::RangeEnd::Excluded => Pat::Str(".."),
668+
},
669+
};
670+
(start, end)
671+
},
672+
PatKind::Slice(_, _, _) => (Pat::Str("["), Pat::Str("]")),
673+
}
674+
}
675+
676+
fn pat_expr_search_pat(expr: &PatExpr<'_>) -> (Pat, Pat) {
677+
match expr.kind {
678+
PatExprKind::Lit { lit, negated } => {
679+
let (start, end) = lit_search_pat(&lit.node);
680+
if negated { (Pat::Str("!"), end) } else { (start, end) }
681+
},
682+
PatExprKind::Path(path) => qpath_search_pat(&path),
683+
}
684+
}
685+
592686
pub trait WithSearchPat<'cx> {
593687
type Context: LintContext;
594688
fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat);
@@ -618,6 +712,7 @@ impl_with_search_pat!((_cx: LateContext<'tcx>, self: Ident) => ident_search_pat(
618712
impl_with_search_pat!((_cx: LateContext<'tcx>, self: Lit) => lit_search_pat(&self.node));
619713
impl_with_search_pat!((_cx: LateContext<'tcx>, self: Path<'_>) => path_search_pat(self));
620714
impl_with_search_pat!((_cx: LateContext<'tcx>, self: PolyTraitRef<'_>) => poly_trait_ref_search_pat(self));
715+
impl_with_search_pat!((cx: LateContext<'tcx>, self: rustc_hir::Pat<'_>) => pat_search_pat(cx.tcx, self));
621716

622717
impl_with_search_pat!((_cx: EarlyContext<'tcx>, self: Attribute) => attr_search_pat(self));
623718
impl_with_search_pat!((_cx: EarlyContext<'tcx>, self: ast::Ty) => ast_ty_search_pat(self));

‎tests/ui/auxiliary/non-exhaustive-struct.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#[non_exhaustive]
2+
#[derive(Default)]
3+
pub struct NonExhaustiveStruct {
4+
pub field1: i32,
5+
pub field2: i32,
6+
_private: i32,
7+
}
8+
9+
#[non_exhaustive]
10+
#[derive(Default)]
11+
pub struct NonExhaustiveStructNoPrivateFields {
12+
pub field: i32,
13+
}

0 commit comments

Comments
 (0)