Skip to content

Commit 3a3f4a7

Browse files
borsbors
committed
Auto merge of rust-lang#67151 - petrochenkov:docomm, r=estebank
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 rust-lang#65750.
2 parents e39ae6f + 3d57b8b commit 3a3f4a7

File tree

9 files changed

+81
-103
lines changed

9 files changed

+81
-103
lines changed

‎src/librustc_lint/builtin.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,10 @@ pub struct MissingDoc {
314314
impl_lint_pass!(MissingDoc => [MISSING_DOCS]);
315315

316316
fn has_doc(attr: &ast::Attribute) -> bool {
317+
if attr.is_doc_comment() {
318+
return true;
319+
}
320+
317321
if !attr.check_name(sym::doc) {
318322
return false;
319323
}
@@ -768,7 +772,7 @@ impl UnusedDocComment {
768772

769773
let span = sugared_span.take().unwrap_or_else(|| attr.span);
770774

771-
if attr.check_name(sym::doc) {
775+
if attr.is_doc_comment() || attr.check_name(sym::doc) {
772776
let mut err = cx.struct_span_lint(UNUSED_DOC_COMMENTS, span, "unused doc comment");
773777

774778
err.span_label(

‎src/librustc_lint/unused.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAttributes {
271271
fn check_attribute(&mut self, cx: &LateContext<'_, '_>, attr: &ast::Attribute) {
272272
debug!("checking attribute: {:?}", attr);
273273

274+
if attr.is_doc_comment() {
275+
return;
276+
}
277+
274278
let attr_info = attr.ident().and_then(|ident| self.builtin_attributes.get(&ident.name));
275279

276280
if let Some(&&(name, ty, ..)) = attr_info {

‎src/librustc_parse/validate_attr.rs

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,17 @@ use crate::parse_in;
44

55
use rustc_errors::{Applicability, PResult};
66
use rustc_feature::{AttributeTemplate, BUILTIN_ATTRIBUTE_MAP};
7-
use syntax::ast::{
8-
self, AttrKind, Attribute, Ident, MacArgs, MacDelimiter, MetaItem, MetaItemKind,
9-
};
10-
use syntax::attr::mk_name_value_item_str;
7+
use syntax::ast::{self, Attribute, MacArgs, MacDelimiter, MetaItem, MetaItemKind};
118
use syntax::early_buffered_lints::ILL_FORMED_ATTRIBUTE_INPUT;
129
use syntax::sess::ParseSess;
1310
use syntax::tokenstream::DelimSpan;
1411
use syntax_pos::{sym, Symbol};
1512

1613
pub fn check_meta(sess: &ParseSess, attr: &Attribute) {
14+
if attr.is_doc_comment() {
15+
return;
16+
}
17+
1718
let attr_info =
1819
attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name)).map(|a| **a);
1920

@@ -33,26 +34,22 @@ pub fn check_meta(sess: &ParseSess, attr: &Attribute) {
3334
}
3435

3536
pub fn parse_meta<'a>(sess: &'a ParseSess, attr: &Attribute) -> PResult<'a, MetaItem> {
36-
Ok(match attr.kind {
37-
AttrKind::Normal(ref item) => MetaItem {
38-
span: attr.span,
39-
path: item.path.clone(),
40-
kind: match &attr.get_normal_item().args {
41-
MacArgs::Empty => MetaItemKind::Word,
42-
MacArgs::Eq(_, t) => {
43-
let v = parse_in(sess, t.clone(), "name value", |p| p.parse_unsuffixed_lit())?;
44-
MetaItemKind::NameValue(v)
45-
}
46-
MacArgs::Delimited(dspan, delim, t) => {
47-
check_meta_bad_delim(sess, *dspan, *delim, "wrong meta list delimiters");
48-
let nmis = parse_in(sess, t.clone(), "meta list", |p| p.parse_meta_seq_top())?;
49-
MetaItemKind::List(nmis)
50-
}
51-
},
37+
let item = attr.get_normal_item();
38+
Ok(MetaItem {
39+
span: attr.span,
40+
path: item.path.clone(),
41+
kind: match &item.args {
42+
MacArgs::Empty => MetaItemKind::Word,
43+
MacArgs::Eq(_, t) => {
44+
let v = parse_in(sess, t.clone(), "name value", |p| p.parse_unsuffixed_lit())?;
45+
MetaItemKind::NameValue(v)
46+
}
47+
MacArgs::Delimited(dspan, delim, t) => {
48+
check_meta_bad_delim(sess, *dspan, *delim, "wrong meta list delimiters");
49+
let nmis = parse_in(sess, t.clone(), "meta list", |p| p.parse_meta_seq_top())?;
50+
MetaItemKind::List(nmis)
51+
}
5252
},
53-
AttrKind::DocComment(comment) => {
54-
mk_name_value_item_str(Ident::new(sym::doc, attr.span), comment, attr.span)
55-
}
5653
})
5754
}
5855

‎src/librustc_save_analysis/lib.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -815,15 +815,15 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> {
815815
let mut result = String::new();
816816

817817
for attr in attrs {
818-
if attr.check_name(sym::doc) {
819-
if let Some(val) = attr.value_str() {
820-
if attr.is_doc_comment() {
821-
result.push_str(&strip_doc_comment_decoration(&val.as_str()));
822-
} else {
823-
result.push_str(&val.as_str());
824-
}
825-
result.push('\n');
826-
} else if let Some(meta_list) = attr.meta_item_list() {
818+
if let Some(val) = attr.doc_str() {
819+
if attr.is_doc_comment() {
820+
result.push_str(&strip_doc_comment_decoration(&val.as_str()));
821+
} else {
822+
result.push_str(&val.as_str());
823+
}
824+
result.push('\n');
825+
} else if attr.check_name(sym::doc) {
826+
if let Some(meta_list) = attr.meta_item_list() {
827827
meta_list
828828
.into_iter()
829829
.filter(|it| it.check_name(sym::include))

‎src/librustdoc/clean/types.rs

Lines changed: 21 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ use rustc::ty::layout::VariantIdx;
1717
use rustc::util::nodemap::{FxHashMap, FxHashSet};
1818
use rustc_index::vec::IndexVec;
1919
use rustc_target::spec::abi::Abi;
20-
use syntax::ast::{self, AttrKind, AttrStyle, Attribute, Ident};
20+
use syntax::ast::{self, AttrStyle, Ident};
2121
use syntax::attr;
2222
use syntax::source_map::DUMMY_SP;
23-
use syntax::util::comments;
23+
use syntax::util::comments::strip_doc_comment_decoration;
2424
use syntax_pos::hygiene::MacroKind;
2525
use syntax_pos::symbol::{sym, Symbol};
2626
use syntax_pos::{self, FileName};
@@ -502,58 +502,33 @@ impl Attributes {
502502
let mut cfg = Cfg::True;
503503
let mut doc_line = 0;
504504

505-
/// If `attr` is a doc comment, strips the leading and (if present)
506-
/// trailing comments symbols, e.g. `///`, `/**`, and `*/`. Otherwise,
507-
/// returns `attr` unchanged.
508-
pub fn with_doc_comment_markers_stripped<T>(
509-
attr: &Attribute,
510-
f: impl FnOnce(&Attribute) -> T,
511-
) -> T {
512-
match attr.kind {
513-
AttrKind::Normal(_) => f(attr),
514-
AttrKind::DocComment(comment) => {
515-
let comment =
516-
Symbol::intern(&comments::strip_doc_comment_decoration(&comment.as_str()));
517-
f(&Attribute {
518-
kind: AttrKind::DocComment(comment),
519-
id: attr.id,
520-
style: attr.style,
521-
span: attr.span,
522-
})
523-
}
524-
}
525-
}
526-
527505
let other_attrs = attrs
528506
.iter()
529507
.filter_map(|attr| {
530-
with_doc_comment_markers_stripped(attr, |attr| {
508+
if let Some(value) = attr.doc_str() {
509+
let (value, mk_fragment): (_, fn(_, _, _) -> _) = if attr.is_doc_comment() {
510+
(strip_doc_comment_decoration(&value.as_str()), DocFragment::SugaredDoc)
511+
} else {
512+
(value.to_string(), DocFragment::RawDoc)
513+
};
514+
515+
let line = doc_line;
516+
doc_line += value.lines().count();
517+
doc_strings.push(mk_fragment(line, attr.span, value));
518+
519+
if sp.is_none() {
520+
sp = Some(attr.span);
521+
}
522+
None
523+
} else {
531524
if attr.check_name(sym::doc) {
532525
if let Some(mi) = attr.meta() {
533-
if let Some(value) = mi.value_str() {
534-
// Extracted #[doc = "..."]
535-
let value = value.to_string();
536-
let line = doc_line;
537-
doc_line += value.lines().count();
538-
539-
if attr.is_doc_comment() {
540-
doc_strings
541-
.push(DocFragment::SugaredDoc(line, attr.span, value));
542-
} else {
543-
doc_strings.push(DocFragment::RawDoc(line, attr.span, value));
544-
}
545-
546-
if sp.is_none() {
547-
sp = Some(attr.span);
548-
}
549-
return None;
550-
} else if let Some(cfg_mi) = Attributes::extract_cfg(&mi) {
526+
if let Some(cfg_mi) = Attributes::extract_cfg(&mi) {
551527
// Extracted #[doc(cfg(...))]
552528
match Cfg::parse(cfg_mi) {
553529
Ok(new_cfg) => cfg &= new_cfg,
554530
Err(e) => diagnostic.span_err(e.span, e.msg),
555531
}
556-
return None;
557532
} else if let Some((filename, contents)) =
558533
Attributes::extract_include(&mi)
559534
{
@@ -566,7 +541,7 @@ impl Attributes {
566541
}
567542
}
568543
Some(attr.clone())
569-
})
544+
}
570545
})
571546
.collect();
572547

@@ -589,7 +564,7 @@ impl Attributes {
589564

590565
let inner_docs = attrs
591566
.iter()
592-
.filter(|a| a.check_name(sym::doc))
567+
.filter(|a| a.doc_str().is_some())
593568
.next()
594569
.map_or(true, |a| a.style == AttrStyle::Inner);
595570

‎src/libsyntax/ast.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2364,10 +2364,6 @@ pub enum AttrKind {
23642364
/// A doc comment (e.g. `/// ...`, `//! ...`, `/** ... */`, `/*! ... */`).
23652365
/// Doc attributes (e.g. `#[doc="..."]`) are represented with the `Normal`
23662366
/// variant (which is much less compact and thus more expensive).
2367-
///
2368-
/// Note: `self.has_name(sym::doc)` and `self.check_name(sym::doc)` succeed
2369-
/// for this variant, but this may change in the future.
2370-
/// ```
23712367
DocComment(Symbol),
23722368
}
23732369

‎src/libsyntax/attr/builtin.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use syntax_pos::{symbol::sym, symbol::Symbol, Span};
1616
use rustc_error_codes::*;
1717

1818
pub fn is_builtin_attr(attr: &Attribute) -> bool {
19-
attr.ident().filter(|ident| is_builtin_attr_name(ident.name)).is_some()
19+
attr.is_doc_comment() || attr.ident().filter(|ident| is_builtin_attr_name(ident.name)).is_some()
2020
}
2121

2222
enum AttrError {

‎src/libsyntax/attr/mod.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ impl Attribute {
139139
pub fn has_name(&self, name: Symbol) -> bool {
140140
match self.kind {
141141
AttrKind::Normal(ref item) => item.path == name,
142-
AttrKind::DocComment(_) => name == sym::doc,
142+
AttrKind::DocComment(_) => false,
143143
}
144144
}
145145

@@ -163,7 +163,7 @@ impl Attribute {
163163
None
164164
}
165165
}
166-
AttrKind::DocComment(_) => Some(Ident::new(sym::doc, self.span)),
166+
AttrKind::DocComment(_) => None,
167167
}
168168
}
169169
pub fn name_or_empty(&self) -> Symbol {
@@ -173,7 +173,7 @@ impl Attribute {
173173
pub fn value_str(&self) -> Option<Symbol> {
174174
match self.kind {
175175
AttrKind::Normal(ref item) => item.meta(self.span).and_then(|meta| meta.value_str()),
176-
AttrKind::DocComment(comment) => Some(comment),
176+
AttrKind::DocComment(..) => None,
177177
}
178178
}
179179

@@ -279,27 +279,35 @@ impl Attribute {
279279
}
280280
}
281281

282+
pub fn doc_str(&self) -> Option<Symbol> {
283+
match self.kind {
284+
AttrKind::DocComment(symbol) => Some(symbol),
285+
AttrKind::Normal(ref item) if item.path == sym::doc => {
286+
item.meta(self.span).and_then(|meta| meta.value_str())
287+
}
288+
_ => None,
289+
}
290+
}
291+
282292
pub fn get_normal_item(&self) -> &AttrItem {
283293
match self.kind {
284294
AttrKind::Normal(ref item) => item,
285-
AttrKind::DocComment(_) => panic!("unexpected sugared doc"),
295+
AttrKind::DocComment(_) => panic!("unexpected doc comment"),
286296
}
287297
}
288298

289299
pub fn unwrap_normal_item(self) -> AttrItem {
290300
match self.kind {
291301
AttrKind::Normal(item) => item,
292-
AttrKind::DocComment(_) => panic!("unexpected sugared doc"),
302+
AttrKind::DocComment(_) => panic!("unexpected doc comment"),
293303
}
294304
}
295305

296306
/// Extracts the MetaItem from inside this Attribute.
297307
pub fn meta(&self) -> Option<MetaItem> {
298308
match self.kind {
299309
AttrKind::Normal(ref item) => item.meta(self.span),
300-
AttrKind::DocComment(comment) => {
301-
Some(mk_name_value_item_str(Ident::new(sym::doc, self.span), comment, self.span))
302-
}
310+
AttrKind::DocComment(..) => None,
303311
}
304312
}
305313
}

‎src/libsyntax_expand/parse/tests.rs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@ use crate::tests::{matches_codepattern, string_to_stream, with_error_checking_pa
33
use errors::PResult;
44
use rustc_parse::new_parser_from_source_str;
55
use syntax::ast::{self, Name, PatKind};
6-
use syntax::attr::first_attr_value_str_by_name;
76
use syntax::print::pprust::item_to_string;
87
use syntax::ptr::P;
98
use syntax::sess::ParseSess;
109
use syntax::source_map::FilePathMapping;
11-
use syntax::symbol::{kw, sym};
10+
use syntax::symbol::{kw, sym, Symbol};
1211
use syntax::token::{self, Token};
1312
use syntax::tokenstream::{DelimSpan, TokenStream, TokenTree};
1413
use syntax::visit;
@@ -244,25 +243,20 @@ fn crlf_doc_comments() {
244243
let name_1 = FileName::Custom("crlf_source_1".to_string());
245244
let source = "/// doc comment\r\nfn foo() {}".to_string();
246245
let item = parse_item_from_source_str(name_1, source, &sess).unwrap().unwrap();
247-
let doc = first_attr_value_str_by_name(&item.attrs, sym::doc).unwrap();
246+
let doc = item.attrs.iter().filter_map(|at| at.doc_str()).next().unwrap();
248247
assert_eq!(doc.as_str(), "/// doc comment");
249248

250249
let name_2 = FileName::Custom("crlf_source_2".to_string());
251250
let source = "/// doc comment\r\n/// line 2\r\nfn foo() {}".to_string();
252251
let item = parse_item_from_source_str(name_2, source, &sess).unwrap().unwrap();
253-
let docs = item
254-
.attrs
255-
.iter()
256-
.filter(|a| a.has_name(sym::doc))
257-
.map(|a| a.value_str().unwrap().to_string())
258-
.collect::<Vec<_>>();
259-
let b: &[_] = &["/// doc comment".to_string(), "/// line 2".to_string()];
252+
let docs = item.attrs.iter().filter_map(|at| at.doc_str()).collect::<Vec<_>>();
253+
let b: &[_] = &[Symbol::intern("/// doc comment"), Symbol::intern("/// line 2")];
260254
assert_eq!(&docs[..], b);
261255

262256
let name_3 = FileName::Custom("clrf_source_3".to_string());
263257
let source = "/** doc comment\r\n * with CRLF */\r\nfn foo() {}".to_string();
264258
let item = parse_item_from_source_str(name_3, source, &sess).unwrap().unwrap();
265-
let doc = first_attr_value_str_by_name(&item.attrs, sym::doc).unwrap();
259+
let doc = item.attrs.iter().filter_map(|at| at.doc_str()).next().unwrap();
266260
assert_eq!(doc.as_str(), "/** doc comment\n * with CRLF */");
267261
});
268262
}

0 commit comments

Comments
 (0)