Skip to content

Commit

Permalink
Auto merge of #96745 - ehuss:even-more-attribute-validation, r=cjgillot
Browse files Browse the repository at this point in the history
Visit attributes in more places.

This adds 3 loosely related changes (I can split PRs if desired):

- Attribute checking on pattern struct fields.
- Attribute checking on struct expression fields.
- Lint level visiting on pattern struct fields, struct expression fields, and generic parameters.

There are still some lints which ignore lint levels in various positions. This is a consequence of how the lints themselves are implemented. For example, lint levels on associated consts don't work with `unused_braces`.
  • Loading branch information
bors committed Aug 15, 2022
2 parents 80ed61f + 900a9d3 commit 6ce7609
Show file tree
Hide file tree
Showing 23 changed files with 1,520 additions and 141 deletions.
4 changes: 3 additions & 1 deletion compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1406,8 +1406,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
}

fn lower_expr_field(&mut self, f: &ExprField) -> hir::ExprField<'hir> {
let hir_id = self.lower_node_id(f.id);
self.lower_attrs(hir_id, &f.attrs);
hir::ExprField {
hir_id: self.next_id(),
hir_id,
ident: self.lower_ident(f.ident),
expr: self.lower_expr(&f.expr),
span: self.lower_span(f.span),
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_ast_lowering/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,13 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
});
}

fn visit_pat_field(&mut self, field: &'hir PatField<'hir>) {
self.insert(field.span, field.hir_id, Node::PatField(field));
self.with_parent(field.hir_id, |this| {
intravisit::walk_pat_field(this, field);
});
}

fn visit_arm(&mut self, arm: &'hir Arm<'hir>) {
let node = Node::Arm(arm);

Expand All @@ -225,6 +232,13 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
});
}

fn visit_expr_field(&mut self, field: &'hir ExprField<'hir>) {
self.insert(field.span, field.hir_id, Node::ExprField(field));
self.with_parent(field.hir_id, |this| {
intravisit::walk_expr_field(this, field);
});
}

fn visit_stmt(&mut self, stmt: &'hir Stmt<'hir>) {
self.insert(stmt.span, stmt.hir_id, Node::Stmt(stmt));

Expand Down
17 changes: 11 additions & 6 deletions compiler/rustc_ast_lowering/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,17 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
ImplTraitContext::Disallowed(ImplTraitPosition::Path),
);

