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

rustdoc: add an --edition flag to compile docs/doctests with a certain edition #49451

Merged
merged 4 commits into from
Apr 1, 2018

Conversation

QuietMisdreavus
Copy link
Member

To correspond with the 2018 edition, this adds a (currently unstable) --edition flag to rustdoc that makes it compile crates and doctests with the given edition. Once this lands, Cargo should be updated to pass this flag when the edition configuration option is given.

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 28, 2018
@QuietMisdreavus
Copy link
Member Author

r? @GuillaumeGomez

@GuillaumeGomez
Copy link
Member

Once CI fixed, r=me.

@QuietMisdreavus
Copy link
Member Author

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Mar 29, 2018

📌 Commit 97aead0 has been approved by GuillaumeGomez

@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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2018
@bors
Copy link
Contributor

bors commented Mar 30, 2018

⌛ Testing commit 97aead0 with merge 7ae31e9...

bors added a commit that referenced this pull request Mar 30, 2018
rustdoc: add an --edition flag to compile docs/doctests with a certain edition

To correspond with the 2018 edition, this adds a (currently unstable) `--edition` flag to rustdoc that makes it compile crates and doctests with the given edition. Once this lands, Cargo should be updated to pass this flag when the edition configuration option is given.
@bors
Copy link
Contributor

bors commented Mar 31, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 31, 2018
@TimNN
Copy link
Contributor

TimNN commented Mar 31, 2018

Your PR failed on Travis. 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.
The job exceeded the maximum time limit for jobs, and has been terminated.

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 @TimNN.

1 similar comment
@TimNN
Copy link
Contributor

TimNN commented Mar 31, 2018

Your PR failed on Travis. 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.
The job exceeded the maximum time limit for jobs, and has been terminated.

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 @TimNN.

@kennytm
Copy link
Member

kennytm commented Apr 1, 2018

@bors retry

3-hour timeout with x86_64-gnu-incremental.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 1, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Apr 1, 2018
…uillaumeGomez

rustdoc: add an --edition flag to compile docs/doctests with a certain edition

To correspond with the 2018 edition, this adds a (currently unstable) `--edition` flag to rustdoc that makes it compile crates and doctests with the given edition. Once this lands, Cargo should be updated to pass this flag when the edition configuration option is given.
bors added a commit that referenced this pull request Apr 1, 2018
Rollup of 3 pull requests

- Successful merges: #49451, #49498, #49549
- Failed merges:
@bors
Copy link
Contributor

bors commented Apr 1, 2018

⌛ Testing commit 97aead0 with merge 06fa27d...

@bors bors merged commit 97aead0 into rust-lang:master Apr 1, 2018
@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 1, 2018
@QuietMisdreavus QuietMisdreavus deleted the epoch-doctests branch April 2, 2018 18:51
@rust-lang rust-lang deleted a comment from bors Apr 15, 2018
@wigy-opensource-developer
Copy link
Contributor

What is missing to get this --edition flag onto stable? 1 year later I still need to use edition2018 on doctests in a crate that has edition="2018" in its Cargo.toml

@QuietMisdreavus
Copy link
Member Author

This flag was stabilized alongside the 2018 edition in #54057. Cargo was passing it to rustdoc based on the Cargo.toml even earlier, starting in rust-lang/cargo#5299. What problems are you having?

@wigy-opensource-developer
Copy link
Contributor

wigy-opensource-developer commented Apr 3, 2019

I have a doctest in an edition2018 crate like:

/// ```
/// # use crate::SomeStruct;
/// let s= SomeStruct::new();
/// ```

which did not find SomeStruct without adding the edition2018 flag explicitly to each doctest with a use crate::. So this should work without the flag since 1.32 latest?

@QuietMisdreavus
Copy link
Member Author

Is that the entire doctest? Where is SomeStruct defined? Doctests are compiled as separate standalone binaries that link to their containing crate, so that exact example shouldn't work in either case. What error are you getting?

@wigy-opensource-developer
Copy link
Contributor

wigy-opensource-developer commented Apr 3, 2019

