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.

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

Highlight edition-specific keywords correctly in code blocks, accounting for code block edition modifiers #80226

Merged
merged 1 commit into from
Dec 25, 2020
Merged

Conversation

ThePuzzlemaker
Copy link
Contributor

Previously, edition-specific keywords (such as async and await) were not highlighted in code blocks, regardless of what edition was set. With this PR, this issue is fixed.

Now, the following behavior happens:

  • When a code block is explicitly set to edition X, keywords from edition X are highlighted
  • When a code block is explicitly set to a version that does not contain those keywords from edition X (e.g. edition Y), keywords from edition X are not highlighted
  • When a code block has no explicit edition, keywords from the edition passed via --edition to rustdoc are highlighted

For example, a project set with edition = "2015" in its Cargo.toml would not highlight async/await unless the code block was set to edition2018. Additionally, a project set with edition = "2018" in its Cargo.toml would highlight async/await unless the code block was set to a version that did not contain those keywords (e.g. edition2015).

This PR fixes #80004.

r? @jyn514

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 20, 2020
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-9 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
.................................................................................................... 9000/11189
.................................................................................................... 9100/11189
................................................................................i......i............ 9200/11189
.................................................................................................... 9300/11189
...................iiiiii..iiiiii.i................................................................. 9400/11189
.................................................................................................... 9600/11189
.................................................................................................... 9700/11189
..........................................................................................test [ui] ui/issues/issue-74564-if-expr-stack-overflow.rs has been running for over 60 seconds
.......... 9800/11189
---
Suite("src/test/assembly") not skipped for "bootstrap::test::Assembly" -- not in ["src/tools/tidy"]
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 27 tests
iiiiiiiiiiiiiiiiiiiiiiiiiii

Suite("src/test/incremental") not skipped for "bootstrap::test::Incremental" -- not in ["src/tools/tidy"]
 finished in 0.071 seconds
Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
Suite("src/test/debuginfo") not skipped for "bootstrap::test::Debuginfo" -- not in ["src/tools/tidy"]
Check compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 116 tests
iiiiiiiiii.i.i...i..i..ii....ii....ii..........iiii.........i.....i...i.......ii.i.ii.....iiii.....i 100/116
test result: ok. 78 passed; 0 failed; 38 ignored; 0 measured; 0 filtered out; finished in 2.39s

 finished in 2.458 seconds
Suite("src/test/ui-fulldeps") not skipped for "bootstrap::test::UiFullDeps" -- not in ["src/tools/tidy"]
---
   Compiling once_cell v1.4.1
   Compiling expect-test v1.0.1
   Compiling rustdoc v0.0.0 (/checkout/src/librustdoc)
error[E0061]: this function takes 3 arguments but 2 arguments were supplied
  --> src/librustdoc/html/highlight/tests.rs:21:9
   |
21 |         write_code(&mut out, src);
   |         |
   |         expected 3 arguments
   |
note: function defined here
note: function defined here
  --> src/librustdoc/html/highlight.rs:50:4
   |
