Report marker diagnostics during parsing, rather than evaluation (#9338)

## Summary

I want to move towards a more normalized marker representation within
the marker tree, which means that the things we warn against will
disappear by the time we get to evaluation. I think it makes more sense
to show these warnings when we create the tree, rather than when we
evaluate it.
This commit is contained in:
Charlie Marsh 2024-11-26 09:15:33 -05:00 committed by GitHub
parent f886d08094
commit 106863069d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 397 additions and 248 deletions

View file

@ -28,7 +28,8 @@ use url::Url;
use cursor::Cursor;
pub use marker::{
ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, MarkerEnvironment,
ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, LoweredMarkerValueExtra,
LoweredMarkerValueString, LoweredMarkerValueVersion, MarkerEnvironment,
MarkerEnvironmentBuilder, MarkerExpression, MarkerOperator, MarkerTree, MarkerTreeContents,
MarkerTreeKind, MarkerValue, MarkerValueExtra, MarkerValueString, MarkerValueVersion,
MarkerWarningKind, StringMarkerTree, StringVersion, VersionMarkerTree,
@ -200,8 +201,6 @@ impl<T: Pep508Url> Serialize for Requirement<T> {
}
}
type MarkerWarning = (MarkerWarningKind, String);
impl<T: Pep508Url> Requirement<T> {
/// Returns whether the markers apply for the given environment
pub fn evaluate_markers(&self, env: &MarkerEnvironment, extras: &[ExtraName]) -> bool {
@ -223,15 +222,6 @@ impl<T: Pep508Url> Requirement<T> {
.evaluate_extras_and_python_version(extras, python_versions)
}
/// Returns whether the markers apply for the given environment.
pub fn evaluate_markers_and_report(
&self,
env: &MarkerEnvironment,
extras: &[ExtraName],
) -> (bool, Vec<MarkerWarning>) {
self.marker.evaluate_collect_warnings(env, extras)
}
/// Return the requirement with an additional marker added, to require the given extra.
///
/// For example, given `flask >= 2.0.2`, calling `with_extra_marker("dotenv")` would return

View file

@ -56,9 +56,12 @@ use std::sync::LazyLock;
use uv_pep440::{release_specifier_to_range, Operator, Version, VersionSpecifier};
use version_ranges::Ranges;
use crate::marker::lowering::{
LoweredMarkerValueExtra, LoweredMarkerValueString, LoweredMarkerValueVersion,
};
use crate::marker::MarkerValueExtra;
use crate::ExtraOperator;
use crate::{MarkerExpression, MarkerOperator, MarkerValueString, MarkerValueVersion};
use crate::{MarkerExpression, MarkerOperator, MarkerValueVersion};
/// The global node interner.
pub(crate) static INTERNER: LazyLock<Interner> = LazyLock::new(Interner::default);
@ -161,7 +164,7 @@ impl InternerGuard<'_> {
specifier,
} => match python_version_to_full_version(normalize_specifier(specifier)) {
Ok(specifier) => (
Variable::Version(MarkerValueVersion::PythonFullVersion),
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
Edges::from_specifier(specifier),
),
Err(node) => return node,
@ -172,16 +175,17 @@ impl InternerGuard<'_> {
negated,
} => match Edges::from_python_versions(versions, negated) {
Ok(edges) => (
Variable::Version(MarkerValueVersion::PythonFullVersion),
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
edges,
),
Err(node) => return node,
},
// A variable representing the output of a version key. Edges correspond
// to disjoint version ranges.
MarkerExpression::Version { key, specifier } => {
(Variable::Version(key), Edges::from_specifier(specifier))
}
MarkerExpression::Version { key, specifier } => (
Variable::Version(key.into()),
Edges::from_specifier(specifier),
),
// A variable representing the output of a version key. Edges correspond
// to disjoint version ranges.
MarkerExpression::VersionIn {
@ -189,7 +193,7 @@ impl InternerGuard<'_> {
versions,
negated,
} => (
Variable::Version(key),
Variable::Version(key.into()),
Edges::from_versions(&versions, negated),
),
// The `in` and `contains` operators are a bit different than other operators.
@ -206,38 +210,76 @@ impl InternerGuard<'_> {
key,
operator: MarkerOperator::In,
value,
} => (Variable::In { key, value }, Edges::from_bool(true)),
} => (
Variable::In {
key: key.into(),
value,
},
Edges::from_bool(true),
),
MarkerExpression::String {
key,
operator: MarkerOperator::NotIn,
value,
} => (Variable::In { key, value }, Edges::from_bool(false)),
} => (
Variable::In {
key: key.into(),
value,
},
Edges::from_bool(false),
),
MarkerExpression::String {
key,
operator: MarkerOperator::Contains,
value,
} => (Variable::Contains { key, value }, Edges::from_bool(true)),
} => (
Variable::Contains {
key: key.into(),
value,
},
Edges::from_bool(true),
),
MarkerExpression::String {
key,
operator: MarkerOperator::NotContains,
value,
} => (Variable::Contains { key, value }, Edges::from_bool(false)),
} => (
Variable::Contains {
key: key.into(),
value,
},
Edges::from_bool(false),
),
// A variable representing the output of a string key. Edges correspond
// to disjoint string ranges.
MarkerExpression::String {
key,
operator,
value,
} => (Variable::String(key), Edges::from_string(operator, value)),
} => (
Variable::String(key.into()),
Edges::from_string(operator, value),
),
// A variable representing the existence or absence of a particular extra.
MarkerExpression::Extra {
name,
name: MarkerValueExtra::Extra(extra),
operator: ExtraOperator::Equal,
} => (Variable::Extra(name), Edges::from_bool(true)),
} => (
Variable::Extra(LoweredMarkerValueExtra::Extra(extra)),
Edges::from_bool(true),
),
MarkerExpression::Extra {
name,
name: MarkerValueExtra::Extra(extra),
operator: ExtraOperator::NotEqual,
} => (Variable::Extra(name), Edges::from_bool(false)),
} => (
Variable::Extra(LoweredMarkerValueExtra::Extra(extra)),
Edges::from_bool(false),
),
// Invalid extras are always `false`.
MarkerExpression::Extra {
name: MarkerValueExtra::Arbitrary(_),
..
} => return NodeId::FALSE,
};
self.create_node(var, children)
@ -391,7 +433,7 @@ impl InternerGuard<'_> {
// Look for a `python_full_version` expression, otherwise
// we recursively simplify.
let Node {
var: Variable::Version(MarkerValueVersion::PythonFullVersion),
var: Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
children: Edges::Version { ref edges },
} = node
else {
@ -464,7 +506,7 @@ impl InternerGuard<'_> {
return NodeId::FALSE;
}
if matches!(i, NodeId::TRUE) {
let var = Variable::Version(MarkerValueVersion::PythonFullVersion);
let var = Variable::Version(LoweredMarkerValueVersion::PythonFullVersion);
let edges = Edges::Version {
edges: Edges::from_range(&py_range),
};
@ -473,7 +515,7 @@ impl InternerGuard<'_> {
let node = self.shared.node(i);
let Node {
var: Variable::Version(MarkerValueVersion::PythonFullVersion),
var: Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
children: Edges::Version { ref edges },
} = node
else {
@ -569,26 +611,26 @@ pub(crate) enum Variable {
///
/// This is the highest order variable as it typically contains the most complex
/// ranges, allowing us to merge ranges at the top-level.
Version(MarkerValueVersion),
Version(LoweredMarkerValueVersion),
/// A string marker, such as `os_name`.
String(MarkerValueString),
String(LoweredMarkerValueString),
/// A variable representing a `<key> in <value>` expression for a particular
/// string marker and value.
In {
key: MarkerValueString,
key: LoweredMarkerValueString,
value: String,
},
/// A variable representing a `<value> in <key>` expression for a particular
/// string marker and value.
Contains {
key: MarkerValueString,
key: LoweredMarkerValueString,
value: String,
},
/// A variable representing the existence or absence of a given extra.
///
/// We keep extras at the leaves of the tree, so when simplifying extras we can
/// trivially remove the leaves without having to reconstruct the entire tree.
Extra(MarkerValueExtra),
Extra(LoweredMarkerValueExtra),
}
/// A decision node in an Algebraic Decision Diagram.

View file

@ -2,7 +2,7 @@ use std::sync::Arc;
use uv_pep440::{Version, VersionParseError};
use crate::{MarkerValueString, MarkerValueVersion, StringVersion};
use crate::{LoweredMarkerValueString, LoweredMarkerValueVersion, StringVersion};
/// The marker values for a python interpreter, normally the current one
///
@ -33,35 +33,36 @@ struct MarkerEnvironmentInner {
impl MarkerEnvironment {
/// Returns of the PEP 440 version typed value of the key in the current environment
pub fn get_version(&self, key: MarkerValueVersion) -> &Version {
pub fn get_version(&self, key: LoweredMarkerValueVersion) -> &Version {
match key {
MarkerValueVersion::ImplementationVersion => &self.implementation_version().version,
MarkerValueVersion::PythonFullVersion => &self.python_full_version().version,
MarkerValueVersion::PythonVersion => &self.python_version().version,
LoweredMarkerValueVersion::ImplementationVersion => {
&self.implementation_version().version
}
LoweredMarkerValueVersion::PythonFullVersion => &self.python_full_version().version,
LoweredMarkerValueVersion::PythonVersion => &self.python_version().version,
}
}
/// Returns of the stringly typed value of the key in the current environment
pub fn get_string(&self, key: MarkerValueString) -> &str {
pub fn get_string(&self, key: LoweredMarkerValueString) -> &str {
match key {
MarkerValueString::ImplementationName => self.implementation_name(),
MarkerValueString::OsName | MarkerValueString::OsNameDeprecated => self.os_name(),
MarkerValueString::PlatformMachine | MarkerValueString::PlatformMachineDeprecated => {
self.platform_machine()
LoweredMarkerValueString::ImplementationName => self.implementation_name(),
LoweredMarkerValueString::OsName | LoweredMarkerValueString::OsNameDeprecated => {
self.os_name()
}
MarkerValueString::PlatformPythonImplementation
| MarkerValueString::PlatformPythonImplementationDeprecated
| MarkerValueString::PythonImplementationDeprecated => {
LoweredMarkerValueString::PlatformMachine
| LoweredMarkerValueString::PlatformMachineDeprecated => self.platform_machine(),
LoweredMarkerValueString::PlatformPythonImplementation
| LoweredMarkerValueString::PlatformPythonImplementationDeprecated
| LoweredMarkerValueString::PythonImplementationDeprecated => {
self.platform_python_implementation()
}
MarkerValueString::PlatformRelease => self.platform_release(),
MarkerValueString::PlatformSystem => self.platform_system(),
MarkerValueString::PlatformVersion | MarkerValueString::PlatformVersionDeprecated => {
self.platform_version()
}
MarkerValueString::SysPlatform | MarkerValueString::SysPlatformDeprecated => {
self.sys_platform()
}
LoweredMarkerValueString::PlatformRelease => self.platform_release(),
LoweredMarkerValueString::PlatformSystem => self.platform_system(),
LoweredMarkerValueString::PlatformVersion
| LoweredMarkerValueString::PlatformVersionDeprecated => self.platform_version(),
LoweredMarkerValueString::SysPlatform
| LoweredMarkerValueString::SysPlatformDeprecated => self.sys_platform(),
}
}
}

View file

@ -0,0 +1,186 @@
use std::fmt::{Display, Formatter};
use uv_normalize::ExtraName;
use crate::{MarkerValueExtra, MarkerValueString, MarkerValueVersion};
/// Those environment markers with a PEP 440 version as value such as `python_version`
#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
#[allow(clippy::enum_variant_names)]
pub enum LoweredMarkerValueVersion {
/// `implementation_version`
ImplementationVersion,
/// `python_full_version`
PythonFullVersion,
/// `python_version`
PythonVersion,
}
impl Display for LoweredMarkerValueVersion {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::ImplementationVersion => f.write_str("implementation_version"),
Self::PythonFullVersion => f.write_str("python_full_version"),
Self::PythonVersion => f.write_str("python_version"),
}
}
}
impl From<MarkerValueVersion> for LoweredMarkerValueVersion {
fn from(value: MarkerValueVersion) -> Self {
match value {
MarkerValueVersion::ImplementationVersion => Self::ImplementationVersion,
MarkerValueVersion::PythonFullVersion => Self::PythonFullVersion,
MarkerValueVersion::PythonVersion => Self::PythonVersion,
}
}
}
impl From<LoweredMarkerValueVersion> for MarkerValueVersion {
fn from(value: LoweredMarkerValueVersion) -> Self {
match value {
LoweredMarkerValueVersion::ImplementationVersion => Self::ImplementationVersion,
LoweredMarkerValueVersion::PythonFullVersion => Self::PythonFullVersion,
LoweredMarkerValueVersion::PythonVersion => Self::PythonVersion,
}
}
}
/// Those environment markers with an arbitrary string as value such as `sys_platform`
#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
pub enum LoweredMarkerValueString {
/// `implementation_name`
ImplementationName,
/// `os_name`
OsName,
/// Deprecated `os.name` from <https://peps.python.org/pep-0345/#environment-markers>
OsNameDeprecated,
/// `platform_machine`
PlatformMachine,
/// Deprecated `platform.machine` from <https://peps.python.org/pep-0345/#environment-markers>
PlatformMachineDeprecated,
/// `platform_python_implementation`
PlatformPythonImplementation,
/// Deprecated `platform.python_implementation` from <https://peps.python.org/pep-0345/#environment-markers>
PlatformPythonImplementationDeprecated,
/// Deprecated `python_implementation` from <https://github.com/pypa/packaging/issues/72>
PythonImplementationDeprecated,
/// `platform_release`
PlatformRelease,
/// `platform_system`
PlatformSystem,
/// `platform_version`
PlatformVersion,
/// Deprecated `platform.version` from <https://peps.python.org/pep-0345/#environment-markers>
PlatformVersionDeprecated,
/// `sys_platform`
SysPlatform,
/// Deprecated `sys.platform` from <https://peps.python.org/pep-0345/#environment-markers>
SysPlatformDeprecated,
}
impl From<MarkerValueString> for LoweredMarkerValueString {
fn from(value: MarkerValueString) -> Self {
match value {
MarkerValueString::ImplementationName => Self::ImplementationName,
MarkerValueString::OsName => Self::OsName,
MarkerValueString::OsNameDeprecated => Self::OsNameDeprecated,
MarkerValueString::PlatformMachine => Self::PlatformMachine,
MarkerValueString::PlatformMachineDeprecated => Self::PlatformMachineDeprecated,
MarkerValueString::PlatformPythonImplementation => Self::PlatformPythonImplementation,
MarkerValueString::PlatformPythonImplementationDeprecated => {
Self::PlatformPythonImplementationDeprecated
}
MarkerValueString::PythonImplementationDeprecated => {
Self::PythonImplementationDeprecated
}
MarkerValueString::PlatformRelease => Self::PlatformRelease,
MarkerValueString::PlatformSystem => Self::PlatformSystem,
MarkerValueString::PlatformVersion => Self::PlatformVersion,
MarkerValueString::PlatformVersionDeprecated => Self::PlatformVersionDeprecated,
MarkerValueString::SysPlatform => Self::SysPlatform,
MarkerValueString::SysPlatformDeprecated => Self::SysPlatformDeprecated,
}
}
}
impl From<LoweredMarkerValueString> for MarkerValueString {
fn from(value: LoweredMarkerValueString) -> Self {
match value {
LoweredMarkerValueString::ImplementationName => Self::ImplementationName,
LoweredMarkerValueString::OsName => Self::OsName,
LoweredMarkerValueString::OsNameDeprecated => Self::OsNameDeprecated,
LoweredMarkerValueString::PlatformMachine => Self::PlatformMachine,
LoweredMarkerValueString::PlatformMachineDeprecated => Self::PlatformMachineDeprecated,
LoweredMarkerValueString::PlatformPythonImplementation => {
Self::PlatformPythonImplementation
}
LoweredMarkerValueString::PlatformPythonImplementationDeprecated => {
Self::PlatformPythonImplementationDeprecated
}
LoweredMarkerValueString::PythonImplementationDeprecated => {
Self::PythonImplementationDeprecated
}
LoweredMarkerValueString::PlatformRelease => Self::PlatformRelease,
LoweredMarkerValueString::PlatformSystem => Self::PlatformSystem,
LoweredMarkerValueString::PlatformVersion => Self::PlatformVersion,
LoweredMarkerValueString::PlatformVersionDeprecated => Self::PlatformVersionDeprecated,
LoweredMarkerValueString::SysPlatform => Self::SysPlatform,
LoweredMarkerValueString::SysPlatformDeprecated => Self::SysPlatformDeprecated,
}
}
}
impl Display for LoweredMarkerValueString {
/// Normalizes deprecated names to the proper ones
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::ImplementationName => f.write_str("implementation_name"),
Self::OsName | Self::OsNameDeprecated => f.write_str("os_name"),
Self::PlatformMachine | Self::PlatformMachineDeprecated => {
f.write_str("platform_machine")
}
Self::PlatformPythonImplementation
| Self::PlatformPythonImplementationDeprecated
| Self::PythonImplementationDeprecated => f.write_str("platform_python_implementation"),
Self::PlatformRelease => f.write_str("platform_release"),
Self::PlatformSystem => f.write_str("platform_system"),
Self::PlatformVersion | Self::PlatformVersionDeprecated => {
f.write_str("platform_version")
}
Self::SysPlatform | Self::SysPlatformDeprecated => f.write_str("sys_platform"),
}
}
}
/// The [`ExtraName`] value used in `extra` markers.
#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
pub enum LoweredMarkerValueExtra {
/// A valid [`ExtraName`].
Extra(ExtraName),
}
impl LoweredMarkerValueExtra {
/// Returns the [`ExtraName`] value.
pub fn extra(&self) -> &ExtraName {
match self {
Self::Extra(extra) => extra,
}
}
}
impl From<LoweredMarkerValueExtra> for MarkerValueExtra {
fn from(value: LoweredMarkerValueExtra) -> Self {
match value {
LoweredMarkerValueExtra::Extra(extra) => Self::Extra(extra),
}
}
}
impl Display for LoweredMarkerValueExtra {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::Extra(extra) => extra.fmt(f),
}
}
}

View file

@ -11,11 +11,13 @@
mod algebra;
mod environment;
mod lowering;
pub(crate) mod parse;
mod simplify;
mod tree;
pub use environment::{MarkerEnvironment, MarkerEnvironmentBuilder};
pub use lowering::{LoweredMarkerValueExtra, LoweredMarkerValueString, LoweredMarkerValueVersion};
pub use tree::{
ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, MarkerExpression,
MarkerOperator, MarkerTree, MarkerTreeContents, MarkerTreeDebugGraph, MarkerTreeKind,

View file

@ -6,8 +6,8 @@ use uv_pep440::{Version, VersionPattern, VersionSpecifier};
use crate::cursor::Cursor;
use crate::marker::MarkerValueExtra;
use crate::{
ExtraOperator, MarkerExpression, MarkerOperator, MarkerTree, MarkerValue, MarkerValueVersion,
MarkerWarningKind, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter,
ExtraOperator, MarkerExpression, MarkerOperator, MarkerTree, MarkerValue, MarkerValueString,
MarkerValueVersion, MarkerWarningKind, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter,
};
/// ```text
@ -72,6 +72,7 @@ fn parse_marker_operator<T: Pep508Url>(
/// '`implementation_version`', 'extra'
pub(crate) fn parse_marker_value<T: Pep508Url>(
cursor: &mut Cursor,
reporter: &mut impl Reporter,
) -> Result<MarkerValue, Pep508Error<T>> {
// > User supplied constants are always encoded as strings with either ' or " quote marks. Note
// > that backslash escapes are not defined, but existing implementations do support them. They
@ -101,14 +102,60 @@ pub(crate) fn parse_marker_value<T: Pep508Url>(
!char.is_whitespace() && !['>', '=', '<', '!', '~', ')'].contains(&char)
});
let key = cursor.slice(start, len);
MarkerValue::from_str(key).map_err(|_| Pep508Error {
message: Pep508ErrorSource::String(format!(
"Expected a quoted string or a valid marker name, found `{key}`"
)),
start,
len,
input: cursor.to_string(),
})
MarkerValue::from_str(key)
.map_err(|_| Pep508Error {
message: Pep508ErrorSource::String(format!(
"Expected a quoted string or a valid marker name, found `{key}`"
)),
start,
len,
input: cursor.to_string(),
})
.inspect(|value| match value {
MarkerValue::MarkerEnvString(MarkerValueString::OsNameDeprecated) => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"os.name is deprecated in favor of os_name".to_string(),
);
}
MarkerValue::MarkerEnvString(MarkerValueString::PlatformMachineDeprecated) => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"platform.machine is deprecated in favor of platform_machine".to_string(),
);
}
MarkerValue::MarkerEnvString(
MarkerValueString::PlatformPythonImplementationDeprecated,
) => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"platform.python_implementation is deprecated in favor of platform_python_implementation".to_string(),
);
}
MarkerValue::MarkerEnvString(
MarkerValueString::PythonImplementationDeprecated,
) => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"python_implementation is deprecated in favor of platform_python_implementation"
.to_string(),
);
}
MarkerValue::MarkerEnvString(MarkerValueString::PlatformVersionDeprecated) => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"platform.version is deprecated in favor of platform_version"
.to_string(),
);
}
MarkerValue::MarkerEnvString(MarkerValueString::SysPlatformDeprecated) => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"sys.platform is deprecated in favor of sys_platform".to_string(),
);
}
_ => {}
})
}
}
}
@ -121,14 +168,14 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
reporter: &mut impl Reporter,
) -> Result<Option<MarkerExpression>, Pep508Error<T>> {
cursor.eat_whitespace();
let l_value = parse_marker_value(cursor)?;
let l_value = parse_marker_value(cursor, reporter)?;
cursor.eat_whitespace();
// "not in" and "in" must be preceded by whitespace. We must already have matched a whitespace
// when we're here because other `parse_marker_key` would have pulled the characters in and
// errored
let operator = parse_marker_operator(cursor)?;
cursor.eat_whitespace();
let r_value = parse_marker_value(cursor)?;
let r_value = parse_marker_value(cursor, reporter)?;
// Convert a `<marker_value> <marker_op> <marker_value>` expression into its
// typed equivalent.

View file

@ -51,7 +51,7 @@ fn collect_dnf(
let current = path.len();
for version in excluded {
path.push(MarkerExpression::Version {
key: marker.key(),
key: marker.key().into(),
specifier: VersionSpecifier::not_equals_version(version.clone()),
});
}
@ -64,7 +64,7 @@ fn collect_dnf(
// Detect whether the range for this edge can be simplified as a star inequality.
if let Some(specifier) = star_range_inequality(&range) {
path.push(MarkerExpression::Version {
key: marker.key(),
key: marker.key().into(),
specifier,
});
@ -77,7 +77,7 @@ fn collect_dnf(
let current = path.len();
for specifier in VersionSpecifier::from_release_only_bounds(bounds) {
path.push(MarkerExpression::Version {
key: marker.key(),
key: marker.key().into(),
specifier,
});
}
@ -94,7 +94,7 @@ fn collect_dnf(
let current = path.len();
for value in excluded {
path.push(MarkerExpression::String {
key: marker.key(),
key: marker.key().into(),
operator: MarkerOperator::NotEqual,
value: value.clone(),
});
@ -109,7 +109,7 @@ fn collect_dnf(
let current = path.len();
for (operator, value) in MarkerOperator::from_bounds(bounds) {
path.push(MarkerExpression::String {
key: marker.key(),
key: marker.key().into(),
operator,
value: value.clone(),
});
@ -129,7 +129,7 @@ fn collect_dnf(
};
let expr = MarkerExpression::String {
key: marker.key(),
key: marker.key().into(),
value: marker.value().to_owned(),
operator,
};
@ -148,7 +148,7 @@ fn collect_dnf(
};
let expr = MarkerExpression::String {
key: marker.key(),
key: marker.key().into(),
value: marker.value().to_owned(),
operator,
};
@ -167,7 +167,7 @@ fn collect_dnf(
};
let expr = MarkerExpression::Extra {
name: marker.name().clone(),
name: marker.name().clone().into(),
operator,
};

View file

@ -10,15 +10,17 @@ use uv_normalize::ExtraName;
use uv_pep440::{Version, VersionParseError, VersionSpecifier};
use version_ranges::Ranges;
use super::algebra::{Edges, NodeId, Variable, INTERNER};
use super::simplify;
use crate::cursor::Cursor;
use crate::marker::lowering::{
LoweredMarkerValueExtra, LoweredMarkerValueString, LoweredMarkerValueVersion,
};
use crate::marker::parse;
use crate::{
MarkerEnvironment, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter, TracingReporter,
};
use super::algebra::{Edges, NodeId, Variable, INTERNER};
use super::simplify;
/// Ways in which marker evaluation can fail
#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
pub enum MarkerWarningKind {
@ -823,7 +825,6 @@ impl MarkerTree {
/// Does this marker apply in the given environment?
pub fn evaluate(&self, env: &MarkerEnvironment, extras: &[ExtraName]) -> bool {
self.report_deprecated_options(&mut TracingReporter);
self.evaluate_reporter_impl(env, extras, &mut TracingReporter)
}
@ -839,7 +840,6 @@ impl MarkerTree {
env: Option<&MarkerEnvironment>,
extras: &[ExtraName],
) -> bool {
self.report_deprecated_options(&mut TracingReporter);
match env {
None => self.evaluate_extras(extras),
Some(env) => self.evaluate_reporter_impl(env, extras, &mut TracingReporter),
@ -854,7 +854,6 @@ impl MarkerTree {
extras: &[ExtraName],
reporter: &mut impl Reporter,
) -> bool {
self.report_deprecated_options(reporter);
self.evaluate_reporter_impl(env, extras, reporter)
}
@ -915,12 +914,7 @@ impl MarkerTree {
}
MarkerTreeKind::Extra(marker) => {
return marker
.edge(
marker
.name()
.as_extra()
.is_some_and(|extra| extras.contains(extra)),
)
.edge(extras.contains(marker.name().extra()))
.evaluate_reporter_impl(env, extras, reporter);
}
}
@ -945,7 +939,7 @@ impl MarkerTree {
MarkerTreeKind::True => true,
MarkerTreeKind::False => false,
MarkerTreeKind::Version(marker) => marker.edges().any(|(range, tree)| {
if marker.key() == MarkerValueVersion::PythonVersion {
if marker.key() == LoweredMarkerValueVersion::PythonVersion {
if !python_versions
.iter()
.any(|version| range.contains(version))
@ -966,12 +960,7 @@ impl MarkerTree {
.children()
.any(|(_, tree)| tree.evaluate_extras_and_python_version(extras, python_versions)),
MarkerTreeKind::Extra(marker) => marker
.edge(
marker
.name()
.as_extra()
.is_some_and(|extra| extras.contains(extra)),
)
.edge(extras.contains(marker.name().extra()))
.evaluate_extras_and_python_version(extras, python_versions),
}
}
@ -996,112 +985,11 @@ impl MarkerTree {
.children()
.any(|(_, tree)| tree.evaluate_extras(extras)),
MarkerTreeKind::Extra(marker) => marker
.edge(
marker
.name()
.as_extra()
.is_some_and(|extra| extras.contains(extra)),
)
.edge(extras.contains(marker.name().extra()))
.evaluate_extras(extras),
}
}
/// Same as [`Self::evaluate`], but instead of using logging to warn, you get a Vec with all
/// warnings collected
pub fn evaluate_collect_warnings(
&self,
env: &MarkerEnvironment,
extras: &[ExtraName],
) -> (bool, Vec<(MarkerWarningKind, String)>) {
let mut warnings = Vec::new();
let mut reporter = |kind, warning| {
warnings.push((kind, warning));
};
self.report_deprecated_options(&mut reporter);
let result = self.evaluate_reporter_impl(env, extras, &mut reporter);
(result, warnings)
}
/// Report the deprecated marker from <https://peps.python.org/pep-0345/#environment-markers>
fn report_deprecated_options(&self, reporter: &mut impl Reporter) {
let string_marker = match self.kind() {
MarkerTreeKind::True | MarkerTreeKind::False => return,
MarkerTreeKind::String(marker) => marker,
MarkerTreeKind::Version(marker) => {
for (_, tree) in marker.edges() {
tree.report_deprecated_options(reporter);
}
return;
}
MarkerTreeKind::In(marker) => {
for (_, tree) in marker.children() {
tree.report_deprecated_options(reporter);
}
return;
}
MarkerTreeKind::Contains(marker) => {
for (_, tree) in marker.children() {
tree.report_deprecated_options(reporter);
}
return;
}
MarkerTreeKind::Extra(marker) => {
for (_, tree) in marker.children() {
tree.report_deprecated_options(reporter);
}
return;
}
};
match string_marker.key() {
MarkerValueString::OsNameDeprecated => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"os.name is deprecated in favor of os_name".to_string(),
);
}
MarkerValueString::PlatformMachineDeprecated => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"platform.machine is deprecated in favor of platform_machine".to_string(),
);
}
MarkerValueString::PlatformPythonImplementationDeprecated => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"platform.python_implementation is deprecated in favor of
platform_python_implementation"
.to_string(),
);
}
MarkerValueString::PythonImplementationDeprecated => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"python_implementation is deprecated in favor of
platform_python_implementation"
.to_string(),
);
}
MarkerValueString::PlatformVersionDeprecated => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"platform.version is deprecated in favor of platform_version".to_string(),
);
}
MarkerValueString::SysPlatformDeprecated => {
reporter.report(
MarkerWarningKind::DeprecatedMarkerName,
"sys.platform is deprecated in favor of sys_platform".to_string(),
);
}
_ => {}
}
for (_, tree) in string_marker.children() {
tree.report_deprecated_options(reporter);
}
}
/// Find a top level `extra == "..."` expression.
///
/// ASSUMPTION: There is one `extra = "..."`, and it's either the only marker or part of the
@ -1240,13 +1128,9 @@ impl MarkerTree {
}
fn simplify_extras_with_impl(self, is_extra: &impl Fn(&ExtraName) -> bool) -> MarkerTree {
MarkerTree(INTERNER.lock().restrict(self.0, &|var| {
match var {
Variable::Extra(name) => name
.as_extra()
.and_then(|name| is_extra(name).then_some(true)),
_ => None,
}
MarkerTree(INTERNER.lock().restrict(self.0, &|var| match var {
Variable::Extra(name) => is_extra(name.extra()).then_some(true),
_ => None,
}))
}
}
@ -1430,13 +1314,13 @@ pub enum MarkerTreeKind<'a> {
#[derive(PartialEq, Eq, Clone, Debug)]
pub struct VersionMarkerTree<'a> {
id: NodeId,
key: MarkerValueVersion,
key: LoweredMarkerValueVersion,
map: &'a [(Ranges<Version>, NodeId)],
}
impl VersionMarkerTree<'_> {
/// The key for this node.
pub fn key(&self) -> MarkerValueVersion {
pub fn key(&self) -> LoweredMarkerValueVersion {
self.key
}
@ -1466,13 +1350,13 @@ impl Ord for VersionMarkerTree<'_> {
#[derive(PartialEq, Eq, Clone, Debug)]
pub struct StringMarkerTree<'a> {
id: NodeId,
key: MarkerValueString,
key: LoweredMarkerValueString,
map: &'a [(Ranges<String>, NodeId)],
}
impl StringMarkerTree<'_> {
/// The key for this node.
pub fn key(&self) -> MarkerValueString {
pub fn key(&self) -> LoweredMarkerValueString {
self.key
}
@ -1501,7 +1385,7 @@ impl Ord for StringMarkerTree<'_> {
/// A string marker node with the `in` operator, such as `os_name in 'WindowsLinux'`.
#[derive(PartialEq, Eq, Clone, Debug)]
pub struct InMarkerTree<'a> {
key: MarkerValueString,
key: LoweredMarkerValueString,
value: &'a str,
high: NodeId,
low: NodeId,
@ -1509,7 +1393,7 @@ pub struct InMarkerTree<'a> {
impl InMarkerTree<'_> {
/// The key (LHS) for this expression.
pub fn key(&self) -> MarkerValueString {
pub fn key(&self) -> LoweredMarkerValueString {
self.key
}
@ -1551,7 +1435,7 @@ impl Ord for InMarkerTree<'_> {
/// A string marker node with inverse of the `in` operator, such as `'nux' in os_name`.
#[derive(PartialEq, Eq, Clone, Debug)]
pub struct ContainsMarkerTree<'a> {
key: MarkerValueString,
key: LoweredMarkerValueString,
value: &'a str,
high: NodeId,
low: NodeId,
@ -1559,7 +1443,7 @@ pub struct ContainsMarkerTree<'a> {
impl ContainsMarkerTree<'_> {
/// The key (LHS) for this expression.
pub fn key(&self) -> MarkerValueString {
pub fn key(&self) -> LoweredMarkerValueString {
self.key
}
@ -1601,14 +1485,14 @@ impl Ord for ContainsMarkerTree<'_> {
/// A node representing the existence or absence of a given extra, such as `extra == 'bar'`.
#[derive(PartialEq, Eq, Clone, Debug)]
pub struct ExtraMarkerTree<'a> {
name: &'a MarkerValueExtra,
name: &'a LoweredMarkerValueExtra,
high: NodeId,
low: NodeId,
}
impl ExtraMarkerTree<'_> {
/// Returns the name of the extra in this expression.
pub fn name(&self) -> &MarkerValueExtra {
pub fn name(&self) -> &LoweredMarkerValueExtra {
self.name
}
@ -2023,14 +1907,15 @@ mod test {
let expected = [
"WARN warnings4: uv_pep508: os.name is deprecated in favor of os_name",
"WARN warnings4: uv_pep508: platform.machine is deprecated in favor of platform_machine",
"WARN warnings4: uv_pep508: platform.python_implementation is deprecated in favor of",
"WARN warnings4: uv_pep508: sys.platform is deprecated in favor of sys_platform",
"WARN warnings4: uv_pep508: platform.python_implementation is deprecated in favor of platform_python_implementation",
"WARN warnings4: uv_pep508: platform.version is deprecated in favor of platform_version",
"WARN warnings4: uv_pep508: sys.platform is deprecated in favor of sys_platform",
"WARN warnings4: uv_pep508: Comparing linux and posix lexicographically"
];
if lines == expected {
Ok(())
} else {
Err(format!("{lines:?}"))
Err(format!("{lines:#?}"))
}
});
}
@ -2043,59 +1928,52 @@ mod test {
#[test]
fn test_marker_version_inverted() {
let env37 = env37();
let (result, warnings) = MarkerTree::from_str("python_version > '3.6'")
let result = MarkerTree::from_str("python_version > '3.6'")
.unwrap()
.evaluate_collect_warnings(&env37, &[]);
assert_eq!(warnings, &[]);
.evaluate(&env37, &[]);
assert!(result);
let (result, warnings) = MarkerTree::from_str("'3.6' > python_version")
let result = MarkerTree::from_str("'3.6' > python_version")
.unwrap()
.evaluate_collect_warnings(&env37, &[]);
assert_eq!(warnings, &[]);
.evaluate(&env37, &[]);
assert!(!result);
// Meaningless expressions are ignored, so this is always true.
let (result, warnings) = MarkerTree::from_str("'3.*' == python_version")
let result = MarkerTree::from_str("'3.*' == python_version")
.unwrap()
.evaluate_collect_warnings(&env37, &[]);
assert_eq!(warnings, &[]);
.evaluate(&env37, &[]);
assert!(result);
}
#[test]
fn test_marker_string_inverted() {
let env37 = env37();
let (result, warnings) = MarkerTree::from_str("'nux' in sys_platform")
let result = MarkerTree::from_str("'nux' in sys_platform")
.unwrap()
.evaluate_collect_warnings(&env37, &[]);
assert_eq!(warnings, &[]);
.evaluate(&env37, &[]);
assert!(result);
let (result, warnings) = MarkerTree::from_str("sys_platform in 'nux'")
let result = MarkerTree::from_str("sys_platform in 'nux'")
.unwrap()
.evaluate_collect_warnings(&env37, &[]);
assert_eq!(warnings, &[]);
.evaluate(&env37, &[]);
assert!(!result);
}
#[test]
fn test_marker_version_star() {
let env37 = env37();
let (result, warnings) = MarkerTree::from_str("python_version == '3.7.*'")
let result = MarkerTree::from_str("python_version == '3.7.*'")
.unwrap()
.evaluate_collect_warnings(&env37, &[]);
assert_eq!(warnings, &[]);
.evaluate(&env37, &[]);
assert!(result);
}
#[test]
fn test_tilde_equal() {
let env37 = env37();
let (result, warnings) = MarkerTree::from_str("python_version ~= '3.7'")
let result = MarkerTree::from_str("python_version ~= '3.7'")
.unwrap()
.evaluate_collect_warnings(&env37, &[]);
assert_eq!(warnings, &[]);
.evaluate(&env37, &[]);
assert!(result);
}

View file

@ -1,7 +1,8 @@
use crate::requires_python::{LowerBound, RequiresPythonRange, UpperBound};
use pubgrub::Range;
use uv_pep440::Version;
use uv_pep508::{MarkerTree, MarkerTreeKind, MarkerValueVersion};
use uv_pep508::{LoweredMarkerValueVersion, MarkerTree, MarkerTreeKind};
use crate::requires_python::{LowerBound, RequiresPythonRange, UpperBound};
/// Returns the bounding Python versions that can satisfy the [`MarkerTree`], if it's constrained.
pub(crate) fn requires_python(tree: &MarkerTree) -> Option<RequiresPythonRange> {
@ -9,14 +10,15 @@ pub(crate) fn requires_python(tree: &MarkerTree) -> Option<RequiresPythonRange>
match tree.kind() {
MarkerTreeKind::True | MarkerTreeKind::False => {}
MarkerTreeKind::Version(marker) => match marker.key() {
MarkerValueVersion::PythonVersion | MarkerValueVersion::PythonFullVersion => {
LoweredMarkerValueVersion::PythonVersion
| LoweredMarkerValueVersion::PythonFullVersion => {
for (range, tree) in marker.edges() {
if !tree.is_false() {
markers.push(range.clone());
}
}
}
MarkerValueVersion::ImplementationVersion => {
LoweredMarkerValueVersion::ImplementationVersion => {
for (_, tree) in marker.edges() {
collect_python_markers(&tree, markers);
}

View file

@ -587,7 +587,8 @@ impl ResolverOutput {
marker_env: &MarkerEnvironment,
) -> Result<MarkerTree, Box<ParsedUrlError>> {
use uv_pep508::{
MarkerExpression, MarkerOperator, MarkerTree, MarkerValueString, MarkerValueVersion,
LoweredMarkerValueString, LoweredMarkerValueVersion, MarkerExpression, MarkerOperator,
MarkerTree,
};
/// A subset of the possible marker values.
@ -597,8 +598,8 @@ impl ResolverOutput {
/// values based on the current marker environment.
#[derive(Debug, Eq, Hash, PartialEq)]
enum MarkerParam {
Version(MarkerValueVersion),
String(MarkerValueString),
Version(LoweredMarkerValueVersion),
String(LoweredMarkerValueString),
}
/// Add all marker parameters from the given tree to the given set.
@ -688,14 +689,14 @@ impl ResolverOutput {
MarkerParam::Version(value_version) => {
let from_env = marker_env.get_version(value_version);
MarkerExpression::Version {
key: value_version,
key: value_version.into(),
specifier: VersionSpecifier::equals_version(from_env.clone()),
}
}
MarkerParam::String(value_string) => {
let from_env = marker_env.get_string(value_string);
MarkerExpression::String {
key: value_string,
key: value_string.into(),
operator: MarkerOperator::Equal,
value: from_env.to_string(),
}