diff --git a/crates/no-mistakes/src/codebase/analysis_session.rs b/crates/no-mistakes/src/codebase/analysis_session.rs index 4783574c7..5ba1d51df 100644 --- a/crates/no-mistakes/src/codebase/analysis_session.rs +++ b/crates/no-mistakes/src/codebase/analysis_session.rs @@ -25,12 +25,22 @@ pub struct AnalysisSession { datasets: DashMap>, supplemental_sources: Arc, resolver_caches: DashMap>, + registry_extension_reports: DashMap, parse_attempts: Option>, } type AnalysisDataset = crate::codebase::analysis_dataset::AnalysisDataset; type DatasetCell = OnceLock>; type SourceReadResult = Result, SourceReadError>; +type RegistryExtensionResult = + Result, Arc>; +type RegistryExtensionCell = Arc>; + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +struct RegistryExtensionKey { + root: PathBuf, + path: PathBuf, +} #[derive(Clone, Debug, PartialEq, Eq)] pub struct SourceReadError { @@ -64,6 +74,7 @@ impl AnalysisSession { datasets: DashMap::new(), supplemental_sources, resolver_caches: DashMap::new(), + registry_extension_reports: DashMap::new(), parse_attempts: collect_keyed_work.then(DashMap::new), }) } @@ -191,6 +202,38 @@ impl AnalysisSession { .map(|(root, cell)| self.dataset_from_cell(&root, &cell).sources_for(&root)) .unwrap_or_else(|| Arc::clone(&self.supplemental_sources)) } + + /// Memoize a request-owned registry-extension report projection for one + /// root/file pair. This is not a canonical TS fact because it is the + /// query's rendered report, but it owns no OXC data and can therefore be + /// reused without parsing the same source again. + pub(crate) fn registry_extension_report( + &self, + root: &Path, + path: &Path, + build: impl FnOnce() -> anyhow::Result, + ) -> anyhow::Result { + let key = RegistryExtensionKey { + root: normalize_path(root), + path: normalize_path(path), + }; + let cell = match self.registry_extension_reports.entry(key) { + Entry::Occupied(entry) => Arc::clone(entry.get()), + Entry::Vacant(entry) => { + let cell = Arc::new(OnceLock::new()); + entry.insert(Arc::clone(&cell)); + cell + } + }; + cell.get_or_init(|| { + build() + .map(Arc::new) + .map_err(|error| Arc::::from(format!("{error:#}"))) + }) + .clone() + .map(|report| (*report).clone()) + .map_err(|error| anyhow::anyhow!(error)) + } } #[cfg(test)] diff --git a/crates/no-mistakes/src/codebase/ts_source/tests/gitignore.rs b/crates/no-mistakes/src/codebase/ts_source/tests/gitignore.rs index d8b4b324e..46fadcd60 100644 --- a/crates/no-mistakes/src/codebase/ts_source/tests/gitignore.rs +++ b/crates/no-mistakes/src/codebase/ts_source/tests/gitignore.rs @@ -68,10 +68,6 @@ fn pass5a_visible_adapters_preserve_fixture_backed_discovery_output() { #[test] fn pass5a_public_wrappers_create_one_request_snapshot() { let cases = [ - ( - include_str!("../../../fetches/pipeline/run.rs"), - "pub(crate) fn run_with_base_root(", - ), ( include_str!("../../../queue/graph.rs"), "pub fn analyze_project(", @@ -92,7 +88,6 @@ fn pass5a_public_wrappers_create_one_request_snapshot() { include_str!("../../../react_traits/pipeline/check.rs"), "pub fn check_enabled(", ), - (include_str!("../../../data_pw_query.rs"), "pub fn run("), (include_str!("../../../ci.rs"), "pub fn impact_report("), (include_str!("../../../ci.rs"), "pub fn env_report("), ( @@ -126,6 +121,55 @@ fn pass5a_public_wrappers_create_one_request_snapshot() { } } +#[test] +fn pass5a_session_owned_queries_do_not_restart_discovery_or_source_reads() { + let data_pw = include_str!("../../../data_pw_query.rs"); + let data_pw_run = function_body(data_pw, "pub fn run("); + let data_pw_prepared = function_body(data_pw, "pub(crate) fn run_with_session("); + assert_eq!(data_pw_run.matches("AnalysisSession::new").count(), 1); + assert!(!data_pw_run.contains("VisiblePathSnapshot::new")); + for forbidden in [ + "VisiblePathSnapshot::new", + "load_v2_config(", + "discover_visible_paths(", + ] { + assert!( + !data_pw_prepared.contains(forbidden), + "data-pw prepared run: {forbidden}" + ); + } + let data_pw_scan = include_str!("../../../data_pw_query/scan.rs"); + assert!(data_pw_scan.contains("session.read_source(path)")); + assert!(!data_pw_scan.contains("std::fs::read_to_string")); + + let registry = include_str!("../../../registry_extension_query.rs"); + let registry_run = function_body(registry, "pub fn run("); + let registry_prepared = function_body(registry, "pub(crate) fn run_with_session("); + assert_eq!(registry_run.matches("AnalysisSession::new").count(), 1); + assert!(!registry_run.contains("std::fs::read_to_string")); + assert!(registry_prepared.contains(".read_source(&path)")); + assert!(registry_prepared.contains(".with_program")); + + let fetches = include_str!("../../../fetches/pipeline/run.rs"); + let fetch_run = function_body(fetches, "pub(crate) fn run_with_base_root("); + let fetch_prepared = function_body(fetches, "pub(crate) fn run_with_base_root_and_session("); + assert_eq!(fetch_run.matches("AnalysisSession::new").count(), 1); + for forbidden in [ + "VisiblePathSnapshot::new", + "load_v2_config(", + "discover_visible_paths(", + ] { + assert!( + !fetch_prepared.contains(forbidden), + "fetches prepared run: {forbidden}" + ); + } + let fetch_facts = include_str!("../../../fetch/file_facts.rs"); + assert!(fetch_facts.contains(".read_source(&abs_path)")); + assert!(fetch_facts.contains(".with_program")); + assert!(!fetch_facts.contains("std::fs::read_to_string")); +} + #[test] fn pass5a_prepared_bodies_do_not_restart_discovery_or_config_loading() { let cases = [ diff --git a/crates/no-mistakes/src/data_pw_query.rs b/crates/no-mistakes/src/data_pw_query.rs index ffb6f76d1..45e3fed35 100644 --- a/crates/no-mistakes/src/data_pw_query.rs +++ b/crates/no-mistakes/src/data_pw_query.rs @@ -22,8 +22,9 @@ use globset::{GlobBuilder, GlobSet, GlobSetBuilder}; use rayon::prelude::*; use serde::Serialize; +use crate::codebase::analysis_session::AnalysisSession; use crate::codebase::ts_source::relative_slash_path; -use crate::config::v2::{load_v2_config_from_visible, ConfigView}; +use crate::config::v2::ConfigView; use crate::playwright::selectors::compile_selector_attribute_value_regex; const SOURCE_EXTENSIONS: &[&str] = &["tsx", "ts", "jsx", "js", "mts", "cts", "mjs", "cjs"]; @@ -115,15 +116,37 @@ pub fn run( attribute_override: &[String], scan_override: &[String], include: &DataPwInclude, +) -> Result { + let session = AnalysisSession::new(crate::diagnostics::current()); + run_with_session( + &session, + root, + config_path, + value, + attribute_override, + scan_override, + include, + ) +} + +/// Run the query using the caller-owned request analysis session. +pub(crate) fn run_with_session( + session: &AnalysisSession, + root: &Path, + config_path: Option<&Path>, + value: &str, + attribute_override: &[String], + scan_override: &[String], + include: &DataPwInclude, ) -> Result { // VisiblePathSnapshot returns lexically normalized paths. Normalize the // matching boundary once too so roots containing `.`/`..` still produce // relative report paths and pass the visibility filter. let root = crate::codebase::ts_source::normalize_discovery_path(root); - let snapshot = crate::codebase::ts_source::VisiblePathSnapshot::new(&root); + let snapshot = session.visible_paths(&root); let visible_paths = snapshot.paths_for(&root); crate::invocation::check_timeout()?; - let config = load_v2_config_from_visible(&root, config_path, &visible_paths)?; + let config = session.config(&root, config_path)?; let view = ConfigView::new(&config); let attributes: Vec = if attribute_override.is_empty() { @@ -177,7 +200,7 @@ pub fn run( }; let files = discover_files_from_visible_paths(&root, &visible_paths, view.skip_directories()); - let hits = scan_files(&files, &root, &scan)?; + let hits = scan_files(&files, &root, &scan, session)?; let mut source: Vec = Vec::new(); let mut test: Vec = Vec::new(); diff --git a/crates/no-mistakes/src/data_pw_query/scan.rs b/crates/no-mistakes/src/data_pw_query/scan.rs index 28a107369..439b973ae 100644 --- a/crates/no-mistakes/src/data_pw_query/scan.rs +++ b/crates/no-mistakes/src/data_pw_query/scan.rs @@ -32,9 +32,8 @@ fn discover_files_from_visible_paths( .is_ok_and(|rel| !rel_path_under_skip_dir(rel, extra_skip)) }) .filter(|path| has_source_extension(path)) - // `WalkDir`'s default (non-link-following) file type never reports a - // symlink as a file, so match that here rather than `Path::is_file`, - // which follows the link. + // Keep the scanner's non-link-following discovery semantics. Source + // text itself is still read only through the request SourceStore. .filter(|path| std::fs::symlink_metadata(path).is_ok_and(|metadata| metadata.is_file())) .cloned() .collect() @@ -87,14 +86,16 @@ fn scan_files( files: &[PathBuf], root: &Path, scan: &ScanConfig<'_>, + session: &crate::codebase::analysis_session::AnalysisSession, ) -> Result> { - scan_files_with_timeout_check(files, root, scan, &crate::invocation::check_timeout) + scan_files_with_timeout_check(files, root, scan, session, &crate::invocation::check_timeout) } fn scan_files_with_timeout_check( files: &[PathBuf], root: &Path, scan: &ScanConfig<'_>, + session: &crate::codebase::analysis_session::AnalysisSession, check_timeout: &(impl Fn() -> Result<()> + Sync), ) -> Result> { files @@ -102,7 +103,7 @@ fn scan_files_with_timeout_check( .try_fold(Vec::new, |mut hits, path| -> Result<_> { check_timeout()?; let rel = relative_slash_path(root, path); - hits.extend(scan_file(path, &rel, scan, check_timeout)?); + hits.extend(scan_file(path, &rel, scan, session, check_timeout)?); Ok(hits) }) .try_reduce(Vec::new, |mut left, mut right| -> Result<_> { @@ -115,6 +116,7 @@ fn scan_file( path: &Path, rel: &str, scan: &ScanConfig, + session: &crate::codebase::analysis_session::AnalysisSession, check_timeout: &(impl Fn() -> Result<()> + Sync), ) -> Result> { check_timeout()?; @@ -141,7 +143,7 @@ fn scan_file( } else { FileKind::Source }; - let Ok(source) = std::fs::read_to_string(path) else { + let Ok(source) = session.read_source(path) else { return Ok(Vec::new()); }; let mut hits = Vec::new(); diff --git a/crates/no-mistakes/src/data_pw_query/tests.rs b/crates/no-mistakes/src/data_pw_query/tests.rs index bc4e4c6f2..2b524f1be 100644 --- a/crates/no-mistakes/src/data_pw_query/tests.rs +++ b/crates/no-mistakes/src/data_pw_query/tests.rs @@ -142,6 +142,26 @@ fn finds_source_and_test_usages() { assert_eq!(test[0].attribute, "data-pw"); } +#[test] +fn prepared_run_reuses_one_visible_inventory_and_source_store() { + let observer = crate::diagnostics::InvocationObserver::new(true); + let session = crate::codebase::analysis_session::AnalysisSession::new(Some(observer)); + let include = DataPwInclude::default(); + + let first = + run_with_session(&session, &fixture(), None, "search-bar", &[], &[], &include).unwrap(); + let second = + run_with_session(&session, &fixture(), None, "search-bar", &[], &[], &include).unwrap(); + + assert_eq!(first, second); + let work = session.work_snapshot(); + assert!(!work.source_reads.is_empty()); + assert!( + work.source_reads.values().all(|count| *count == 1), + "source reads must be memoized by the request session: {work:?}" + ); +} + #[test] fn expired_deadline_returns_timeout_instead_of_a_partial_report() { let _deadline = crate::invocation::install_test_deadline(std::time::Duration::ZERO).unwrap(); @@ -268,6 +288,7 @@ fn is_skip_dir_honors_defaults_and_config() { #[test] fn scan_file_ignores_unreadable_path() { + let session = crate::codebase::analysis_session::AnalysisSession::disabled(); let regex = compile_selector_attribute_value_regex(&["data-pw".to_string()]).unwrap(); let globs = build_globset(&[]).unwrap(); let scan = ScanConfig { @@ -283,6 +304,7 @@ fn scan_file_ignores_unreadable_path() { Path::new("/no/such/file.tsx"), "x.tsx", &scan, + &session, &crate::invocation::check_timeout, ) .unwrap(); @@ -291,6 +313,7 @@ fn scan_file_ignores_unreadable_path() { #[test] fn parallel_scan_propagates_a_deadline_that_expires_inside_a_worker() { + let session = crate::codebase::analysis_session::AnalysisSession::disabled(); let root = fixture(); let regex = compile_selector_attribute_value_regex(&["data-pw".to_string()]).unwrap(); let globs = build_globset(&[]).unwrap(); @@ -321,6 +344,7 @@ fn parallel_scan_propagates_a_deadline_that_expires_inside_a_worker() { &[root.join("app/search.tsx")], &root, &scan, + &session, &expire_mid_scan, ) .unwrap_err(); diff --git a/crates/no-mistakes/src/fetch/file_analysis.rs b/crates/no-mistakes/src/fetch/file_analysis.rs index 24b5a92ec..3f609f592 100644 --- a/crates/no-mistakes/src/fetch/file_analysis.rs +++ b/crates/no-mistakes/src/fetch/file_analysis.rs @@ -55,6 +55,7 @@ pub(crate) fn analyze_file_from_visible( } pub(crate) struct VisibleFileAnalysis<'a> { + pub session: &'a crate::codebase::analysis_session::AnalysisSession, pub root: &'a Path, pub visited: &'a mut HashSet<(PathBuf, bool, bool)>, pub fetches: &'a mut Vec, @@ -69,6 +70,7 @@ pub(crate) fn analyze_file_from_visible_with_facts( context: &mut VisibleFileAnalysis<'_>, ) -> Result { let VisibleFileAnalysis { + session, root, visited, fetches, @@ -94,7 +96,13 @@ pub(crate) fn analyze_file_from_visible_with_facts( return Ok(cached_fetches.is_client); } - let facts = parsed_files.load(&abs_path, root, &mut cache.imports, visible_files)?; + let facts = parsed_files.load_with_session( + session, + &abs_path, + root, + &mut cache.imports, + visible_files, + )?; let is_client = !inherited_is_route_handler && !facts.has_use_server_directive && (inherited_is_client || facts.has_use_client_directive); @@ -112,6 +120,7 @@ pub(crate) fn analyze_file_from_visible_with_facts( &import, (is_client, inherited_is_route_handler), &mut VisibleFileAnalysis { + session, root, visited, fetches: &mut file_fetches, diff --git a/crates/no-mistakes/src/fetch/file_facts.rs b/crates/no-mistakes/src/fetch/file_facts.rs index 5db879abc..7ea87bee0 100644 --- a/crates/no-mistakes/src/fetch/file_facts.rs +++ b/crates/no-mistakes/src/fetch/file_facts.rs @@ -25,8 +25,9 @@ pub struct ParsedFileCache { } impl ParsedFileCache { - pub(crate) fn load( + pub(crate) fn load_with_session( &mut self, + session: &crate::codebase::analysis_session::AnalysisSession, path: &Path, root: &Path, import_cache: &mut HashMap>, @@ -38,8 +39,10 @@ impl ParsedFileCache { } let result = (|| { - let source = std::fs::read_to_string(&abs_path)?; - crate::ast::with_program(&abs_path, &source, |program, _| { + let source = session + .read_source(&abs_path) + .map_err(|error| anyhow::anyhow!(error.to_string()))?; + session.with_program(&abs_path, &source, |program, _| { ParsedFileFacts::from_program( &abs_path, root, diff --git a/crates/no-mistakes/src/fetch/import_routes.rs b/crates/no-mistakes/src/fetch/import_routes.rs index 6cdfbac02..de6a58836 100644 --- a/crates/no-mistakes/src/fetch/import_routes.rs +++ b/crates/no-mistakes/src/fetch/import_routes.rs @@ -38,46 +38,64 @@ pub fn route_reaches_target_from_visible_with_facts( parsed_files: &mut ParsedFileCache, visible_files: &HashSet, ) -> Result { - let abs_target = crate::codebase::ts_resolver::normalize_path(target); - route_reaches_target_with_facts_inner( - path, - &abs_target, + let session = crate::codebase::analysis_session::AnalysisSession::disabled(); + let mut facts = RouteTargetFacts { root, visited, import_cache, parsed_files, visible_files, - ) + }; + route_reaches_target_from_visible_with_facts_and_session(&session, path, target, &mut facts) +} + +/// Prepared state shared by a route-to-target traversal. +#[doc(hidden)] +pub struct RouteTargetFacts<'a> { + pub root: &'a Path, + pub visited: &'a mut HashSet, + pub import_cache: &'a mut HashMap>, + pub parsed_files: &'a mut ParsedFileCache, + pub visible_files: &'a HashSet, +} + +#[doc(hidden)] +pub fn route_reaches_target_from_visible_with_facts_and_session( + session: &crate::codebase::analysis_session::AnalysisSession, + path: &Path, + target: &Path, + facts: &mut RouteTargetFacts<'_>, +) -> Result { + let abs_target = crate::codebase::ts_resolver::normalize_path(target); + route_reaches_target_with_facts_inner(session, path, &abs_target, facts) } fn route_reaches_target_with_facts_inner( + session: &crate::codebase::analysis_session::AnalysisSession, path: &Path, abs_target: &Path, - root: &Path, - visited: &mut HashSet, - import_cache: &mut HashMap>, - parsed_files: &mut ParsedFileCache, - visible_files: &HashSet, + facts: &mut RouteTargetFacts<'_>, ) -> Result { let abs_path = crate::codebase::ts_resolver::normalize_path(path); if abs_path == abs_target { return Ok(true); } - if !visible_files.contains(&abs_path) || !visited.insert(abs_path.clone()) { + if !facts.visible_files.contains(&abs_path) || !facts.visited.insert(abs_path.clone()) { return Ok(false); } - let facts = parsed_files.load(&abs_path, root, import_cache, visible_files)?; - for import in facts.imports { - if route_reaches_target_with_facts_inner( - &import, - abs_target, - root, - visited, - import_cache, - parsed_files, - visible_files, - )? { + let imports = facts + .parsed_files + .load_with_session( + session, + &abs_path, + facts.root, + facts.import_cache, + facts.visible_files, + )? + .imports; + for import in imports { + if route_reaches_target_with_facts_inner(session, &import, abs_target, facts)? { return Ok(true); } } diff --git a/crates/no-mistakes/src/fetch/imports/tests.rs b/crates/no-mistakes/src/fetch/imports/tests.rs index 334efc69c..cf3618d7c 100644 --- a/crates/no-mistakes/src/fetch/imports/tests.rs +++ b/crates/no-mistakes/src/fetch/imports/tests.rs @@ -1,6 +1,7 @@ use super::*; use crate::ast; use std::collections::{HashMap, HashSet}; +use std::path::PathBuf; #[test] fn test_collect_runtime_imports_from_program() { @@ -176,3 +177,34 @@ fn pass4a_ignored_import_candidate_does_not_shadow_visible_route_fallback() { assert!(reaches); } + +#[test] +fn visible_facts_route_traversal_compatibility_wrapper_reaches_an_import() { + let root = crate::codebase::ts_resolver::normalize_path( + &PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../../fixtures/fetch/visible-facts-route-wrapper"), + ); + let route = root.join("route.ts"); + let target = root.join("target.ts"); + let visible_files = crate::codebase::ts_source::discover_visible_paths(&root) + .into_iter() + .collect(); + let mut visited = HashSet::new(); + let mut import_cache = HashMap::new(); + let mut parsed_files = crate::fetch::ParsedFileCache::default(); + + // Keep this on the legacy wrapper: session-aware pipeline tests cover the + // prepared path, while this protects the compatibility adapter itself. + let reaches = crate::fetch::import_routes::route_reaches_target_from_visible_with_facts( + &route, + &target, + &root, + &mut visited, + &mut import_cache, + &mut parsed_files, + &visible_files, + ) + .unwrap(); + + assert!(reaches); +} diff --git a/crates/no-mistakes/src/fetch/mod.rs b/crates/no-mistakes/src/fetch/mod.rs index 2a2b61fc5..f10160a35 100644 --- a/crates/no-mistakes/src/fetch/mod.rs +++ b/crates/no-mistakes/src/fetch/mod.rs @@ -16,6 +16,12 @@ pub mod visitor_state; #[doc(hidden)] pub use file_facts::ParsedFileCache; #[doc(hidden)] -pub use import_routes::route_reaches_target_from_visible_with_facts; +pub use import_routes::{ + route_reaches_target_from_visible_with_facts, + route_reaches_target_from_visible_with_facts_and_session, RouteTargetFacts, +}; #[doc(hidden)] -pub use route_analysis::collect_route_fetches_from_visible_with_facts; +pub use route_analysis::{ + collect_route_fetches_from_visible_with_facts, + collect_route_fetches_from_visible_with_facts_and_session, +}; diff --git a/crates/no-mistakes/src/fetch/route_analysis.rs b/crates/no-mistakes/src/fetch/route_analysis.rs index ce56500a0..633d686e0 100644 --- a/crates/no-mistakes/src/fetch/route_analysis.rs +++ b/crates/no-mistakes/src/fetch/route_analysis.rs @@ -17,7 +17,8 @@ pub fn collect_route_fetches( root: &Path, cache: &mut Cache, ) -> Result> { - collect_route_fetches_inner(route, frontend_root, root, cache, None, None) + let session = crate::codebase::analysis_session::AnalysisSession::disabled(); + collect_route_fetches_inner(&session, route, frontend_root, root, cache, None, None) } pub fn collect_route_fetches_from_visible( @@ -27,7 +28,16 @@ pub fn collect_route_fetches_from_visible( cache: &mut Cache, visible_files: &HashSet, ) -> Result> { - collect_route_fetches_inner(route, frontend_root, root, cache, Some(visible_files), None) + let session = crate::codebase::analysis_session::AnalysisSession::disabled(); + collect_route_fetches_inner( + &session, + route, + frontend_root, + root, + cache, + Some(visible_files), + None, + ) } #[doc(hidden)] @@ -38,8 +48,32 @@ pub fn collect_route_fetches_from_visible_with_facts( cache: &mut Cache, parsed_files: &mut ParsedFileCache, visible_files: &HashSet, +) -> Result> { + let session = crate::codebase::analysis_session::AnalysisSession::disabled(); + collect_route_fetches_from_visible_with_facts_and_session( + &session, + route, + frontend_root, + root, + cache, + parsed_files, + visible_files, + ) +} + +/// Traverse prepared fetch facts using the caller-owned source/parse session. +#[doc(hidden)] +pub fn collect_route_fetches_from_visible_with_facts_and_session( + session: &crate::codebase::analysis_session::AnalysisSession, + route: &Route, + frontend_root: &Path, + root: &Path, + cache: &mut Cache, + parsed_files: &mut ParsedFileCache, + visible_files: &HashSet, ) -> Result> { collect_route_fetches_inner( + session, route, frontend_root, root, @@ -50,6 +84,7 @@ pub fn collect_route_fetches_from_visible_with_facts( } fn collect_route_fetches_inner( + session: &crate::codebase::analysis_session::AnalysisSession, route: &Route, frontend_root: &Path, root: &Path, @@ -62,7 +97,6 @@ fn collect_route_fetches_inner( let mut visited = HashSet::new(); let mut fetches = Vec::new(); - let mut traversal = FetchTraversal { root, visited: &mut visited, @@ -70,6 +104,7 @@ fn collect_route_fetches_inner( cache, visible_files, parsed_files, + session, }; let _route_is_client = traversal.analyze(&route.file, (false, route_is_route_handler))?; @@ -116,6 +151,7 @@ struct FetchTraversal<'a> { cache: &'a mut Cache, visible_files: Option<&'a HashSet>, parsed_files: Option<&'a mut ParsedFileCache>, + session: &'a crate::codebase::analysis_session::AnalysisSession, } impl FetchTraversal<'_> { @@ -125,6 +161,7 @@ impl FetchTraversal<'_> { path, inherited, &mut VisibleFileAnalysis { + session: self.session, root: self.root, visited: self.visited, fetches: self.fetches, diff --git a/crates/no-mistakes/src/fetches/pipeline/route_analysis.rs b/crates/no-mistakes/src/fetches/pipeline/route_analysis.rs index d67f3cc7d..f9cd2736a 100644 --- a/crates/no-mistakes/src/fetches/pipeline/route_analysis.rs +++ b/crates/no-mistakes/src/fetches/pipeline/route_analysis.rs @@ -8,10 +8,7 @@ pub(crate) fn check_route_matches( route: &no_mistakes::routes::Route, target_specs: &[TargetSpec], wrapper_files: &[PathBuf], - cache: &mut Cache, - parsed_files: &mut no_mistakes::fetch::ParsedFileCache, - root: &Path, - visible_files: &HashSet, + mut context: RouteMatchContext<'_>, ) -> Result<(bool, Vec)> { let mut newly_matched = Vec::new(); @@ -28,14 +25,7 @@ pub(crate) fn check_route_matches( } if let Some(target_file) = &target.file { - let reaches_route_target = reaches_target( - &route.file, - target_file, - root, - cache, - parsed_files, - visible_files, - )?; + let reaches_route_target = reaches_target(&route.file, target_file, &mut context)?; if reaches_route_target { matched = true; newly_matched.push(target.raw.clone()); @@ -49,14 +39,8 @@ pub(crate) fn check_route_matches( break; } - let reaches_wrapper_target = reaches_target( - wrapper_file, - target_file, - root, - cache, - parsed_files, - visible_files, - )?; + let reaches_wrapper_target = + reaches_target(wrapper_file, target_file, &mut context)?; if reaches_wrapper_target { wrapper_file_matches = true; break; @@ -74,22 +58,31 @@ pub(crate) fn check_route_matches( Ok((matched, newly_matched)) } +pub(crate) struct RouteMatchContext<'a> { + pub(crate) cache: &'a mut Cache, + pub(crate) session: &'a no_mistakes::codebase::analysis_session::AnalysisSession, + pub(crate) parsed_files: &'a mut no_mistakes::fetch::ParsedFileCache, + pub(crate) root: &'a Path, + pub(crate) visible_files: &'a HashSet, +} + fn reaches_target( source_file: &Path, target_file: &Path, - root: &Path, - cache: &mut Cache, - parsed_files: &mut no_mistakes::fetch::ParsedFileCache, - visible_files: &HashSet, + context: &mut RouteMatchContext<'_>, ) -> Result { let mut visited_targets = HashSet::new(); - no_mistakes::fetch::route_reaches_target_from_visible_with_facts( + let mut facts = no_mistakes::fetch::RouteTargetFacts { + root: context.root, + visited: &mut visited_targets, + import_cache: &mut context.cache.imports, + parsed_files: &mut *context.parsed_files, + visible_files: context.visible_files, + }; + no_mistakes::fetch::route_reaches_target_from_visible_with_facts_and_session( + context.session, source_file, target_file, - root, - &mut visited_targets, - &mut cache.imports, - parsed_files, - visible_files, + &mut facts, ) } diff --git a/crates/no-mistakes/src/fetches/pipeline/run.rs b/crates/no-mistakes/src/fetches/pipeline/run.rs index daaf5e6cd..a79af55ae 100644 --- a/crates/no-mistakes/src/fetches/pipeline/run.rs +++ b/crates/no-mistakes/src/fetches/pipeline/run.rs @@ -3,7 +3,7 @@ use crate::fetches::analyze::routes::collect_layout_chain_files_from_visible; use crate::fetches::cli::Cli; use crate::fetches::pipeline::aggregate::build_final_report; use crate::fetches::pipeline::cache::Cache; -use crate::fetches::pipeline::route_analysis::check_route_matches; +use crate::fetches::pipeline::route_analysis::{check_route_matches, RouteMatchContext}; use crate::fetches::pipeline::target::{resolve_target_file, TargetSpec}; use crate::fetches::report::types::{FinalReport, RouteReport}; use anyhow::Result; @@ -12,6 +12,17 @@ use std::collections::{HashMap, HashSet}; use std::path::Path; pub(crate) fn run_with_base_root(base_root: &Path, cli: &Cli) -> Result { + let session = no_mistakes::codebase::analysis_session::AnalysisSession::new( + no_mistakes::diagnostics::current(), + ); + run_with_base_root_and_session(base_root, cli, &session) +} + +pub(crate) fn run_with_base_root_and_session( + base_root: &Path, + cli: &Cli, + session: &no_mistakes::codebase::analysis_session::AnalysisSession, +) -> Result { let requested_root = base_root.join(&cli.root); let root = requested_root .canonicalize() @@ -20,13 +31,9 @@ pub(crate) fn run_with_base_root(base_root: &Path, cli: &Cli) -> Result/app` app when nothing is configured or // inferable, matching the pre-existing zero-signal default; a genuinely // ambiguous multi-app repository with no binding still errors (see @@ -64,12 +71,15 @@ pub(crate) fn run_with_base_root(base_root: &Path, cli: &Cli) -> Result Result< Ok(target_specs) } +struct AnalyzeRoutesContext<'a> { + target_specs: &'a [TargetSpec], + frontend_root: &'a Path, + root: &'a Path, + cache: &'a mut Cache, + session: &'a no_mistakes::codebase::analysis_session::AnalysisSession, + parsed_files: &'a mut no_mistakes::fetch::ParsedFileCache, + visible_files: &'a HashSet, +} + fn analyze_routes( all_routes: Vec, - target_specs: &[TargetSpec], - frontend_root: &Path, - root: &Path, - cache: &mut Cache, - parsed_files: &mut no_mistakes::fetch::ParsedFileCache, - visible_files: &HashSet, + context: AnalyzeRoutesContext<'_>, ) -> Result<(Vec, HashSet)> { + let AnalyzeRoutesContext { + target_specs, + frontend_root, + root, + cache, + session, + parsed_files, + visible_files, + } = context; let mut reports = Vec::new(); let mut matched_targets: HashSet = HashSet::new(); @@ -126,10 +150,13 @@ fn analyze_routes( &route, target_specs, &wrapper_files, - cache, - parsed_files, - root, - visible_files, + RouteMatchContext { + cache, + session, + parsed_files, + root, + visible_files, + }, )?; for t in newly_matched { @@ -140,14 +167,16 @@ fn analyze_routes( continue; } - let fetches = no_mistakes::fetch::collect_route_fetches_from_visible_with_facts( - &route, - frontend_root, - root, - cache, - parsed_files, - visible_files, - )?; + let fetches = + no_mistakes::fetch::collect_route_fetches_from_visible_with_facts_and_session( + session, + &route, + frontend_root, + root, + cache, + parsed_files, + visible_files, + )?; reports.push(RouteReport { route: route.pattern, diff --git a/crates/no-mistakes/src/fetches/tests/run_with_base_root_tests.rs b/crates/no-mistakes/src/fetches/tests/run_with_base_root_tests.rs index d4cc29cef..64fa4b9cf 100644 --- a/crates/no-mistakes/src/fetches/tests/run_with_base_root_tests.rs +++ b/crates/no-mistakes/src/fetches/tests/run_with_base_root_tests.rs @@ -1,5 +1,5 @@ use crate::fetches::cli::Cli; -use crate::fetches::pipeline::run::run_with_base_root; +use crate::fetches::pipeline::run::{run_with_base_root, run_with_base_root_and_session}; use no_mistakes::cli::Format; use std::fs; use std::path::PathBuf; @@ -56,6 +56,49 @@ fn target_matching_and_fetch_analysis_parse_each_fixture_file_once() { assert!(counts.values().all(|count| *count == 1), "{counts:?}"); } +#[test] +fn prepared_fetch_run_reuses_the_session_for_target_matching_and_traversal() { + let root = fixture("nextjs-fetches", "parse-sharing"); + let cli = Cli { + root: PathBuf::from("."), + config: None, + format: Format::Human, + json: false, + targets: vec!["app/users.ts".to_string()], + }; + let observer = no_mistakes::diagnostics::InvocationObserver::new(true); + let session = no_mistakes::codebase::analysis_session::AnalysisSession::new(Some(observer)); + + let report = run_with_base_root_and_session(&root, &cli, &session).unwrap(); + + assert_eq!(report.routes.len(), 1); + assert_eq!(report.routes[0].api_calls.len(), 1); + assert_eq!(report.routes[0].api_calls[0].path, "/api/users"); + let work = session.work_snapshot(); + let expected_paths = ["app/page.tsx", "app/users.ts"] + .into_iter() + .map(|path| no_mistakes::codebase::ts_resolver::normalize_path(&root.join(path))) + .collect::>(); + assert_eq!( + work.source_reads.keys().cloned().collect::>(), + expected_paths, + "target matching and traversal must read the expected prepared sources: {work:?}" + ); + assert_eq!( + work.parse_attempts.keys().cloned().collect::>(), + expected_paths, + "target matching and traversal must parse the expected prepared sources: {work:?}" + ); + assert!( + work.source_reads.values().all(|count| *count == 1), + "source reads must be memoized by the request session: {work:?}" + ); + assert!( + work.parse_attempts.values().all(|count| *count == 1), + "target matching and traversal must share parsed facts: {work:?}" + ); +} + #[test] fn test_run_with_base_root_errors_when_route_target_matcher_fails() { let root = tempdir().unwrap(); diff --git a/crates/no-mistakes/src/registry_extension_query.rs b/crates/no-mistakes/src/registry_extension_query.rs index da29b0d04..caa76697f 100644 --- a/crates/no-mistakes/src/registry_extension_query.rs +++ b/crates/no-mistakes/src/registry_extension_query.rs @@ -26,6 +26,7 @@ use oxc_ast_visit::{walk, Visit}; use oxc_span::GetSpan; use serde::Serialize; +use crate::codebase::analysis_session::AnalysisSession; use crate::codebase::ts_source::{byte_offset_to_line, relative_slash_path}; /// The import backing a registry entry. @@ -65,19 +66,32 @@ pub struct RegistryExtensionReport { /// Run the `registry-extension` query against a single file. pub fn run(root: &Path, registry_file: &Path) -> Result { + let session = AnalysisSession::new(crate::diagnostics::current()); + run_with_session(&session, root, registry_file) +} + +/// Run the query using the caller-owned request analysis session. +pub(crate) fn run_with_session( + session: &AnalysisSession, + root: &Path, + registry_file: &Path, +) -> Result { let path = if registry_file.is_absolute() { registry_file.to_path_buf() } else { root.join(registry_file) }; - let source = std::fs::read_to_string(&path) - .map_err(|error| anyhow::anyhow!("cannot read {}: {error}", path.display()))?; - let rel = relative_slash_path(root, &path); - - crate::ast::with_program(&path, &source, |program, source| { - analyze(program, source, rel) + session.registry_extension_report(root, &path, || { + let source = session + .read_source(&path) + .map_err(|error| anyhow::anyhow!("cannot read {}: {error}", path.display()))?; + let rel = relative_slash_path(root, &path); + session + .with_program(&path, &source, |program, source| { + analyze(program, source, rel) + }) + .map_err(|error| anyhow::anyhow!("{error}"))? }) - .map_err(|error| anyhow::anyhow!("{error}"))? } fn analyze( diff --git a/crates/no-mistakes/src/registry_extension_query/tests.rs b/crates/no-mistakes/src/registry_extension_query/tests.rs index 68deb1a54..d3debb5c0 100644 --- a/crates/no-mistakes/src/registry_extension_query/tests.rs +++ b/crates/no-mistakes/src/registry_extension_query/tests.rs @@ -31,6 +31,38 @@ fn detects_register_call_pattern() { ); } +#[test] +fn prepared_run_reuses_one_source_read_and_parse_attempt() { + let observer = crate::diagnostics::InvocationObserver::new(true); + let session = crate::codebase::analysis_session::AnalysisSession::new(Some(observer)); + let file = fixture().join("register-call.ts"); + + let first = run_with_session(&session, &fixture(), Path::new("register-call.ts")).unwrap(); + let second = run_with_session(&session, &fixture(), Path::new("register-call.ts")).unwrap(); + + assert_eq!(first, second); + let work = session.work_snapshot(); + let file = crate::codebase::ts_resolver::normalize_path(&file); + assert_eq!(work.source_reads[&file], 1); + assert_eq!(work.parse_attempts[&file], 1); +} + +#[test] +fn prepared_run_memoizes_parse_failures_with_the_same_error() { + let observer = crate::diagnostics::InvocationObserver::new(true); + let session = crate::codebase::analysis_session::AnalysisSession::new(Some(observer)); + let file = fixture().join("unparseable.ts"); + + let first = run_with_session(&session, &fixture(), Path::new("unparseable.ts")).unwrap_err(); + let second = run_with_session(&session, &fixture(), Path::new("unparseable.ts")).unwrap_err(); + + assert_eq!(first.to_string(), second.to_string()); + let work = session.work_snapshot(); + let file = crate::codebase::ts_resolver::normalize_path(&file); + assert_eq!(work.source_reads[&file], 1); + assert_eq!(work.parse_attempts[&file], 1); +} + #[test] fn detects_container_array() { let report = report("container-array.ts"); diff --git a/fixtures/fetch/visible-facts-route-wrapper/route.ts b/fixtures/fetch/visible-facts-route-wrapper/route.ts new file mode 100644 index 000000000..3009e9984 --- /dev/null +++ b/fixtures/fetch/visible-facts-route-wrapper/route.ts @@ -0,0 +1,5 @@ +// Keep this direct import: it exercises the compatibility wrapper separately +// from the session-aware fetch pipeline's target traversal. +import { target } from './target'; + +target(); diff --git a/fixtures/fetch/visible-facts-route-wrapper/target.ts b/fixtures/fetch/visible-facts-route-wrapper/target.ts new file mode 100644 index 000000000..07403ac28 --- /dev/null +++ b/fixtures/fetch/visible-facts-route-wrapper/target.ts @@ -0,0 +1 @@ +export const target = () => undefined;