Skip to content

Commit 2d1c3a6

Browse files
rust-timerrust-timer
authored
Unrolled build for #142237
Rollup merge of #142237 - benschulz:unused-parens-fn, r=fee1-dead Detect more cases of unused_parens around types With this change, more unused parentheses around bounds and types nested within bounds are detected.
2 parents 6677875 + 7d6764a commit 2d1c3a6

File tree

19 files changed

+596
-83
lines changed

19 files changed

+596
-83
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,6 +1390,7 @@ impl Expr {
13901390
path.clone(),
13911391
TraitBoundModifiers::NONE,
13921392
self.span,
1393+
Parens::No,
13931394
))),
13941395
_ => None,
13951396
}
@@ -3366,6 +3367,13 @@ pub struct TraitRef {
33663367
pub ref_id: NodeId,
33673368
}
33683369

3370+
/// Whether enclosing parentheses are present or not.
3371+
#[derive(Clone, Encodable, Decodable, Debug)]
3372+
pub enum Parens {
3373+
Yes,
3374+
No,
3375+
}
3376+
33693377
#[derive(Clone, Encodable, Decodable, Debug)]
33703378
pub struct PolyTraitRef {
33713379
/// The `'a` in `for<'a> Foo<&'a T>`.
@@ -3378,6 +3386,10 @@ pub struct PolyTraitRef {
33783386
pub trait_ref: TraitRef,
33793387

33803388
pub span: Span,
3389+
3390+
/// When `Yes`, the first and last character of `span` are an opening
3391+
/// and a closing paren respectively.
3392+
pub parens: Parens,
33813393
}
33823394

33833395
impl PolyTraitRef {
@@ -3386,12 +3398,14 @@ impl PolyTraitRef {
33863398
path: Path,
33873399
modifiers: TraitBoundModifiers,
33883400
span: Span,
3401+
parens: Parens,
33893402
) -> Self {
33903403
PolyTraitRef {
33913404
bound_generic_params: generic_params,
33923405
modifiers,
33933406
trait_ref: TraitRef { path, ref_id: DUMMY_NODE_ID },
33943407
span,
3408+
parens,
33953409
}
33963410
}
33973411
}

‎compiler/rustc_ast/src/visit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1142,7 +1142,7 @@ macro_rules! common_visitor_and_walkers {
11421142
vis: &mut V,
11431143
p: &$($lt)? $($mut)? PolyTraitRef,
11441144
) -> V::Result {
1145-
let PolyTraitRef { bound_generic_params, modifiers, trait_ref, span } = p;
1145+
let PolyTraitRef { bound_generic_params, modifiers, trait_ref, span, parens: _ } = p;
11461146
try_visit!(visit_modifiers(vis, modifiers));
11471147
try_visit!(visit_generic_params(vis, bound_generic_params));
11481148
try_visit!(vis.visit_trait_ref(trait_ref));

‎compiler/rustc_ast_lowering/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,6 +1209,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
12091209
modifiers: TraitBoundModifiers::NONE,
12101210
trait_ref: TraitRef { path: path.clone(), ref_id: t.id },
12111211
span: t.span,
1212+
parens: ast::Parens::No,
12121213
},
12131214
itctx,
12141215
);

‎compiler/rustc_expand/src/build.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ impl<'a> ExtCtxt<'a> {
195195
},
196196
trait_ref: self.trait_ref(path),
197197
span,
198+
parens: ast::Parens::No,
198199
}
199200
}
200201

‎compiler/rustc_lint/src/unused.rs

Lines changed: 118 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ use std::iter;
33
use rustc_ast::util::{classify, parser};
44
use rustc_ast::{self as ast, ExprKind, HasAttrs as _, StmtKind};
55
use rustc_attr_data_structures::{AttributeKind, find_attr};
6+
use rustc_data_structures::fx::FxHashMap;
67
use rustc_errors::{MultiSpan, pluralize};
78
use rustc_hir::def::{DefKind, Res};
89
use rustc_hir::def_id::DefId;
910
use rustc_hir::{self as hir, LangItem};
1011
use rustc_infer::traits::util::elaborate;
1112
use rustc_middle::ty::{self, Ty, adjustment};
1213
use rustc_session::{declare_lint, declare_lint_pass, impl_lint_pass};
14+
use rustc_span::edition::Edition::Edition2015;
1315
use rustc_span::{BytePos, Span, Symbol, kw, sym};
1416
use tracing::instrument;
1517