let fs = self.arena.alloc_from_iter(fields.iter().map(|f| hir::PatField {
hir_id: self.next_id(),
ident: self.lower_ident(f.ident),
pat: self.lower_pat(&f.pat),
is_shorthand: f.is_shorthand,
span: self.lower_span(f.span),
let fs = self.arena.alloc_from_iter(fields.iter().map(|f| {
let hir_id = self.lower_node_id(f.id);
self.lower_attrs(hir_id, &f.attrs);

hir::PatField {
hir_id,
ident: self.lower_ident(f.ident),
pat: self.lower_pat(&f.pat),
is_shorthand: f.is_shorthand,
span: self.lower_span(f.span),
}
}));
break hir::PatKind::Struct(qpath, fs, etc);
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3332,12 +3332,14 @@ pub enum Node<'hir> {
Field(&'hir FieldDef<'hir>),
AnonConst(&'hir AnonConst),
Expr(&'hir Expr<'hir>),
ExprField(&'hir ExprField<'hir>),
Stmt(&'hir Stmt<'hir>),
PathSegment(&'hir PathSegment<'hir>),
Ty(&'hir Ty<'hir>),
TypeBinding(&'hir TypeBinding<'hir>),
TraitRef(&'hir TraitRef<'hir>),
Pat(&'hir Pat<'hir>),
PatField(&'hir PatField<'hir>),
Arm(&'hir Arm<'hir>),
Block(&'hir Block<'hir>),
Local(&'hir Local<'hir>),
Expand Down Expand Up @@ -3388,6 +3390,8 @@ impl<'hir> Node<'hir> {
| Node::Block(..)
| Node::Ctor(..)
| Node::Pat(..)
| Node::PatField(..)
| Node::ExprField(..)
| Node::Arm(..)
| Node::Local(..)
| Node::Crate(..)
Expand Down
30 changes: 20 additions & 10 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,9 @@ pub trait Visitor<'v>: Sized {
fn visit_pat(&mut self, p: &'v Pat<'v>) {
walk_pat(self, p)
}
fn visit_pat_field(&mut self, f: &'v PatField<'v>) {
walk_pat_field(self, f)
}
fn visit_array_length(&mut self, len: &'v ArrayLen) {
walk_array_len(self, len)
}
Expand All @@ -337,6 +340,9 @@ pub trait Visitor<'v>: Sized {
fn visit_let_expr(&mut self, lex: &'v Let<'v>) {
walk_let_expr(self, lex)
}
fn visit_expr_field(&mut self, field: &'v ExprField<'v>) {
walk_expr_field(self, field)
}
fn visit_ty(&mut self, t: &'v Ty<'v>) {
walk_ty(self, t)
}
Expand Down Expand Up @@ -761,11 +767,7 @@ pub fn walk_pat<'v, V: Visitor<'v>>(visitor: &mut V, pattern: &'v Pat<'v>) {
}
PatKind::Struct(ref qpath, fields, _) => {
visitor.visit_qpath(qpath, pattern.hir_id, pattern.span);
for field in fields {
visitor.visit_id(field.hir_id);
visitor.visit_ident(field.ident);
visitor.visit_pat(&field.pat)
}
walk_list!(visitor, visit_pat_field, fields);
}
PatKind::Or(pats) => walk_list!(visitor, visit_pat, pats),
PatKind::Tuple(tuple_elements, _) => {
Expand All @@ -792,6 +794,12 @@ pub fn walk_pat<'v, V: Visitor<'v>>(visitor: &mut V, pattern: &'v Pat<'v>) {
}
}

pub fn walk_pat_field<'v, V: Visitor<'v>>(visitor: &mut V, field: &'v PatField<'v>) {
visitor.visit_id(field.hir_id);
visitor.visit_ident(field.ident);
visitor.visit_pat(&field.pat)
}

pub fn walk_foreign_item<'v, V: Visitor<'v>>(visitor: &mut V, foreign_item: &'v ForeignItem<'v>) {
visitor.visit_id(foreign_item.hir_id());
visitor.visit_ident(foreign_item.ident);
Expand Down Expand Up @@ -1059,6 +1067,12 @@ pub fn walk_let_expr<'v, V: Visitor<'v>>(visitor: &mut V, let_expr: &'v Let<'v>)
walk_list!(visitor, visit_ty, let_expr.ty);
}

pub fn walk_expr_field<'v, V: Visitor<'v>>(visitor: &mut V, field: &'v ExprField<'v>) {
visitor.visit_id(field.hir_id);
visitor.visit_ident(field.ident);
visitor.visit_expr(&field.expr)
}

pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>) {
visitor.visit_id(expression.hir_id);
match expression.kind {
Expand All @@ -1073,11 +1087,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
}
ExprKind::Struct(ref qpath, fields, ref optional_base) => {
visitor.visit_qpath(qpath, expression.hir_id, expression.span);
for field in fields {
visitor.visit_id(field.hir_id);
visitor.visit_ident(field.ident);
visitor.visit_expr(&field.expr)
}
walk_list!(visitor, visit_expr_field, fields);
walk_list!(visitor, visit_expr, optional_base);
}
ExprKind::Tup(subexpressions) => {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir/src/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ pub enum Target {
GenericParam(GenericParamKind),
MacroDef,
Param,
PatField,
ExprField,
}

impl Display for Target {
Expand Down Expand Up @@ -183,6 +185,8 @@ impl Target {
},
Target::MacroDef => "macro def",
Target::Param => "function param",
Target::PatField => "pattern field",
Target::ExprField => "struct field",
}
}
}
60 changes: 32 additions & 28 deletions compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,14 @@ impl<'a> State<'a> {
Node::Variant(a) => self.print_variant(a),
Node::AnonConst(a) => self.print_anon_const(a),
Node::Expr(a) => self.print_expr(a),
Node::ExprField(a) => self.print_expr_field(&a),
Node::Stmt(a) => self.print_stmt(a),
Node::PathSegment(a) => self.print_path_segment(a),
Node::Ty(a) => self.print_type(a),
Node::TypeBinding(a) => self.print_type_binding(a),
Node::TraitRef(a) => self.print_trait_ref(a),
Node::Pat(a) => self.print_pat(a),
Node::PatField(a) => self.print_patfield(&a),
Node::Arm(a) => self.print_arm(a),
Node::Infer(_) => self.word("_"),
Node::Block(a) => {
Expand Down Expand Up @@ -1127,20 +1129,7 @@ impl<'a> State<'a> {
) {
self.print_qpath(qpath, true);
self.word("{");
self.commasep_cmnt(
Consistent,
fields,
|s, field| {
s.ibox(INDENT_UNIT);
if !field.is_shorthand {
s.print_ident(field.ident);
s.word_space(":");
}
s.print_expr(field.expr);
s.end()
},
|f| f.span,
);
self.commasep_cmnt(Consistent, fields, |s, field| s.print_expr_field(field), |f| f.span);
if let Some(expr) = wth {
self.ibox(INDENT_UNIT);
if !fields.is_empty() {
Expand All @@ -1157,6 +1146,20 @@ impl<'a> State<'a> {
self.word("}");
}

fn print_expr_field(&mut self, field: &hir::ExprField<'_>) {
if self.attrs(field.hir_id).is_empty() {
self.space();
}
self.cbox(INDENT_UNIT);
self.print_outer_attributes(&self.attrs(field.hir_id));
if !field.is_shorthand {
self.print_ident(field.ident);
self.word_space(":");
}
self.print_expr(&field.expr);
self.end()
}

fn print_expr_tup(&mut self, exprs: &[hir::Expr<'_>]) {
self.popen();
self.commasep_exprs(Inconsistent, exprs);
Expand Down Expand Up @@ -1803,20 +1806,7 @@ impl<'a> State<'a> {
if !empty {
self.space();
}
self.commasep_cmnt(
Consistent,
fields,
|s, f| {
s.cbox(INDENT_UNIT);
if !f.is_shorthand {
s.print_ident(f.ident);
s.word_nbsp(":");
}
s.print_pat(f.pat);
s.end()
},
|f| f.pat.span,
);
self.commasep_cmnt(Consistent, &fields, |s, f| s.print_patfield(f), |f| f.pat.span);
if etc {
if !fields.is_empty() {
self.word_space(",");
Expand Down Expand Up @@ -1911,6 +1901,20 @@ impl<'a> State<'a> {
self.ann.post(self, AnnNode::Pat(pat))
}

pub fn print_patfield(&mut self, field: &hir::PatField<'_>) {
if self.attrs(field.hir_id).is_empty() {
self.space();
}
self.cbox(INDENT_UNIT);
self.print_outer_attributes(&self.attrs(field.hir_id));
if !field.is_shorthand {
self.print_ident(field.ident);
self.word_nbsp(":");
}
self.print_pat(field.pat);
self.end();
}

pub fn print_param(&mut self, arg: &hir::Param<'_>) {
self.print_outer_attributes(self.attrs(arg.hir_id));
self.print_pat(arg.pat);
Expand Down
13 changes: 10 additions & 3 deletions compiler/rustc_lint/src/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
run_early_pass!(self, check_pat_post, p);
}

fn visit_pat_field(&mut self, field: &'a ast::PatField) {
self.with_lint_attrs(field.id, &field.attrs, |cx| {
ast_visit::walk_pat_field(cx, field);
});
}

fn visit_anon_const(&mut self, c: &'a ast::AnonConst) {
self.check_id(c.id);
ast_visit::walk_anon_const(self, c);
Expand Down Expand Up @@ -219,9 +225,10 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
}

fn visit_generic_param(&mut self, param: &'a ast::GenericParam) {
run_early_pass!(self, check_generic_param, param);
self.check_id(param.id);
ast_visit::walk_generic_param(self, param);
self.with_lint_attrs(param.id, &param.attrs, |cx| {
run_early_pass!(cx, check_generic_param, param);
ast_visit::walk_generic_param(cx, param);
});
}

fn visit_generics(&mut self, g: &'a ast::Generics) {
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,12 @@ impl<'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'tcx> {
})
}

fn visit_expr_field(&mut self, field: &'tcx hir::ExprField<'tcx>) {
self.with_lint_attrs(field.hir_id, |builder| {
intravisit::walk_expr_field(builder, field);
})
}

fn visit_field_def(&mut self, s: &'tcx hir::FieldDef<'tcx>) {
self.with_lint_attrs(s.hir_id, |builder| {
intravisit::walk_field_def(builder, s);
Expand Down Expand Up @@ -801,6 +807,18 @@ impl<'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'tcx> {
intravisit::walk_impl_item(builder, impl_item);
});
}

fn visit_pat_field(&mut self, field: &'tcx hir::PatField<'tcx>) {
self.with_lint_attrs(field.hir_id, |builder| {
intravisit::walk_pat_field(builder, field);
})
}

fn visit_generic_param(&mut self, p: &'tcx hir::GenericParam<'tcx>) {
self.with_lint_attrs(p.hir_id, |builder| {
intravisit::walk_generic_param(builder, p);
});
}
}

pub fn provide(providers: &mut Providers) {
Expand Down
17 changes: 6 additions & 11 deletions compiler/rustc_lint/src/nonstandard_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,19 +437,14 @@ impl<'tcx> LateLintPass<'tcx> for NonSnakeCase {

fn check_pat(&mut self, cx: &LateContext<'_>, p: &hir::Pat<'_>) {
if let PatKind::Binding(_, hid, ident, _) = p.kind {
if let hir::Node::Pat(parent_pat) = cx.tcx.hir().get(cx.tcx.hir().get_parent_node(hid))
if let hir::Node::PatField(field) = cx.tcx.hir().get(cx.tcx.hir().get_parent_node(hid))
{
if let PatKind::Struct(_, field_pats, _) = &parent_pat.kind {
if field_pats
.iter()
.any(|field| !field.is_shorthand && field.pat.hir_id == p.hir_id)
{
// Only check if a new name has been introduced, to avoid warning
// on both the struct definition and this pattern.
self.check_snake_case(cx, "variable", &ident);
}
return;
if !field.is_shorthand {
// Only check if a new name has been introduced, to avoid warning
// on both the struct definition and this pattern.
self.check_snake_case(cx, "variable", &ident);
}
return;
}
self.check_snake_case(cx, "variable", &ident);
}
Expand Down
Loading

0 comments on commit 6ce7609

Please sign in to comment.