Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
2 changes: 1 addition & 1 deletion crates/no-mistakes/src/check_runner.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
pub(crate) mod enabled;
mod forbidden_plan;
mod graph_plan;
pub(crate) mod prepared;
mod results;
mod run_all;
Expand Down
25 changes: 13 additions & 12 deletions crates/no-mistakes/src/check_runner/run_all.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{
complete_domain_checks, empty_results, enabled, forbidden_plan, prepared, results, CheckResults,
complete_domain_checks, empty_results, enabled, graph_plan, prepared, results, CheckResults,
};
use crate::check_parallel::{run_domain_checks, DomainCheckInputs};
use crate::check_tasks;
Expand Down Expand Up @@ -29,11 +29,10 @@ pub(crate) fn run_all(
let unique_exports_enabled = check_tasks::unique_exports_configured(config);
let enabled = enabled::ConfiguredChecks::from_config(config);
let filesystem_rules_enabled = check_tasks::filesystem_rules_configured(config);
let forbidden_deps_enabled = check_tasks::forbidden_dependencies_configured(config);
let forbidden_graph_plan = forbidden_deps_enabled
.then(|| no_mistakes::codebase::rules::forbidden_dependencies::graph_plan(config))
.flatten();
let playwright_consumers = forbidden_graph_plan
let canonical_graph_plan = no_mistakes::codebase::rules::canonical_graph_plan(config);
let graph_requires_full_file_universe =
no_mistakes::codebase::rules::canonical_graph_requires_full_file_universe(config);
let playwright_consumers = canonical_graph_plan
.map(
|plan| no_mistakes::playwright::rules::PlaywrightFactConsumers {
graph_selectors: plan.playwright_selectors,
Expand Down Expand Up @@ -76,21 +75,22 @@ pub(crate) fn run_all(
),
));
}
let prepared_graph = forbidden_plan::prepare(
let prepared_graph = graph_plan::prepare(
&root,
config,
forbidden_plan::PreparedInputs {
graph_plan::PreparedInputs {
codebase_config: &prepared.codebase_config,
tsconfig: &prepared.tsconfig,
visible_paths: prepared.visible_paths.as_ref(),
workflow_documents: prepared.workflow_documents.as_ref(),
},
forbidden_graph_plan,
canonical_graph_plan,
&mut playwright_fact_plan,
&mut plan,
)?;
let needs_shared_facts =
forbidden_deps_enabled || playwright_fact_plan.is_some() || plan_requests_facts(&plan);
let needs_shared_facts = canonical_graph_plan.is_some()
|| playwright_fact_plan.is_some()
|| plan_requests_facts(&plan);
if !needs_shared_facts
&& !filesystem_rules_enabled
&& !no_mistakes::playwright::rules::configured(config)
Expand All @@ -110,7 +110,8 @@ pub(crate) fn run_all(
)
},
);
let needs_full_graph_files = forbidden_graph_plan.is_some() || playwright_fact_plan.is_some();
let needs_full_graph_files =
graph_requires_full_file_universe || playwright_fact_plan.is_some();
let needs_graph_files =
needs_shared_facts && (needs_full_graph_files || enabled.dynamic_import_rules);
let (discovered, graph_files) = if needs_full_graph_files {
Expand Down
34 changes: 1 addition & 33 deletions crates/no-mistakes/src/check_runner/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use std::time::Duration;

mod architecture;
mod config_path;
mod graph_scope;
mod integration_gitignore;
#[cfg(feature = "test-instrumentation")]
mod prepared_parser_cache;
Expand Down Expand Up @@ -330,39 +331,6 @@ fn run_all_skips_discovery_for_forbidden_deps_only() {
);
}

#[test]
fn run_all_keeps_forbidden_graph_files_outside_filesystem_skips() {
let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("../../test-cases/check-runner/forbidden-deps-ignores-filesystem-skip/fixture");
let config = root.join(".no-mistakes.yml");
let results = run_all(root, Some(config), None).unwrap();

assert!(
results
.rules
.iter()
.any(|f| f.rule == no_mistakes::codebase::rules::FORBIDDEN_DEPENDENCIES),
"expected forbidden-dependencies finding for file under filesystem skip"
);
}

