-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[rustdoc] Do not emit redundant_explicit_links lint if the doc comment comes from expansion #141648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
fbbda9e
2723a5d
9ac0d61
0ada37b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -49,6 +49,9 @@ pub struct DocFragment { | |||||||||
pub doc: Symbol, | ||||||||||
pub kind: DocFragmentKind, | ||||||||||
pub indent: usize, | ||||||||||
/// Because we temper with the spans context, this information cannot be correctly retrieved | ||||||||||
/// later on. So instead, we compute it and store it here. | ||||||||||
pub from_expansion: bool, | ||||||||||
} | ||||||||||
|
||||||||||
#[derive(Clone, Copy, Debug)] | ||||||||||
|
@@ -208,17 +211,18 @@ pub fn attrs_to_doc_fragments<'a, A: AttributeExt + Clone + 'a>( | |||||||||
for (attr, item_id) in attrs { | ||||||||||
if let Some((doc_str, comment_kind)) = attr.doc_str_and_comment_kind() { | ||||||||||
let doc = beautify_doc_string(doc_str, comment_kind); | ||||||||||
let (span, kind) = if attr.is_doc_comment() { | ||||||||||
(attr.span(), DocFragmentKind::SugaredDoc) | ||||||||||
let (span, kind, from_expansion) = if attr.is_doc_comment() { | ||||||||||
let span = attr.span(); | ||||||||||
(span, DocFragmentKind::SugaredDoc, span.from_expansion()) | ||||||||||
} else { | ||||||||||
( | ||||||||||
attr.value_span() | ||||||||||
.map(|i| i.with_ctxt(attr.span().ctxt())) | ||||||||||
.unwrap_or(attr.span()), | ||||||||||
DocFragmentKind::RawDoc, | ||||||||||
) | ||||||||||
let attr_span = attr.span(); | ||||||||||
let (span, from_expansion) = match attr.value_span() { | ||||||||||
Some(sp) => (sp.with_ctxt(attr_span.ctxt()), sp.from_expansion()), | ||||||||||
None => (attr_span, attr_span.from_expansion()), | ||||||||||
}; | ||||||||||
(span, DocFragmentKind::RawDoc, from_expansion) | ||||||||||
}; | ||||||||||
let fragment = DocFragment { span, doc, kind, item_id, indent: 0 }; | ||||||||||
let fragment = DocFragment { span, doc, kind, item_id, indent: 0, from_expansion }; | ||||||||||
doc_fragments.push(fragment); | ||||||||||
} else if !doc_only { | ||||||||||
other_attrs.push(attr.clone()); | ||||||||||
|
@@ -501,16 +505,21 @@ fn collect_link_data<'input, F: BrokenLinkCallback<'input>>( | |||||||||
} | ||||||||||
|
||||||||||
/// Returns a span encompassing all the document fragments. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Boolean return values can sometimes be unclear which state represents what, |
||||||||||
pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> { | ||||||||||
if fragments.is_empty() { | ||||||||||
return None; | ||||||||||
} | ||||||||||
let start = fragments[0].span; | ||||||||||
if start == DUMMY_SP { | ||||||||||
pub fn span_of_fragments_with_expansion(fragments: &[DocFragment]) -> Option<(Span, bool)> { | ||||||||||
let Some(first_fragment) = fragments.first() else { return None }; | ||||||||||
if first_fragment.span == DUMMY_SP { | ||||||||||
return None; | ||||||||||
} | ||||||||||
let end = fragments.last().expect("no doc strings provided").span; | ||||||||||
Some(start.to(end)) | ||||||||||
let last_fragment = fragments.last().expect("no doc strings provided"); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: this |
||||||||||
Some(( | ||||||||||
first_fragment.span.to(last_fragment.span), | ||||||||||
first_fragment.from_expansion || last_fragment.from_expansion, | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
In case only the middle fragment is from an expansion (we should also add a testcase for this). |
||||||||||
)) | ||||||||||
} | ||||||||||
|
||||||||||
/// Returns a span encompassing all the document fragments. | ||||||||||
pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> { | ||||||||||
span_of_fragments_with_expansion(fragments).map(|(sp, _)| sp) | ||||||||||
} | ||||||||||
|
||||||||||
/// Attempts to match a range of bytes from parsed markdown to a `Span` in the source code. | ||||||||||
|
@@ -535,7 +544,7 @@ pub fn source_span_for_markdown_range( | |||||||||
markdown: &str, | ||||||||||
md_range: &Range<usize>, | ||||||||||
fragments: &[DocFragment], | ||||||||||
) -> Option<Span> { | ||||||||||
) -> Option<(Span, bool)> { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also document the meaning of the boolean return on this function too. |
||||||||||
let map = tcx.sess.source_map(); | ||||||||||
source_span_for_markdown_range_inner(map, markdown, md_range, fragments) | ||||||||||
} | ||||||||||
|
@@ -546,7 +555,7 @@ pub fn source_span_for_markdown_range_inner( | |||||||||
markdown: &str, | ||||||||||
md_range: &Range<usize>, | ||||||||||
fragments: &[DocFragment], | ||||||||||
) -> Option<Span> { | ||||||||||
) -> Option<(Span, bool)> { | ||||||||||
use rustc_span::BytePos; | ||||||||||
|
||||||||||
if let &[fragment] = &fragments | ||||||||||
|
@@ -557,11 +566,14 @@ pub fn source_span_for_markdown_range_inner( | |||||||||
&& let Ok(md_range_hi) = u32::try_from(md_range.end) | ||||||||||
{ | ||||||||||
// Single fragment with string that contains same bytes as doc. | ||||||||||
return Some(Span::new( | ||||||||||
fragment.span.lo() + rustc_span::BytePos(md_range_lo), | ||||||||||
fragment.span.lo() + rustc_span::BytePos(md_range_hi), | ||||||||||
fragment.span.ctxt(), | ||||||||||
fragment.span.parent(), | ||||||||||
return Some(( | ||||||||||
Span::new( | ||||||||||
fragment.span.lo() + rustc_span::BytePos(md_range_lo), | ||||||||||
fragment.span.lo() + rustc_span::BytePos(md_range_hi), | ||||||||||
fragment.span.ctxt(), | ||||||||||
fragment.span.parent(), | ||||||||||
), | ||||||||||
fragment.from_expansion, | ||||||||||
)); | ||||||||||
} | ||||||||||
|
||||||||||
|
@@ -593,19 +605,21 @@ pub fn source_span_for_markdown_range_inner( | |||||||||
{ | ||||||||||
match_data = Some((i, match_start)); | ||||||||||
} else { | ||||||||||
// Heirustic produced ambiguity, return nothing. | ||||||||||
// Heuristic produced ambiguity, return nothing. | ||||||||||
return None; | ||||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
if let Some((i, match_start)) = match_data { | ||||||||||
let sp = fragments[i].span; | ||||||||||
let fragment = &fragments[i]; | ||||||||||
let sp = fragment.span; | ||||||||||
// we need to calculate the span start, | ||||||||||
// then use that in our calulations for the span end | ||||||||||
let lo = sp.lo() + BytePos(match_start as u32); | ||||||||||
return Some( | ||||||||||
return Some(( | ||||||||||
sp.with_lo(lo).with_hi(lo + BytePos((md_range.end - md_range.start) as u32)), | ||||||||||
); | ||||||||||
fragment.from_expansion, | ||||||||||
)); | ||||||||||
} | ||||||||||
return None; | ||||||||||
} | ||||||||||
|
@@ -659,8 +673,12 @@ pub fn source_span_for_markdown_range_inner( | |||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
Some(span_of_fragments(fragments)?.from_inner(InnerSpan::new( | ||||||||||
md_range.start + start_bytes, | ||||||||||
md_range.end + start_bytes + end_bytes, | ||||||||||
))) | ||||||||||
let (span, from_expansion) = span_of_fragments_with_expansion(fragments)?; | ||||||||||
Some(( | ||||||||||
span.from_inner(InnerSpan::new( | ||||||||||
md_range.start + start_bytes, | ||||||||||
md_range.end + start_bytes + end_bytes, | ||||||||||
)), | ||||||||||
from_expansion, | ||||||||||
)) | ||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -161,15 +161,26 @@ fn check_inline_or_reference_unknown_redundancy( | |
|
||
if dest_res == display_res { | ||
let link_span = | ||
source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings) | ||
.unwrap_or(item.attr_span(cx.tcx)); | ||
let explicit_span = source_span_for_markdown_range( | ||
match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings) | ||
{ | ||
Some((sp, from_expansion)) => { | ||
if from_expansion { | ||
return None; | ||
} | ||
sp | ||
} | ||
None => item.attr_span(cx.tcx), | ||
}; | ||
let (explicit_span, from_expansion) = source_span_for_markdown_range( | ||
cx.tcx, | ||
doc, | ||
&offset_explicit_range(doc, link_range, open, close), | ||
&item.attrs.doc_strings, | ||
)?; | ||
let display_span = source_span_for_markdown_range( | ||
if from_expansion { | ||
return None; | ||
} | ||
let (display_span, _) = source_span_for_markdown_range( | ||
cx.tcx, | ||
doc, | ||
resolvable_link_range, | ||
|
@@ -206,21 +217,32 @@ fn check_reference_redundancy( | |
|
||
if dest_res == display_res { | ||
let link_span = | ||
source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings) | ||
.unwrap_or(item.attr_span(cx.tcx)); | ||
let explicit_span = source_span_for_markdown_range( | ||
match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings) | ||
{ | ||
Some((sp, from_expansion)) => { | ||
if from_expansion { | ||
return None; | ||
} | ||
sp | ||
} | ||
None => item.attr_span(cx.tcx), | ||
}; | ||
let (explicit_span, from_expansion) = source_span_for_markdown_range( | ||
cx.tcx, | ||
doc, | ||
&offset_explicit_range(doc, link_range.clone(), b'[', b']'), | ||
&item.attrs.doc_strings, | ||
)?; | ||
let display_span = source_span_for_markdown_range( | ||
if from_expansion { | ||
return None; | ||
} | ||
let (display_span, _) = source_span_for_markdown_range( | ||
Comment on lines
+230
to
+239
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are we checking alternatively, if we really cared about saving branches, we could probably get the display span by subtracting the explicit span from the total span, eliminating a call to also, the use of For example, this produces no warnings: /// [bar](bar)
#[doc = ""]
pub fn foo() {}
pub fn bar() {} Technically this is an unrelated and preexisting bug, so I can spin this off into a separate issue and send a followup PR if you want, but it might be easier to just squash it now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, gonna handle this case here. |
||
cx.tcx, | ||
doc, | ||
resolvable_link_range, | ||
&item.attrs.doc_strings, | ||
)?; | ||
let def_span = source_span_for_markdown_range( | ||
let (def_span, _) = source_span_for_markdown_range( | ||
cx.tcx, | ||
doc, | ||
&offset_reference_def_range(doc, dest, link_range), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.