I have compiled this minimal example, please run it with cargo test --doc:

#![allow(dead_code)]

pub struct SomeStruct {}

impl SomeStruct {
    /// Always returns true
    ///
    /// # Example
    ///
    /// ```edition2018
    /// # use crate::SomeStruct;
    /// let s = SomeStruct {};
    /// assert!(!s.foo());
    /// ```
    pub fn foo(&self) -> bool {
        true
    }
}

fn main() {}

Playground

Works fine without any errors. If I remove edition2018 from line 10 though, it fails with

error[E0432]: unresolved import `crate::SomeStruct`
 --> src/lib.rs:11:5
  |
3 | use crate::SomeStruct;
  |     ^^^^^^^^^^^^^^^^^ no `SomeStruct` in the root

If I go back to the edition2015 import format, that also fixes the error:

#![allow(dead_code)]

pub struct SomeStruct {}

impl SomeStruct {
    /// Always returns true
    ///
    /// # Example
    ///
    /// ```
    /// # use playground::SomeStruct;
    /// let s = SomeStruct {};
    /// assert!(s.foo());
    /// ```
    pub fn foo(&self) -> bool {
        true
    }
}

fn main() {}

Playground

@QuietMisdreavus
Copy link
Member Author

Adding just edition2018 to the doctest actually causes the test to be ignored. Using the proper rust,edition2018 marker causes rustdoc to give the proper output:

$ rustdoc +stable --test f.rs --extern f=libf.rlib

running 1 test
test f.rs - SomeStruct::foo (line 10) ... FAILED

failures:

---- f.rs - SomeStruct::foo (line 10) stdout ----
error[E0432]: unresolved import `crate::SomeStruct`
 --> f.rs:11:5
  |
4 | use crate::SomeStruct;
  |     ^^^^^^^^^^^^^^^^^ no `SomeStruct` in the root

thread 'f.rs - SomeStruct::foo (line 10)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:321:13
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.


failures:
    f.rs - SomeStruct::foo (line 10)

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

Note that this is the same as taking the marker off the doctest and compiling with --edition=2018 passed directly to rustdoc:

$ rustdoc +stable --test f.rs --extern f=libf.rlib --edition=2018

running 1 test
test f.rs - SomeStruct::foo (line 10) ... FAILED

failures:

---- f.rs - SomeStruct::foo (line 10) stdout ----
error[E0432]: unresolved import `crate::SomeStruct`
 --> f.rs:11:5
  |
4 | use crate::SomeStruct;
  |     ^^^^^^^^^^^^^^^^^ no `SomeStruct` in the root

thread 'f.rs - SomeStruct::foo (line 10)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:321:13
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.


failures:
    f.rs - SomeStruct::foo (line 10)

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

For completeness, here is the output i received with your original sample:

$ rustdoc +stable --test f.rs --extern f=libf.rlib

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

@QuietMisdreavus
Copy link
Member Author

QuietMisdreavus commented Apr 3, 2019

The reason the test fails anyway is because the path crate::SomeStruct is not a valid path within that doctest - it's a separate crate, and SomeStruct is not declared within that "crate". You have to use the name of your original crate to properly reference SomeStruct.

EDIT: It's also worth noting that even playground::SomeStruct isn't a valid path for that doctest, since SomeStruct isn't even publicly exported in the source crate.

@wigy-opensource-developer
Copy link
Contributor

wigy-opensource-developer commented Apr 3, 2019

Wow, that was a big oversight from me. 🙍‍♂️ Thanks for helping out. Is there a way to reduce sensitivity of doctest code to crate renames? I tried super:: without success, and it feels wrong to copy-paste the crate name all around its own crate. (the whole reason why edition 2018 introduced the crate path)

@QuietMisdreavus
Copy link
Member Author

Not that i know of - since the doctests are meant to be example code listed in documentation, it fits that they would need to use the public name for everything. (And the setup of always being compiled as a standalone binary means that no "relative" path would work, either - the items in the original crate are always treated as an external crate.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants