intra-doc links: Links with unexpected characters are silently ignored

[dylni]

Intra-doc links with unexpected characters are ignored here:

rust/src/librustdoc/passes/collect_intra_doc_links.rs

Lines 931 to 933 in 78a0894

	if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ch == ':' || ch == '_')) {
	return;
	}

However, it would be better if they caused a warning with --display-doctest-warnings. Developers might expect [`&[String]`] to link to the slice primitive, but that link would be ignored without a warning.

cc @jyn514
cc #76106 (comment)

[camelid]

FWIW, I think this is less of an issue now, since more characters are allowed in intra-doc links, and thus not ignored. Also, IIRC --display-warnings has now been renamed to --display-doctest-warnings, so that flag shouldn't be used here. Maybe we could add another lint or something? But like I said, it's less of an issue now.

[dylni]

@camelid Thanks, I updated the option name. However, I am still not seeing a warning for [`&[String]`], which was my primary concern. It would be ideal if bracketed text could either be properly linked or give a warning in all cases.

[camelid]

I've changed the title since --display-doctest-warnings is being removed and is not the right way to expose this anyway.

[dylni]

That's great. Enabling the warnings by default is much better.

[camelid]

That's great. Enabling the warnings by default is much better.

I see the advantage of enabling them by default, but rustdoc intentionally silently ignores some links to avoid tons of lints being emitted for existing docs.

[jyn514]

This is a duplicate of #54191.

[dylni]

rustdoc intentionally silently ignores some links

Is there a plan to allow requesting stricter behavior when --display-doctest-warnings is removed? I read through #73314, but there seems to be some confusion about the many purposes the option serves, which I assume is why it's being removed.

[jyn514]

@dylni what do you mean by stricter behavior? If you mean what's in this issue, I don't think it's a good idea for rustdoc to decide whether to resolve a link based on an option; it already does that for --document-private-items and it's pretty confusing IMO. If you mean something else, please open a new issue; I'm ok with adding additional lints but I'm not a fan of additional configuration.

[dylni]

@jyn514 camelid mentioned

--display-doctest-warnings is being removed and is not the right way to expose this anyway.

I was wondering if there was a particular replacement planned for the option. In the past, it enabled warnings when intra-doc links failed to resolve, but that part seems to be enabled by default now. I was wondering if there's any additional strictness/warnings that would be lost by removing the option.

I don't think it's a good idea for rustdoc to decide whether to resolve a link based on an option

I agree. It doesn't matter to me if these links are resolved or fail. I would just prefer a warning when they're unresolved.

[camelid]

I was wondering if there was a particular replacement planned for the option. In the past, it enabled warnings when intra-doc links failed to resolve, but that part seems to be enabled by default now. I was wondering if there's any additional strictness/warnings that would be lost by removing the option.

Hmm, I didn't realize the flag used to enable intra-doc warnings. My understanding was that it was always used to show warnings in doctests (i.e., ``` code examples).

[dylni]

Hmm, I didn't realize the flag used to enable intra-doc warnings.

That was the only reason I mentioned it in this issue. IIRC, at the time this issue was created, not using --display-warnings caused no warnings to be displayed for all broken intra-doc links.

[jyn514]

IIRC, at the time this issue was created, not using --display-warnings caused no warnings to be displayed for all broken intra-doc links.

That can't be correct. I started working on intra-doc links about 2 months before you opened the issue and they still gave warnings at that time; AFAIK they've never been behind any sort of feature gate other than originally only being available on nightly.

[jyn514]

Anyway, I think this is a distraction. CLI options seem like the wrong model for this and I don't think we should add more; this should be a new lint if we add it at all.

[jyn514]

I'm going to close this in favor of #54191.

[dylni]

AFAIK they've never been behind any sort of feature gate.

It's entirely possible this is correct, and you would know better than I. That is just what I recall.

Anyway, I think this is a distraction.

Right, this got a little off-topic.

I'm going to close this in favor of #54191.

Thanks for pointing out that issue!

[camelid]

@dylni Perhaps you were thinking of --deny warnings?

[dylni]

@camelid After investigating a bit, I think I may have falsely attributed the option with showing intra-doc warnings in addition to doctest warnings. Cached compilation not re-issuing warnings was likely what I was seeing instead. Sorry for the confusion.