#[test]
fn run_all_keeps_playwright_graph_files_outside_filesystem_skips() {
let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("../../test-cases/check-runner/playwright-graph-ignores-filesystem-skip/fixture");
let config = root.join(".no-mistakes.yml");
let results = run_all(root, Some(config), None).unwrap();

assert!(!results.rules.iter().any(|finding| {
finding.rule == no_mistakes::playwright::rules::PLAYWRIGHT_COVERAGE
&& finding.target.as_deref() == Some("data-testid=save")
}));
assert!(results.rules.iter().any(|finding| {
finding.rule == no_mistakes::playwright::rules::PLAYWRIGHT_COVERAGE
&& finding.target.as_deref() == Some("data-testid=delete")
}));
}

#[test]
fn run_all_dynamic_import_graph_excludes_gitignored_targets() {
let dir = crate::test_support::materialize_gitignore_fixture("transitive-visibility");
Expand Down
6 changes: 3 additions & 3 deletions crates/no-mistakes/src/check_runner/tests/architecture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ fn aggregate_check_injects_prepared_config_into_every_domain() {
include_str!("../run_all.rs"),
);
let prepared = include_str!("../prepared.rs");
let forbidden_plan = include_str!("../forbidden_plan.rs");
let graph_plan = include_str!("../graph_plan.rs");
let parallel = include_str!("../../check_parallel.rs");
let tasks = check_task_sources();

Expand Down Expand Up @@ -55,8 +55,8 @@ fn aggregate_check_injects_prepared_config_into_every_domain() {
1
);
assert!(!prepared.contains("resolve_tsconfig_from_visible"));
assert!(forbidden_plan.contains("prepare_graph_config"));
assert!(forbidden_plan.contains("ts_fact_plan_and_context_for_plan_with_prepared"));
assert!(graph_plan.contains("prepare_graph_config"));
assert!(graph_plan.contains("ts_fact_plan_and_context_for_plan_with_prepared"));
assert!(!runner.contains("react_traits::check_enabled"));
assert!(prepared.contains("prepare_from_snapshot_with_catalog"));
assert!(!tasks.contains("queue::analyze_project_with_prepared_facts("));
Expand Down
65 changes: 65 additions & 0 deletions crates/no-mistakes/src/check_runner/tests/graph_scope.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use super::*;

#[test]
fn run_all_keeps_forbidden_graph_files_outside_filesystem_skips() {
let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("../../test-cases/check-runner/forbidden-deps-ignores-filesystem-skip/fixture");
let config = root.join(".no-mistakes.yml");
let results = run_all(root, Some(config), None).unwrap();

assert!(
results
.rules
.iter()
.any(|f| f.rule == no_mistakes::codebase::rules::FORBIDDEN_DEPENDENCIES),
"expected forbidden-dependencies finding for file under filesystem skip"
);
}

#[test]
fn run_all_keeps_dynamic_import_graph_within_filesystem_skips() {
let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("../../test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture");
let config = root.join(".no-mistakes.yml");
let results = run_all(root, Some(config), None).unwrap();
let finding = results
.rules
.iter()
.find(|finding| finding.file == "tests/scoped.test.ts")
.expect("skipped dynamic import remains reportable as unresolved");

assert_eq!(finding.import.as_deref(), Some("../skipped/target"));
assert_eq!(finding.target, None);
}

#[test]
fn run_all_keeps_reachability_sources_outside_filesystem_skips() {
let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(
"../../test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture",
);
let config = root.join(".no-mistakes.yml");
let results = run_all(root, Some(config), None).unwrap();

assert!(results.rules.iter().any(|finding| {
finding.rule == no_mistakes::codebase::rules::REQUIRED_ENTRYPOINT_REACHABILITY
&& finding.file == "sources/unreachable.ts"
&& finding.message.contains("not runtime-reachable")
}));
}

#[test]
fn run_all_keeps_playwright_graph_files_outside_filesystem_skips() {
let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("../../test-cases/check-runner/playwright-graph-ignores-filesystem-skip/fixture");
let config = root.join(".no-mistakes.yml");
let results = run_all(root, Some(config), None).unwrap();

assert!(!results.rules.iter().any(|finding| {
finding.rule == no_mistakes::playwright::rules::PLAYWRIGHT_COVERAGE
&& finding.target.as_deref() == Some("data-testid=save")
}));
assert!(results.rules.iter().any(|finding| {
finding.rule == no_mistakes::playwright::rules::PLAYWRIGHT_COVERAGE
&& finding.target.as_deref() == Some("data-testid=delete")
}));
}
4 changes: 0 additions & 4 deletions crates/no-mistakes/src/check_tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,6 @@ pub(crate) fn queues_configured(config: &NoMistakesConfig) -> bool {
.any(|project| !project.queues.enqueues.is_empty() || !project.queues.workers.is_empty())
}

pub(crate) fn forbidden_dependencies_configured(config: &NoMistakesConfig) -> bool {
rule_configured(config, rules::FORBIDDEN_DEPENDENCIES)
}

pub(crate) fn unique_exports_configured(config: &NoMistakesConfig) -> bool {
rule_configured(config, unique_exports::RULE_ID)
}
Expand Down
4 changes: 4 additions & 0 deletions crates/no-mistakes/src/codebase/check_facts/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ impl CheckFactMap {
self.view_with_supplemental(supplemental, graph_files)
}

pub(crate) fn with_graph_file_universe(&self, graph_files: Vec<PathBuf>) -> Self {
self.view_with_supplemental(&Self::default(), graph_files)
}

fn view_with_supplemental(&self, supplemental: &Self, graph_files: Vec<PathBuf>) -> Self {
let mut ts = self.ts.clone();
ts.extend(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,17 @@ fn non_workflow_relationship_edges(relationship: &RelationshipArg) -> &'static [
EdgeKind::TypeImport,
EdgeKind::DynamicImport,
EdgeKind::Require,
EdgeKind::RequireResolve,
],
RelationshipArg::ImportStatic => &[EdgeKind::Import],
RelationshipArg::ImportDynamic => &[EdgeKind::DynamicImport],
RelationshipArg::ImportType => &[EdgeKind::TypeImport],
RelationshipArg::ImportRequire => &[EdgeKind::Require],
RelationshipArg::ImportRequire => &[EdgeKind::Require, EdgeKind::RequireResolve],
RelationshipArg::RouteImport => &[EdgeKind::RouteImport],
RelationshipArg::Workspace => &[EdgeKind::WorkspaceImport],
RelationshipArg::Workspace => &[
EdgeKind::WorkspaceImport,
EdgeKind::WorkspaceTypeImport,
],
RelationshipArg::Package => &[EdgeKind::PackageDependency],
RelationshipArg::Test => &[
EdgeKind::TestOf,
Expand Down Expand Up @@ -94,6 +98,7 @@ fn standard_relationship_edges() -> std::collections::HashSet<EdgeKind> {
EdgeKind::TypeImport,
EdgeKind::DynamicImport,
EdgeKind::Require,
EdgeKind::RequireResolve,
EdgeKind::TestOf,
EdgeKind::VitestSetup(
crate::codebase::dependencies::graph::VitestSetupField::SetupFiles,
Expand All @@ -108,6 +113,7 @@ fn standard_relationship_edges() -> std::collections::HashSet<EdgeKind> {
EdgeKind::Layout,
EdgeKind::MarkdownLink,
EdgeKind::WorkspaceImport,
EdgeKind::WorkspaceTypeImport,
EdgeKind::PackageDependency,
EdgeKind::CiInvocation,
EdgeKind::WorkflowJob,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,12 @@ impl GraphBuildPlan {
imports: allowed.contains(&EdgeKind::Import)
|| allowed.contains(&EdgeKind::TypeImport)
|| allowed.contains(&EdgeKind::DynamicImport)
|| allowed.contains(&EdgeKind::Require),
|| allowed.contains(&EdgeKind::Require)
|| allowed.contains(&EdgeKind::RequireResolve),
route_imports: allowed.contains(&EdgeKind::RouteImport),
workspace: allowed.contains(&EdgeKind::WorkspaceImport),
workspace: allowed.contains(&EdgeKind::WorkspaceImport)
|| allowed.contains(&EdgeKind::WorkspaceTypeImport)
|| allowed.contains(&EdgeKind::RequireResolve),
package: allowed.contains(&EdgeKind::PackageDependency),
tests: allowed.contains(&EdgeKind::TestOf)
|| allowed.contains(&EdgeKind::VitestSetup(VitestSetupField::SetupFiles))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ fn edge_kind_for_import(import: &ExtractedImport) -> EdgeKind {
ImportKind::Static => EdgeKind::Import,
ImportKind::Type => EdgeKind::TypeImport,
ImportKind::Dynamic => EdgeKind::DynamicImport,
ImportKind::Require | ImportKind::RequireResolve => EdgeKind::Require,
ImportKind::Require => EdgeKind::Require,
ImportKind::RequireResolve => EdgeKind::RequireResolve,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fn collect_import_edges(
);
if let Some(target) = classification.resolver_path() {
let target = graph_files.visible_path(target)?;
return is_indexable(target).then(|| {
return (is_indexable(target) || kind == EdgeKind::RequireResolve).then(|| {
(
NodeId::File((*path).clone()),
NodeId::File(target.to_path_buf()),
Expand Down Expand Up @@ -67,6 +67,7 @@ fn collect_asset_edges(
.imports
.iter()
.filter(|imp| import_is_reachable(imp, facts, reachable))
.filter(|imp| !matches!(imp.kind, ImportKind::Type | ImportKind::RequireResolve))
.filter(|imp| imp.specifier.starts_with('.') || imp.specifier.starts_with('/'))
.filter_map(|imp| {
resolver.resolve(&imp.specifier, path).and_then(|target| {
Expand Down Expand Up @@ -113,10 +114,15 @@ fn collect_workspace_edges(
.workspace_path()
.and_then(|entry| graph_files.visible_path(entry))
.map(|entry| {
let kind = match imp.kind {
ImportKind::Type => EdgeKind::WorkspaceTypeImport,
ImportKind::RequireResolve => EdgeKind::RequireResolve,
_ => EdgeKind::WorkspaceImport,
};
(
NodeId::File((*path).clone()),
NodeId::File(entry.to_path_buf()),
EdgeKind::WorkspaceImport,
kind,
)
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,11 @@ fn collect_direct_reexport_edge(
inputs.visible_files,
) {
let Some(target) = inputs.graph_files.visible_path(&target) else { return; };
let kind = workspace_symbol_edge_kind(
export.is_type_only || target_export_is_type(target, imported, inputs.facts),
);
if imported == "*" {
edges.push((from, NodeId::File(target.to_path_buf()), EdgeKind::WorkspaceImport));
edges.push((from, NodeId::File(target.to_path_buf()), kind));
return;
}
edges.push((
Expand All @@ -99,7 +102,7 @@ fn collect_direct_reexport_edge(
file: target.to_path_buf(),
symbol: imported.clone(),
},
EdgeKind::WorkspaceImport,
kind,
));
} else if !inputs
.workspace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ fn imported_symbol_map(
ImportedSymbolTarget::Symbol {
file: target,
symbol: import.imported.clone(),
kind: EdgeKind::WorkspaceImport,
kind: workspace_symbol_edge_kind(import.is_type_only),
}
} else if workspace.recognizes_specifier_from(&import.source, path) {
continue;
Expand Down Expand Up @@ -130,7 +130,7 @@ fn namespace_import_map(
ImportedSymbolTarget::Symbol {
file,
symbol: "*".to_string(),
kind: EdgeKind::WorkspaceImport,
kind: workspace_symbol_edge_kind(import.is_type_only),
}
} else if workspace.recognizes_specifier_from(&import.source, path) {
continue;
Expand Down Expand Up @@ -175,3 +175,21 @@ fn symbol_edge_kind(is_type_only: bool) -> EdgeKind {
EdgeKind::Import
}
}

fn workspace_symbol_edge_kind(is_type_only: bool) -> EdgeKind {
if is_type_only {
EdgeKind::WorkspaceTypeImport
} else {
EdgeKind::WorkspaceImport
}
}

fn with_type_only_edge_kind(kind: EdgeKind, is_type_only: bool) -> EdgeKind {
if !is_type_only {
kind
} else if matches!(kind, EdgeKind::WorkspaceImport | EdgeKind::WorkspaceTypeImport) {
EdgeKind::WorkspaceTypeImport
} else {
EdgeKind::TypeImport
}
}
Loading
Loading