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

Allow the edition specifier to be an integer #12301

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
73 changes: 68 additions & 5 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,22 @@ impl StringOrVec {
}
}

#[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
#[serde(untagged, expecting = "expected a string or an integer")]
pub enum StringOrI64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Other areas I suspect we should update if we move forward with this

  • cargo new
  • embedded manifests
  • documentation

Copy link
Member

Choose a reason for hiding this comment

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

@epage We shouldn't update cargo new until 2024, to avoid MSRV implications.

Copy link
Member Author

@est31 est31 Jun 28, 2023

Choose a reason for hiding this comment

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

There has been some discussion on this in #12301 (review) and #12301 (comment) . I agree that cargo new should be updated only well into the 2023 edition, or even later, not just for MSRV but also for better error messages from cargos that don't support the 2023 edition.

String(String),
I64(i64),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why an i64 instead of a u64?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have tried an u32 at the start and it didn't work, as in it compiled but the tests as added by this PR failed. The problem was I think that serde represents toml integers via i64 in its data model. In the end I don't think it matters that much: a negative number will still give an error one way or another.

}

impl Display for StringOrI64 {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
StringOrI64::String(s) => f.write_str(s),
StringOrI64::I64(v) => write!(f, "{v}"),
}
}
}

#[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
#[serde(untagged, expecting = "expected a boolean or a string")]
pub enum StringOrBool {
Expand Down Expand Up @@ -1262,6 +1278,50 @@ impl TomlWorkspaceDependency {
//. This already has a `Deserialize` impl from version_trim_whitespace
type MaybeWorkspaceSemverVersion = MaybeWorkspace<semver::Version, TomlWorkspaceField>;

type MaybeWorkspaceStringOrI64 = MaybeWorkspace<StringOrI64, TomlWorkspaceField>;
impl<'de> de::Deserialize<'de> for MaybeWorkspaceStringOrI64 {
fn deserialize<D>(d: D) -> Result<Self, D::Error>
where
D: de::Deserializer<'de>,
{
struct Visitor;

impl<'de> de::Visitor<'de> for Visitor {
type Value = MaybeWorkspaceStringOrI64;

fn expecting(&self, f: &mut fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
f.write_str("a string, integer or workspace")
}

fn visit_i64<E>(self, value: i64) -> Result<Self::Value, E>
where
E: de::Error,
{
let int = de::value::I64Deserializer::new(value);
StringOrI64::deserialize(int).map(MaybeWorkspace::Defined)
}

fn visit_string<E>(self, value: String) -> Result<Self::Value, E>
where
E: de::Error,
{
let string = de::value::StringDeserializer::new(value);
StringOrI64::deserialize(string).map(MaybeWorkspace::Defined)
}

fn visit_map<V>(self, map: V) -> Result<Self::Value, V::Error>
where
V: de::MapAccess<'de>,
{
let mvd = de::value::MapAccessDeserializer::new(map);
TomlWorkspaceField::deserialize(mvd).map(MaybeWorkspace::Workspace)
}
}

d.deserialize_any(Visitor)
}
}

type MaybeWorkspaceString = MaybeWorkspace<String, TomlWorkspaceField>;
impl<'de> de::Deserialize<'de> for MaybeWorkspaceString {
fn deserialize<D>(d: D) -> Result<Self, D::Error>
Expand Down Expand Up @@ -1501,7 +1561,7 @@ impl WorkspaceInherit for TomlWorkspaceField {
#[derive(Deserialize, Serialize, Clone, Debug)]
#[serde(rename_all = "kebab-case")]
pub struct TomlPackage {
edition: Option<MaybeWorkspaceString>,
edition: Option<MaybeWorkspaceStringOrI64>,
rust_version: Option<MaybeWorkspaceString>,
name: InternedString,
#[serde(deserialize_with = "version_trim_whitespace")]
Expand Down Expand Up @@ -1583,7 +1643,7 @@ pub struct InheritableFields {
license_file: Option<String>,
repository: Option<String>,
publish: Option<VecStringOrBool>,
edition: Option<String>,
edition: Option<StringOrI64>,
badges: Option<BTreeMap<String, BTreeMap<String, String>>>,
exclude: Option<Vec<String>>,
include: Option<Vec<String>>,
Expand Down Expand Up @@ -1620,7 +1680,7 @@ impl InheritableFields {
("package.categories", categories -> Vec<String>),
("package.description", description -> String),
("package.documentation", documentation -> String),
("package.edition", edition -> String),
("package.edition", edition -> StringOrI64),
("package.exclude", exclude -> Vec<String>),
("package.homepage", homepage -> String),
("package.include", include -> Vec<String>),
Expand Down Expand Up @@ -1728,7 +1788,7 @@ impl TomlManifest {
.edition
.as_ref()
.and_then(|e| e.as_defined())
.map(|e| Edition::from_str(e))
.map(|e| Edition::from_str(&e.to_string()))
.unwrap_or(Ok(Edition::Edition2015))
.map(|e| e.default_resolve_behavior())
})?;
Expand Down Expand Up @@ -2040,9 +2100,12 @@ impl TomlManifest {
let edition = if let Some(edition) = package.edition.clone() {
let edition: Edition = edition
.resolve("edition", || inherit()?.edition())?
.to_string()
.parse()
.with_context(|| "failed to parse the `edition` key")?;
package.edition = Some(MaybeWorkspace::Defined(edition.to_string()));
package.edition = Some(MaybeWorkspace::Defined(StringOrI64::String(
edition.to_string(),
)));
edition
} else {
Edition::Edition2015
Expand Down
2 changes: 2 additions & 0 deletions src/doc/src/reference/manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ examples, etc.
edition = '2021'
```

For the value of the `edition` key, both strings and integers are supported.

Most manifests have the `edition` field filled in automatically by [`cargo new`]
with the latest stable edition. By default `cargo new` creates a manifest with
the 2021 edition currently.
Expand Down
65 changes: 65 additions & 0 deletions tests/testsuite/edition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,71 @@ fn edition_works_for_build_script() {
p.cargo("check -v").run();
}

#[cargo_test]
fn edition_works_as_integer() {
const NEEDS_2021: &str = "pub fn foo() { let hi: u8 = 0i32.try_into().unwrap(); }";
let p_2021 = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = 2021
"#,
)
.file("src/lib.rs", NEEDS_2021)
.build();
p_2021.cargo("check").run();

let p_2018 = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = 2018
"#,
)
.file("src/lib.rs", NEEDS_2021)
.build();
p_2018
.cargo("check")
.with_status(101)
.with_stderr_contains("[..] is included in the prelude starting in Edition 2021")
.run();
}

#[cargo_test]
fn edition_breaks_as_float() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = 2021.0
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check -v")
.with_status(101)
.with_stderr(
"\
[ERROR] failed to parse manifest at `[..]/foo/Cargo.toml`

Caused by:
invalid type: floating point `2021`, expected a string, integer or workspace
in `package.edition`
",
)
.run();
}

#[cargo_test]
fn edition_unstable_gated() {
// During the period where a new edition is coming up, but not yet stable,
Expand Down