Skip to content

Commit 50e9c09

Browse files
Erk-Erk-
committed
handle feedback from review
1 parent c3e9310 commit 50e9c09

File tree

5 files changed

+96
-64
lines changed

5 files changed

+96
-64
lines changed

‎clippy_lints/src/rest_when_destructuring_struct.rs‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use clippy_utils::is_from_proc_macro;
33
use itertools::Itertools;
44
use rustc_abi::VariantIdx;
55
use rustc_lint::LateLintPass;
6-
use rustc_middle::ty::{self, Visibility};
6+
use rustc_middle::ty;
77
use rustc_session::declare_lint_pass;
88

99
declare_clippy_lint! {
@@ -39,7 +39,7 @@ declare_clippy_lint! {
3939
/// ```
4040
#[clippy::version = "1.89.0"]
4141
pub REST_WHEN_DESTRUCTURING_STRUCT,
42-
nursery,
42+
restriction,
4343
"rest (..) in destructuring expression"
4444
}
4545
declare_lint_pass!(RestWhenDestructuringStruct => [REST_WHEN_DESTRUCTURING_STRUCT]);
@@ -57,21 +57,17 @@ impl<'tcx> LateLintPass<'tcx> for RestWhenDestructuringStruct {
5757
.opt_def_id()
5858
.map_or(VariantIdx::ZERO, |x| a.variant_index_with_id(x));
5959

60-
let leave_dotdot = a.variants()[vid].field_list_has_applicable_non_exhaustive();
60+
let leave_dotdot = a.variants()[vid]
61+
.fields
62+
.iter()
63+
.any(|f| !f.vis.is_accessible_from(cx.tcx.parent_module(pat.hir_id), cx.tcx));
6164

6265
let mut rest_fields = a.variants()[vid]
6366
.fields
6467
.iter()
65-
.filter(|f| {
66-
if a.did().is_local() {
67-
true
68-
} else {
69-
matches!(f.vis, Visibility::Public)
70-
}
71-
})
72-
.map(|field| field.ident(cx.tcx))
73-
.filter(|pf| !fields.iter().any(|x| x.ident == *pf))
74-
.map(|x| format!("{x}: _"));
68+
.filter(|f| f.vis.is_accessible_from(cx.tcx.parent_module(pat.hir_id), cx.tcx))
69+
.filter(|pf| !fields.iter().any(|x| x.ident.name == pf.name))
70+
.map(|x| format!("{}: _", x.ident(cx.tcx)));
7571

7672
let mut fmt_fields = rest_fields.join(", ");
7773

‎clippy_utils/src/check_proc_macro.rs‎

Lines changed: 41 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@ 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, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, FnRetTy, HirId, Impl,
25-
ImplItem, ImplItemImplKind, ImplItemKind, IsAuto, Item, ItemKind, Lit, LoopSource, MatchSource, MutTy, Node, Path,
26-
QPath, Safety, TraitImplHeader, TraitItem, TraitItemKind, Ty, TyKind, UnOp, UnsafeSource, Variant, VariantData,
27-
YieldSource,
25+
ImplItem, ImplItemImplKind, ImplItemKind, IsAuto, Item, ItemKind, Lit, LoopSource, MatchSource, MutTy, Node,
26+
PatExpr, PatExprKind, PatKind, Path, QPath, Safety, TraitImplHeader, TraitItem, TraitItemKind, Ty, TyKind, UnOp,
27+
UnsafeSource, Variant, VariantData, YieldSource,
2828
};
2929
use rustc_lint::{EarlyContext, LateContext, LintContext};
3030
use rustc_middle::ty::TyCtxt;
@@ -542,97 +542,94 @@ fn ident_search_pat(ident: Ident) -> (Pat, Pat) {
542542

543543
fn pat_search_pat(tcx: TyCtxt<'_>, pat: &rustc_hir::Pat<'_>) -> (Pat, Pat) {
544544
match pat.kind {
545-
rustc_hir::PatKind::Missing | rustc_hir::PatKind::Err(_) => (Pat::Str(""), Pat::Str("")),
546-
rustc_hir::PatKind::Wild => (Pat::Sym(kw::Underscore), Pat::Sym(kw::Underscore)),
547-
rustc_hir::PatKind::Binding(binding_mode, _, ident, Some(end_pat)) => {
548-
let start = match binding_mode.0 {
549-
rustc_hir::ByRef::Yes(rustc_hir::Mutability::Not) => Pat::Str("ref"),
550-
rustc_hir::ByRef::Yes(rustc_hir::Mutability::Mut) => Pat::Str("ref mut"),
551-
rustc_hir::ByRef::No => {
552-
let (s, _) = ident_search_pat(ident);
553-
s
554-
},
545+
PatKind::Missing | PatKind::Err(_) => (Pat::Str(""), Pat::Str("")),
546+
PatKind::Wild => (Pat::Sym(kw::Underscore), Pat::Sym(kw::Underscore)),
547+
PatKind::Binding(binding_mode, _, ident, Some(end_pat)) => {
548+
let start = if binding_mode == BindingMode::NONE {
549+
ident_search_pat(ident).0
550+
} else {
551+
Pat::Str(binding_mode.prefix_str())
555552
};
556553

557554
let (_, end) = pat_search_pat(tcx, end_pat);
558555
(start, end)
559556
},
560-
rustc_hir::PatKind::Binding(binding_mode, _, ident, None) => {
557+
PatKind::Binding(binding_mode, _, ident, None) => {
561558
let (s, end) = ident_search_pat(ident);
562-
let start = match binding_mode.0 {
563-
rustc_hir::ByRef::Yes(rustc_hir::Mutability::Not) => Pat::Str("ref"),
564-
rustc_hir::ByRef::Yes(rustc_hir::Mutability::Mut) => Pat::Str("ref mut"),
565-
rustc_hir::ByRef::No => s,
559+
let start = if binding_mode == BindingMode::NONE {
560+
s
561+
} else {
562+
Pat::Str(binding_mode.prefix_str())
566563
};
567564

568565
(start, end)
569566
},
570-
rustc_hir::PatKind::Struct(path, _, _) => {
567+
PatKind::Struct(path, _, _) => {
571568
let (start, _) = qpath_search_pat(&path);
572569
(start, Pat::Str("}"))
573570
},
574-
rustc_hir::PatKind::TupleStruct(path, _, _) => {
571+
PatKind::TupleStruct(path, _, _) => {
575572
let (start, _) = qpath_search_pat(&path);
576573
(start, Pat::Str(")"))
577574
},
578-
rustc_hir::PatKind::Or(plist) => {
575+
PatKind::Or(plist) => {
579576
// documented invariant
580577
debug_assert!(plist.len() >= 2);
581578
let (start, _) = pat_search_pat(tcx, plist.first().unwrap());
582579
let (_, end) = pat_search_pat(tcx, plist.last().unwrap());
583580
(start, end)
584581
},
585-
rustc_hir::PatKind::Never => (Pat::Str("!"), Pat::Str("")),
586-
rustc_hir::PatKind::Tuple(_, _) => (Pat::Str("("), Pat::Str(")")),
587-
rustc_hir::PatKind::Box(p) => {
582+
PatKind::Never => (Pat::Str("!"), Pat::Str("")),
583+
PatKind::Tuple(_, _) => (Pat::Str("("), Pat::Str(")")),
584+
PatKind::Box(p) => {
588585
let (_, end) = pat_search_pat(tcx, p);
589586
(Pat::Str("box"), end)
590587
},
591-
rustc_hir::PatKind::Deref(_) => (Pat::Str("deref!("), Pat::Str(")")),
592-
rustc_hir::PatKind::Ref(p, _) => {
588+
PatKind::Deref(_) => (Pat::Str("deref!("), Pat::Str(")")),
589+
PatKind::Ref(p, _) => {
593590
let (_, end) = pat_search_pat(tcx, p);
594591
(Pat::Str("&"), end)
595592
},
596-
rustc_hir::PatKind::Expr(expr) => pat_search_pat_expr_kind(expr),
597-
rustc_hir::PatKind::Guard(pat, guard) => {
593+
PatKind::Expr(expr) => pat_search_pat_expr_kind(expr),
594+
PatKind::Guard(pat, guard) => {
598595
let (start, _) = pat_search_pat(tcx, pat);
599596
let (_, end) = expr_search_pat(tcx, guard);
600597
(start, end)
601598
},
602-
rustc_hir::PatKind::Range(None, None, range) => match range {
599+
PatKind::Range(None, None, range) => match range {
603600
rustc_hir::RangeEnd::Included => (Pat::Str("..="), Pat::Str("")),
604601
rustc_hir::RangeEnd::Excluded => (Pat::Str(".."), Pat::Str("")),
605602
},
606-
rustc_hir::PatKind::Range(r_start, r_end, range) => {
607-
let (start, _) = match r_start {
608-
Some(e) => pat_search_pat_expr_kind(e),
603+
PatKind::Range(r_start, r_end, range) => {
604+
let start = match r_start {
605+
Some(e) => pat_search_pat_expr_kind(e).0,
609606
None => match range {
610-
rustc_hir::RangeEnd::Included => (Pat::Str("..="), Pat::Str("")),
611-
rustc_hir::RangeEnd::Excluded => (Pat::Str(".."), Pat::Str("")),
607+
rustc_hir::RangeEnd::Included => Pat::Str("..="),
608+
rustc_hir::RangeEnd::Excluded => Pat::Str(".."),
612609
},
613610
};
614611

615-
let (_, end) = match r_end {
616-
Some(e) => pat_search_pat_expr_kind(e),
612+
let end = match r_end {
613+
Some(e) => pat_search_pat_expr_kind(e).1,
617614
None => match range {
618-
rustc_hir::RangeEnd::Included => (Pat::Str(""), Pat::Str("..=")),
619-
rustc_hir::RangeEnd::Excluded => (Pat::Str(""), Pat::Str("..")),
615+
rustc_hir::RangeEnd::Included => Pat::Str("..="),
616+
rustc_hir::RangeEnd::Excluded => Pat::Str(".."),
620617
},
621618
};
622619
(start, end)
623620
},
624-
rustc_hir::PatKind::Slice(_, _, _) => (Pat::Str("["), Pat::Str("]")),
621+
PatKind::Slice(_, _, _) => (Pat::Str("["), Pat::Str("]")),
625622
}
626623
}
627624

628-
fn pat_search_pat_expr_kind(expr: &crate::PatExpr<'_>) -> (Pat, Pat) {
625+
fn pat_search_pat_expr_kind(expr: &PatExpr<'_>) -> (Pat, Pat) {
629626
match expr.kind {
630-
crate::PatExprKind::Lit { lit, negated } => {
627+
PatExprKind::Lit { lit, negated } => {
631628
let (start, end) = lit_search_pat(&lit.node);
632629
if negated { (Pat::Str("!"), end) } else { (start, end) }
633630
},
634-
crate::PatExprKind::ConstBlock(_block) => (Pat::Str("const {"), Pat::Str("}")),
635-
crate::PatExprKind::Path(path) => qpath_search_pat(&path),
631+
PatExprKind::ConstBlock(_block) => (Pat::Str("const {"), Pat::Str("}")),
632+
PatExprKind::Path(path) => qpath_search_pat(&path),
636633
}
637634
}
638635

‎tests/ui/rest_when_destructuring_struct.fixed‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ enum E {
2222
C {},
2323
}
2424

25+
mod m {
26+
#[derive(Default)]
27+
pub struct Sm {
28+
pub a: u8,
29+
pub(crate) b: u8,
30+
c: u8,
31+
}
32+
}
33+
2534
fn main() {
2635
let s = S { a: 1, b: 2, c: 3 };
2736

@@ -67,4 +76,9 @@ fn main() {
6776
let NonExhaustiveStruct {
6877
field1: _, field2: _, ..
6978
} = ne;
79+
80+
use m::Sm;
81+
82+
let Sm { a: _, b: _, .. } = Sm::default();
83+
//~^ rest_when_destructuring_struct
7084
}

‎tests/ui/rest_when_destructuring_struct.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ enum E {
2222
C {},
2323
}
2424

25+
mod m {
26+
#[derive(Default)]
27+
pub struct Sm {
28+
pub a: u8,
29+
pub(crate) b: u8,
30+
c: u8,
31+
}
32+
}
33+
2534
fn main() {
2635
let s = S { a: 1, b: 2, c: 3 };
2736

@@ -67,4 +76,9 @@ fn main() {
6776
let NonExhaustiveStruct {
6877
field1: _, field2: _, ..
6978
} = ne;
79+
80+
use m::Sm;
81+
82+
let Sm { .. } = Sm::default();
83+
//~^ rest_when_destructuring_struct
7084
}

‎tests/ui/rest_when_destructuring_struct.stderr‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: struct destructuring with rest (..)
2-
--> tests/ui/rest_when_destructuring_struct.rs:28:9
2+
--> tests/ui/rest_when_destructuring_struct.rs:37:9
33
|
44
LL | let S { a, b, .. } = s;
55
| ^^^^^^^^^^^^^^
@@ -13,7 +13,7 @@ LL + let S { a, b, c: _ } = s;
1313
|
1414

1515
error: struct destructuring with rest (..)
16-
--> tests/ui/rest_when_destructuring_struct.rs:31:9
16+
--> tests/ui/rest_when_destructuring_struct.rs:40:9
1717
|
1818
LL | let S { a, b, c, .. } = s;
1919
| ^^^^^^^^^^^^^^^^^
@@ -25,7 +25,7 @@ LL + let S { a, b, c, } = s;
2525
|
2626

2727
error: struct destructuring with rest (..)
28-
--> tests/ui/rest_when_destructuring_struct.rs:38:9
28+
--> tests/ui/rest_when_destructuring_struct.rs:47:9
2929
|
3030
LL | E::B { .. } => (),
3131
| ^^^^^^^^^^^
@@ -37,7 +37,7 @@ LL + E::B { b1: _, b2: _ } => (),
3737
|
3838

3939
error: struct destructuring with rest (..)
40-
--> tests/ui/rest_when_destructuring_struct.rs:40:9
40+
--> tests/ui/rest_when_destructuring_struct.rs:49:9
4141
|
4242
LL | E::C { .. } => (),
4343
| ^^^^^^^^^^^
@@ -49,7 +49,7 @@ LL + E::C { } => (),
4949
|
5050

5151
error: struct destructuring with rest (..)
52-
--> tests/ui/rest_when_destructuring_struct.rs:46:9
52+
--> tests/ui/rest_when_destructuring_struct.rs:55:9
5353
|
5454
LL | E::B { b1: _, .. } => (),
5555
| ^^^^^^^^^^^^^^^^^^
@@ -61,7 +61,7 @@ LL + E::B { b1: _, b2: _ } => (),
6161
|
6262

6363
error: struct destructuring with rest (..)
64-
--> tests/ui/rest_when_destructuring_struct.rs:63:9
64+
--> tests/ui/rest_when_destructuring_struct.rs:72:9
6565
|
6666
LL | let NonExhaustiveStruct { field1: _, .. } = ne;
6767
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -71,5 +71,16 @@ help: consider explicitly ignoring remaining fields with wildcard patterns (x: _
7171
LL | let NonExhaustiveStruct { field1: _, field2: _, .. } = ne;
7272
| ++++++++++
7373

74-
error: aborting due to 6 previous errors
74+
error: struct destructuring with rest (..)
75+
--> tests/ui/rest_when_destructuring_struct.rs:82:9
76+
|
77+
LL | let Sm { .. } = Sm::default();
78+
| ^^^^^^^^^
79+
|
80+
help: consider explicitly ignoring remaining fields with wildcard patterns (x: _)
81+
|
82+
LL | let Sm { a: _, b: _, .. } = Sm::default();
83+
| +++++++++++
84+
85+
error: aborting due to 7 previous errors
7586

0 commit comments

Comments
 (0)