From 5011ec7fedffe34d943654ffb4308875fc5ec8f3 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 11 Oct 2019 21:00:09 +0200 Subject: [PATCH] move attr meta grammar to parse::validate_atr + ast_validation --- src/librustc_passes/ast_validation.rs | 5 ++ src/libsyntax/attr/builtin.rs | 69 +-------------- src/libsyntax/attr/mod.rs | 19 ---- src/libsyntax/config.rs | 4 +- src/libsyntax/feature_gate/check.rs | 23 +---- src/libsyntax/parse/mod.rs | 1 + src/libsyntax/parse/validate_attr.rs | 112 ++++++++++++++++++++++++ src/libsyntax_expand/expand.rs | 4 +- src/libsyntax_ext/util.rs | 5 +- src/test/ui/proc-macro/attribute.stderr | 24 ++--- 10 files changed, 142 insertions(+), 124 deletions(-) create mode 100644 src/libsyntax/parse/validate_attr.rs diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 8cf83d41ac2da..a45035f123f56 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -15,6 +15,7 @@ use syntax::ast::*; use syntax::attr; use syntax::expand::is_proc_macro_attr; use syntax::feature_gate::is_builtin_attr; +use syntax::parse::validate_attr; use syntax::source_map::Spanned; use syntax::symbol::{kw, sym}; use syntax::visit::{self, Visitor}; @@ -369,6 +370,10 @@ fn validate_generics_order<'a>( } impl<'a> Visitor<'a> for AstValidator<'a> { + fn visit_attribute(&mut self, attr: &Attribute) { + validate_attr::check_meta(&self.session.parse_sess, attr); + } + fn visit_expr(&mut self, expr: &'a Expr) { match &expr.kind { ExprKind::Closure(_, _, _, fn_decl, _, _) => { diff --git a/src/libsyntax/attr/builtin.rs b/src/libsyntax/attr/builtin.rs index 787d69f5e9964..3ac9abb722361 100644 --- a/src/libsyntax/attr/builtin.rs +++ b/src/libsyntax/attr/builtin.rs @@ -1,7 +1,6 @@ //! Parsing and validation of builtin attributes use crate::ast::{self, Attribute, MetaItem, NestedMetaItem}; -use crate::early_buffered_lints::BufferedEarlyLintId; use crate::feature_gate::{Features, GatedCfg}; use crate::print::pprust; use crate::sess::ParseSess; @@ -36,7 +35,7 @@ impl AttributeTemplate { } /// Checks that the given meta-item is compatible with this template. - fn compatible(&self, meta_item_kind: &ast::MetaItemKind) -> bool { + pub fn compatible(&self, meta_item_kind: &ast::MetaItemKind) -> bool { match meta_item_kind { ast::MetaItemKind::Word => self.word, ast::MetaItemKind::List(..) => self.list.is_some(), @@ -938,69 +937,3 @@ pub fn find_transparency( let fallback = if is_legacy { Transparency::SemiTransparent } else { Transparency::Opaque }; (transparency.map_or(fallback, |t| t.0), error) } - -pub fn check_builtin_attribute( - sess: &ParseSess, attr: &ast::Attribute, name: Symbol, template: AttributeTemplate -) { - // Some special attributes like `cfg` must be checked - // before the generic check, so we skip them here. - let should_skip = |name| name == sym::cfg; - // Some of previously accepted forms were used in practice, - // report them as warnings for now. - let should_warn = |name| name == sym::doc || name == sym::ignore || - name == sym::inline || name == sym::link || - name == sym::test || name == sym::bench; - - match attr.parse_meta(sess) { - Ok(meta) => if !should_skip(name) && !template.compatible(&meta.kind) { - let error_msg = format!("malformed `{}` attribute input", name); - let mut msg = "attribute must be of the form ".to_owned(); - let mut suggestions = vec![]; - let mut first = true; - if template.word { - first = false; - let code = format!("#[{}]", name); - msg.push_str(&format!("`{}`", &code)); - suggestions.push(code); - } - if let Some(descr) = template.list { - if !first { - msg.push_str(" or "); - } - first = false; - let code = format!("#[{}({})]", name, descr); - msg.push_str(&format!("`{}`", &code)); - suggestions.push(code); - } - if let Some(descr) = template.name_value_str { - if !first { - msg.push_str(" or "); - } - let code = format!("#[{} = \"{}\"]", name, descr); - msg.push_str(&format!("`{}`", &code)); - suggestions.push(code); - } - if should_warn(name) { - sess.buffer_lint( - BufferedEarlyLintId::IllFormedAttributeInput, - meta.span, - ast::CRATE_NODE_ID, - &msg, - ); - } else { - sess.span_diagnostic.struct_span_err(meta.span, &error_msg) - .span_suggestions( - meta.span, - if suggestions.len() == 1 { - "must be of the form" - } else { - "the following are the possible correct uses" - }, - suggestions.into_iter(), - Applicability::HasPlaceholders, - ).emit(); - } - } - Err(mut err) => err.emit(), - } -} diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index c639431794c50..2c011b0647004 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -14,17 +14,13 @@ use crate::ast::{MetaItem, MetaItemKind, NestedMetaItem}; use crate::ast::{Lit, LitKind, Expr, Item, Local, Stmt, StmtKind, GenericParam}; use crate::mut_visit::visit_clobber; use crate::source_map::{BytePos, Spanned}; -use crate::parse; use crate::token::{self, Token}; use crate::ptr::P; -use crate::sess::ParseSess; use crate::symbol::{sym, Symbol}; use crate::ThinVec; use crate::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint}; use crate::GLOBALS; -use errors::PResult; - use log::debug; use syntax_pos::Span; @@ -328,21 +324,6 @@ impl Attribute { Some(mk_name_value_item_str(Ident::new(sym::doc, self.span), comment, self.span)), } } - - pub fn parse_meta<'a>(&self, sess: &'a ParseSess) -> PResult<'a, MetaItem> { - match self.kind { - AttrKind::Normal(ref item) => { - Ok(MetaItem { - path: item.path.clone(), - kind: parse::parse_in_attr(sess, self, |parser| parser.parse_meta_item_kind())?, - span: self.span, - }) - } - AttrKind::DocComment(comment) => { - Ok(mk_name_value_item_str(Ident::new(sym::doc, self.span), comment, self.span)) - } - } - } } /* Constructors */ diff --git a/src/libsyntax/config.rs b/src/libsyntax/config.rs index 5f89ed36e2a7d..a460fc2b99af3 100644 --- a/src/libsyntax/config.rs +++ b/src/libsyntax/config.rs @@ -10,7 +10,7 @@ use crate::attr; use crate::ast; use crate::edition::Edition; use crate::mut_visit::*; -use crate::parse; +use crate::parse::{self, validate_attr}; use crate::ptr::P; use crate::sess::ParseSess; use crate::symbol::sym; @@ -168,7 +168,7 @@ impl<'a> StripUnconfigured<'a> { true }; - let meta_item = match attr.parse_meta(self.sess) { + let meta_item = match validate_attr::parse_meta(self.sess, attr) { Ok(meta_item) => meta_item, Err(mut err) => { err.emit(); return true; } }; diff --git a/src/libsyntax/feature_gate/check.rs b/src/libsyntax/feature_gate/check.rs index ecff89ad59bd9..4742e01d7f4ea 100644 --- a/src/libsyntax/feature_gate/check.rs +++ b/src/libsyntax/feature_gate/check.rs @@ -3,18 +3,14 @@ use super::accepted::ACCEPTED_FEATURES; use super::removed::{REMOVED_FEATURES, STABLE_REMOVED_FEATURES}; use super::builtin_attrs::{AttributeGate, BUILTIN_ATTRIBUTE_MAP}; -use crate::ast::{ - self, AssocTyConstraint, AssocTyConstraintKind, NodeId, GenericParam, GenericParamKind, - PatKind, RangeEnd, VariantData, -}; -use crate::attr::{self, check_builtin_attribute}; +use crate::ast::{self, AssocTyConstraint, AssocTyConstraintKind, NodeId}; +use crate::ast::{GenericParam, GenericParamKind, PatKind, RangeEnd, VariantData}; +use crate::attr; use crate::source_map::Spanned; use crate::edition::{ALL_EDITIONS, Edition}; use crate::visit::{self, FnKind, Visitor}; -use crate::token; use crate::sess::ParseSess; use crate::symbol::{Symbol, sym}; -use crate::tokenstream::TokenTree; use errors::{Applicability, DiagnosticBuilder, Handler}; use rustc_data_structures::fx::FxHashMap; @@ -331,19 +327,6 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { if let Some((.., AttributeGate::Gated(_, name, descr, has_feature))) = attr_info { gate_feature_fn!(self, has_feature, attr.span, name, descr, GateStrength::Hard); } - // Check input tokens for built-in and key-value attributes. - match attr_info { - // `rustc_dummy` doesn't have any restrictions specific to built-in attributes. - Some((name, _, template, _)) if name != sym::rustc_dummy => - check_builtin_attribute(self.parse_sess, attr, name, template), - _ => if let Some(TokenTree::Token(token)) = - attr.get_normal_item().tokens.trees().next() { - if token == token::Eq { - // All key-value attributes are restricted to meta-item syntax. - attr.parse_meta(self.parse_sess).map_err(|mut err| err.emit()).ok(); - } - } - } // Check unstable flavors of the `#[doc]` attribute. if attr.check_name(sym::doc) { for nested_meta in attr.meta_item_list().unwrap_or_default() { diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index b54f4862f1220..9155cbe5dd8e3 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -23,6 +23,7 @@ mod tests; #[macro_use] pub mod parser; pub mod lexer; +pub mod validate_attr; #[derive(Clone)] pub struct Directory<'a> { diff --git a/src/libsyntax/parse/validate_attr.rs b/src/libsyntax/parse/validate_attr.rs new file mode 100644 index 0000000000000..6cc8a1ec33e50 --- /dev/null +++ b/src/libsyntax/parse/validate_attr.rs @@ -0,0 +1,112 @@ +//! Meta-syntax validation logic of attributes for post-expansion. + +use crate::ast::{self, Attribute, AttrKind, Ident, MetaItem}; +use crate::attr::{AttributeTemplate, mk_name_value_item_str}; +use crate::sess::ParseSess; +use crate::feature_gate::BUILTIN_ATTRIBUTE_MAP; +use crate::early_buffered_lints::BufferedEarlyLintId; +use crate::token; +use crate::tokenstream::TokenTree; + +use errors::{PResult, Applicability}; +use syntax_pos::{Symbol, sym}; + +pub fn check_meta(sess: &ParseSess, attr: &Attribute) { + let attr_info = + attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name)).map(|a| **a); + + // Check input tokens for built-in and key-value attributes. + match attr_info { + // `rustc_dummy` doesn't have any restrictions specific to built-in attributes. + Some((name, _, template, _)) if name != sym::rustc_dummy => + check_builtin_attribute(sess, attr, name, template), + _ => if let Some(TokenTree::Token(token)) = attr.get_normal_item().tokens.trees().next() { + if token == token::Eq { + // All key-value attributes are restricted to meta-item syntax. + parse_meta(sess, attr).map_err(|mut err| err.emit()).ok(); + } + } + } +} + +pub fn parse_meta<'a>(sess: &'a ParseSess, attr: &Attribute) -> PResult<'a, MetaItem> { + Ok(match attr.kind { + AttrKind::Normal(ref item) => MetaItem { + path: item.path.clone(), + kind: super::parse_in_attr(sess, attr, |p| p.parse_meta_item_kind())?, + span: attr.span, + }, + AttrKind::DocComment(comment) => { + mk_name_value_item_str(Ident::new(sym::doc, attr.span), comment, attr.span) + } + }) +} + +pub fn check_builtin_attribute( + sess: &ParseSess, + attr: &Attribute, + name: Symbol, + template: AttributeTemplate, +) { + // Some special attributes like `cfg` must be checked + // before the generic check, so we skip them here. + let should_skip = |name| name == sym::cfg; + // Some of previously accepted forms were used in practice, + // report them as warnings for now. + let should_warn = |name| name == sym::doc || name == sym::ignore || + name == sym::inline || name == sym::link || + name == sym::test || name == sym::bench; + + match parse_meta(sess, attr) { + Ok(meta) => if !should_skip(name) && !template.compatible(&meta.kind) { + let error_msg = format!("malformed `{}` attribute input", name); + let mut msg = "attribute must be of the form ".to_owned(); + let mut suggestions = vec![]; + let mut first = true; + if template.word { + first = false; + let code = format!("#[{}]", name); + msg.push_str(&format!("`{}`", &code)); + suggestions.push(code); + } + if let Some(descr) = template.list { + if !first { + msg.push_str(" or "); + } + first = false; + let code = format!("#[{}({})]", name, descr); + msg.push_str(&format!("`{}`", &code)); + suggestions.push(code); + } + if let Some(descr) = template.name_value_str { + if !first { + msg.push_str(" or "); + } + let code = format!("#[{} = \"{}\"]", name, descr); + msg.push_str(&format!("`{}`", &code)); + suggestions.push(code); + } + if should_warn(name) { + sess.buffer_lint( + BufferedEarlyLintId::IllFormedAttributeInput, + meta.span, + ast::CRATE_NODE_ID, + &msg, + ); + } else { + sess.span_diagnostic.struct_span_err(meta.span, &error_msg) + .span_suggestions( + meta.span, + if suggestions.len() == 1 { + "must be of the form" + } else { + "the following are the possible correct uses" + }, + suggestions.into_iter(), + Applicability::HasPlaceholders, + ).emit(); + } + } + Err(mut err) => err.emit(), + } +} diff --git a/src/libsyntax_expand/expand.rs b/src/libsyntax_expand/expand.rs index 516b284cecd60..d0b60fa308dd6 100644 --- a/src/libsyntax_expand/expand.rs +++ b/src/libsyntax_expand/expand.rs @@ -14,6 +14,7 @@ use syntax::feature_gate::{self, Features, GateIssue, is_builtin_attr, emit_feat use syntax::mut_visit::*; use syntax::parse::DirectoryOwnership; use syntax::parse::parser::Parser; +use syntax::parse::validate_attr; use syntax::print::pprust; use syntax::ptr::P; use syntax::sess::ParseSess; @@ -640,7 +641,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { self.parse_ast_fragment(tok_result, fragment_kind, &item.path, span) } SyntaxExtensionKind::LegacyAttr(expander) => { - match attr.parse_meta(self.cx.parse_sess) { + match validate_attr::parse_meta(self.cx.parse_sess, &attr) { Ok(meta) => { let item = expander.expand(self.cx, span, &meta, item); fragment_kind.expect_from_annotatables(item) @@ -1031,6 +1032,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { let features = self.cx.ecfg.features.unwrap(); for attr in attrs.iter() { feature_gate::check_attribute(attr, self.cx.parse_sess, features); + validate_attr::check_meta(self.cx.parse_sess, attr); // macros are expanded before any lint passes so this warning has to be hardcoded if attr.has_name(sym::derive) { diff --git a/src/libsyntax_ext/util.rs b/src/libsyntax_ext/util.rs index d84fe19b3eab2..c8f0eb4ad25fc 100644 --- a/src/libsyntax_ext/util.rs +++ b/src/libsyntax_ext/util.rs @@ -1,11 +1,12 @@ use syntax_pos::Symbol; use syntax::ast::MetaItem; -use syntax::attr::{check_builtin_attribute, AttributeTemplate}; +use syntax::attr::AttributeTemplate; +use syntax::parse::validate_attr; use syntax_expand::base::ExtCtxt; pub fn check_builtin_macro_attribute(ecx: &ExtCtxt<'_>, meta_item: &MetaItem, name: Symbol) { // All the built-in macro attributes are "words" at the moment. let template = AttributeTemplate::only_word(); let attr = ecx.attribute(meta_item.clone()); - check_builtin_attribute(ecx.parse_sess, &attr, name, template); + validate_attr::check_builtin_attribute(ecx.parse_sess, &attr, name, template); } diff --git a/src/test/ui/proc-macro/attribute.stderr b/src/test/ui/proc-macro/attribute.stderr index 1503f62cb6c1a..021e7cad09b69 100644 --- a/src/test/ui/proc-macro/attribute.stderr +++ b/src/test/ui/proc-macro/attribute.stderr @@ -1,3 +1,15 @@ +error: malformed `proc_macro_derive` attribute input + --> $DIR/attribute.rs:9:1 + | +LL | #[proc_macro_derive] + | ^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[proc_macro_derive(TraitName, /*opt*/ attributes(name1, name2, ...))]` + +error: malformed `proc_macro_derive` attribute input + --> $DIR/attribute.rs:12:1 + | +LL | #[proc_macro_derive = ""] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[proc_macro_derive(TraitName, /*opt*/ attributes(name1, name2, ...))]` + error: attribute must have either one or two arguments --> $DIR/attribute.rs:15:1 | @@ -88,17 +100,5 @@ error: `self` cannot be a name of derive helper attribute LL | #[proc_macro_derive(d17, attributes(self))] | ^^^^ -error: malformed `proc_macro_derive` attribute input - --> $DIR/attribute.rs:9:1 - | -LL | #[proc_macro_derive] - | ^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[proc_macro_derive(TraitName, /*opt*/ attributes(name1, name2, ...))]` - -error: malformed `proc_macro_derive` attribute input - --> $DIR/attribute.rs:12:1 - | -LL | #[proc_macro_derive = ""] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[proc_macro_derive(TraitName, /*opt*/ attributes(name1, name2, ...))]` - error: aborting due to 17 previous errors