Report an ambiguity if both modules and primitives are in scope for intra-doc links

[jyn514]

Closes #75381

• Add a new prim@ disambiguator, since both modules and primitives are in the same namespace
• Refactor report_ambiguity into a closure

Additionally, I noticed that rustdoc would previously allow [struct@char] if char resolved to a primitive (not if it had a DefId). I fixed that and added a test case.

I also need to update libstd to use prim@char instead of type@char. If possible I would also like to refactor ambiguity_error to use Disambiguator instead of its own hand-rolled match - that ran into issues with prim@ (I updated one and not the other) and it would be better for them to be in sync.

[rust-highfive]

Some changes occurred in intra-doc-links.

cc @jyn514

[rust-highfive]

r? @GuillaumeGomez

(rust_highfive has picked a reviewer for you, use r? to override)

[jyn514]

Wow, that's more errors than I expected.

error: `char` is both a module and a builtin type
    --> library/core/src/str/mod.rs:4231:62
     |
4231 |     /// The [pattern] can be a `&str`, [`char`], a slice of [`char`]s, or a
     |                                                              ^^^^^^ ambiguous link
     |
help: to link to the module, prefix with the item type
     |
4231 |     /// The [pattern] can be a `&str`, [`char`], a slice of [`module@char`]s, or a
     |                                                              ^^^^^^^^^^^^^
help: to link to the builtin type, prefix with the item type
     |
4231 |     /// The [pattern] can be a `&str`, [`char`], a slice of [`prim@char`]s, or a
     |                                                              ^^^^^^^^^^^

error: aborting due to 62 previous errors

[jyn514]

If possible I would also like to refactor ambiguity_error to use Disambiguator instead of its own hand-rolled match - that ran into issues with prim@ (I updated one and not the other) and it would be better for them to be in sync.

I just realized ambiguity_error also duplicates the 'use this disambiguator' text but better 🤦 fixing it will be a larger change than I expected so saving it for a follow-up PR.

[jyn514]

This is ready for review.

[GuillaumeGomez]

Thanks!

@bors: r+

[bors]

📌 Commit cdffc57b2c5f0c494b4b94726088b3a228f5f2cc has been approved by GuillaumeGomez

[bors]

⌛ Testing commit cdffc57b2c5f0c494b4b94726088b3a228f5f2cc with merge 907ba2b614a2d1b1384fc7d0a8a987e676d5d184...

[rust-log-analyzer]

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

+ /scripts/validate-toolstate.sh
Cloning into 'rust-toolstate'...
<Nothing changed>
Traceback (most recent call last):
  File "../../src/tools/publish_toolstate.py", line 283, in <module>
    validate_maintainers(repo, github_token)
  File "../../src/tools/publish_toolstate.py", line 84, in validate_maintainers
    'Accept': 'application/vnd.github.hellcat-preview+json',
  File "/usr/lib/python3.6/urllib/request.py", line 223, in urlopen
    return opener.open(url, data, timeout)
  File "/usr/lib/python3.6/urllib/request.py", line 532, in open
    response = meth(req, response)
  File "/usr/lib/python3.6/urllib/request.py", line 642, in http_response
    'http', request, response, code, msg, hdrs)
  File "/usr/lib/python3.6/urllib/request.py", line 570, in error
    return self._call_chain(*args)
  File "/usr/lib/python3.6/urllib/request.py", line 504, in _call_chain
    result = func(*args)
  File "/usr/lib/python3.6/urllib/request.py", line 650, in http_error_default
    raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 403: Forbidden
  local time: Sun Aug 23 16:57:45 UTC 2020
  network time: Sun, 23 Aug 2020 16:57:46 GMT
== end clock drift check ==
== end clock drift check ==
##[error]Process completed with exit code 1.
Terminate orphan process: pid (9567) (node)
Terminate orphan process: pid (9595) (python)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

[bors]

💔 Test failed - checks-actions

[jyn514]

Network error.

@bors retry

Although I expect this to conflict with some of the intra-doc link PRs.

[bors]

💔 Test failed - checks-actions

[jyn514]

@bors r=guillaumegomez

[bors]

📌 Commit 25cfd57 has been approved by guillaumegomez

[bors]

⌛ Testing commit 25cfd57 with merge aa7010d...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: guillaumegomez
Pushing aa7010d to master...

[chris-morgan]

I very much dislike this use of “prim”, which feels unprecedented as a keyword (I dunno, maybe the compiler uses it, maybe not, but as a user of the language I haven’t seen anyone do it this way). Could we change it to the full word “primitive”? That would match Rust’s general style of identifiers and the filenames rustdoc emits for the primitives (primitive.u8.html, &c.).

[jyn514]

I very much dislike this use of “prim”, which feels unprecedented as a keyword (I dunno, maybe the compiler uses it, maybe not, but as a user of the language I haven’t seen anyone do it this way). Could we change it to the full word “primitive”? That would match Rust’s general style of identifiers and the filenames rustdoc emits for the primitives (primitive.u8.html, &c.).

@chris-morgan you can already use primitive@ and it will work fine: https://github.com/rust-lang/rust/pull/75815/files#diff-2bcad7b16b56ef2ebb92f8e60ae33773R1041

[chris-morgan]

OK, I didn’t read carefully enough to notice that; thanks for pointing it out.

But my point still mostly stands, because the I-think-unprecedented “prim” is still both supported and what the note suggests. If I submit another PR that drops prim in favour of primitive, would it be accepted?

[jyn514]

If you drop prim, go ahead and drop the rest of the duplicate disambiguators at the same time: #74430 (comment)

I don't feel strongly either way, I think it would need FCP though since they already exist on nightly but haven't hit stable. I can start an FCP if you make a PR.

[jyn514]

@chris-morgan are you still interested in working on this? The beta branch is in a couple weeks and I'd prefer to make breaking changes before intra-doc links stabilize.