rustdoc: improve refdef handling in the unresolved link lint #136363

notriddle · 2025-01-31T19:06:23Z

This commit takes advantage of a feature in pulldown-cmark that makes the list of link definitions available to the consuming application. It produces unresolved link warnings for refdefs that aren't used, and can now produce exact spans for the dest even when it has escapes.

Closes #133150 since this lint would have caught the mistake in that issue, and, along with rust-lang/rust-clippy#13707, most mistakes in this class should produce a warning from one of them.

rustbot · 2025-01-31T19:06:31Z

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

notriddle · 2025-01-31T19:09:45Z

tests/rustdoc-ui/intra-doc/weird-syntax.rs

-/// The [cln][] link here is going to be unresolved, because `struct@Clone` gets //~ERROR link
-/// rejected in Markdown for not being URL-shaped enough.


This comment was wrong.

struct@Clone is URL-ish enough to match CommonMark's refdef grammar. The reason it was rejected before was because the //~ERROR link hot comment got fed to the markdown parser, and, as a result, caused the refdef to not get parsed there.

By using a below-the-line comment, I can work around that and demonstrate the change correctly.

petrochenkov · 2025-02-03T20:18:57Z

compiler/rustc_resolve/src/rustdoc.rs


                links.push(preprocess_link(&dest_url));
            }
            _ => {}
        }
    }

+    for (label, refdef) in event_iter.reference_definitions().iter() {


This logic is run on a compiler's happy path, when rustdoc and its lints are not involved.
Could you do a perf run before merging this?

notriddle · 2025-02-03T21:13:21Z

Not a problem.

@bors try @rust-timer queue

…unused-refdef, r=<try> rustdoc: improve refdef handling in the unresolved link lint This commit takes advantage of a feature in pulldown-cmark that makes the list of link definitions available to the consuming application. It produces unresolved link warnings for refdefs that aren't used, and can now produce exact spans for the dest even when it has escapes. Closes rust-lang#133150 since this lint would have caught the mistake in that issue, and, along with rust-lang/rust-clippy#13707, most mistakes in this class should produce a warning from one of them.

bors · 2025-02-03T21:14:34Z

⌛ Trying commit d15dfe7 with merge 230bb73...

bors · 2025-02-03T23:03:12Z

☀️ Try build successful - checks-actions
Build commit: 230bb73 (230bb733f167bb2561c8450c5669374b46ef4209)

rust-timer · 2025-02-04T01:40:28Z

Finished benchmarking commit (230bb73): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.2%, 0.2%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.2%, 0.2%]	2

Max RSS (memory usage)

Results (primary -1.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.6%	[-1.6%, -1.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.6%	[-1.6%, -1.6%]	1

Cycles

Results (primary -2.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.1%	[-2.1%, -2.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.1%	[-2.1%, -2.1%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 779.425s -> 779.101s (-0.04%)
Artifact size: 328.79 MiB -> 328.83 MiB (0.01%)

notriddle · 2025-02-04T16:52:20Z

The two regressions are both doc builds, tiny, and they come with a diagnostic that can help people out of a pretty bad syntax hole.

notriddle · 2025-02-14T16:02:43Z

r? @GuillaumeGomez

GuillaumeGomez · 2025-02-14T16:05:14Z

It's really cool, great idea! Perf impact seems acceptable to me. Not sure if we are waiting on petrochenkov or not so I'll let you r+.

notriddle · 2025-02-14T17:01:53Z

This logic is run on a compiler's happy path, when rustdoc and its lints are not involved.
Could you do a perf run before merging this?

The result of the lint is no perf impacts other than rustdoc, so it shouldn't be a problem.

@bors r=GuillaumeGomez

bors · 2025-02-14T17:01:58Z

📌 Commit d15dfe7 has been approved by GuillaumeGomez

It is now in the queue for this repository.

bors · 2025-02-14T20:17:06Z

⌛ Testing commit d15dfe7 with merge c3a99e7...

bors · 2025-02-14T20:18:32Z

💔 Test failed - checks-actions

fmease · 2025-02-14T21:46:55Z

@bors retry (after #137041)

…unused-refdef, r=GuillaumeGomez rustdoc: improve refdef handling in the unresolved link lint This commit takes advantage of a feature in pulldown-cmark that makes the list of link definitions available to the consuming application. It produces unresolved link warnings for refdefs that aren't used, and can now produce exact spans for the dest even when it has escapes. Closes rust-lang#133150 since this lint would have caught the mistake in that issue, and, along with rust-lang/rust-clippy#13707, most mistakes in this class should produce a warning from one of them.

bors · 2025-02-15T10:59:18Z

⌛ Testing commit d15dfe7 with merge bd3b5cb...

bors · 2025-02-15T11:56:08Z

💔 Test failed - checks-actions

This commit takes advantage of a feature in pulldown-cmark that makes the list of link definitions available to the consuming application. It produces unresolved link warnings for refdefs that aren't used, and can now produce exact spans for the dest even when it has escapes.

notriddle · 2025-02-15T21:25:29Z

Rebased it. This change required weird-syntax.stderr to be re-blessed because of a minor output change:

- LL | /// [cln]: trait@Clone
-    |            ~~~~~~
+ LL - /// [cln]: struct@Clone
+ LL + /// [cln]: trait@Clone

@bors r=GuillaumeGomez

bors · 2025-02-15T21:25:31Z

📌 Commit 4d551dd has been approved by GuillaumeGomez

It is now in the queue for this repository.

bors · 2025-02-16T10:59:45Z

⌛ Testing commit 4d551dd with merge 23032f3...

bors · 2025-02-16T14:01:00Z

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 23032f3 to master...

rust-timer · 2025-02-16T15:29:42Z

Finished benchmarking commit (23032f3): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.2%, 0.3%]	2
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.2%, 0.3%]	2

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 790.759s -> 789.651s (-0.14%)
Artifact size: 350.03 MiB -> 350.07 MiB (0.01%)

rustbot assigned fmease Jan 31, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 31, 2025

notriddle commented Jan 31, 2025

View reviewed changes