Skip to content

redundant_explicit_links rustdoc lint should have macro guard #141553

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
Open
Manishearth opened this issue May 25, 2025 · 9 comments · May be fixed by #141648
Open

redundant_explicit_links rustdoc lint should have macro guard #141553

Manishearth opened this issue May 25, 2025 · 9 comments · May be fixed by #141648
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Manishearth
Copy link
Member

On ICU4X main, with nightly, cargo doc --all-features in provider/core

error: redundant explicit link target
   --> provider/core/src/constructors.rs:106:47
    |
106 |               "provided by a [`BufferProvider`](icu_provider::buf::BufferProvider).\n\n",
    |                               ----------------  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ explicit target is redundant
    |                               |
    |                               because label contains path that resolves to same destination
    |
   ::: provider/core/src/hello_world.rs:302:5
    |
302 | /     icu_provider::gen_buffer_data_constructors!((prefs: HelloWorldFormatterPreferences) -> error: DataError,
303 | |         functions: [
304 | |             try_new: skip,
305 | |             try_new_with_buffer_provider,
306 | |             try_new_unstable,
307 | |             Self,
308 | |     ]);
    | |______- in this macro invocation
    |
    = note: when a link's destination is not specified,
            the label is used to resolve intra-doc links
    = note: `-D rustdoc::redundant-explicit-links` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(rustdoc::redundant_explicit_links)]`
    = note: this error originates in the macro `$crate::gen_buffer_data_constructors` which comes from the expansion of the macro `icu_provider::gen_buffer_data_constructors` (in Nightly builds, run with -Z macro-backtrace for more info)

These docs are macro-generated, which means even if that import is around in some contexts, it isn't around in all of them, and we can't fix that in the macro.

https://github.com/unicode-org/icu4x/blob/f47044e3de1ba0c47a43b95968bd503902328188/provider/core/src/constructors.rs#L107

Given that they are macro generated docs, it is not possible to stick an allow() statement in the macro to silence the lint (without enabling attributes-on-expressions). So this code is permabroken.

This lint should probably get a macro guard, or even be reverted until this can be figured out: the lint is mostly a cleanup lint and should not cause such problems.
@Manishearth Manishearth added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label May 25, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 25, 2025
@Manishearth
Copy link
Member Author 

$ rustc +nightly -V
rustc 1.89.0-nightly (5e16c6620 2025-05-24)

@Manishearth
Copy link
Member Author

cc @rust-lang/rustdoc

Seems like this lint isn't actually new, but it started firing here anew? Not sure what's going on here.

@Manishearth Manishearth added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 25, 2025
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 25, 2025
@Manishearth
Copy link
Member Author

Tagging as a regression since it's an unsilenceable lint issue, in context.

@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name labels May 25, 2025
@Manishearth
Copy link
Member Author

Manishearth commented May 26, 2025 •
edited
Loading

Regression range from CI is 2eef478..3e674b0 (between nightly-2025-05-22 and nightly-2025-05-23)

@Nemo157
Copy link
Member

Nemo157 commented May 26, 2025

#136400 seems potentially relevant, if it affected this lint too.

@Manishearth
Copy link
Member Author

Ah, it probbaly made this lint trigger where it didn't before because the lint previously didn't handle macro fragments. Makes sense.

The redundant links lint should just get a macro check, I think.

@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 26, 2025
@GuillaumeGomez
Copy link
Member

Adding a fix.

@GuillaumeGomez
Copy link
Member

Maybe I talked too fast. After looking at it for way too long, I tried on this code:

macro_rules! mac1 {
    () => {
        "provided by a [`BufferProvider`](crate::BufferProvider)."
    };
}

macro_rules! mac2 {
    () => {
        #[doc = mac1!()]
	pub struct BufferProvider;
    }
}

#[doc = mac1!()]
pub struct Foo;

mac2!{}

To check that both doc spans were from expansion... Except both are considered to not be from expansion. It comes from this. Removing this line breaks a few other things and make the output a bit less good, so I decided to instead check if the item itself was from expansion... and once again I'm deeply surprised to discover that BufferProvider span is not considered coming from expansion. At this point, I'm really wondering what I'm missing.

Anyway, I'll try to take a deeper look to what I missed because I'm sure it's something obvious.

@GuillaumeGomez
Copy link
Member

I think we need to keep the "is expanded" information when converting the spans. Gonna give it a try tomorrow.

@GuillaumeGomez GuillaumeGomez linked a pull request May 27, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
6 participants
@Nemo157 @Manishearth @GuillaumeGomez @apiraino @fmease @rustbot