From 3d57b8bcc0a6a0378a9cea0291bb76d44bec6ff8 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 7 Dec 2019 21:28:29 +0300 Subject: [PATCH] doc comments: Less attribute mimicking --- src/librustc_lint/builtin.rs | 6 ++- src/librustc_lint/unused.rs | 4 ++ src/librustc_parse/validate_attr.rs | 43 +++++++++--------- src/librustc_save_analysis/lib.rs | 18 ++++---- src/librustdoc/clean/types.rs | 67 +++++++++-------------------- src/libsyntax/ast.rs | 4 -- src/libsyntax/attr/builtin.rs | 2 +- src/libsyntax/attr/mod.rs | 24 +++++++---- src/libsyntax_expand/parse/tests.rs | 16 +++---- 9 files changed, 81 insertions(+), 103 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 2d9e960716f..fe53a2574bc 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -314,6 +314,10 @@ pub struct MissingDoc { impl_lint_pass!(MissingDoc => [MISSING_DOCS]); fn has_doc(attr: &ast::Attribute) -> bool { + if attr.is_doc_comment() { + return true; + } + if !attr.check_name(sym::doc) { return false; } @@ -768,7 +772,7 @@ impl UnusedDocComment { let span = sugared_span.take().unwrap_or_else(|| attr.span); - if attr.check_name(sym::doc) { + if attr.is_doc_comment() || attr.check_name(sym::doc) { let mut err = cx.struct_span_lint(UNUSED_DOC_COMMENTS, span, "unused doc comment"); err.span_label( diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 5edb81c1e51..1501e7084a4 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -271,6 +271,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAttributes { fn check_attribute(&mut self, cx: &LateContext<'_, '_>, attr: &ast::Attribute) { debug!("checking attribute: {:?}", attr); + if attr.is_doc_comment() { + return; + } + let attr_info = attr.ident().and_then(|ident| self.builtin_attributes.get(&ident.name)); if let Some(&&(name, ty, ..)) = attr_info { diff --git a/src/librustc_parse/validate_attr.rs b/src/librustc_parse/validate_attr.rs index ef9b8cba43b..9b96702fa1f 100644 --- a/src/librustc_parse/validate_attr.rs +++ b/src/librustc_parse/validate_attr.rs @@ -4,16 +4,17 @@ use crate::parse_in; use rustc_errors::{Applicability, PResult}; use rustc_feature::{AttributeTemplate, BUILTIN_ATTRIBUTE_MAP}; -use syntax::ast::{ - self, AttrKind, Attribute, Ident, MacArgs, MacDelimiter, MetaItem, MetaItemKind, -}; -use syntax::attr::mk_name_value_item_str; +use syntax::ast::{self, Attribute, MacArgs, MacDelimiter, MetaItem, MetaItemKind}; use syntax::early_buffered_lints::ILL_FORMED_ATTRIBUTE_INPUT; use syntax::sess::ParseSess; use syntax::tokenstream::DelimSpan; use syntax_pos::{sym, Symbol}; pub fn check_meta(sess: &ParseSess, attr: &Attribute) { + if attr.is_doc_comment() { + return; + } + let attr_info = attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name)).map(|a| **a); @@ -33,26 +34,22 @@ pub fn check_meta(sess: &ParseSess, attr: &Attribute) { } pub fn parse_meta<'a>(sess: &'a ParseSess, attr: &Attribute) -> PResult<'a, MetaItem> { - Ok(match attr.kind { - AttrKind::Normal(ref item) => MetaItem { - span: attr.span, - path: item.path.clone(), - kind: match &attr.get_normal_item().args { - MacArgs::Empty => MetaItemKind::Word, - MacArgs::Eq(_, t) => { - let v = parse_in(sess, t.clone(), "name value", |p| p.parse_unsuffixed_lit())?; - MetaItemKind::NameValue(v) - } - MacArgs::Delimited(dspan, delim, t) => { - check_meta_bad_delim(sess, *dspan, *delim, "wrong meta list delimiters"); - let nmis = parse_in(sess, t.clone(), "meta list", |p| p.parse_meta_seq_top())?; - MetaItemKind::List(nmis) - } - }, + let item = attr.get_normal_item(); + Ok(MetaItem { + span: attr.span, + path: item.path.clone(), + kind: match &item.args { + MacArgs::Empty => MetaItemKind::Word, + MacArgs::Eq(_, t) => { + let v = parse_in(sess, t.clone(), "name value", |p| p.parse_unsuffixed_lit())?; + MetaItemKind::NameValue(v) + } + MacArgs::Delimited(dspan, delim, t) => { + check_meta_bad_delim(sess, *dspan, *delim, "wrong meta list delimiters"); + let nmis = parse_in(sess, t.clone(), "meta list", |p| p.parse_meta_seq_top())?; + MetaItemKind::List(nmis) + } }, - AttrKind::DocComment(comment) => { - mk_name_value_item_str(Ident::new(sym::doc, attr.span), comment, attr.span) - } }) } diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index 27d4eef34cb..9f4fcb1f330 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -815,15 +815,15 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> { let mut result = String::new(); for attr in attrs { - if attr.check_name(sym::doc) { - if let Some(val) = attr.value_str() { - if attr.is_doc_comment() { - result.push_str(&strip_doc_comment_decoration(&val.as_str())); - } else { - result.push_str(&val.as_str()); - } - result.push('\n'); - } else if let Some(meta_list) = attr.meta_item_list() { + if let Some(val) = attr.doc_str() { + if attr.is_doc_comment() { + result.push_str(&strip_doc_comment_decoration(&val.as_str())); + } else { + result.push_str(&val.as_str()); + } + result.push('\n'); + } else if attr.check_name(sym::doc) { + if let Some(meta_list) = attr.meta_item_list() { meta_list .into_iter() .filter(|it| it.check_name(sym::include)) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index e7b9964d465..a8781604622 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -17,10 +17,10 @@ use rustc::ty::layout::VariantIdx; use rustc::util::nodemap::{FxHashMap, FxHashSet}; use rustc_index::vec::IndexVec; use rustc_target::spec::abi::Abi; -use syntax::ast::{self, AttrKind, AttrStyle, Attribute, Ident}; +use syntax::ast::{self, AttrStyle, Ident}; use syntax::attr; use syntax::source_map::DUMMY_SP; -use syntax::util::comments; +use syntax::util::comments::strip_doc_comment_decoration; use syntax_pos::hygiene::MacroKind; use syntax_pos::symbol::{sym, Symbol}; use syntax_pos::{self, FileName}; @@ -502,58 +502,33 @@ impl Attributes { let mut cfg = Cfg::True; let mut doc_line = 0; - /// If `attr` is a doc comment, strips the leading and (if present) - /// trailing comments symbols, e.g. `///`, `/**`, and `*/`. Otherwise, - /// returns `attr` unchanged. - pub fn with_doc_comment_markers_stripped( - attr: &Attribute, - f: impl FnOnce(&Attribute) -> T, - ) -> T { - match attr.kind { - AttrKind::Normal(_) => f(attr), - AttrKind::DocComment(comment) => { - let comment = - Symbol::intern(&comments::strip_doc_comment_decoration(&comment.as_str())); - f(&Attribute { - kind: AttrKind::DocComment(comment), - id: attr.id, - style: attr.style, - span: attr.span, - }) - } - } - } - let other_attrs = attrs .iter() .filter_map(|attr| { - with_doc_comment_markers_stripped(attr, |attr| { + if let Some(value) = attr.doc_str() { + let (value, mk_fragment): (_, fn(_, _, _) -> _) = if attr.is_doc_comment() { + (strip_doc_comment_decoration(&value.as_str()), DocFragment::SugaredDoc) + } else { + (value.to_string(), DocFragment::RawDoc) + }; + + let line = doc_line; + doc_line += value.lines().count(); + doc_strings.push(mk_fragment(line, attr.span, value)); + + if sp.is_none() { + sp = Some(attr.span); + } + None + } else { if attr.check_name(sym::doc) { if let Some(mi) = attr.meta() { - if let Some(value) = mi.value_str() { - // Extracted #[doc = "..."] - let value = value.to_string(); - let line = doc_line; - doc_line += value.lines().count(); - - if attr.is_doc_comment() { - doc_strings - .push(DocFragment::SugaredDoc(line, attr.span, value)); - } else { - doc_strings.push(DocFragment::RawDoc(line, attr.span, value)); - } - - if sp.is_none() { - sp = Some(attr.span); - } - return None; - } else if let Some(cfg_mi) = Attributes::extract_cfg(&mi) { + if let Some(cfg_mi) = Attributes::extract_cfg(&mi) { // Extracted #[doc(cfg(...))] match Cfg::parse(cfg_mi) { Ok(new_cfg) => cfg &= new_cfg, Err(e) => diagnostic.span_err(e.span, e.msg), } - return None; } else if let Some((filename, contents)) = Attributes::extract_include(&mi) { @@ -566,7 +541,7 @@ impl Attributes { } } Some(attr.clone()) - }) + } }) .collect(); @@ -589,7 +564,7 @@ impl Attributes { let inner_docs = attrs .iter() - .filter(|a| a.check_name(sym::doc)) + .filter(|a| a.doc_str().is_some()) .next() .map_or(true, |a| a.style == AttrStyle::Inner); diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index c98942abaf3..0a2004a8229 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -2364,10 +2364,6 @@ pub enum AttrKind { /// A doc comment (e.g. `/// ...`, `//! ...`, `/** ... */`, `/*! ... */`). /// Doc attributes (e.g. `#[doc="..."]`) are represented with the `Normal` /// variant (which is much less compact and thus more expensive). - /// - /// Note: `self.has_name(sym::doc)` and `self.check_name(sym::doc)` succeed - /// for this variant, but this may change in the future. - /// ``` DocComment(Symbol), } diff --git a/src/libsyntax/attr/builtin.rs b/src/libsyntax/attr/builtin.rs index 65b67981474..bf64333830e 100644 --- a/src/libsyntax/attr/builtin.rs +++ b/src/libsyntax/attr/builtin.rs @@ -16,7 +16,7 @@ use syntax_pos::{symbol::sym, symbol::Symbol, Span}; use rustc_error_codes::*; pub fn is_builtin_attr(attr: &Attribute) -> bool { - attr.ident().filter(|ident| is_builtin_attr_name(ident.name)).is_some() + attr.is_doc_comment() || attr.ident().filter(|ident| is_builtin_attr_name(ident.name)).is_some() } enum AttrError { diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index 82685e98386..0cd300384f8 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -139,7 +139,7 @@ impl Attribute { pub fn has_name(&self, name: Symbol) -> bool { match self.kind { AttrKind::Normal(ref item) => item.path == name, - AttrKind::DocComment(_) => name == sym::doc, + AttrKind::DocComment(_) => false, } } @@ -163,7 +163,7 @@ impl Attribute { None } } - AttrKind::DocComment(_) => Some(Ident::new(sym::doc, self.span)), + AttrKind::DocComment(_) => None, } } pub fn name_or_empty(&self) -> Symbol { @@ -173,7 +173,7 @@ impl Attribute { pub fn value_str(&self) -> Option { match self.kind { AttrKind::Normal(ref item) => item.meta(self.span).and_then(|meta| meta.value_str()), - AttrKind::DocComment(comment) => Some(comment), + AttrKind::DocComment(..) => None, } } @@ -279,17 +279,27 @@ impl Attribute { } } + pub fn doc_str(&self) -> Option { + match self.kind { + AttrKind::DocComment(symbol) => Some(symbol), + AttrKind::Normal(ref item) if item.path == sym::doc => { + item.meta(self.span).and_then(|meta| meta.value_str()) + } + _ => None, + } + } + pub fn get_normal_item(&self) -> &AttrItem { match self.kind { AttrKind::Normal(ref item) => item, - AttrKind::DocComment(_) => panic!("unexpected sugared doc"), + AttrKind::DocComment(_) => panic!("unexpected doc comment"), } } pub fn unwrap_normal_item(self) -> AttrItem { match self.kind { AttrKind::Normal(item) => item, - AttrKind::DocComment(_) => panic!("unexpected sugared doc"), + AttrKind::DocComment(_) => panic!("unexpected doc comment"), } } @@ -297,9 +307,7 @@ impl Attribute { pub fn meta(&self) -> Option { match self.kind { AttrKind::Normal(ref item) => item.meta(self.span), - AttrKind::DocComment(comment) => { - Some(mk_name_value_item_str(Ident::new(sym::doc, self.span), comment, self.span)) - } + AttrKind::DocComment(..) => None, } } } diff --git a/src/libsyntax_expand/parse/tests.rs b/src/libsyntax_expand/parse/tests.rs index 154ccb25621..833fda6a2eb 100644 --- a/src/libsyntax_expand/parse/tests.rs +++ b/src/libsyntax_expand/parse/tests.rs @@ -3,12 +3,11 @@ use crate::tests::{matches_codepattern, string_to_stream, with_error_checking_pa use errors::PResult; use rustc_parse::new_parser_from_source_str; use syntax::ast::{self, Name, PatKind}; -use syntax::attr::first_attr_value_str_by_name; use syntax::print::pprust::item_to_string; use syntax::ptr::P; use syntax::sess::ParseSess; use syntax::source_map::FilePathMapping; -use syntax::symbol::{kw, sym}; +use syntax::symbol::{kw, sym, Symbol}; use syntax::token::{self, Token}; use syntax::tokenstream::{DelimSpan, TokenStream, TokenTree}; use syntax::visit; @@ -244,25 +243,20 @@ fn crlf_doc_comments() { let name_1 = FileName::Custom("crlf_source_1".to_string()); let source = "/// doc comment\r\nfn foo() {}".to_string(); let item = parse_item_from_source_str(name_1, source, &sess).unwrap().unwrap(); - let doc = first_attr_value_str_by_name(&item.attrs, sym::doc).unwrap(); + let doc = item.attrs.iter().filter_map(|at| at.doc_str()).next().unwrap(); assert_eq!(doc.as_str(), "/// doc comment"); let name_2 = FileName::Custom("crlf_source_2".to_string()); let source = "/// doc comment\r\n/// line 2\r\nfn foo() {}".to_string(); let item = parse_item_from_source_str(name_2, source, &sess).unwrap().unwrap(); - let docs = item - .attrs - .iter() - .filter(|a| a.has_name(sym::doc)) - .map(|a| a.value_str().unwrap().to_string()) - .collect::>(); - let b: &[_] = &["/// doc comment".to_string(), "/// line 2".to_string()]; + let docs = item.attrs.iter().filter_map(|at| at.doc_str()).collect::>(); + let b: &[_] = &[Symbol::intern("/// doc comment"), Symbol::intern("/// line 2")]; assert_eq!(&docs[..], b); let name_3 = FileName::Custom("clrf_source_3".to_string()); let source = "/** doc comment\r\n * with CRLF */\r\nfn foo() {}".to_string(); let item = parse_item_from_source_str(name_3, source, &sess).unwrap().unwrap(); - let doc = first_attr_value_str_by_name(&item.attrs, sym::doc).unwrap(); + let doc = item.attrs.iter().filter_map(|at| at.doc_str()).next().unwrap(); assert_eq!(doc.as_str(), "/** doc comment\n * with CRLF */"); }); }