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

feat(toml): Warn on unset Edition #13505

Merged
merged 6 commits into from
Mar 1, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix(toml): Give MSRV-aware guidance
epage committed Feb 29, 2024
commit 768c70a744429f678a7cd6e88e9670f833e5e58f
11 changes: 10 additions & 1 deletion src/cargo/core/features.rs
Original file line number Diff line number Diff line change
@@ -180,9 +180,12 @@ pub type AllowFeatures = BTreeSet<String>;
/// [`is_stable`]: Edition::is_stable
/// [`toml::to_real_manifest`]: crate::util::toml::to_real_manifest
/// [`features!`]: macro.features.html
#[derive(Clone, Copy, Debug, Hash, PartialOrd, Ord, Eq, PartialEq, Serialize, Deserialize)]
#[derive(
Default, Clone, Copy, Debug, Hash, PartialOrd, Ord, Eq, PartialEq, Serialize, Deserialize,
)]
pub enum Edition {
/// The 2015 edition
#[default]
Edition2015,
/// The 2018 edition
Edition2018,
@@ -199,6 +202,12 @@ impl Edition {
pub const LATEST_UNSTABLE: Option<Edition> = Some(Edition::Edition2024);
/// The latest stable edition.
pub const LATEST_STABLE: Edition = Edition::Edition2021;
pub const ALL: &'static [Edition] = &[
Self::Edition2015,
Self::Edition2018,
Self::Edition2021,
Self::Edition2024,
];
/// Possible values allowed for the `--edition` CLI flag.
///
/// This requires a static value due to the way clap works, otherwise I
34 changes: 30 additions & 4 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ use crate::AlreadyPrintedError;
use anyhow::{anyhow, bail, Context as _};
use cargo_platform::Platform;
use cargo_util::paths;
use cargo_util_schemas::core::PartialVersion;
use cargo_util_schemas::manifest;
use cargo_util_schemas::manifest::RustVersion;
use itertools::Itertools;
@@ -595,12 +596,37 @@ pub fn to_real_manifest(
}
edition
} else {
let msrv_edition = if let Some(rust_version) = &rust_version {
Edition::ALL
.iter()
.filter(|e| {
e.first_version()
.map(|e| {
let e = PartialVersion::from(e);
e <= **rust_version
})
.unwrap_or_default()
})
.max()
.copied()
} else {
None
}
.unwrap_or_default();
let default_edition = Edition::default();
let latest_edition = Edition::LATEST_STABLE;

let tip = if msrv_edition == default_edition {
String::new()
} else if msrv_edition == latest_edition {
format!(" while the latest is {latest_edition}")
} else {
format!(" while {msrv_edition} is compatible with `rust-version`")
};
warnings.push(format!(
"no edition set: defaulting to the {} edition while the latest is {}",
Edition::Edition2015,
Edition::LATEST_STABLE
"no edition set: defaulting to the {default_edition} edition{tip}",
));
Edition::Edition2015
default_edition
};
// Add these lines if start a new unstable edition.
// ```
29 changes: 28 additions & 1 deletion tests/testsuite/edition.rs
Original file line number Diff line number Diff line change
@@ -123,6 +123,33 @@ fn edition_unstable() {
.run();
}

#[cargo_test]
fn unset_edition_works_with_no_newer_compatible_edition() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = 'foo'
version = '0.1.0'
rust-version = "1.0"
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check -v")
.with_stderr(
"\
[WARNING] no edition set: defaulting to the 2015 edition
[CHECKING] foo [..]
[RUNNING] `rustc [..] --edition=2015 [..]`
[FINISHED] [..]
",
)
.run();
}

#[cargo_test]
fn unset_edition_works_on_old_msrv() {
let p = project()
@@ -141,7 +168,7 @@ fn unset_edition_works_on_old_msrv() {
p.cargo("check -v")
.with_stderr(
"\
[WARNING] no edition set: defaulting to the 2015 edition while the latest is 2021
[WARNING] no edition set: defaulting to the 2015 edition while 2018 is compatible with `rust-version`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call out which package it is? There might be a workspace containing tons of members.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like its done for us:

                        // In a workspace, it can be confusing where a warning
                        // originated, so include the path.
                        format!("{}: {}", path.display(), warning.message)

The other warnings I looked at do not mention the package which is why I dug into it to find that code. The test case must be only for a single package so it gets elided.

[CHECKING] foo [..]
[RUNNING] `rustc [..] --edition=2015 [..]`
[FINISHED] [..]