Skip to content

Commit

Permalink
Auto merge of #67151 - petrochenkov:docomm, r=estebank
Browse files Browse the repository at this point in the history 
doc comments: Less attribute mimicking

Make sure doc comments are not converted into intermediate meta-items, or not mixed with `doc(inline)` or something like that.

Follow-up to #65750.
  • Loading branch information
@bors
bors committed Dec 28, 2019
2 parents e39ae6f + 3d57b8b commit 3a3f4a7
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 103 deletions.
6 changes: 5 additions & 1 deletion src/librustc_lint/builtin.rs
View file
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_lint/unused.rs
View file
Expand Up @@ -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 {
Expand Down
43 changes: 20 additions & 23 deletions src/librustc_parse/validate_attr.rs
View file
Expand Up @@ -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);

Expand All @@ -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)
}
})
}

Expand Down
18 changes: 9 additions & 9 deletions src/librustc_save_analysis/lib.rs
View file
Expand Up @@ -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))
Expand Down
67 changes: 21 additions & 46 deletions src/librustdoc/clean/types.rs
View file
Expand Up @@ -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};
Expand Down Expand Up @@ -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<T>(
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)
{
Expand All @@ -566,7 +541,7 @@ impl Attributes {
}
}
Some(attr.clone())
})
}
})
.collect();

Expand All @@ -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);

Expand Down
4 changes: 0 additions & 4 deletions src/libsyntax/ast.rs
View file
Expand Up @@ -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),
}

Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/attr/builtin.rs
View file
Expand Up @@ -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 {
Expand Down
24 changes: 16 additions & 8 deletions src/libsyntax/attr/mod.rs
View file
Expand Up @@ -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,
}
}

Expand All @@ -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 {
Expand All @@ -173,7 +173,7 @@ impl Attribute {
pub fn value_str(&self) -> Option<Symbol> {
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,
}
}

Expand Down Expand Up @@ -279,27 +279,35 @@ impl Attribute {
}
}

pub fn doc_str(&self) -> Option<Symbol> {
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"),
}
}

/// Extracts the MetaItem from inside this Attribute.
pub fn meta(&self) -> Option<MetaItem> {
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,
}
}
}
Expand Down
16 changes: 5 additions & 11 deletions src/libsyntax_expand/parse/tests.rs
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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::<Vec<_>>();
let b: &[_] = &["/// doc comment".to_string(), "/// line 2".to_string()];
let docs = item.attrs.iter().filter_map(|at| at.doc_str()).collect::<Vec<_>>();
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 */");
});
}
Expand Down

0 comments on commit 3a3f4a7

Please sign in to comment.