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.

Sign up for GitHub

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

Report marker diagnostics during parsing, rather than evaluation #9338

Merged
merged 2 commits into from
Nov 26, 2024
Merged

Report marker diagnostics during parsing, rather than evaluation #9338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96b269a
Report marker diagnostics during parsing, rather than evaluation
charliermarsh Nov 21, 2024
274b7db
Add a lowered representation for markers (#9341)
charliermarsh Nov 25, 2024
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
14 changes: 2 additions & 12 deletions crates/uv-pep508/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
90 changes: 66 additions & 24 deletions crates/uv-pep508/src/marker/algebra.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -172,24 +175,25 @@ 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 {
key,
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.
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
};
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
43 changes: 22 additions & 21 deletions crates/uv-pep508/src/marker/environment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -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(),
}
}
}
Expand Down
Loading
Loading