book icon indicating copy to clipboard operation
book copied to clipboard

the implied `edition` for playground needs to be `2021` in ch09-02 (at least)

Open correabuscar opened this issue 1 year ago • 4 comments
trafficstars

  • I have searched open and closed issues and pull requests for duplicates, using these search terms:
    • 2021
    • ch9 (found this https://github.com/rust-lang/book/issues/1262 )
    • ch09
  • I have checked the latest main branch to see if this has already been fixed, in this file:
    • https://github.com/rust-lang/book/blob/ee5c7ec70496cc625b467012c44caa167eb60309/src/ch09-02-recoverable-errors-with-result.md?plain=1#L65C23-L65C76
    • https://github.com/rust-lang/book/blob/ee5c7ec70496cc625b467012c44caa167eb60309/listings/ch09-error-handling/listing-09-04/src/main.rs#L1

URL to the section(s) of the book with this problem: https://doc.rust-lang.org/book/ch09-02-recoverable-errors-with-result.html#recoverable-errors-with-result https://doc.rust-lang.org/nightly/book/ch09-02-recoverable-errors-with-result.html#recoverable-errors-with-result

Description of the problem: There's an example there, this one:

use std::fs::File;

fn main() {
    let greeting_file_result = File::open("hello.txt");

    let greeting_file = match greeting_file_result {
        Ok(file) => file,
        Err(error) => panic!("Problem opening the file: {error:?}"),
    };
}

which when you press the Play button, it clearly uses one of 2015 or 2018 editions instead of the 2021 one because the errors are these:

Compiling playground v0.0.1 (/playground)
warning: unused variable: `greeting_file`
 --> src/main.rs:6:9
  |
6 |     let greeting_file = match greeting_file_result {
  |         ^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_greeting_file`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `error`
 --> src/main.rs:8:13
  |
8 |         Err(error) => panic!("Problem opening the file: {error:?}"),
  |             ^^^^^ help: if this is intentional, prefix it with an underscore: `_error`

warning: panic message contains an unused formatting placeholder
 --> src/main.rs:8:57
  |
8 |         Err(error) => panic!("Problem opening the file: {error:?}"),
  |                                                         ^^^^^^^^^
  |
  = note: this message is not used as a format string when given without arguments, but will be in Rust 2021
  = note: `#[warn(non_fmt_panics)]` on by default
help: add the missing argument
  |
8 |         Err(error) => panic!("Problem opening the file: {error:?}", ...),
  |                                                                   +++++
help: or add a "{}" format string to use the message literally
  |
8 |         Err(error) => panic!("{}", "Problem opening the file: {error:?}"),
  |                              +++++

warning: `playground` (bin "playground") generated 3 warnings (run `cargo fix --bin "playground"` to apply 1 suggestion)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.74s
     Running `target/debug/playground`
thread 'main' panicked at src/main.rs:8:23:
Problem opening the file: {error:?}
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Also note that there's no button to open that code block in playground, there's only copy code and play button(which shows the output inline, while viewing the book), so to try it in playground, one must manually open the already-known playground url then paste the copied code there and see that it actually works (due to 2021 edition being default in playground).

Here's 2018 edition on playground Here's 2021 edition on playground The latter works as expected.

Suggested fix: I don't know where this edition is enforced for playground(it doesn't seem like this knows), but, at least for this example, it should be edition=2021, because 2015 and 2018 show the above warnings and as you can see, it doesn't display correct output Problem opening the file: {error:?} which should be Problem opening the file: Os { code: 2, kind: NotFound, message: "No such file or directory" }.

I've checked the linked file and it is in a project with edition 2021, however, that doesn't mean that the play button knows to use that edition for playground.

It would seem that the workaround[^1] is quite possibly adding edition2021 here just like it's done in https://github.com/rust-lang/book/blob/ee5c7ec70496cc625b467012c44caa167eb60309/src/ch04-02-references-and-borrowing.md?plain=1#L173 [^1]: it's just a workaround because who knows in how many other places this is needed; ideally, that edition value would be gotten from Cargo.toml instead, somehow, everywhere where there's a play button...

A built book.js has 2015 as the default edition:

        let text = playground_text(code_block);
        let classes = code_block.querySelector('code').classList;
        let edition = "2015";
        if(classes.contains("edition2018")) {
            edition = "2018";
        } else if(classes.contains("edition2021")) {
            edition = "2021";
        }

well this default is from mdBook and been there since 2021 or earlier.

correabuscar avatar Jul 08 '24 07:07 correabuscar

Thanks for reporting this! I agree that we should make sure that all the playground links get generated using the edition in the Cargo.toml file. We can do that using the relevant mdBook config, and then we’ll also need to audit to make sure everything still works as expected.

chriskrycho avatar Jul 10 '24 18:07 chriskrycho

Thanks for reporting this! I agree that we should make sure that all the playground links get generated using the edition in the Cargo.toml file. We can do that using the relevant mdBook config, and then we’ll also need to audit to make sure everything still works as expected.

How interesting! Thank you for this unexpected(to me) suggestion!

A quick test with it in root bool.toml, like:

diff --git a/book.toml b/book.toml
index b73ff03b7a272f44..03f18be9de120c3c 100644
--- a/book.toml
+++ b/book.toml
@@ -15,3 +15,7 @@ git-repository-url = "https://github.com
 
 [preprocessor.trpl-listing]
 output-mode = "default"
+
+[rust]
+edition="2021"
+

shows 618 changes, that would, apparently, have to be tested(or audited as you say) if they still work as intended. But the original issue, in OP, is definitely fixed with this as:

   Compiling playground v0.0.1 (/playground)
warning: unused variable: `greeting_file`
 --> src/main.rs:6:9
  |
6 |     let greeting_file = match greeting_file_result {
  |         ^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_greeting_file`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: `playground` (bin "playground") generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.62s
     Running `target/debug/playground`
thread 'main' panicked at src/main.rs:8:23:
Problem opening the file: Os { code: 2, kind: NotFound, message: "No such file or directory" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

That warning is still not ideal, but that's another matter.

The other books have their own book.toml which I didn't try to change:

./first-edition/book.toml
./2018-edition/book.toml
./nostarch/book.toml
./second-edition/book.toml

from what I can tell, only the last two might be candidates for change as well, but I know nothing about them.

correabuscar avatar Jul 11 '24 11:07 correabuscar

I merged a baseline change for this in #3974, as it passed CI so things which have a hard requirement on it all pass. We likely still want to do a further pass to check the behavior of the other listings, and we will also want to figure out why this one (and any others like it!) passed CI (which is the really 😬 bit).

chriskrycho avatar Jul 16 '24 14:07 chriskrycho

we will also want to figure out why this one (and any others like it!) passed CI

it doesn't fail because they're only warnings not errors, when using edition 2015 instead of edition 2021.

It would need a #![deny(rust_2021_compatibility)] for that specific example(in OP) to fail, if edition 2015 or 2018 were used. However, I haven't found a global way to deny that for ALL examples.


I guess mdbook test is the one that's supposed to fail? and yet it does not fail even with:

diff --git a/book.toml b/book.toml
index 800adcf0f5bcd753..90f9cdebce43fff7 100644
--- a/book.toml
+++ b/book.toml
@@ -17,4 +17,4 @@ git-repository-url = "https://github.com
 output-mode = "default"
 
 [rust]
-edition = "2021"
+edition = "2015"

So we can't trust it. A preliminary test I did shows that the edition isn't used at all by mdbook test (so it must be using whatever the default edition is for the current rustc, unsure) , I'll look more into it.

Ok so apparently mdbook test handles the {#rustdoc_include ../listings/ch09-error-handling/listing-09-04/src/main.rs} which makes the code become:

```rust,should_panic
#![allow(unused_variables)]
use std::fs::File;

fn main() {
    let greeting_file_result = File::open("hello.txt");

    let greeting_file = match greeting_file_result {
        Ok(file) => file,
        Err(error) => panic!("Problem opening the file: {error:?}"),
    };
}
\```

and then mdbook test calls out to rustdoc (as per this info) which is a call like this: /tmp/mdbook-F1QUCF/ch09-02-recoverable-errors-with-result.md --test --edition 2021 (the 2021 is the one from book.toml), this seems to be enough in theory, but for some reason rustdoc doesn't fail because they're only warnings not errors, when using edition 2015 instead of edition 2021.

correabuscar avatar Jul 16 '24 15:07 correabuscar

I just circled back to this and checked on things, and I think the fix I landed to handle this did indeed handle everything we needed here. We will need to remember to hit that on future updates, like when the 2024 Edition lands in a few months, so I opened #4064 to help us remember! Going to go ahead and close this out now. Thanks again!

chriskrycho avatar Oct 10 '24 13:10 chriskrycho