Skip to content

Commit

Permalink
Code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Aug 21, 2024
1 parent 8c41162 commit ba1a840
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 41 deletions.
2 changes: 1 addition & 1 deletion crates/red_knot/tests/file_watching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ fn changed_versions_file() -> anyhow::Result<()> {
},
)?;

// Remove site packages from the search path settings.
// Unset the custom typeshed directory.
assert_eq!(
resolve_module(case.db(), ModuleName::new("os").unwrap()),
None
Expand Down
6 changes: 3 additions & 3 deletions crates/red_knot_python_semantic/src/module_resolver/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ pub(crate) enum SearchPathValidationError {
NoStdlibSubdirectory(SystemPathBuf),

/// The typeshed path provided by the user is a directory,
/// but `stdlib/VERSIONS` could not be read
/// but `stdlib/VERSIONS` could not be read.
/// (This is only relevant for stdlib search paths.)
FailedToReadVersionsFile {
path: SystemPathBuf,
Expand All @@ -312,7 +312,7 @@ pub(crate) enum SearchPathValidationError {
/// (This is only relevant for stdlib search paths.)
VersionsParseError(TypeshedVersionsParseError),

/// Failed to discover the site packages for the configured virtual environment.
/// Failed to discover the site-packages for the configured virtual environment.
SitePackagesDiscovery(SitePackagesDiscoveryError),
}

Expand All @@ -331,7 +331,7 @@ impl fmt::Display for SearchPathValidationError {
}
Self::VersionsParseError(underlying_error) => underlying_error.fmt(f),
SearchPathValidationError::SitePackagesDiscovery(error) => {
write!(f, "Failed to discover the site packages: {error}")
write!(f, "Failed to discover the site-packages directory: {error}")
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions crates/red_knot_python_semantic/src/module_resolver/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ pub(crate) struct SearchPaths {
/// for the first `site-packages` path
site_packages: Vec<SearchPath>,

typeshed_versions: Typeshed,
typeshed_versions: ResolvedTypeshedVersions,
}

impl SearchPaths {
Expand Down Expand Up @@ -202,11 +202,11 @@ impl SearchPaths {

let search_path = SearchPath::custom_stdlib(db, &custom_typeshed)?;

(Typeshed::Custom(parsed), search_path)
(ResolvedTypeshedVersions::Custom(parsed), search_path)
} else {
tracing::debug!("Using vendored stdlib");
(
Typeshed::Vendored(vendored_typeshed_versions()),
ResolvedTypeshedVersions::Vendored(vendored_typeshed_versions()),
SearchPath::vendored_stdlib(),
)
};
Expand Down Expand Up @@ -280,18 +280,18 @@ impl SearchPaths {
}

#[derive(Debug, PartialEq, Eq)]
enum Typeshed {
enum ResolvedTypeshedVersions {
Vendored(&'static TypeshedVersions),
Custom(TypeshedVersions),
}

impl Deref for Typeshed {
impl Deref for ResolvedTypeshedVersions {
type Target = TypeshedVersions;

fn deref(&self) -> &Self::Target {
match self {
Typeshed::Vendored(versions) => versions,
Typeshed::Custom(versions) => versions,
ResolvedTypeshedVersions::Vendored(versions) => versions,
ResolvedTypeshedVersions::Custom(versions) => versions,
}
}
}
Expand Down Expand Up @@ -693,8 +693,8 @@ impl PackageKind {
}

pub(super) struct ResolverContext<'db> {
pub(crate) db: &'db dyn Db,
pub(crate) target_version: PythonVersion,
pub(super) db: &'db dyn Db,
pub(super) target_version: PythonVersion,
}

impl<'db> ResolverContext<'db> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl TypeshedVersions {
}

#[must_use]
pub(crate) fn query_module(
pub(in crate::module_resolver) fn query_module(
&self,
module: &ModuleName,
target_version: PythonVersion,
Expand Down
9 changes: 2 additions & 7 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,14 +392,9 @@ mod tests {

Program::from_settings(
&db,
ProgramSettings {
&ProgramSettings {
target_version: PythonVersion::default(),
search_paths: SearchPathSettings {
extra_paths: Vec::new(),
src_root: SystemPathBuf::from("/src"),
site_packages: vec![],
custom_typeshed: None,
},
search_paths: SearchPathSettings::new(SystemPathBuf::from("/src")),
},
)
.expect("Valid search path settings");
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_workspace/src/db/changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl RootDatabase {
.map(|path| path.join("VERSIONS"));

let mut workspace_change = false;
// Changes to a custom stdlib path's VERSION
// Changes to a custom stdlib path's VERSIONS
let mut custom_stdlib_change = false;
// Packages that need reloading
let mut changed_packages = FxHashSet::default();
Expand Down
16 changes: 6 additions & 10 deletions crates/red_knot_workspace/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@ use rustc_hash::{FxBuildHasher, FxHashSet};
use salsa::{Durability, Setter as _};

pub use metadata::{PackageMetadata, WorkspaceMetadata};
use red_knot_python_semantic::SearchPathSettings;
use red_knot_python_semantic::types::check_types;
use red_knot_python_semantic::SearchPathSettings;
use ruff_db::source::{line_index, source_text, SourceDiagnostic};
use ruff_db::{
files::{File, system_path_to_file},
system::{SystemPath, SystemPathBuf, walk_directory::WalkState},
files::{system_path_to_file, File},
system::{walk_directory::WalkState, SystemPath, SystemPathBuf},
};
use ruff_db::source::{line_index, source_text, SourceDiagnostic};
use ruff_python_ast::{name::Name, PySourceType};
use ruff_text_size::Ranged;

use crate::workspace::files::{Index, Indexed, PackageFiles};
use crate::{
db::Db,
lint::{lint_semantic, lint_syntax},
};
use crate::workspace::files::{Index, Indexed, PackageFiles};

mod files;
mod metadata;
Expand Down Expand Up @@ -346,11 +346,7 @@ impl Package {
tracing::debug_span!("index_package_files", package = %self.name(db)).entered();

let files = discover_package_files(db, self.root(db));
tracing::info!(
"Indexed {} files for package '{}'",
files.len(),
self.name(db)
);
tracing::info!("Found {} files in package '{}'", files.len(), self.name(db));
vacant.set(files)
}
Index::Indexed(indexed) => indexed,
Expand Down
4 changes: 2 additions & 2 deletions crates/red_knot_workspace/tests/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ use std::path::PathBuf;
use red_knot_python_semantic::{HasTy, SemanticModel};
use red_knot_workspace::db::RootDatabase;
use red_knot_workspace::workspace::WorkspaceMetadata;
use ruff_db::files::{File, system_path_to_file};
use ruff_db::files::{system_path_to_file, File};
use ruff_db::parsed::parsed_module;
use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf};
use ruff_python_ast::{Alias, Expr, Parameter, ParameterWithDefault, Stmt};
use ruff_python_ast::visitor::source_order;
use ruff_python_ast::visitor::source_order::SourceOrderVisitor;
use ruff_python_ast::{Alias, Expr, Parameter, ParameterWithDefault, Stmt};

fn setup_db(workspace_root: &SystemPath) -> anyhow::Result<RootDatabase> {
let system = OsSystem::new(workspace_root);
Expand Down
17 changes: 10 additions & 7 deletions crates/ruff_benchmark/benches/red_knot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

use red_knot_python_semantic::PythonVersion;
use red_knot_workspace::db::RootDatabase;
use red_knot_workspace::watch::{ChangedKind, ChangeEvent};
use red_knot_workspace::watch::{ChangeEvent, ChangedKind};
use red_knot_workspace::workspace::settings::Configuration;
use red_knot_workspace::workspace::WorkspaceMetadata;
use ruff_benchmark::criterion::{BatchSize, Criterion, criterion_group, criterion_main};
use ruff_benchmark::criterion::{criterion_group, criterion_main, BatchSize, Criterion};
use ruff_benchmark::TestFile;
use ruff_db::files::{File, system_path_to_file};
use ruff_db::files::{system_path_to_file, File};
use ruff_db::source::source_text;
use ruff_db::system::{MemoryFileSystem, SystemPath, TestSystem};

Expand Down Expand Up @@ -131,10 +131,13 @@ fn benchmark_incremental(criterion: &mut Criterion) {
|case| {
let Case { db, .. } = case;

db.apply_changes(vec![ChangeEvent::Changed {
path: case.re_path.to_path_buf(),
kind: ChangedKind::FileContent,
}]);
db.apply_changes(
vec![ChangeEvent::Changed {
path: case.re_path.to_path_buf(),
kind: ChangedKind::FileContent,
}],
None,
);

let result = db.check().unwrap();

Expand Down

0 comments on commit ba1a840

Please sign in to comment.