Skip to content

Commit

Permalink
Move red_knot_python_semantic::PythonVersion to the `ruff_python_as…
Browse files Browse the repository at this point in the history
…t` crate (#16147)

## Summary

This PR moves the `PythonVersion` struct from the
`red_knot_python_semantic` crate to the `ruff_python_ast` crate so that
it can be used more easily in the syntax error detection work. Compared
to that [prototype](#16090) these
changes reduce us from 2 `PythonVersion` structs to 1.

This does not unify any of the `PythonVersion` *enums*, but I hope to
make some progress on that in a follow-up.

## Test Plan

Existing tests, this should not change any external behavior.

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
  • Loading branch information
ntBre and AlexWaygood authored Feb 14, 2025
1 parent fa28dc5 commit f58a54f
Show file tree
Hide file tree
Showing 27 changed files with 62 additions and 59 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/red_knot/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ red_knot_python_semantic = { workspace = true }
red_knot_project = { workspace = true, features = ["zstd"] }
red_knot_server = { workspace = true }
ruff_db = { workspace = true, features = ["os", "cache"] }
ruff_python_ast = { workspace = true }

anyhow = { workspace = true }
chrono = { workspace = true }
Expand Down
6 changes: 3 additions & 3 deletions crates/red_knot/src/python_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl std::fmt::Display for PythonVersion {
}
}

impl From<PythonVersion> for red_knot_python_semantic::PythonVersion {
impl From<PythonVersion> for ruff_python_ast::python_version::PythonVersion {
fn from(value: PythonVersion) -> Self {
match value {
PythonVersion::Py37 => Self::PY37,
Expand All @@ -61,8 +61,8 @@ mod tests {
#[test]
fn same_default_as_python_version() {
assert_eq!(
red_knot_python_semantic::PythonVersion::from(PythonVersion::default()),
red_knot_python_semantic::PythonVersion::default()
ruff_python_ast::python_version::PythonVersion::from(PythonVersion::default()),
ruff_python_ast::python_version::PythonVersion::default()
);
}
}
3 changes: 2 additions & 1 deletion crates/red_knot/tests/file_watching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ use red_knot_project::metadata::pyproject::{PyProject, Tool};
use red_knot_project::metadata::value::{RangedValue, RelativePathBuf};
use red_knot_project::watch::{directory_watcher, ChangeEvent, ProjectWatcher};
use red_knot_project::{Db, ProjectDatabase, ProjectMetadata};
use red_knot_python_semantic::{resolve_module, ModuleName, PythonPlatform, PythonVersion};
use red_knot_python_semantic::{resolve_module, ModuleName, PythonPlatform};
use ruff_db::files::{system_path_to_file, File, FileError};
use ruff_db::source::source_text;
use ruff_db::system::{
OsSystem, System, SystemPath, SystemPathBuf, UserConfigDirectoryOverrideGuard,
};
use ruff_db::Upcast;
use ruff_python_ast::python_version::PythonVersion;

struct TestCase {
db: ProjectDatabase,
Expand Down
3 changes: 2 additions & 1 deletion crates/red_knot_project/src/combine.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::{collections::HashMap, hash::BuildHasher};

use red_knot_python_semantic::{PythonPlatform, PythonVersion, SitePackages};
use red_knot_python_semantic::{PythonPlatform, SitePackages};
use ruff_db::system::SystemPathBuf;
use ruff_python_ast::python_version::PythonVersion;

/// Combine two values, preferring the values in `self`.
///
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_project/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,8 @@ mod tests {
use anyhow::{anyhow, Context};
use insta::assert_ron_snapshot;
use red_knot_python_semantic::PythonVersion;
use ruff_db::system::{SystemPathBuf, TestSystem};
use ruff_python_ast::python_version::PythonVersion;

use crate::{ProjectDiscoveryError, ProjectMetadata};

Expand Down
5 changes: 2 additions & 3 deletions crates/red_knot_project/src/metadata/options.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use crate::metadata::value::{RangedValue, RelativePathBuf, ValueSource, ValueSourceGuard};
use crate::Db;
use red_knot_python_semantic::lint::{GetLintError, Level, LintSource, RuleSelection};
use red_knot_python_semantic::{
ProgramSettings, PythonPlatform, PythonVersion, SearchPathSettings, SitePackages,
};
use red_knot_python_semantic::{ProgramSettings, PythonPlatform, SearchPathSettings, SitePackages};
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity, Span};
use ruff_db::files::system_path_to_file;
use ruff_db::system::{System, SystemPath};
use ruff_macros::Combine;
use ruff_python_ast::python_version::PythonVersion;
use rustc_hash::FxHashMap;
use serde::{Deserialize, Serialize};
use std::borrow::Cow;
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_project/src/metadata/pyproject.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::metadata::options::Options;
use crate::metadata::value::{RangedValue, ValueSource, ValueSourceGuard};
use pep440_rs::{release_specifiers_to_ranges, Version, VersionSpecifiers};
use red_knot_python_semantic::PythonVersion;
use ruff_python_ast::python_version::PythonVersion;
use serde::{Deserialize, Deserializer, Serialize};
use std::collections::Bound;
use std::ops::Deref;
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ pub(crate) mod tests {
use std::sync::Arc;

use crate::program::{Program, SearchPathSettings};
use crate::python_version::PythonVersion;
use crate::{default_lint_registry, ProgramSettings, PythonPlatform};

use super::Db;
Expand All @@ -29,6 +28,7 @@ pub(crate) mod tests {
use ruff_db::system::{DbWithTestSystem, System, SystemPathBuf, TestSystem};
use ruff_db::vendored::VendoredFileSystem;
use ruff_db::{Db as SourceDb, Upcast};
use ruff_python_ast::python_version::PythonVersion;

#[salsa::db]
#[derive(Clone)]
Expand Down
2 changes: 0 additions & 2 deletions crates/red_knot_python_semantic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ pub use module_name::ModuleName;
pub use module_resolver::{resolve_module, system_module_search_paths, KnownModule, Module};
pub use program::{Program, ProgramSettings, SearchPathSettings, SitePackages};
pub use python_platform::PythonPlatform;
pub use python_version::PythonVersion;
pub use semantic_model::{HasType, SemanticModel};

pub mod ast_node_ref;
Expand All @@ -20,7 +19,6 @@ mod module_resolver;
mod node_key;
mod program;
mod python_platform;
mod python_version;
pub mod semantic_index;
mod semantic_model;
pub(crate) mod site_packages;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,10 +631,10 @@ impl PartialEq<SearchPath> for VendoredPathBuf {
#[cfg(test)]
mod tests {
use ruff_db::Db;
use ruff_python_ast::python_version::PythonVersion;

use crate::db::tests::TestDb;
use crate::module_resolver::testing::{FileSpec, MockedTypeshed, TestCase, TestCaseBuilder};
use crate::python_version::PythonVersion;

use super::*;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ use rustc_hash::{FxBuildHasher, FxHashSet};
use ruff_db::files::{File, FilePath, FileRootKind};
use ruff_db::system::{DirectoryEntry, System, SystemPath, SystemPathBuf};
use ruff_db::vendored::{VendoredFileSystem, VendoredPath};
use ruff_python_ast::python_version::PythonVersion;

use crate::db::Db;
use crate::module_name::ModuleName;
use crate::module_resolver::typeshed::{vendored_typeshed_versions, TypeshedVersions};
use crate::site_packages::VirtualEnvironment;
use crate::{Program, PythonVersion, SearchPathSettings, SitePackages};
use crate::{Program, SearchPathSettings, SitePackages};

use super::module::{Module, ModuleKind};
use super::path::{ModulePath, SearchPath, SearchPathValidationError};
Expand Down Expand Up @@ -724,12 +725,12 @@ mod tests {
assert_const_function_query_was_not_run, assert_function_query_was_not_run,
};
use ruff_db::Db;
use ruff_python_ast::python_version::PythonVersion;

use crate::db::tests::TestDb;
use crate::module_name::ModuleName;
use crate::module_resolver::module::ModuleKind;
use crate::module_resolver::testing::{FileSpec, MockedTypeshed, TestCase, TestCaseBuilder};
use crate::PythonVersion;
use crate::{ProgramSettings, PythonPlatform};

use super::*;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use ruff_db::system::{DbWithTestSystem, SystemPath, SystemPathBuf};
use ruff_db::vendored::VendoredPathBuf;
use ruff_python_ast::python_version::PythonVersion;

use crate::db::tests::TestDb;
use crate::program::{Program, SearchPathSettings};
use crate::python_version::PythonVersion;
use crate::{ProgramSettings, PythonPlatform, SitePackages};

/// A test case for the module resolver.
Expand Down
39 changes: 20 additions & 19 deletions crates/red_knot_python_semantic/src/module_resolver/typeshed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ use std::num::{NonZeroU16, NonZeroUsize};
use std::ops::{RangeFrom, RangeInclusive};
use std::str::FromStr;

use ruff_python_ast::python_version::PythonVersion;
use rustc_hash::FxHashMap;

use crate::db::Db;
use crate::module_name::ModuleName;
use crate::{Program, PythonVersion};
use crate::Program;

pub(in crate::module_resolver) fn vendored_typeshed_versions(db: &dyn Db) -> TypeshedVersions {
TypeshedVersions::from_str(
Expand Down Expand Up @@ -278,12 +279,12 @@ impl FromStr for PyVersionRange {
let mut parts = s.split('-').map(str::trim);
match (parts.next(), parts.next(), parts.next()) {
(Some(lower), Some(""), None) => {
let lower = PythonVersion::from_versions_file_string(lower)?;
let lower = python_version_from_versions_file_string(lower)?;
Ok(Self::AvailableFrom(lower..))
}
(Some(lower), Some(upper), None) => {
let lower = PythonVersion::from_versions_file_string(lower)?;
let upper = PythonVersion::from_versions_file_string(upper)?;
let lower = python_version_from_versions_file_string(lower)?;
let upper = python_version_from_versions_file_string(upper)?;
Ok(Self::AvailableWithin(lower..=upper))
}
_ => Err(TypeshedVersionsParseErrorKind::UnexpectedNumberOfHyphens),
Expand All @@ -302,21 +303,21 @@ impl fmt::Display for PyVersionRange {
}
}

impl PythonVersion {
fn from_versions_file_string(s: &str) -> Result<Self, TypeshedVersionsParseErrorKind> {
let mut parts = s.split('.').map(str::trim);
let (Some(major), Some(minor), None) = (parts.next(), parts.next(), parts.next()) else {
return Err(TypeshedVersionsParseErrorKind::UnexpectedNumberOfPeriods(
s.to_string(),
));
};
PythonVersion::try_from((major, minor)).map_err(|int_parse_error| {
TypeshedVersionsParseErrorKind::IntegerParsingFailure {
version: s.to_string(),
err: int_parse_error,
}
})
}
fn python_version_from_versions_file_string(
s: &str,
) -> Result<PythonVersion, TypeshedVersionsParseErrorKind> {
let mut parts = s.split('.').map(str::trim);
let (Some(major), Some(minor), None) = (parts.next(), parts.next(), parts.next()) else {
return Err(TypeshedVersionsParseErrorKind::UnexpectedNumberOfPeriods(
s.to_string(),
));
};
PythonVersion::try_from((major, minor)).map_err(|int_parse_error| {
TypeshedVersionsParseErrorKind::IntegerParsingFailure {
version: s.to_string(),
err: int_parse_error,
}
})
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/src/program.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::module_resolver::SearchPaths;
use crate::python_platform::PythonPlatform;
use crate::python_version::PythonVersion;
use crate::Db;

use anyhow::Context;
use ruff_db::system::{SystemPath, SystemPathBuf};
use ruff_python_ast::python_version::PythonVersion;
use salsa::Durability;
use salsa::Setter;

Expand Down
3 changes: 1 addition & 2 deletions crates/red_knot_python_semantic/src/site_packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ use std::num::NonZeroUsize;
use std::ops::Deref;

use ruff_db::system::{System, SystemPath, SystemPathBuf};

use crate::PythonVersion;
use ruff_python_ast::python_version::PythonVersion;

type SitePackagesDiscoveryResult<T> = Result<T, SitePackagesDiscoveryError>;

Expand Down
5 changes: 3 additions & 2 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use itertools::Itertools;
use ruff_db::diagnostic::Severity;
use ruff_db::files::File;
use ruff_python_ast as ast;
use ruff_python_ast::python_version::PythonVersion;
use type_ordering::union_elements_ordering;

pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder};
Expand Down Expand Up @@ -43,7 +44,7 @@ use crate::types::diagnostic::INVALID_TYPE_FORM;
use crate::types::infer::infer_unpack_types;
use crate::types::mro::{Mro, MroError, MroIterator};
use crate::types::narrow::narrowing_constraint;
use crate::{Db, FxOrderSet, Module, Program, PythonVersion};
use crate::{Db, FxOrderSet, Module, Program};

mod builder;
mod call;
Expand Down Expand Up @@ -4949,12 +4950,12 @@ pub(crate) mod tests {
use super::*;
use crate::db::tests::{setup_db, TestDbBuilder};
use crate::stdlib::typing_symbol;
use crate::PythonVersion;
use ruff_db::files::system_path_to_file;
use ruff_db::parsed::parsed_module;
use ruff_db::system::DbWithTestSystem;
use ruff_db::testing::assert_function_query_was_not_run;
use ruff_python_ast as ast;
use ruff_python_ast::python_version::PythonVersion;
use test_case::test_case;

/// Explicitly test for Python version <3.13 and >=3.13, to ensure that
Expand Down
3 changes: 2 additions & 1 deletion crates/red_knot_test/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
//! ```
use anyhow::Context;
use red_knot_python_semantic::{PythonPlatform, PythonVersion};
use red_knot_python_semantic::PythonPlatform;
use ruff_python_ast::python_version::PythonVersion;
use serde::Deserialize;

#[derive(Deserialize, Debug, Default, Clone)]
Expand Down
3 changes: 2 additions & 1 deletion crates/red_knot_test/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ use std::sync::Arc;
use red_knot_python_semantic::lint::{LintRegistry, RuleSelection};
use red_knot_python_semantic::{
default_lint_registry, Db as SemanticDb, Program, ProgramSettings, PythonPlatform,
PythonVersion, SearchPathSettings,
SearchPathSettings,
};
use ruff_db::files::{File, Files};
use ruff_db::system::{DbWithTestSystem, System, SystemPath, SystemPathBuf, TestSystem};
use ruff_db::vendored::VendoredFileSystem;
use ruff_db::{Db as SourceDb, Upcast};
use ruff_python_ast::python_version::PythonVersion;

#[salsa::db]
#[derive(Clone)]
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ doctest = false
default = ["console_error_panic_hook"]

[dependencies]
red_knot_python_semantic = { workspace = true }
red_knot_project = { workspace = true, default-features = false, features = ["deflate"] }

ruff_db = { workspace = true, default-features = false, features = [] }
ruff_python_ast = { workspace = true }
ruff_notebook = { workspace = true }

console_error_panic_hook = { workspace = true, optional = true }
Expand Down
6 changes: 3 additions & 3 deletions crates/red_knot_wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ pub enum PythonVersion {
Py313,
}

impl From<PythonVersion> for red_knot_python_semantic::PythonVersion {
impl From<PythonVersion> for ruff_python_ast::python_version::PythonVersion {
fn from(value: PythonVersion) -> Self {
match value {
PythonVersion::Py37 => Self::PY37,
Expand Down Expand Up @@ -308,8 +308,8 @@ mod tests {
#[test]
fn same_default_as_python_version() {
assert_eq!(
red_knot_python_semantic::PythonVersion::from(PythonVersion::default()),
red_knot_python_semantic::PythonVersion::default()
ruff_python_ast::python_version::PythonVersion::from(PythonVersion::default()),
ruff_python_ast::python_version::PythonVersion::default()
);
}
}
1 change: 0 additions & 1 deletion crates/ruff_benchmark/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ ruff_python_ast = { workspace = true }
ruff_python_formatter = { workspace = true }
ruff_python_parser = { workspace = true }
ruff_python_trivia = { workspace = true }
red_knot_python_semantic = { workspace = true }
red_knot_project = { workspace = true }

[lints]
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_benchmark/benches/red_knot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ use red_knot_project::metadata::options::{EnvironmentOptions, Options};
use red_knot_project::metadata::value::RangedValue;
use red_knot_project::watch::{ChangeEvent, ChangedKind};
use red_knot_project::{Db, ProjectDatabase, ProjectMetadata};
use red_knot_python_semantic::PythonVersion;
use ruff_benchmark::criterion::{criterion_group, criterion_main, BatchSize, Criterion};
use ruff_benchmark::TestFile;
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity};
use ruff_db::files::{system_path_to_file, File};
use ruff_db::source::source_text;
use ruff_db::system::{MemoryFileSystem, SystemPath, SystemPathBuf, TestSystem};
use ruff_python_ast::python_version::PythonVersion;
use rustc_hash::FxHashSet;

struct Case {
Expand Down
Loading

0 comments on commit f58a54f

Please sign in to comment.