50 | fn write_code(out: &mut String, src: &str, edition: Edition) {

error[E0061]: this function takes 3 arguments but 2 arguments were supplied
error[E0061]: this function takes 3 arguments but 2 arguments were supplied
  --> src/librustdoc/html/highlight/tests.rs:33:5
33 |     write_code(&mut html, src);
   |     ^^^^^^^^^^ ---------  --- supplied 2 arguments
   |     |
   |     expected 3 arguments
   |     expected 3 arguments
   |
note: function defined here
  --> src/librustdoc/html/highlight.rs:50:4
   |
50 | fn write_code(out: &mut String, src: &str, edition: Edition) {

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0061`.
For more information about this error, try `rustc --explain E0061`.
error: could not compile `rustdoc`

To learn more, run the command again with --verbose.


command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--manifest-path" "/checkout/src/tools/rustdoc/Cargo.toml" "-p" "rustdoc:0.0.0" "--" "--quiet"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test --exclude src/tools/tidy
Build completed unsuccessfully in 0:29:20

@petrochenkov petrochenkov self-assigned this Dec 20, 2020
@jyn514 jyn514 added A-frontend Area: frontend (errors, parsing and HIR) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 20, 2020
@jyn514
Copy link
Member

jyn514 commented Dec 20, 2020

Hmm, I see - the issue is that the edition is not necessarily related to the span, so even if you pass in item.source, someone could have an edition2018 lang string on the code block.

The rustdoc side of things seems reasonable to me but I think this should have a reviewer from T-compiler.

r? @petrochenkov

@jyn514
Copy link
Member

jyn514 commented Dec 20, 2020

Also you need to fix the unit tests:

error[E0061]: this function takes 3 arguments but 2 arguments were supplied
  --> src/librustdoc/html/highlight/tests.rs:21:9
   |
21 |         write_code(&mut out, src);
   |         ^^^^^^^^^^ --------  --- supplied 2 arguments
   |         |
   |         expected 3 arguments

@jyn514 jyn514 added A-doctests Area: Documentation tests, run by rustdoc T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 20, 2020
@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Dec 20, 2020

Hmm, I see - the issue is that the edition is not necessarily related to the span, so even if you pass in item.source, someone could have an edition2018 lang string on the code block.

@jyn514 I'm not sure what you're talking about. The only time I think I can see something like item.source is in this code: https://github.com/rust-lang/rust/blob/ffa98d6fb1144ca42ee92d0465b7bc1b669d64c1/src/librustdoc/html/render/mod.rs#L4717-L4728
I'm pretty sure that this code is only for highlighting macro definitions (e.g. at the top of dbg!).

@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Dec 20, 2020

As for code blocks, they use either the explicit edition attribute or the edition given by --edition.
At least, that's how I've interpreted the code (please correct me though if I'm wrong).

@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 21, 2020

One more question - I assumed that rustdoc "compiles" separate code blocks as independent programs, creating independent compilation sessions, so each session can have its own default edition possibly depending on the block's annotations (which will be used by Ident::from_str(text).is_reserved()).
Why doesn't it happen here?

@ThePuzzlemaker
Copy link
Contributor Author

One more question - I assumed that rustdoc "compiles" separate code blocks as independent programs, creating independent compilation sessions, so each session can have its own default edition possibly depending on the block's annotations (which will be used by Ident::from_str(text).is_reserved()).
Why doesn't it happen here?

@petrochenkov I’m pretty sure this is just simple lexing for highlighting and rendering—individual sessions for each code block/doctest would probably be part of the doctest logic rather than rendering as that’d seem expensive for simple lexing.

@petrochenkov petrochenkov added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 21, 2020
@rust-log-analyzer

This comment has been minimized.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 22, 2020
rustc_span: Provide a reserved identifier check for a specific edition

while keeping edition evaluation lazy because it may be expensive.

Needed for rust-lang#80226.
@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Dec 23, 2020

#80272 merged; no longer blocked. Will update and re-test.
@rustbot modify labels: -S-blocked

@rustbot rustbot removed the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Dec 23, 2020
@ThePuzzlemaker
Copy link
Contributor Author

Updated, rebased, and squashed. Working again 😄

@ThePuzzlemaker
Copy link
Contributor Author

I used Symbol::intern(...).is_reserved rather than Ident::from_str(...).name.is_reserved to prevent unnecessary uses of a bunch of dummy spans

@petrochenkov
Copy link
Contributor

@bors r=jyn514,petrochenkov

@bors
Copy link
Contributor

bors commented Dec 23, 2020

📌 Commit d14ef17 has been approved by jyn514,petrochenkov

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 23, 2020
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 23, 2020
…yn514,petrochenkov

Highlight edition-specific keywords correctly in code blocks, accounting for code block edition modifiers

Previously, edition-specific keywords (such as `async` and `await`) were not highlighted in code blocks, regardless of what edition was set. With this PR, this issue is fixed.

Now, the following behavior happens:
- When a code block is explicitly set to edition X, keywords from edition X are highlighted
- When a code block is explicitly set to a version that does not contain those keywords from edition X (e.g. edition Y), keywords from edition X are **not** highlighted
- When a code block has no explicit edition, keywords from the edition passed via `--edition` to rustdoc are highlighted

For example, a project set with `edition = "2015"` in its `Cargo.toml` would not highlight `async`/`await` unless the code block was set to `edition2018`. Additionally, a project set with `edition = "2018"` in its `Cargo.toml` *would* highlight `async`/`await` unless the code block was set to a version that did not contain those keywords (e.g. `edition2015`).

This PR fixes rust-lang#80004.

r? `@jyn514`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 24, 2020
…yn514,petrochenkov

Highlight edition-specific keywords correctly in code blocks, accounting for code block edition modifiers

Previously, edition-specific keywords (such as `async` and `await`) were not highlighted in code blocks, regardless of what edition was set. With this PR, this issue is fixed.

Now, the following behavior happens:
- When a code block is explicitly set to edition X, keywords from edition X are highlighted
- When a code block is explicitly set to a version that does not contain those keywords from edition X (e.g. edition Y), keywords from edition X are **not** highlighted
- When a code block has no explicit edition, keywords from the edition passed via `--edition` to rustdoc are highlighted

For example, a project set with `edition = "2015"` in its `Cargo.toml` would not highlight `async`/`await` unless the code block was set to `edition2018`. Additionally, a project set with `edition = "2018"` in its `Cargo.toml` *would* highlight `async`/`await` unless the code block was set to a version that did not contain those keywords (e.g. `edition2015`).

This PR fixes rust-lang#80004.

r? ```@jyn514```
@Dylan-DPC-zz
Copy link

has conflicts

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 24, 2020
@ThePuzzlemaker
Copy link
Contributor Author

Rebased and fixed merge conflicts.

@rust-log-analyzer

This comment has been minimized.

…, accounting for code block edition modifiers

This is a squash of these commits:
- Highlight edition-specific keywords correctly in code blocks,
accounting for code block edition modifiers
- Fix unit tests
- Revert changes to rustc_span::symbol to prepare for merge of #80272
- Use new Symbol::is_reserved API from #80272
- Remove unused import added by accident when merging
@ThePuzzlemaker
Copy link
Contributor Author

Fixed.

@petrochenkov
Copy link
Contributor

@bors r=jyn514,petrochenkov

@bors
Copy link
Contributor

bors commented Dec 24, 2020

📌 Commit db1451c has been approved by jyn514,petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 24, 2020
@bors
Copy link
Contributor

bors commented Dec 25, 2020

⌛ Testing commit db1451c with merge ab10778...

@bors
Copy link
Contributor

bors commented Dec 25, 2020

☀️ Test successful - checks-actions
Approved by: jyn514,petrochenkov
Pushing ab10778 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 25, 2020
@bors bors merged commit ab10778 into rust-lang:master Dec 25, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 25, 2020
@ThePuzzlemaker ThePuzzlemaker deleted the issue-80004-fix branch December 25, 2020 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-doctests Area: Documentation tests, run by rustdoc A-frontend Area: frontend (errors, parsing and HIR) merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

async and other Rust 2018+ keywords not recognized as so in Rustdoc highlighting
8 participants