Auto merge of #12786 - epage:version, r=weihanglo

feat(toml): Allow version-less manifests

### What does this PR try to resolve?

Expected behavior with this PR:
- `package.version` defaults to `0.0.0`
- `package.publish` is defaulted to `version.is_some()`

This also updates "cargo script" to rely on this new behavior.

My motivation is to find ways to close the gap between "cargo script" and `Cargo.toml`.  With "cargo script", we want to allow people to only write however much of a manifest is directly needed for the work they are doing (which includes having no manifest).  Each difference between "cargo script" and `Cargo.toml` is a cost we have to pay in our documentation and a hurdle in a users understanding of what is happening.

There has been other interest in this which I also find of interest (from #9829):
- Lower boilerplate, whether for [cargo xtasks](https://github.com/matklad/cargo-xtask), nested packages (rust-lang/rfcs#3452), etc
- Unmet expectations from users because this field is primarily targeted at registry operations when they want it for their marketing version (#6583).
- Make "unpublished" packages stand out

This then unblocks unifying `package.publish` by making the field's default based on the presence of a version as inspired by the proposal in #9829.  Without this change, we were trading one form of boilerplate (`version = "0.0.0"`) for another (`publish = false`).

Fixes #9829
Fixes #12690
Fixes #6153

### How should we test and review this PR?

The initial commit has test cases I thought would be relevant for this change and you can see how each commit affects those or existing test cases.  Would definitely be interested in hearing of other troubling cases to test

Implementation wise, I made `MaybeWorkspaceVersion` deserializer trim spaces so I could more easily handle the field being an `Option`.  This is in its own commit.

### Additional information

Alternatives considered
- Making the default version "stand out more" with it being something like `0.0.0+HEAD`.  The extra noise didn't seem worth it and people would contend over what the metadata field *should be*
- Make the default version the lowest version possible (`0.0.0-0`?).  Unsure if this will ever really matter especially since you can't publish
- Defer defaulting `package.publish` and instead error
  - Further unifying more fields made this too compelling for me :)
- Put this behind `-Zscript` and make it a part of rust-lang/rfcs#3502
  - Having an affect outside of that RFC, I wanted to make sure this got the attention it deserved rather than getting lost in the noise of a large RFC.
- Don't just default the version but make packages versionless
  - I extended the concept of versionless to `PackageId`'s internals and saw no observable difference
  - I then started to examine the idea of version being optional everywhere (via `PackageId`s API) and ... things got messy especially when starting to look at the resolver.  This would have also added a lot of error checks / asserts for "the version must be set here".  Overall, the gains seemed questionable and the cost high, so I held off.

Author: bors

src/cargo/ops/registry/publish.rs

@@ -103,7 +103,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
         if allowed_registries.is_empty() {
             bail!(
                 "`{}` cannot be published.\n\
-                 `package.publish` is set to `false` or an empty list in Cargo.toml and prevents publishing.",
+                 `package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish.",
                 pkg.name(),
             );
         } else if !allowed_registries.contains(&reg_name) {

src/cargo/util/toml/embedded.rs

@@ -6,8 +6,6 @@ use crate::Config;
 
 const DEFAULT_EDITION: crate::core::features::Edition =
     crate::core::features::Edition::LATEST_STABLE;
-const DEFAULT_VERSION: &str = "0.0.0";
-const DEFAULT_PUBLISH: bool = false;
 const AUTO_FIELDS: &[&str] = &["autobins", "autoexamples", "autotests", "autobenches"];
 
 pub fn expand_manifest(
@@ -123,9 +121,6 @@ fn expand_manifest_(
     package
         .entry("name".to_owned())
         .or_insert(toml::Value::String(name));
-    package
-        .entry("version".to_owned())
-        .or_insert_with(|| toml::Value::String(DEFAULT_VERSION.to_owned()));
     package.entry("edition".to_owned()).or_insert_with(|| {
         let _ = config.shell().warn(format_args!(
             "`package.edition` is unspecified, defaulting to `{}`",
@@ -136,9 +131,6 @@ fn expand_manifest_(
     package
         .entry("build".to_owned())
         .or_insert_with(|| toml::Value::Boolean(false));
-    package
-        .entry("publish".to_owned())
-        .or_insert_with(|| toml::Value::Boolean(DEFAULT_PUBLISH));
     for field in AUTO_FIELDS {
         package
             .entry(field.to_owned())
@@ -621,8 +613,6 @@ autotests = false
 build = false
 edition = "2021"
 name = "test-"
-publish = false
-version = "0.0.0"
 
 [profile.release]
 strip = true
@@ -651,8 +641,6 @@ autotests = false
 build = false
 edition = "2021"
 name = "test-"
-publish = false
-version = "0.0.0"
 
 [profile.release]
 strip = true

src/cargo/util/toml/mod.rs

@@ -550,11 +550,17 @@ impl TomlManifest {
         let version = package
             .version
             .clone()
-            .resolve("version", || inherit()?.version())?;
+            .map(|version| version.resolve("version", || inherit()?.version()))
+            .transpose()?;
 
-        package.version = MaybeWorkspace::Defined(version.clone());
+        package.version = version.clone().map(MaybeWorkspace::Defined);
 
-        let pkgid = package.to_package_id(source_id, version)?;
+        let pkgid = package.to_package_id(
+            source_id,
+            version
+                .clone()
+                .unwrap_or_else(|| semver::Version::new(0, 0, 0)),
+        )?;
 
         let edition = if let Some(edition) = package.edition.clone() {
             let edition: Edition = edition
@@ -1006,9 +1012,14 @@ impl TomlManifest {
         let publish = match publish {
             Some(VecStringOrBool::VecString(ref vecstring)) => Some(vecstring.clone()),
             Some(VecStringOrBool::Bool(false)) => Some(vec![]),
-            None | Some(VecStringOrBool::Bool(true)) => None,
+            Some(VecStringOrBool::Bool(true)) => None,
+            None => version.is_none().then_some(vec![]),
         };
 
+        if version.is_none() && publish != Some(vec![]) {
+            bail!("`package.publish` requires `package.version` be specified");
+        }
+
         if summary.features().contains_key("default-features") {
             warnings.push(
                 "`default-features = [\"..\"]` was found in [features]. \
@@ -1659,8 +1670,7 @@ pub struct TomlPackage {
     edition: Option<MaybeWorkspaceString>,
     rust_version: Option<MaybeWorkspaceRustVersion>,
     name: InternedString,
-    #[serde(deserialize_with = "version_trim_whitespace")]
-    version: MaybeWorkspaceSemverVersion,
+    version: Option<MaybeWorkspaceSemverVersion>,
     authors: Option<MaybeWorkspaceVecString>,
     build: Option<StringOrBool>,
     metabuild: Option<StringOrVec>,
@@ -1709,22 +1719,6 @@ impl TomlPackage {
     }
 }
 
-fn version_trim_whitespace<'de, D>(deserializer: D) -> Result<MaybeWorkspaceSemverVersion, D::Error>
-where
-    D: de::Deserializer<'de>,
-{
-    UntaggedEnumVisitor::new()
-        .expecting("SemVer version")
-        .string(
-            |value| match value.trim().parse().map_err(de::Error::custom) {
-                Ok(parsed) => Ok(MaybeWorkspace::Defined(parsed)),
-                Err(e) => Err(e),
-            },
-        )
-        .map(|value| value.deserialize().map(MaybeWorkspace::Workspace))
-        .deserialize(deserializer)
-}
-
 /// This Trait exists to make [`MaybeWorkspace::Workspace`] generic. It makes deserialization of
 /// [`MaybeWorkspace`] much easier, as well as making error messages for
 /// [`MaybeWorkspace::resolve`] much nicer
@@ -1793,6 +1787,23 @@ impl<T, W: WorkspaceInherit> MaybeWorkspace<T, W> {
 
 //. This already has a `Deserialize` impl from version_trim_whitespace
 type MaybeWorkspaceSemverVersion = MaybeWorkspace<semver::Version, TomlWorkspaceField>;
+impl<'de> de::Deserialize<'de> for MaybeWorkspaceSemverVersion {
+    fn deserialize<D>(d: D) -> Result<Self, D::Error>
+    where
+        D: de::Deserializer<'de>,
+    {
+        UntaggedEnumVisitor::new()
+            .expecting("SemVer version")
+            .string(
+                |value| match value.trim().parse().map_err(de::Error::custom) {
+                    Ok(parsed) => Ok(MaybeWorkspace::Defined(parsed)),
+                    Err(e) => Err(e),
+                },
+            )
+            .map(|value| value.deserialize().map(MaybeWorkspace::Workspace))
+            .deserialize(d)
+    }
+}
 
 type MaybeWorkspaceString = MaybeWorkspace<String, TomlWorkspaceField>;
 impl<'de> de::Deserialize<'de> for MaybeWorkspaceString {

src/doc/src/reference/manifest.md

@@ -109,6 +109,8 @@ resolve dependencies, and for guidelines on setting your own version. See the
 [SemVer compatibility] chapter for more details on exactly what constitutes a
 breaking change.
 
+This field is optional and defaults to `0.0.0`.  The field is required for publishing packages.
+
 [Resolver]: resolver.md
 [SemVer compatibility]: semver.md
 
@@ -470,23 +472,22 @@ if any of those files change.
 
 ### The `publish` field
 
-The `publish` field can be used to prevent a package from being published to a
-package registry (like *crates.io*) by mistake, for instance to keep a package
-private in a company.
-
+The `publish` field can be used to control which registries names the package
+may be published to:
 ```toml
 [package]
 # ...
-publish = false
+publish = ["some-registry-name"]
 ```
 
-The value may also be an array of strings which are registry names that are
-allowed to be published to.
-
+To prevent a package from being published to a registry (like crates.io) by mistake,
+for instance to keep a package private in a company,
+you can omit the [`version`](#the-version-field) field.
+If you'd like to be more explicit, you can disable publishing:
 ```toml
 [package]
 # ...
-publish = ["some-registry-name"]
+publish = false
 ```
 
 If publish array contains a single registry, `cargo publish` command will use

src/doc/src/reference/unstable.md

@@ -1221,9 +1221,6 @@ at the start of the infostring at the top of the file.
 
 Inferred / defaulted manifest fields:
 - `package.name = <slugified file stem>`
-- `package.version = "0.0.0"` to [call attention to this crate being used in unexpected places](https://matklad.github.io/2021/08/22/large-rust-workspaces.html#Smaller-Tips)
-- `package.publish = false` to avoid accidental publishes, particularly if we
-  later add support for including them in a workspace.
 - `package.edition = <current>` to avoid always having to add an embedded
   manifest at the cost of potentially breaking scripts on rust upgrades
   - Warn when `edition` is unspecified to raise awareness of this

tests/testsuite/check.rs

@@ -1496,3 +1496,26 @@ fn check_unused_manifest_keys() {
         )
         .run();
 }
+
+#[cargo_test]
+fn versionless_package() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                description = "foo"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+    p.cargo("check")
+        .with_stderr(
+            "\
+[CHECKING] foo v0.0.0 ([CWD])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+",
+        )
+        .run();
+}