Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2302,7 +2302,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
self.check_param_uses_if_mcg(ct, tcx.hir_span(path_hir_id), false)
}

/// Lower a [`hir::ConstArg`] to a (type-level) [`ty::Const`](Const).
/// Lower a [`hir::ConstArg`] to a (type-level) [`ty::Const`].
#[instrument(skip(self), level = "debug")]
pub fn lower_const_arg(&self, const_arg: &hir::ConstArg<'tcx>, ty: Ty<'tcx>) -> Const<'tcx> {
let tcx = self.tcx();
Expand Down
73 changes: 65 additions & 8 deletions compiler/rustc_resolve/src/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,11 +641,59 @@ pub fn source_span_for_markdown_range_inner(
let mut start_bytes = 0;
let mut end_bytes = 0;

let span_of_all_fragments: Span = span_of_fragments(fragments)?;

let mut prev_lines_bytes = 0;
'outer: for (line_no, md_line) in md_lines.enumerate() {
loop {
let source_line = src_lines.next()?;
match source_line.find(md_line) {
Some(offset) => {
let source_line_len = u32::try_from(source_line.len()).unwrap();
let source_line_span =
span_of_all_fragments.split_at(prev_lines_bytes).1.split_at(source_line_len).0;
let fragment = fragments
.iter()
// `source_line_span` might contain indentation that `fragment.span` doesn't contain
.find(|fragment| fragment.span.overlaps(source_line_span));
// Since we're counting bytes, `prev_line_bytes` includes the "\n".
prev_lines_bytes += source_line_len + 1;
if let Some(fragment) = fragment
&& let Some(offset) = source_line.find(md_line)
{
if fragment.span.lo() > source_line_span.lo()
&& source_line
[..usize::try_from(fragment.span.lo().0 - source_line_span.lo().0).unwrap()]
.chars()
.any(|c| !c.is_whitespace())
{
// Make sure anything between the start of this line and the fragment itself is just indentation.
// Because source_line is built by splitting the span that covers all fragments, this only finds
// characters *between* doc comments, not characters before or after doc comments.
//
// 1| /** doc */
// 2| #[inline] /** doc2 */
// ^^^^^^^^^
// | this
//
// 3| fn foo() {}
return None;
}
if fragment.span.hi() < source_line_span.hi()
&& source_line
[usize::try_from(fragment.span.hi().0 - source_line_span.lo().0).unwrap()..]
.chars()
.any(|c| !c.is_whitespace())
{
// Make sure anything between the start of this line and the fragment itself is just indentation.
// Because source_line is built by splitting the span that covers all fragments, this only finds
// characters *between* doc comments, not characters before or after doc comments.
// 1| /** doc */ #[inline]
// ^^^^^^^^^
// | this
//
// 2| /** doc2 */
// 3| fn foo() {}
return None;
}
if line_no == starting_line {
start_bytes += offset;

Expand All @@ -661,22 +709,31 @@ pub fn source_span_for_markdown_range_inner(
end_bytes += source_line.len() - md_line.len();
}
break;
}
None => {
} else {
// Since this is a source line that doesn't include a markdown line,
// we have to count the newline that we split from earlier.
// we have to count it and its newline as non-markdown bytes.
if line_no <= starting_line {
start_bytes += source_line.len() + 1;
} else if source_line.chars().any(|c| !c.is_whitespace()) {
// We're past the first line, but haven't found the last line,
// but we found a non-empty non-markdown line.
// This could be an attribute, and we don't want a diagnostic
// suggesting to delete that attribute, so we return None to be safe.
Comment thread
notriddle marked this conversation as resolved.
// 1| /** doc */
// 2 | #[inline]
// ^^^^^^^^^
// | this
// 3| /** doc2 */
// 4| fn foo() {}
return None;
Comment on lines +717 to +728

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else if source_line.chars().any(|c| !c.is_whitespace()) {
// If we're past the first line, but haven't found the last line,
// we can only return a contiguous span if every line is either
// part of the doc comment or blank.
return None;
} else if source_line.chars().any(|c| !c.is_whitespace()) {
// We're past the first line, but haven't found the last line,
// but we found a non-empty non-markdown line.
// This could be an attribute, and we don't want a diagnostic
// suggesting to delete that attribute, so we return None to be safe.
return None;

} else {
end_bytes += source_line.len() + 1;
}
}
}
}
}

let span = span_of_fragments(fragments)?;
let src_span = span.from_inner(InnerSpan::new(
let src_span = span_of_all_fragments.from_inner(InnerSpan::new(
md_range.start + start_bytes,
md_range.end + start_bytes + end_bytes,
));
Expand Down
164 changes: 123 additions & 41 deletions src/librustdoc/passes/lint/redundant_explicit_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fn check_redundant_explicit_link<'md>(
hir_id: HirId,
doc: &'md str,
resolutions: &DocLinkResMap,
) -> Option<()> {
) {
let mut broken_line_callback = |link: BrokenLink<'md>| Some((link.reference, "".into()));
let mut offset_iter = Parser::new_with_broken_link_callback(
doc,
Expand Down Expand Up @@ -115,7 +115,7 @@ fn check_redundant_explicit_link<'md>(
}

if dest_url.ends_with(resolvable_link) || resolvable_link.ends_with(&*dest_url) {
match link_type {
let check_result = match link_type {
LinkType::Inline | LinkType::ReferenceUnknown => {
check_inline_or_reference_unknown_redundancy(
cx,
Expand All @@ -127,10 +127,9 @@ fn check_redundant_explicit_link<'md>(
dest_url.to_string(),
link_data,
if link_type == LinkType::Inline { (b'(', b')') } else { (b'[', b']') },
);
)
}
LinkType::Reference => {
check_reference_redundancy(
LinkType::Reference => check_reference_redundancy(
cx,
item,
hir_id,
Expand All @@ -139,15 +138,41 @@ fn check_redundant_explicit_link<'md>(
link_range,
&dest_url,
link_data,
),
_ => Ok(()),
};
if let Err(lint) = check_result {
cx.tcx.emit_node_span_lint(
crate::lint::REDUNDANT_EXPLICIT_LINKS,
hir_id,
item.attr_span(cx.tcx),
lint,
);
}
_ => {}
}
}
}
}

None
struct RedundantExplicitLinksWithoutSuggestion {
attr_span: Span,
display_link: String,
dest_link: String,
}

impl<'a> Diagnostic<'a, ()> for RedundantExplicitLinksWithoutSuggestion {
fn into_diag(self, dcx: DiagCtxtHandle<'a>, level: Level) -> Diag<'a, ()> {
let Self { attr_span, display_link, dest_link } = self;

Diag::new(dcx, level, "redundant explicit link target")
.with_span_label(
attr_span,
format!("explicit target `{dest_link}` is redundant because label `{display_link}` resolves to same destination")
)
.with_note(
"when a link's destination is not specified,\nthe label is used to resolve intra-doc links"
)
}
}

/// FIXME(ChAoSUnItY): Too many arguments.
Expand All @@ -161,7 +186,7 @@ fn check_inline_or_reference_unknown_redundancy(
dest: String,
link_data: LinkData,
(open, close): (u8, u8),
) -> Option<()> {
) -> Result<(), RedundantExplicitLinksWithoutSuggestion> {
struct RedundantExplicitLinks {
explicit_span: Span,
display_span: Span,
Expand Down Expand Up @@ -196,42 +221,65 @@ fn check_inline_or_reference_unknown_redundancy(
}
}

let (resolvable_link, resolvable_link_range) =
(&link_data.resolvable_link?, &link_data.resolvable_link_range?);
let (dest_res, display_res) =
(find_resolution(resolutions, &dest)?, find_resolution(resolutions, resolvable_link)?);
let (Some(resolvable_link), Some(resolvable_link_range)) =
(&link_data.resolvable_link, &link_data.resolvable_link_range)
else {
return Ok(());
};
let (Some(dest_res), Some(display_res)) =
(find_resolution(resolutions, &dest), find_resolution(resolutions, resolvable_link))
else {
return Ok(());
};

if dest_res == display_res {
let attr_span = item.attr_span(cx.tcx);
let link_span =
match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings)
{
Some((sp, from_expansion)) => {
if from_expansion {
return None;
return Ok(());
}
sp
}
None => item.attr_span(cx.tcx),
None => attr_span,
};
let (explicit_span, false) = source_span_for_markdown_range(
let explicit_span = match source_span_for_markdown_range(
cx.tcx,
doc,
&offset_explicit_range(doc, link_range, open, close),
&item.attrs.doc_strings,
)?
else {
) {
Some((explicit_span, false)) => explicit_span,
// This `span` comes from macro expansion so skipping it.
return None;
Some((_, true)) => return Ok(()),
// Cannot give a contiguous span for this link.
None => {
return Err(RedundantExplicitLinksWithoutSuggestion {
display_link: resolvable_link.clone(),
dest_link: dest.to_string(),
attr_span,
});
}
};
let (display_span, false) = source_span_for_markdown_range(
let display_span = match source_span_for_markdown_range(
cx.tcx,
doc,
resolvable_link_range,
&item.attrs.doc_strings,
)?
else {
) {
Some((display_span, false)) => display_span,
// This `span` comes from macro expansion so skipping it.
return None;
Some((_, true)) => return Ok(()),
// Cannot give a contiguous span for this link.
None => {
return Err(RedundantExplicitLinksWithoutSuggestion {
display_link: resolvable_link.clone(),
dest_link: dest.to_string(),
attr_span,
});
}
};

cx.tcx.emit_node_span_lint(
Expand All @@ -247,7 +295,7 @@ fn check_inline_or_reference_unknown_redundancy(
);
}

None
Ok(())
}

/// FIXME(ChAoSUnItY): Too many arguments.
Expand All @@ -260,7 +308,7 @@ fn check_reference_redundancy(
link_range: Range<usize>,
dest: &CowStr<'_>,
link_data: LinkData,
) -> Option<()> {
) -> Result<(), RedundantExplicitLinksWithoutSuggestion> {
struct RedundantExplicitLinkTarget {
explicit_span: Span,
display_span: Span,
Expand Down Expand Up @@ -294,49 +342,83 @@ fn check_reference_redundancy(
}
}

let (resolvable_link, resolvable_link_range) =
(&link_data.resolvable_link?, &link_data.resolvable_link_range?);
let (dest_res, display_res) =
(find_resolution(resolutions, dest)?, find_resolution(resolutions, resolvable_link)?);
let (Some(resolvable_link), Some(resolvable_link_range)) =
(&link_data.resolvable_link, &link_data.resolvable_link_range)
else {
return Ok(());
};
let (Some(dest_res), Some(display_res)) =
(find_resolution(resolutions, dest), find_resolution(resolutions, resolvable_link))
else {
return Ok(());
};

if dest_res == display_res {
let attr_span = item.attr_span(cx.tcx);
let link_span =
match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings)
{
Some((sp, from_expansion)) => {
if from_expansion {
return None;
// This `span` comes from macro expansion so skipping it.
return Ok(());
}
sp
}
None => item.attr_span(cx.tcx),
None => attr_span,
};
let (explicit_span, false) = source_span_for_markdown_range(
let explicit_span = match source_span_for_markdown_range(
cx.tcx,
doc,
&offset_explicit_range(doc, link_range.clone(), b'[', b']'),
&item.attrs.doc_strings,
)?
else {
) {
Some((explicit_span, false)) => explicit_span,
// This `span` comes from macro expansion so skipping it.
return None;
Some((_, true)) => return Ok(()),
// Cannot give a contiguous span for this link.
None => {
return Err(RedundantExplicitLinksWithoutSuggestion {
display_link: resolvable_link.clone(),
dest_link: dest.to_string(),
attr_span,
});
}
};
let (display_span, false) = source_span_for_markdown_range(
let display_span = match source_span_for_markdown_range(
cx.tcx,
doc,
resolvable_link_range,
&item.attrs.doc_strings,
)?
else {
) {
Some((display_span, false)) => display_span,
// This `span` comes from macro expansion so skipping it.
return None;
Some((_, true)) => return Ok(()),
// Cannot give a contiguous span for this link.
None => {
return Err(RedundantExplicitLinksWithoutSuggestion {
display_link: resolvable_link.clone(),
dest_link: dest.to_string(),
attr_span,
});
}
};
let (def_span, _) = source_span_for_markdown_range(
let def_span = match source_span_for_markdown_range(
cx.tcx,
doc,
&offset_reference_def_range(doc, dest, link_range),
&item.attrs.doc_strings,
)?;
) {
Some((def_span, _)) => def_span,
// Cannot give a contiguous span for this link.
None => {
return Err(RedundantExplicitLinksWithoutSuggestion {
display_link: resolvable_link.clone(),
dest_link: dest.to_string(),
attr_span,
});
}
};

cx.tcx.emit_node_span_lint(
crate::lint::REDUNDANT_EXPLICIT_LINKS,
Expand All @@ -352,7 +434,7 @@ fn check_reference_redundancy(
);
}

None
Ok(())
}

fn find_resolution(resolutions: &DocLinkResMap, path: &str) -> Option<Res<NodeId>> {
Expand Down
Loading
Loading