Warn when patch version in requires-python is implicitly 0

[ianpaul10]

When patch version isn't specified and a matching version is referenced, it will default patch to 0 which could be unclear/confusing. This PR warns the user of that default.

Summary

The first part of this issue #7426. Will tackle the second part mentioned (~=) in a separate PR once I know this is the correct way to warn users.

Test Plan

Unit tests were added

[zanieb]

I'll need to take a closer look, but I was originally thinking we'd only do this for requires-python and that we'd enforce it when we read that value. It'd be nice to add it for other requirements too, but I think we could add those incrementally. If you implement the warning here, there's no way to describe the associated package / field we've read the version from which seems important for having a good message.

You'll also want to use warn_user_once!.

Does that make sense?

cc @BurntSushi would you be interested in being a primary reviewer for this one?

[ianpaul10]

Ah yes that makes sense, my mistake. So by "enforce it when we read that value", do you mean check it somewhere in requires_python.rs? I couldn't see exactly where it's being read for a given project.

[BurntSushi]

@ianpaul10 I don't think we want to do it at a super low level like on RequiresPython itself. It's hard to say exactly when that will get used and whether a warning in all such cases would be appropriate. It's probably better to go higher level. I can see two different spots for this. One is every time we load a pyproject.toml:

uv/crates/uv-workspace/src/pyproject.rs

Lines 56 to 70 in 7b80b18

	/// Parse a `PyProjectToml` from a raw TOML string.
	pub fn from_string(raw: String) -> Result<Self, PyprojectTomlError> {
	let pyproject: toml_edit::ImDocument<_> =
	toml_edit::ImDocument::from_str(&raw).map_err(PyprojectTomlError::TomlSyntax)?;
	let pyproject =
	PyProjectToml::deserialize(pyproject.into_deserializer()).map_err(|err| {
	// TODO(konsti): A typed error would be nicer, this can break on toml upgrades.
	if err.message().contains("missing field `name`") {
	PyprojectTomlError::MissingName(err)
	} else {
	PyprojectTomlError::TomlSchema(err)
	}
	})?;
	Ok(PyProjectToml { raw, ..pyproject })
	}

Another spot is only when the user runs uv lock, which is also where we have some other warnings related to requires-python:

uv/crates/uv/src/commands/project/lock.rs

Lines 332 to 346 in 7b80b18

	let requires_python = if let Some(requires_python) = requires_python {
	if requires_python.is_unbounded() {
	let default =
	RequiresPython::greater_than_equal_version(&interpreter.python_minor_version());
	warn_user_once!("The workspace `requires-python` value does not contain a lower bound: `{requires_python}`. Set a lower bound to indicate the minimum compatible Python version (e.g., `{default}`).");
	}
	requires_python
	} else {
	let default =
	RequiresPython::greater_than_equal_version(&interpreter.python_minor_version());
	warn_user_once!(
	"No `requires-python` value found in the workspace. Defaulting to `{default}`."
	);
	default
	};

I think I would go the latter option. And if you add the warning there, it should show up when running uv sync as well, automatically (among any other commands that try and do a lock). Otherwise, we'd be issuing this warning for almost all uv commands, and I'm not sure we want to go that route.

[ianpaul10]

Thanks a ton for the in-depth comment. That all makes sense, I didn't really know the best place to put it so that colour is helpful. I took your advice and added it into the lock.rs file.

Let me know if that's different than what you had in mind, and if the tests I included are appropriate.

[BurntSushi]

Can you move this to the group of non-std crate-external imports below? With uv_pep440 and uv_distribution_filename.

The general pattern in the ecosystem is to put imports in three groups, in this order: std imports, non-crate imports, crate imports. (I also usually separate core and alloc imports too, but that's only for no_std libraries.)

[BurntSushi]

I know we're using this macro in at least one other place too, but I'd skip on this and just write out the code instead.

#[test]
fn is_matching_without_patch() {
    let test_cases = [
        ("==3.12", true),
        ("==2.7", true),
        ("3.12.1", false),
        // ...
    ];
    for (version, expected) in test_cases {
        let version_specifiers = VersionSpecifiers::from_str(version).unwrap();
        let requires_python = RequiresPython::from_specifiers(&version_specifiers).unwrap();
        assert_eq!(requires_python.is_matching_without_patch(), expected, "version: {version}");
    }
}

The main reason for this is that it would be great to reduce our dependence on proc macros in service of helping compile times.

[BurntSushi]

The phrasing seems a touch off here. Maybe this instead? Not totally sure.

Suggested change

	warn_user_once!("The workspace `requires-python` value is a matching reference without a patch version: `{requires_python}`. It will be interpreted as `{requires_python}.0`. Did you mean `{requires_python}.*`?");
	warn_user_once!("The workspace `requires-python` value does not have a patch version: `{requires_python}`. It will be interpreted as `{requires_python}.0`. Did you mean `{requires_python}.*`?");

[BurntSushi]

Perhaps an odd case, but this makes me think of something like, ==3.10, <3.11. I think that should return true?

[ianpaul10]

Ok thanks for all the comments, I believe I addressed them all, let me know if there's anything I missed.

Also, I'm not sure what's happening with the windows tests and mkdocs, but it appears to be happening on other PRs 🤔 Are they normally flaky or should I investigate further?

[zanieb]

I'll look into that, thanks!

[zanieb]

Thank you and nice work!

I made some minor tweaks, let me know if you have any questions or concerns.

Addressed

[ianpaul10]

Thanks for reviewing and improving the language! Those all make sense to me 👍