add new lint: rest_when_destructuring_struct

CHANGELOG.md

@@ -6736,6 +6736,7 @@ Released 2018-09-13
 [`repr_packed_without_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#repr_packed_without_abi
 [`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization
 [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
+[`rest_when_destructuring_struct`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_when_destructuring_struct
 [`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
 [`result_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_filter_map
 [`result_large_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err

clippy_lints/src/declared_lints.rs

@@ -657,6 +657,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::repeat_vec_with_capacity::REPEAT_VEC_WITH_CAPACITY_INFO,
     crate::replace_box::REPLACE_BOX_INFO,
     crate::reserve_after_initialization::RESERVE_AFTER_INITIALIZATION_INFO,
+    crate::rest_when_destructuring_struct::REST_WHEN_DESTRUCTURING_STRUCT_INFO,
     crate::return_self_not_must_use::RETURN_SELF_NOT_MUST_USE_INFO,
     crate::returns::LET_AND_RETURN_INFO,
     crate::returns::NEEDLESS_RETURN_INFO,

clippy_lints/src/lib.rs

@@ -325,6 +325,7 @@ mod regex;
 mod repeat_vec_with_capacity;
 mod replace_box;
 mod reserve_after_initialization;
+mod rest_when_destructuring_struct;
 mod return_self_not_must_use;
 mod returns;
 mod same_name_method;
@@ -834,5 +835,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg));
     store.register_late_pass(|_| Box::new(volatile_composites::VolatileComposites));
     store.register_late_pass(|_| Box::new(replace_box::ReplaceBox));
+    store.register_late_pass(|_| Box::new(rest_when_destructuring_struct::RestWhenDestructuringStruct));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/rest_when_destructuring_struct.rs

@@ -0,0 +1,107 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::is_from_proc_macro;
+use itertools::Itertools;
+use rustc_abi::VariantIdx;
+use rustc_lint::LateLintPass;
+use rustc_middle::ty;
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Disallows the use of rest patterns when destructuring structs.
+    ///
+    /// ### Why is this bad?
+    /// It might lead to unhandled fields when the struct changes.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// struct S {
+    ///     a: u8,
+    ///     b: u8,
+    ///     c: u8,
+    /// }
+    ///
+    /// let s = S { a: 1, b: 2, c: 3 };
+    ///
+    /// let S { a, b, .. } = s;
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// struct S {
+    ///     a: u8,
+    ///     b: u8,
+    ///     c: u8,
+    /// }
+    ///
+    /// let s = S { a: 1, b: 2, c: 3 };
+    ///
+    /// let S { a, b, c: _ } = s;
+    /// ```
+    #[clippy::version = "1.89.0"]
+    pub REST_WHEN_DESTRUCTURING_STRUCT,
+    restriction,
+    "rest (..) in destructuring expression"
+}
+declare_lint_pass!(RestWhenDestructuringStruct => [REST_WHEN_DESTRUCTURING_STRUCT]);
+
+impl<'tcx> LateLintPass<'tcx> for RestWhenDestructuringStruct {
+    fn check_pat(&mut self, cx: &rustc_lint::LateContext<'tcx>, pat: &'tcx rustc_hir::Pat<'tcx>) {
+        if let rustc_hir::PatKind::Struct(path, fields, Some(dotdot)) = pat.kind
+            && !pat.span.in_external_macro(cx.tcx.sess.source_map())
+            && !is_from_proc_macro(cx, pat)
+            && let qty = cx.typeck_results().qpath_res(&path, pat.hir_id)
+            && let ty = cx.typeck_results().pat_ty(pat)
+            && let ty::Adt(a, _) = ty.kind()
+        {
+            let vid = qty
+                .opt_def_id()
+                .map_or(VariantIdx::ZERO, |x| a.variant_index_with_id(x));
+
+            let leave_dotdot = a.variants()[vid]
+                .fields
+                .iter()
+                .any(|f| !f.vis.is_accessible_from(cx.tcx.parent_module(pat.hir_id), cx.tcx));
+
+            let mut rest_fields = a.variants()[vid]
+                .fields
+                .iter()
+                .filter(|f| f.vis.is_accessible_from(cx.tcx.parent_module(pat.hir_id), cx.tcx))
+                .filter(|pf| !fields.iter().any(|x| x.ident.name == pf.name))
+                .map(|x| format!("{}: _", x.ident(cx.tcx)));
+
+            let mut fmt_fields = rest_fields.join(", ");
+
+            if fmt_fields.is_empty() && leave_dotdot {
+                // The struct is non_exhaustive, from a non-local crate and all public fields are explicitly named.
+                return;
+            }
+
+            if leave_dotdot {
+                fmt_fields.push_str(", ..");
+            }
+
+            let message = if a.variants()[vid].fields.is_empty() {
+                "consider removing the rest pattern (`..`)"
+            } else if fields.is_empty() {
+                "consider explicitly ignoring fields with wildcard patterns (`x: _`)"
+            } else {
+                "consider explicitly ignoring remaining fields with wildcard patterns (`x: _`)"
+            };
+
+            span_lint_and_then(
+                cx,
+                REST_WHEN_DESTRUCTURING_STRUCT,
+                pat.span,
+                "struct destructuring with rest (`..`)",
+                |diag| {
+                    diag.span_suggestion_verbose(
+                        dotdot,
+                        message,
+                        fmt_fields,
+                        rustc_errors::Applicability::MachineApplicable,
+                    );
+                },
+            );
+        }
+    }
+}

clippy_utils/src/check_proc_macro.rs

@@ -16,15 +16,15 @@ use rustc_abi::ExternAbi;
 use rustc_ast as ast;
 use rustc_ast::AttrStyle;
 use rustc_ast::ast::{
-    AttrKind, Attribute, GenericArgs, IntTy, LitIntType, LitKind, StrStyle, TraitObjectSyntax, UintTy,
+    AttrKind, Attribute, BindingMode, GenericArgs, IntTy, LitIntType, LitKind, StrStyle, TraitObjectSyntax, UintTy,
 };
 use rustc_ast::token::CommentKind;
 use rustc_hir::intravisit::FnKind;
 use rustc_hir::{
     Block, BlockCheckMode, Body, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, FnRetTy, HirId, Impl,
-    ImplItem, ImplItemImplKind, ImplItemKind, IsAuto, Item, ItemKind, Lit, LoopSource, MatchSource, MutTy, Node, Path,
-    QPath, Safety, TraitImplHeader, TraitItem, TraitItemKind, Ty, TyKind, UnOp, UnsafeSource, Variant, VariantData,
-    YieldSource,
+    ImplItem, ImplItemImplKind, ImplItemKind, IsAuto, Item, ItemKind, Lit, LoopSource, MatchSource, MutTy, Node,
+    PatExpr, PatExprKind, PatKind, Path, QPath, Safety, TraitImplHeader, TraitItem, TraitItemKind, Ty, TyKind, UnOp,
+    UnsafeSource, Variant, VariantData, YieldSource,
 };
 use rustc_lint::{EarlyContext, LateContext, LintContext};
 use rustc_middle::ty::TyCtxt;
@@ -541,6 +541,99 @@ fn ident_search_pat(ident: Ident) -> (Pat, Pat) {
     (Pat::Sym(ident.name), Pat::Sym(ident.name))
 }
 
+fn pat_search_pat(tcx: TyCtxt<'_>, pat: &rustc_hir::Pat<'_>) -> (Pat, Pat) {
+    match pat.kind {
+        PatKind::Missing | PatKind::Err(_) => (Pat::Str(""), Pat::Str("")),
+        PatKind::Wild => (Pat::Sym(kw::Underscore), Pat::Sym(kw::Underscore)),
+        PatKind::Binding(binding_mode, _, ident, Some(end_pat)) => {
+            let start = if binding_mode == BindingMode::NONE {
+                ident_search_pat(ident).0
+            } else {
+                Pat::Str(binding_mode.prefix_str())
+            };
+
+            let (_, end) = pat_search_pat(tcx, end_pat);
+            (start, end)
+        },
+        PatKind::Binding(binding_mode, _, ident, None) => {
+            let (s, end) = ident_search_pat(ident);
+            let start = if binding_mode == BindingMode::NONE {
+                s
+            } else {
+                Pat::Str(binding_mode.prefix_str())
+            };
+
+            (start, end)
+        },
+        PatKind::Struct(path, _, _) => {
+            let (start, _) = qpath_search_pat(&path);
+            (start, Pat::Str("}"))
+        },
+        PatKind::TupleStruct(path, _, _) => {
+            let (start, _) = qpath_search_pat(&path);
+            (start, Pat::Str(")"))
+        },
+        PatKind::Or(plist) => {
+            // documented invariant
+            debug_assert!(plist.len() >= 2);
+            let (start, _) = pat_search_pat(tcx, plist.first().unwrap());
+            let (_, end) = pat_search_pat(tcx, plist.last().unwrap());
+            (start, end)
+        },
+        PatKind::Never => (Pat::Str("!"), Pat::Str("")),
+        PatKind::Tuple(_, _) => (Pat::Str("("), Pat::Str(")")),
+        PatKind::Box(p) => {
+            let (_, end) = pat_search_pat(tcx, p);
+            (Pat::Str("box"), end)
+        },
+        PatKind::Deref(_) => (Pat::Str("deref!("), Pat::Str(")")),
+        PatKind::Ref(p, _) => {
+            let (_, end) = pat_search_pat(tcx, p);
+            (Pat::Str("&"), end)
+        },
+        PatKind::Expr(expr) => pat_expr_search_pat(expr),
+        PatKind::Guard(pat, guard) => {
+            let (start, _) = pat_search_pat(tcx, pat);
+            let (_, end) = expr_search_pat(tcx, guard);
+            (start, end)
+        },
+        PatKind::Range(None, None, range) => match range {
+            rustc_hir::RangeEnd::Included => (Pat::Str("..="), Pat::Str("")),
+            rustc_hir::RangeEnd::Excluded => (Pat::Str(".."), Pat::Str("")),
+        },
+        PatKind::Range(r_start, r_end, range) => {
+            let start = match r_start {
+                Some(e) => pat_expr_search_pat(e).0,
+                None => match range {
+                    rustc_hir::RangeEnd::Included => Pat::Str("..="),
+                    rustc_hir::RangeEnd::Excluded => Pat::Str(".."),
+                },
+            };
+
+            let end = match r_end {
+                Some(e) => pat_expr_search_pat(e).1,
+                None => match range {
+                    rustc_hir::RangeEnd::Included => Pat::Str("..="),
+                    rustc_hir::RangeEnd::Excluded => Pat::Str(".."),
+                },
+            };
+            (start, end)
+        },
+        PatKind::Slice(_, _, _) => (Pat::Str("["), Pat::Str("]")),
+    }
+}
+
+fn pat_expr_search_pat(expr: &PatExpr<'_>) -> (Pat, Pat) {
+    match expr.kind {
+        PatExprKind::Lit { lit, negated } => {
+            let (start, end) = lit_search_pat(&lit.node);
+            if negated { (Pat::Str("!"), end) } else { (start, end) }
+        },
+        PatExprKind::ConstBlock(_block) => (Pat::Str("const {"), Pat::Str("}")),
+        PatExprKind::Path(path) => qpath_search_pat(&path),
+    }
+}
+
 pub trait WithSearchPat<'cx> {
     type Context: LintContext;
     fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat);
@@ -569,6 +662,7 @@ impl_with_search_pat!((_cx: LateContext<'tcx>, self: Ty<'_>) => ty_search_pat(se
 impl_with_search_pat!((_cx: LateContext<'tcx>, self: Ident) => ident_search_pat(*self));
 impl_with_search_pat!((_cx: LateContext<'tcx>, self: Lit) => lit_search_pat(&self.node));
 impl_with_search_pat!((_cx: LateContext<'tcx>, self: Path<'_>) => path_search_pat(self));
+impl_with_search_pat!((cx: LateContext<'tcx>, self: rustc_hir::Pat<'_>) => pat_search_pat(cx.tcx, self));
 
 impl_with_search_pat!((_cx: EarlyContext<'tcx>, self: Attribute) => attr_search_pat(self));
 impl_with_search_pat!((_cx: EarlyContext<'tcx>, self: ast::Ty) => ast_ty_search_pat(self));

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

@@ -0,0 +1,7 @@
+#[non_exhaustive]
+#[derive(Default)]
+pub struct NonExhaustiveStruct {
+    pub field1: i32,
+    pub field2: i32,
+    _private: i32,
+}

tests/ui/rest_when_destructuring_struct.fixed

@@ -0,0 +1,78 @@
+//@aux-build:proc_macros.rs
+//@aux-build:non-exhaustive-struct.rs
+#![warn(clippy::rest_when_destructuring_struct)]
+
+use non_exhaustive_struct::NonExhaustiveStruct;
+
+struct S {
+    a: u8,
+    b: u8,
+    c: u8,
+}
+
+enum E {
+    A { a1: u8, a2: u8 },
+    B { b1: u8, b2: u8 },
+    C {},
+}
+
+mod m {
+    #[derive(Default)]
+    pub struct Sm {
+        pub a: u8,
+        pub(crate) b: u8,
+        c: u8,
+    }
+}
+
+fn main() {
+    let s = S { a: 1, b: 2, c: 3 };
+
+    let S { a, b, c: _ } = s;
+    //~^ rest_when_destructuring_struct
+
+    let S { a, b, c,  } = s;
+    //~^ rest_when_destructuring_struct
+
+    let e = E::A { a1: 1, a2: 2 };
+
+    match e {
+        E::A { a1, a2 } => (),
+        E::B { b1: _, b2: _ } => (),
+        //~^ rest_when_destructuring_struct
+        E::C {  } => (),
+        //~^ rest_when_destructuring_struct
+    }
+
+    match e {
+        E::A { a1: _, a2: _ } => (),
+        E::B { b1: _, b2: _ } => (),
+        //~^ rest_when_destructuring_struct
+        E::C {} => (),
+    }
+
+    proc_macros::external! {
+        let s1 = S { a: 1, b: 2, c: 3 };
+        let S { a, b, .. } = s1;
+    }
+
+    proc_macros::with_span! {
+        span
+        let s2 = S { a: 1, b: 2, c: 3 };
+        let S { a, b, .. } = s2;
+    }
+
+    let ne = NonExhaustiveStruct::default();
+    let NonExhaustiveStruct { field1: _, field2: _, .. } = ne;
+    //~^ rest_when_destructuring_struct
+
+    let ne = NonExhaustiveStruct::default();
+    let NonExhaustiveStruct {
+        field1: _, field2: _, ..
+    } = ne;
+
+    use m::Sm;
+
+    let Sm { a: _, b: _, .. } = Sm::default();
+    //~^ rest_when_destructuring_struct
+}