Skip to content

Commit 018c3f9

Browse files
authored
Unrolled build for #141648
Rollup merge of #141648 - GuillaumeGomez:redundant_explicit_links-expansion, r=lolbinarycat [rustdoc] Do not emit redundant_explicit_links lint if the doc comment comes from expansion Fixes #141553. The problem was that we change the context for the attributes in some cases to get better error output, preventing us to detect if the attribute comes from expansion. Most of the changes are about keeping track of the "does this span comes from expansion" information. r? ```@Manishearth```
2 parents b03b3a7 + 3b5525b commit 018c3f9

File tree

12 files changed

+212
-65
lines changed

12 files changed

+212
-65
lines changed

compiler/rustc_resolve/src/rustdoc.rs

Lines changed: 62 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ pub struct DocFragment {
4949
pub doc: Symbol,
5050
pub kind: DocFragmentKind,
5151
pub indent: usize,
52+
/// Because we tamper with the spans context, this information cannot be correctly retrieved
53+
/// later on. So instead, we compute it and store it here.
54+
pub from_expansion: bool,
5255
}
5356

5457
#[derive(Clone, Copy, Debug)]
@@ -208,17 +211,18 @@ pub fn attrs_to_doc_fragments<'a, A: AttributeExt + Clone + 'a>(
208211
for (attr, item_id) in attrs {
209212
if let Some((doc_str, comment_kind)) = attr.doc_str_and_comment_kind() {
210213
let doc = beautify_doc_string(doc_str, comment_kind);
211-
let (span, kind) = if attr.is_doc_comment() {
212-
(attr.span(), DocFragmentKind::SugaredDoc)
214+
let (span, kind, from_expansion) = if attr.is_doc_comment() {
215+
let span = attr.span();
216+
(span, DocFragmentKind::SugaredDoc, span.from_expansion())
213217
} else {
214-
(
215-
attr.value_span()
216-
.map(|i| i.with_ctxt(attr.span().ctxt()))
217-
.unwrap_or(attr.span()),
218-
DocFragmentKind::RawDoc,
219-
)
218+
let attr_span = attr.span();
219+
let (span, from_expansion) = match attr.value_span() {
220+
Some(sp) => (sp.with_ctxt(attr_span.ctxt()), sp.from_expansion()),
221+
None => (attr_span, attr_span.from_expansion()),
222+
};
223+
(span, DocFragmentKind::RawDoc, from_expansion)
220224
};
221-
let fragment = DocFragment { span, doc, kind, item_id, indent: 0 };
225+
let fragment = DocFragment { span, doc, kind, item_id, indent: 0, from_expansion };
222226
doc_fragments.push(fragment);
223227
} else if !doc_only {
224228
other_attrs.push(attr.clone());
@@ -505,17 +509,26 @@ fn collect_link_data<'input, F: BrokenLinkCallback<'input>>(
505509
display_text.map(String::into_boxed_str)
506510
}
507511

508-
/// Returns a span encompassing all the document fragments.
509-
pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> {
510-
if fragments.is_empty() {
511-
return None;
512-
}
513-
let start = fragments[0].span;
514-
if start == DUMMY_SP {
512+
/// Returns a tuple containing a span encompassing all the document fragments and a boolean that is
513+
/// `true` if any of the fragments are from a macro expansion.
514+
pub fn span_of_fragments_with_expansion(fragments: &[DocFragment]) -> Option<(Span, bool)> {
515+
let (first_fragment, last_fragment) = match fragments {
516+
[] => return None,
517+
[first, .., last] => (first, last),
518+
[first] => (first, first),
519+
};
520+
if first_fragment.span == DUMMY_SP {
515521
return None;
516522
}
517-
let end = fragments.last().expect("no doc strings provided").span;
518-
Some(start.to(end))
523+
Some((
524+
first_fragment.span.to(last_fragment.span),
525+
fragments.iter().any(|frag| frag.from_expansion),
526+
))
527+
}
528+
529+
/// Returns a span encompassing all the document fragments.
530+
pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> {
531+
span_of_fragments_with_expansion(fragments).map(|(sp, _)| sp)
519532
}
520533

521534
/// Attempts to match a range of bytes from parsed markdown to a `Span` in the source code.
@@ -529,18 +542,22 @@ pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> {
529542
/// This method will return `Some` only if one of the following is true:
530543
///
531544
/// - The doc is made entirely from sugared doc comments, which cannot contain escapes
532-
/// - The doc is entirely from a single doc fragment with a string literal exactly equal to `markdown`.
545+
/// - The doc is entirely from a single doc fragment with a string literal exactly equal to
546+
/// `markdown`.
533547
/// - The doc comes from `include_str!`
534-
/// - The doc includes exactly one substring matching `markdown[md_range]` which is contained in a single doc fragment.
548+
/// - The doc includes exactly one substring matching `markdown[md_range]` which is contained in a
549+
/// single doc fragment.
550+
///
551+
/// This function is defined in the compiler so it can be used by both `rustdoc` and `clippy`.
535552
///
536-
/// This function is defined in the compiler so it can be used by
537-
/// both `rustdoc` and `clippy`.
553+
/// It returns a tuple containing a span encompassing all the document fragments and a boolean that
554+
/// is `true` if any of the *matched* fragments are from a macro expansion.
538555
pub fn source_span_for_markdown_range(
539556
tcx: TyCtxt<'_>,
540557
markdown: &str,
541558
md_range: &Range<usize>,
542559
fragments: &[DocFragment],
543-
) -> Option<Span> {
560+
) -> Option<(Span, bool)> {
544561
let map = tcx.sess.source_map();
545562
source_span_for_markdown_range_inner(map, markdown, md_range, fragments)
546563
}
@@ -551,7 +568,7 @@ pub fn source_span_for_markdown_range_inner(
551568
markdown: &str,
552569
md_range: &Range<usize>,
553570
fragments: &[DocFragment],
554-
) -> Option<Span> {
571+
) -> Option<(Span, bool)> {
555572
use rustc_span::BytePos;
556573

557574
if let &[fragment] = &fragments
@@ -562,11 +579,14 @@ pub fn source_span_for_markdown_range_inner(
562579
&& let Ok(md_range_hi) = u32::try_from(md_range.end)
563580
{
564581
// Single fragment with string that contains same bytes as doc.
565-
return Some(Span::new(
566-
fragment.span.lo() + rustc_span::BytePos(md_range_lo),
567-
fragment.span.lo() + rustc_span::BytePos(md_range_hi),
568-
fragment.span.ctxt(),
569-
fragment.span.parent(),
582+
return Some((
583+
Span::new(
584+
fragment.span.lo() + rustc_span::BytePos(md_range_lo),
585+
fragment.span.lo() + rustc_span::BytePos(md_range_hi),
586+
fragment.span.ctxt(),
587+
fragment.span.parent(),
588+
),
589+
fragment.from_expansion,
570590
));
571591
}
572592

@@ -598,19 +618,21 @@ pub fn source_span_for_markdown_range_inner(
598618
{
599619
match_data = Some((i, match_start));
600620
} else {
601-
// Heirustic produced ambiguity, return nothing.
621+
// Heuristic produced ambiguity, return nothing.
602622
return None;
603623
}
604624
}
605625
}
606626
if let Some((i, match_start)) = match_data {
607-
let sp = fragments[i].span;
627+
let fragment = &fragments[i];
628+
let sp = fragment.span;
608629
// we need to calculate the span start,
609630
// then use that in our calulations for the span end
610631
let lo = sp.lo() + BytePos(match_start as u32);
611-
return Some(
632+
return Some((
612633
sp.with_lo(lo).with_hi(lo + BytePos((md_range.end - md_range.start) as u32)),
613-
);
634+
fragment.from_expansion,
635+
));
614636
}
615637
return None;
616638
}
@@ -664,8 +686,13 @@ pub fn source_span_for_markdown_range_inner(
664686
}
665687
}
666688

667-
Some(span_of_fragments(fragments)?.from_inner(InnerSpan::new(
689+
let (span, _) = span_of_fragments_with_expansion(fragments)?;
690+
let src_span = span.from_inner(InnerSpan::new(
668691
md_range.start + start_bytes,
669692
md_range.end + start_bytes + end_bytes,
670-
)))
693+
));
694+
Some((
695+
src_span,
696+
fragments.iter().any(|frag| frag.span.overlaps(src_span) && frag.from_expansion),
697+
))
671698
}

compiler/rustc_resolve/src/rustdoc/tests.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use super::{DocFragment, DocFragmentKind, source_span_for_markdown_range_inner};
1010
fn single_backtick() {
1111
let sm = SourceMap::new(FilePathMapping::empty());
1212
sm.new_source_file(PathBuf::from("foo.rs").into(), r#"#[doc = "`"] fn foo() {}"#.to_string());
13-
let span = source_span_for_markdown_range_inner(
13+
let (span, _) = source_span_for_markdown_range_inner(
1414
&sm,
1515
"`",
1616
&(0..1),
@@ -20,6 +20,7 @@ fn single_backtick() {
2020
kind: DocFragmentKind::RawDoc,
2121
doc: sym::empty, // unused placeholder
2222
indent: 0,
23+
from_expansion: false,
2324
}],
2425
)
2526
.unwrap();
@@ -32,7 +33,7 @@ fn utf8() {
3233
// regression test for https://github.com/rust-lang/rust/issues/141665
3334
let sm = SourceMap::new(FilePathMapping::empty());
3435
sm.new_source_file(PathBuf::from("foo.rs").into(), r#"#[doc = "⚠"] fn foo() {}"#.to_string());
35-
let span = source_span_for_markdown_range_inner(
36+
let (span, _) = source_span_for_markdown_range_inner(
3637
&sm,
3738
"⚠",
3839
&(0..3),
@@ -42,6 +43,7 @@ fn utf8() {
4243
kind: DocFragmentKind::RawDoc,
4344
doc: sym::empty, // unused placeholder
4445
indent: 0,
46+
from_expansion: false,
4547
}],
4648
)
4749
.unwrap();

src/librustdoc/clean/types/tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ fn create_doc_fragment(s: &str) -> Vec<DocFragment> {
1010
doc: Symbol::intern(s),
1111
kind: DocFragmentKind::SugaredDoc,
1212
indent: 0,
13+
from_expansion: false,
1314
}]
1415
}
1516

src/librustdoc/passes/collect_intra_doc_links.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,13 +1387,15 @@ impl LinkCollector<'_, '_> {
13871387
ori_link: &MarkdownLinkRange,
13881388
item: &Item,
13891389
) {
1390-
let span = source_span_for_markdown_range(
1390+
let span = match source_span_for_markdown_range(
13911391
self.cx.tcx,
13921392
dox,
13931393
ori_link.inner_range(),
13941394
&item.attrs.doc_strings,
1395-
)
1396-
.unwrap_or_else(|| item.attr_span(self.cx.tcx));
1395+
) {
1396+
Some((sp, _)) => sp,
1397+
None => item.attr_span(self.cx.tcx),
1398+
};
13971399
rustc_session::parse::feature_err(
13981400
self.cx.tcx.sess,
13991401
sym::intra_doc_pointers,
@@ -1836,7 +1838,7 @@ fn report_diagnostic(
18361838
let mut md_range = md_range.clone();
18371839
let sp =
18381840
source_span_for_markdown_range(tcx, dox, &md_range, &item.attrs.doc_strings)
1839-
.map(|mut sp| {
1841+
.map(|(mut sp, _)| {
18401842
while dox.as_bytes().get(md_range.start) == Some(&b' ')
18411843
|| dox.as_bytes().get(md_range.start) == Some(&b'`')
18421844
{
@@ -1854,7 +1856,8 @@ fn report_diagnostic(
18541856
(sp, MarkdownLinkRange::Destination(md_range))
18551857
}
18561858
MarkdownLinkRange::WholeLink(md_range) => (
1857-
source_span_for_markdown_range(tcx, dox, md_range, &item.attrs.doc_strings),
1859+
source_span_for_markdown_range(tcx, dox, md_range, &item.attrs.doc_strings)
1860+
.map(|(sp, _)| sp),
18581861
link_range.clone(),
18591862
),
18601863
};

src/librustdoc/passes/lint/bare_urls.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ use crate::html::markdown::main_body_opts;
1818

1919
pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) {
2020
let report_diag = |cx: &DocContext<'_>, msg: &'static str, range: Range<usize>| {
21-
let maybe_sp = source_span_for_markdown_range(cx.tcx, dox, &range, &item.attrs.doc_strings);
21+
let maybe_sp = source_span_for_markdown_range(cx.tcx, dox, &range, &item.attrs.doc_strings)
22+
.map(|(sp, _)| sp);
2223
let sp = maybe_sp.unwrap_or_else(|| item.attr_span(cx.tcx));
2324
cx.tcx.node_span_lint(crate::lint::BARE_URLS, hir_id, sp, |lint| {
2425
lint.primary_message(msg)

src/librustdoc/passes/lint/check_code_block_syntax.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ fn check_rust_syntax(
8787
&code_block.range,
8888
&item.attrs.doc_strings,
8989
) {
90-
Some(sp) => (sp, true),
90+
Some((sp, _)) => (sp, true),
9191
None => (item.attr_span(cx.tcx), false),
9292
};
9393

src/librustdoc/passes/lint/html_tags.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &
1616
let tcx = cx.tcx;
1717
let report_diag = |msg: String, range: &Range<usize>, is_open_tag: bool| {
1818
let sp = match source_span_for_markdown_range(tcx, dox, range, &item.attrs.doc_strings) {
19-
Some(sp) => sp,
19+
Some((sp, _)) => sp,
2020
None => item.attr_span(tcx),
2121
};
2222
tcx.node_span_lint(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| {
@@ -55,7 +55,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &
5555
&(generics_start..generics_end),
5656
&item.attrs.doc_strings,
5757
) {
58-
Some(sp) => sp,
58+
Some((sp, _)) => sp,
5959
None => item.attr_span(tcx),
6060
};
6161
// Sometimes, we only extract part of a path. For example, consider this:

src/librustdoc/passes/lint/redundant_explicit_links.rs

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -161,20 +161,36 @@ fn check_inline_or_reference_unknown_redundancy(
161161

162162
if dest_res == display_res {
163163
let link_span =
164-
source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings)
165-
.unwrap_or(item.attr_span(cx.tcx));
166-
let explicit_span = source_span_for_markdown_range(
164+
match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings)
165+
{
166+
Some((sp, from_expansion)) => {
167+
if from_expansion {
168+
return None;
169+
}
170+
sp
171+
}
172+
None => item.attr_span(cx.tcx),
173+
};
174+
let (explicit_span, false) = source_span_for_markdown_range(
167175
cx.tcx,
168176
doc,
169177
&offset_explicit_range(doc, link_range, open, close),
170178
&item.attrs.doc_strings,
171-
)?;
172-
let display_span = source_span_for_markdown_range(
179+
)?
180+
else {
181+
// This `span` comes from macro expansion so skipping it.
182+
return None;
183+
};
184+
let (display_span, false) = source_span_for_markdown_range(
173185
cx.tcx,
174186
doc,
175187
resolvable_link_range,
176188
&item.attrs.doc_strings,
177-
)?;
189+
)?
190+
else {
191+
// This `span` comes from macro expansion so skipping it.
192+
return None;
193+
};
178194

179195
cx.tcx.node_span_lint(crate::lint::REDUNDANT_EXPLICIT_LINKS, hir_id, explicit_span, |lint| {
180196
lint.primary_message("redundant explicit link target")
@@ -206,21 +222,37 @@ fn check_reference_redundancy(
206222

207223
if dest_res == display_res {
208224
let link_span =
209-
source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings)
210-
.unwrap_or(item.attr_span(cx.tcx));
211-
let explicit_span = source_span_for_markdown_range(
225+
match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings)
226+
{
227+
Some((sp, from_expansion)) => {
228+
if from_expansion {
229+
return None;
230+
}
231+
sp
232+
}
233+
None => item.attr_span(cx.tcx),
234+
};
235+
let (explicit_span, false) = source_span_for_markdown_range(
212236
cx.tcx,
213237
doc,
214238
&offset_explicit_range(doc, link_range.clone(), b'[', b']'),
215239
&item.attrs.doc_strings,
216-
)?;
217-
let display_span = source_span_for_markdown_range(
240+
)?
241+
else {
242+
// This `span` comes from macro expansion so skipping it.
243+
return None;
244+
};
245+
let (display_span, false) = source_span_for_markdown_range(
218246
cx.tcx,
219247
doc,
220248
resolvable_link_range,
221249
&item.attrs.doc_strings,
222-
)?;
223-
let def_span = source_span_for_markdown_range(
250+
)?
251+
else {
252+
// This `span` comes from macro expansion so skipping it.
253+
return None;
254+
};
255+
let (def_span, _) = source_span_for_markdown_range(
224256
cx.tcx,
225257
doc,
226258
&offset_reference_def_range(doc, dest, link_range),

0 commit comments

Comments
 (0)