Skip to content

Commit b11c268

Browse files
Erk-Erk-
committed
handle feedback from review
1 parent 6e0ce3f commit b11c268

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;
@@ -548,97 +548,94 @@ fn ident_search_pat(ident: Ident) -> (Pat, Pat) {
548548

549549
fn pat_search_pat(tcx: TyCtxt<'_>, pat: &rustc_hir::Pat<'_>) -> (Pat, Pat) {
550550
match pat.kind {
551-
rustc_hir::PatKind::Missing | rustc_hir::PatKind::Err(_) => (Pat::Str(""), Pat::Str("")),
552-
rustc_hir::PatKind::Wild => (Pat::Sym(kw::Underscore), Pat::Sym(kw::Underscore)),
553-
rustc_hir::PatKind::Binding(binding_mode, _, ident, Some(end_pat)) => {
554-
let start = match binding_mode.0 {
555-
rustc_hir::ByRef::Yes(rustc_hir::Mutability::Not) => Pat::Str("ref"),
556-
rustc_hir::ByRef::Yes(rustc_hir::Mutability::Mut) => Pat::Str("ref mut"),
557-
rustc_hir::ByRef::No => {
558-
let (s, _) = ident_search_pat(ident);
559-
s
560-
},
551+
PatKind::Missing | PatKind::Err(_) => (Pat::Str(""), Pat::Str("")),
552+
PatKind::Wild => (Pat::Sym(kw::Underscore), Pat::Sym(kw::Underscore)),
553+
PatKind::Binding(binding_mode, _, ident, Some(end_pat)) => {
554+
let start = if binding_mode == BindingMode::NONE {
555+
ident_search_pat(ident).0
556+
} else {
557+
Pat::Str(binding_mode.prefix_str())
561558
};
562559

563560
let (_, end) = pat_search_pat(tcx, end_pat);
564561
(start, end)
565562
},
566-
rustc_hir::PatKind::Binding(binding_mode, _, ident, None) => {
563+
PatKind::Binding(binding_mode, _, ident, None) => {
567564
let (s, end) = ident_search_pat(ident);
568-
let start = match binding_mode.0 {
569-
rustc_hir::ByRef::Yes(rustc_hir::Mutability::Not) => Pat::Str("ref"),
570-
rustc_hir::ByRef::Yes(rustc_hir::Mutability::Mut) => Pat::Str("ref mut"),
571-
rustc_hir::ByRef::No => s,
565+
let start = if binding_mode == BindingMode::NONE {
566+
s
567+
} else {
568+
Pat::Str(binding_mode.prefix_str())
572569
};
573570

574571
(start, end)
575572
},
576-
rustc_hir::PatKind::Struct(path, _, _) => {
573+
PatKind::Struct(path, _, _) => {
577574
let (start, _) = qpath_search_pat(&path);
578575
(start, Pat::Str("}"))
579576
},
580-
rustc_hir::PatKind::TupleStruct(path, _, _) => {
577+
PatKind::TupleStruct(path, _, _) => {
581578
let (start, _) = qpath_search_pat(&path);
582579
(start, Pat::Str(")"))
583580
},
584-
rustc_hir::PatKind::Or(plist) => {
581+
PatKind::Or(plist) => {
585582
// documented invariant
586583
debug_assert!(plist.len() >= 2);
587584
let (start, _) = pat_search_pat(tcx, plist.first().unwrap());
588585
let (_, end) = pat_search_pat(tcx, plist.last().unwrap());
589586
(start, end)
590587
},
591-
rustc_hir::PatKind::Never => (Pat::Str("!"), Pat::Str("")),
592-
rustc_hir::PatKind::Tuple(_, _) => (Pat::Str("("), Pat::Str(")")),
593-
rustc_hir::PatKind::Box(p) => {
588+
PatKind::Never => (Pat::Str("!"), Pat::Str("")),
589+
PatKind::Tuple(_, _) => (Pat::Str("("), Pat::Str(")")),
590+
PatKind::Box(p) => {
594591
let (_, end) = pat_search_pat(tcx, p);
595592
(Pat::Str("box"), end)
596593
},
597-
rustc_hir::PatKind::Deref(_) => (Pat::Str("deref!("), Pat::Str(")")),
598-
rustc_hir::PatKind::Ref(p, _) => {
594+
PatKind::Deref(_) => (Pat::Str("deref!("), Pat::Str(")")),
595+
PatKind::Ref(p, _) => {
599596
let (_, end) = pat_search_pat(tcx, p);
600597
(Pat::Str("&"), end)
601598
},
602-
rustc_hir::PatKind::Expr(expr) => pat_search_pat_expr_kind(expr),
603-
rustc_hir::PatKind::Guard(pat, guard) => {
599+
PatKind::Expr(expr) => pat_search_pat_expr_kind(expr),
600+
PatKind::Guard(pat, guard) => {
604601
let (start, _) = pat_search_pat(tcx, pat);
605602
let (_, end) = expr_search_pat(tcx, guard);
606603
(start, end)
607604
},
608-
rustc_hir::PatKind::Range(None, None, range) => match range {
605+
PatKind::Range(None, None, range) => match range {
609606
rustc_hir::RangeEnd::Included => (Pat::Str("..="), Pat::Str("")),
610607
rustc_hir::RangeEnd::Excluded => (Pat::Str(".."), Pat::Str("")),
611608
},
612-
rustc_hir::PatKind::Range(r_start, r_end, range) => {
613-
let (start, _) = match r_start {
614-
Some(e) => pat_search_pat_expr_kind(e),
609+
PatKind::Range(r_start, r_end, range) => {
610+
let start = match r_start {
611+
Some(e) => pat_search_pat_expr_kind(e).0,
615612
None => match range {
616-
rustc_hir::RangeEnd::Included => (Pat::Str("..="), Pat::Str("")),
617-
rustc_hir::RangeEnd::Excluded => (Pat::Str(".."), Pat::Str("")),
613+
rustc_hir::RangeEnd::Included => Pat::Str("..="),
614+
rustc_hir::RangeEnd::Excluded => Pat::Str(".."),
618615
},
619616
};
620617

621-
let (_, end) = match r_end {
622-
Some(e) => pat_search_pat_expr_kind(e),
618+
let end = match r_end {
619+
Some(e) => pat_search_pat_expr_kind(e).1,
623620
None => match range {
624-
rustc_hir::RangeEnd::Included => (Pat::Str(""), Pat::Str("..=")),
625-
rustc_hir::RangeEnd::Excluded => (Pat::Str(""), Pat::Str("..")),
621+
rustc_hir::RangeEnd::Included => Pat::Str("..="),
622+
rustc_hir::RangeEnd::Excluded => Pat::Str(".."),
626623
},
627624
};
628625
(start, end)
629626
},
630-
rustc_hir::PatKind::Slice(_, _, _) => (Pat::Str("["), Pat::Str("]")),
627+
PatKind::Slice(_, _, _) => (Pat::Str("["), Pat::Str("]")),
631628
}
632629
}
633630

634-
fn pat_search_pat_expr_kind(expr: &crate::PatExpr<'_>) -> (Pat, Pat) {
631+
fn pat_search_pat_expr_kind(expr: &PatExpr<'_>) -> (Pat, Pat) {
635632
match expr.kind {
636-
crate::PatExprKind::Lit { lit, negated } => {
633+
PatExprKind::Lit { lit, negated } => {
637634
let (start, end) = lit_search_pat(&lit.node);
638635
if negated { (Pat::Str("!"), end) } else { (start, end) }
639636
},
640-
crate::PatExprKind::ConstBlock(_block) => (Pat::Str("const {"), Pat::Str("}")),
641-
crate::PatExprKind::Path(path) => qpath_search_pat(&path),
637+
PatExprKind::ConstBlock(_block) => (Pat::Str("const {"), Pat::Str("}")),
638+
PatExprKind::Path(path) => qpath_search_pat(&path),
642639
}
643640
}
644641

‎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)