Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Rustdoc: suggest removing disambiguator if linking to field #87078

Merged
merged 1 commit into from
Jul 13, 2021
Merged

Rustdoc: suggest removing disambiguator if linking to field #87078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions src/librustdoc/passes/collect_intra_doc_links.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1610,6 +1610,8 @@ impl Disambiguator {
return Suggestion::Macro;
} else if kind == DefKind::Fn || kind == DefKind::AssocFn {
return Suggestion::Function;
} else if kind == DefKind::Field {
return Suggestion::RemoveDisambiguator;
}

let prefix = match kind {
Expand Down Expand Up @@ -1674,6 +1676,8 @@ enum Suggestion {
Function,
/// `m!`
Macro,
/// `foo` without any disambiguator
RemoveDisambiguator,
}

impl Suggestion {
Expand All @@ -1682,6 +1686,7 @@ impl Suggestion {
Self::Prefix(x) => format!("prefix with `{}@`", x).into(),
Self::Function => "add parentheses".into(),
Self::Macro => "add an exclamation mark".into(),
Self::RemoveDisambiguator => "remove the disambiguator".into(),
}
}

Expand All @@ -1691,6 +1696,7 @@ impl Suggestion {
Self::Prefix(prefix) => format!("{}@{}", prefix, path_str),
Self::Function => format!("{}()", path_str),
Self::Macro => format!("{}!", path_str),
Self::RemoveDisambiguator => path_str.into(),
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions src/test/rustdoc-ui/intra-doc/field-ice.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![deny(rustdoc::broken_intra_doc_links)]
//~^NOTE the lint level is defined here

/// [`Foo::bar`]
/// [`Foo::bar()`]
//~^ERROR incompatible link kind for `Foo::bar`
//~|HELP to link to the field, remove the disambiguator
//~|NOTE this link resolved to a field, which is not a function
pub struct Foo {
pub bar: u8
}
15 changes: 15 additions & 0 deletions src/test/rustdoc-ui/intra-doc/field-ice.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: incompatible link kind for `Foo::bar`
--> $DIR/field-ice.rs:5:6
|
LL | /// [`Foo::bar()`]
| ^^^^^^^^^^^^ help: to link to the field, remove the disambiguator: ``Foo::bar``
Copy link
Member

@GuillaumeGomez GuillaumeGomez Jul 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The message is really not obvious. What about:

help: to link to the field, remove the parenthesis: `Foo::bar`

? (without the extra backquotes too)

Copy link
Member Author

Choose a reason for hiding this comment

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

the prefix and suffix are all mapped to Disambiguator::Kind(DefKind::Fn):

fn@foo, function@foo, method@foo, and foo().

There are two ways to approach this, either we only allow one kind of those four to exist, or we need to refactor to include that information inside Disambiguator.

Copy link
Member

Choose a reason for hiding this comment

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

either we only allow one kind of those four to exist

This is a breaking change, it's not allowed

|
note: the lint level is defined here
--> $DIR/field-ice.rs:1:9
|
LL | #![deny(rustdoc::broken_intra_doc_links)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: this link resolved to a field, which is not a function

error: aborting due to previous error