@@ -1034,6 +1036,31 @@ pub(crate) struct UnusedParens {
10341036
/// `1 as (i32) < 2` parses to ExprKind::Lt
10351037
/// `1 as i32 < 2` parses to i32::<2[missing angle bracket]
10361038
parens_in_cast_in_lt: Vec<ast::NodeId>,
1039+
/// Ty nodes in this map are in TypeNoBounds position. Any bounds they
1040+
/// contain may be ambiguous w/r/t trailing `+` operators.
1041+
in_no_bounds_pos: FxHashMap<ast::NodeId, NoBoundsException>,
1042+
}
1043+
1044+
/// Whether parentheses may be omitted from a type without resulting in ambiguity.
1045+
///
1046+
/// ```
1047+
/// type Example = Box<dyn Fn() -> &'static (dyn Send) + Sync>;
1048+
/// ```
1049+
///
1050+
/// Here, `&'static (dyn Send) + Sync` is a `TypeNoBounds`. As such, it may not directly
1051+
/// contain `ImplTraitType` or `TraitObjectType` which is why `(dyn Send)` is parenthesized.
1052+
/// However, an exception is made for `ImplTraitTypeOneBound` and `TraitObjectTypeOneBound`.
1053+
/// The following is accepted because there is no `+`.
1054+
///
1055+
/// ```
1056+
/// type Example = Box<dyn Fn() -> &'static dyn Send>;
1057+
/// ```
1058+
enum NoBoundsException {
1059+
/// The type must be parenthesized.
1060+
None,
1061+
/// The type is the last bound of the containing type expression. If it has exactly one bound,
1062+
/// parentheses around the type are unnecessary.
1063+
OneBound,
10371064
}
10381065

10391066
impl_lint_pass!(UnusedParens => [UNUSED_PARENS]);
@@ -1277,23 +1304,100 @@ impl EarlyLintPass for UnusedParens {
12771304
);
12781305
}
12791306
ast::TyKind::Paren(r) => {
1280-
match &r.kind {
1281-
ast::TyKind::TraitObject(..) => {}
1282-
ast::TyKind::BareFn(b)
1283-
if self.with_self_ty_parens && b.generic_params.len() > 0 => {}
1284-
ast::TyKind::ImplTrait(_, bounds) if bounds.len() > 1 => {}
1285-
_ => {
1286-
let spans = if !ty.span.from_expansion() {
1307+
let unused_parens = match &r.kind {
1308+
ast::TyKind::ImplTrait(_, bounds) | ast::TyKind::TraitObject(bounds, _) => {
1309+
match self.in_no_bounds_pos.get(&ty.id) {
1310+
Some(NoBoundsException::None) => false,
1311+
Some(NoBoundsException::OneBound) => bounds.len() <= 1,
1312+
None => true,
1313+
}
1314+
}
1315+
ast::TyKind::BareFn(b) => {
1316+
!self.with_self_ty_parens || b.generic_params.is_empty()
1317+
}
1318+
_ => true,
1319+
};
1320+
1321+
if unused_parens {
1322+
let spans = (!ty.span.from_expansion())
1323+
.then(|| {
12871324
r.span
12881325
.find_ancestor_inside(ty.span)
12891326
.map(|r| (ty.span.with_hi(r.lo()), ty.span.with_lo(r.hi())))
1327+
})
1328+
.flatten();
1329+
1330+
self.emit_unused_delims(cx, ty.span, spans, "type", (false, false), false);
1331+
}
1332+
1333+
self.with_self_ty_parens = false;
1334+
}
1335+
ast::TyKind::Ref(_, mut_ty) | ast::TyKind::Ptr(mut_ty) => {
1336+
self.in_no_bounds_pos.insert(mut_ty.ty.id, NoBoundsException::OneBound);
1337+
}
1338+
ast::TyKind::TraitObject(bounds, _) | ast::TyKind::ImplTrait(_, bounds) => {
1339+
for i in 0..bounds.len() {
1340+
let is_last = i == bounds.len() - 1;
1341+
1342+
if let ast::GenericBound::Trait(poly_trait_ref) = &bounds[i] {
1343+
let fn_with_explicit_ret_ty = if let [.., segment] =
1344+
&*poly_trait_ref.trait_ref.path.segments
1345+
&& let Some(args) = segment.args.as_ref()
1346+
&& let ast::GenericArgs::Parenthesized(paren_args) = &**args
1347+
&& let ast::FnRetTy::Ty(ret_ty) = &paren_args.output
1348+
{
1349+
self.in_no_bounds_pos.insert(
1350+
ret_ty.id,
1351+
if is_last {
1352+
NoBoundsException::OneBound
1353+
} else {
1354+
NoBoundsException::None
1355+
},
1356+
);
1357+
1358+
true
12901359
} else {
1291-
None
1360+
false
12921361
};
1293-
self.emit_unused_delims(cx, ty.span, spans, "type", (false, false), false);
1362+
1363+
// In edition 2015, dyn is a contextual keyword and `dyn::foo::Bar` is
1364+
// parsed as a path, so parens are necessary to disambiguate. See
1365+
// - tests/ui/lint/unused/unused-parens-trait-obj-e2015.rs and
1366+
// - https://doc.rust-lang.org/reference/types/trait-object.html#r-type.trait-object.syntax-edition2018
1367+
let dyn2015_exception = cx.sess().psess.edition == Edition2015
1368+
&& matches!(ty.kind, ast::TyKind::TraitObject(..))
1369+
&& i == 0
1370+
&& poly_trait_ref
1371+
.trait_ref
1372+
.path
1373+
.segments
1374+
.first()
1375+
.map(|s| s.ident.name == kw::PathRoot)
1376+
.unwrap_or(false);
1377+
1378+
if let ast::Parens::Yes = poly_trait_ref.parens
1379+
&& (is_last || !fn_with_explicit_ret_ty)
1380+
&& !dyn2015_exception
1381+
{
1382+
let s = poly_trait_ref.span;
1383+
let spans = (!s.from_expansion()).then(|| {
1384+
(
1385+
s.with_hi(s.lo() + rustc_span::BytePos(1)),
1386+
s.with_lo(s.hi() - rustc_span::BytePos(1)),
1387+
)
1388+
});
1389+
1390+
self.emit_unused_delims(
1391+
cx,
1392+
poly_trait_ref.span,
1393+
spans,
1394+
"type",
1395+
(false, false),
1396+
false,
1397+
);
1398+
}
12941399
}
12951400
}
1296-
self.with_self_ty_parens = false;
12971401
}
12981402
_ => {}
12991403
}
@@ -1303,6 +1407,10 @@ impl EarlyLintPass for UnusedParens {
13031407
<Self as UnusedDelimLint>::check_item(self, cx, item)
13041408
}
13051409

1410+
fn check_item_post(&mut self, _: &EarlyContext<'_>, _: &rustc_ast::Item) {
1411+
self.in_no_bounds_pos.clear();
1412+
}
1413+
13061414
fn enter_where_predicate(&mut self, _: &EarlyContext<'_>, pred: &ast::WherePredicate) {
13071415
use rustc_ast::{WhereBoundPredicate, WherePredicateKind};
13081416
if let WherePredicateKind::BoundPredicate(WhereBoundPredicate {

‎compiler/rustc_parse/src/parser/ty.rs

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,13 @@ impl<'a> Parser<'a> {
305305
let removal_span = kw.span.with_hi(self.token.span.lo());
306306
let path = self.parse_path(PathStyle::Type)?;
307307
let parse_plus = allow_plus == AllowPlus::Yes && self.check_plus();
308-
let kind =
309-
self.parse_remaining_bounds_path(lifetime_defs, path, lo, parse_plus)?;
308+
let kind = self.parse_remaining_bounds_path(
309+
lifetime_defs,
310+
path,
311+
lo,
312+
parse_plus,
313+
ast::Parens::No,
314+
)?;
310315
let err = self.dcx().create_err(errors::TransposeDynOrImpl {
311316
span: kw.span,
312317
kw: kw.name.as_str(),
@@ -333,7 +338,13 @@ impl<'a> Parser<'a> {
333338
} else {
334339
let path = self.parse_path(PathStyle::Type)?;
335340
let parse_plus = allow_plus == AllowPlus::Yes && self.check_plus();
336-
self.parse_remaining_bounds_path(lifetime_defs, path, lo, parse_plus)?
341+
self.parse_remaining_bounds_path(
342+
lifetime_defs,
343+
path,
344+
lo,
345+
parse_plus,
346+
ast::Parens::No,
347+
)?
337348
}
338349
}
339350
} else if self.eat_keyword(exp!(Impl)) {
@@ -413,9 +424,13 @@ impl<'a> Parser<'a> {
413424
let maybe_bounds = allow_plus == AllowPlus::Yes && self.token.is_like_plus();
414425
match ty.kind {
415426
// `(TY_BOUND_NOPAREN) + BOUND + ...`.
416-
TyKind::Path(None, path) if maybe_bounds => {
417-
self.parse_remaining_bounds_path(ThinVec::new(), path, lo, true)
418-
}
427+
TyKind::Path(None, path) if maybe_bounds => self.parse_remaining_bounds_path(
428+
ThinVec::new(),
429+
path,
430+
lo,
431+
true,
432+
ast::Parens::Yes,
433+
),
419434
// For `('a) + …`, we know that `'a` in type position already lead to an error being
420435
// emitted. To reduce output, let's indirectly suppress E0178 (bad `+` in type) and
421436
// other irrelevant consequential errors.
@@ -495,12 +510,14 @@ impl<'a> Parser<'a> {
495510
path: ast::Path,
496511
lo: Span,
497512
parse_plus: bool,
513+
parens: ast::Parens,
498514
) -> PResult<'a, TyKind> {
499515
let poly_trait_ref = PolyTraitRef::new(
500516
generic_params,
501517
path,
502518
TraitBoundModifiers::NONE,
503519
lo.to(self.prev_token.span),
520+
parens,
504521
);
505522
let bounds = vec![GenericBound::Trait(poly_trait_ref)];
506523
self.parse_remaining_bounds(bounds, parse_plus)
@@ -826,7 +843,7 @@ impl<'a> Parser<'a> {
826843
Ok(TyKind::MacCall(P(MacCall { path, args: self.parse_delim_args()? })))
827844
} else if allow_plus == AllowPlus::Yes && self.check_plus() {
828845
// `Trait1 + Trait2 + 'a`
829-
self.parse_remaining_bounds_path(ThinVec::new(), path, lo, true)
846+
self.parse_remaining_bounds_path(ThinVec::new(), path, lo, true, ast::Parens::No)
830847
} else {
831848
// Just a type path.
832849
Ok(TyKind::Path(None, path))
@@ -892,10 +909,10 @@ impl<'a> Parser<'a> {
892909
fn parse_generic_bound(&mut self) -> PResult<'a, GenericBound> {
893910
let lo = self.token.span;
894911
let leading_token = self.prev_token;
895-
let has_parens = self.eat(exp!(OpenParen));
912+
let parens = if self.eat(exp!(OpenParen)) { ast::Parens::Yes } else { ast::Parens::No };
896913

897914
let bound = if self.token.is_lifetime() {
898-
self.parse_generic_lt_bound(lo, has_parens)?
915+
self.parse_generic_lt_bound(lo, parens)?
899916
} else if self.eat_keyword(exp!(Use)) {
900917
// parse precise captures, if any. This is `use<'lt, 'lt, P, P>`; a list of
901918
// lifetimes and ident params (including SelfUpper). These are validated later
@@ -904,7 +921,7 @@ impl<'a> Parser<'a> {
904921
let (args, args_span) = self.parse_precise_capturing_args()?;
905922
GenericBound::Use(args, use_span.to(args_span))
906923
} else {
907-
self.parse_generic_ty_bound(lo, has_parens, &leading_token)?
924+
self.parse_generic_ty_bound(lo, parens, &leading_token)?
908925
};
909926

910927
Ok(bound)
@@ -914,10 +931,14 @@ impl<'a> Parser<'a> {
914931
/// ```ebnf
915932
/// LT_BOUND = LIFETIME
916933
/// ```
917-
fn parse_generic_lt_bound(&mut self, lo: Span, has_parens: bool) -> PResult<'a, GenericBound> {
934+
fn parse_generic_lt_bound(
935+
&mut self,
936+
lo: Span,
937+
parens: ast::Parens,
938+
) -> PResult<'a, GenericBound> {
918939
let lt = self.expect_lifetime();
919940
let bound = GenericBound::Outlives(lt);
920-
if has_parens {
941+
if let ast::Parens::Yes = parens {
921942
// FIXME(Centril): Consider not erroring here and accepting `('lt)` instead,
922943
// possibly introducing `GenericBound::Paren(P<GenericBound>)`?
923944
self.recover_paren_lifetime(lo)?;
@@ -1090,7 +1111,7 @@ impl<'a> Parser<'a> {
10901111
fn parse_generic_ty_bound(
10911112
&mut self,
10921113
lo: Span,
1093-
has_parens: bool,
1114+
parens: ast::Parens,
10941115
leading_token: &Token,
10951116
) -> PResult<'a, GenericBound> {
10961117
let (mut lifetime_defs, binder_span) = self.parse_late_bound_lifetime_defs()?;
@@ -1116,7 +1137,7 @@ impl<'a> Parser<'a> {
11161137
// e.g. `T: for<'a> 'a` or `T: [const] 'a`.
11171138
if self.token.is_lifetime() {
11181139
let _: ErrorGuaranteed = self.error_lt_bound_with_modifiers(modifiers, binder_span);
1119-
return self.parse_generic_lt_bound(lo, has_parens);
1140+
return self.parse_generic_lt_bound(lo, parens);
11201141
}
11211142

11221143
if let (more_lifetime_defs, Some(binder_span)) = self.parse_late_bound_lifetime_defs()? {
@@ -1183,7 +1204,7 @@ impl<'a> Parser<'a> {
11831204
self.recover_fn_trait_with_lifetime_params(&mut path, &mut lifetime_defs)?;
11841205
}
11851206

1186-
if has_parens {
1207+
if let ast::Parens::Yes = parens {
11871208
// Someone has written something like `&dyn (Trait + Other)`. The correct code
11881209
// would be `&(dyn Trait + Other)`
11891210
if self.token.is_like_plus() && leading_token.is_keyword(kw::Dyn) {
@@ -1203,7 +1224,7 @@ impl<'a> Parser<'a> {
12031224
}
12041225

12051226
let poly_trait =
1206-
PolyTraitRef::new(lifetime_defs, path, modifiers, lo.to(self.prev_token.span));
1227+
PolyTraitRef::new(lifetime_defs, path, modifiers, lo.to(self.prev_token.span), parens);
12071228
Ok(GenericBound::Trait(poly_trait))
12081229
}
12091230

‎compiler/rustc_resolve/src/late/diagnostics.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3832,6 +3832,7 @@ fn mk_where_bound_predicate(
38323832
ref_id: DUMMY_NODE_ID,
38333833
},
38343834
span: DUMMY_SP,
3835+
parens: ast::Parens::No,
38353836
})],
38363837
};
38373838

0 commit comments

Comments
 (0)