Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
22 changes: 10 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,8 @@ 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 playwright_consumers = canonical_graph_plan
.map(
|plan| no_mistakes::playwright::rules::PlaywrightFactConsumers {
graph_selectors: plan.playwright_selectors,
Expand Down Expand Up @@ -76,21 +73,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 +108,7 @@ 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 = canonical_graph_plan.is_some() || playwright_fact_plan.is_some();
let needs_graph_files =
needs_shared_facts && (needs_full_graph_files || enabled.dynamic_import_rules);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
let (discovered, graph_files) = if needs_full_graph_files {
Comment thread
jonathanong marked this conversation as resolved.
Outdated
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
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
1 change: 1 addition & 0 deletions crates/no-mistakes/src/codebase/rules/ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub use super::require_files_in_subdirs::RULE_ID as REQUIRE_FILES_IN_SUBDIRS;
pub use super::require_storybook_stories::RULE_ID as REQUIRE_STORYBOOK_STORIES;
pub use super::require_test_per_subdir::RULE_ID as REQUIRE_TEST_PER_SUBDIR;
pub use super::required_companion_imports::RULE_ID as REQUIRED_COMPANION_IMPORTS;
pub use super::required_entrypoint_reachability::RULE_ID as REQUIRED_ENTRYPOINT_REACHABILITY;
pub use super::required_local_docs::REQUIRED_DOC_SECTION_RULE_ID as REQUIRED_DOC_SECTION;
pub use super::required_local_docs::RULE_ID as REQUIRED_LOCAL_DOCS;
pub use super::rust_max_lines_per_file::RULE_ID as RUST_MAX_LINES_PER_FILE;
Expand Down
4 changes: 3 additions & 1 deletion crates/no-mistakes/src/codebase/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub mod require_files_in_subdirs;
pub mod require_storybook_stories;
pub mod require_test_per_subdir;
pub mod required_companion_imports;
pub mod required_entrypoint_reachability;
pub mod required_local_docs;
pub mod rust_max_lines_per_file;
pub mod rust_no_inline_allows;
Expand Down Expand Up @@ -65,7 +66,8 @@ pub use filesystem_dispatch::{
run_filesystem_rules_with_visible_and_snapshot,
};
pub use ids::*;
pub(crate) use run::canonical_graph_plan;
#[doc(hidden)]
pub use run::canonical_graph_plan;
#[doc(hidden)]
pub use run::run_check_with_config_facts_playwright_and_graph;
pub use run::{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
use super::RuleFinding;
use crate::codebase::dependencies::graph::{DepGraph, EdgeKind, GraphBuildPlan, NodeId};
use crate::codebase::ts_source::relative_slash_path;
use crate::config::v2::NoMistakesConfig;
use anyhow::Result;
use serde::Deserialize;
use std::collections::HashSet;
use std::path::{Path, PathBuf};

pub const RULE_ID: &str = "required-entrypoint-reachability";

#[derive(Debug, Deserialize, Default)]
#[serde(default, rename_all = "camelCase")]
pub(crate) struct Options {
pub(crate) source_globs: Vec<String>,
pub(crate) entrypoints: Vec<String>,
pub(crate) max_depth: Option<usize>,
}

pub(crate) fn graph_plan(config: &NoMistakesConfig) -> Option<GraphBuildPlan> {
config
.rule_configured(RULE_ID)
.then(GraphBuildPlan::imports_and_workspace)
Comment thread
jonathanong marked this conversation as resolved.
Outdated
}

pub(crate) fn check_with_graph_and_inferred(
root: &Path,
config: &NoMistakesConfig,
files: &[PathBuf],
graph: &DepGraph,
inferred_roots: Option<&crate::codebase::config::InferredRoots>,
) -> Result<Vec<RuleFinding>> {
let file_universe = files
.iter()
.map(|path| crate::codebase::ts_resolver::normalize_path(path))
.collect::<HashSet<_>>();
let mut findings = Vec::new();
for rule in config.rule_applications(RULE_ID) {
let options: Options = rule.rule_options();
let mut inferred_roots = inferred_roots.cloned().unwrap_or_default();
let source_filter = super::path_filter::RulePathFilter::new_with_inferred(
root,
config,
rule,
&mut inferred_roots,
)?;
let scoped_files = files
.iter()
.filter(|path| source_filter.is_match(path))
.cloned()
.collect::<Vec<_>>();
let target_roots =
super::target_roots_with_inferred(root, config, rule, &mut inferred_roots);
findings.extend(check_rule_application(
root,
&options,
&scoped_files,
&target_roots,
graph,
&file_universe,
));
}
super::sort_findings(&mut findings);
Ok(findings)
}

fn check_rule_application(
root: &Path,
options: &Options,
scoped_files: &[PathBuf],
target_roots: &[PathBuf],
graph: &DepGraph,
file_universe: &HashSet<PathBuf>,
) -> Vec<RuleFinding> {
let mut findings = Vec::new();
if options.source_globs.is_empty() {
findings.push(config_finding(
"each rule entry requires at least one sourceGlobs pattern",
None,
));
}
if options.entrypoints.is_empty() {
findings.push(config_finding(
"each rule entry requires at least one entrypoint",
None,
));
}

let mut source_files = HashSet::new();
for pattern in &options.source_globs {
match super::matching_files(
root,
std::slice::from_ref(pattern),
scoped_files,
target_roots,
) {
Ok(matches) if matches.is_empty() => findings.push(config_finding(
&format!("sourceGlobs pattern `{pattern}` matched no files"),
Some(pattern.clone()),
)),
Ok(matches) => source_files.extend(matches),
Err(error) => findings.push(config_finding(
&format!("invalid sourceGlobs pattern `{pattern}`: {error}"),
Some(pattern.clone()),
)),
}
}

let mut entrypoint_paths = Vec::new();
let mut entrypoint_labels = Vec::new();
for configured in &options.entrypoints {
let path = resolve_entrypoint(root, configured);
if path
.as_ref()
.is_none_or(|path| !file_universe.contains(path) || !graph.contains_file(path))
{
findings.push(config_finding(
&format!("entrypoint `{configured}` does not exist"),
Some(configured.clone()),
));
continue;
}
let path = path.expect("validated entrypoint path");
entrypoint_labels.push(relative_slash_path(root, &path));
entrypoint_paths.push(path);
}
entrypoint_labels.sort();
entrypoint_labels.dedup();
entrypoint_paths.sort();
entrypoint_paths.dedup();

if !entrypoint_paths.is_empty() {
let allowed = runtime_edge_kinds();
let roots = entrypoint_paths
.iter()
.cloned()
.map(NodeId::File)
.collect::<Vec<_>>();
let mut reachable = graph
.deps_of_in_file_universe(&roots, options.max_depth, Some(&allowed), file_universe)
.into_iter()
.filter_map(|entry| entry.node.as_file().map(Path::to_path_buf))
.collect::<HashSet<_>>();
reachable.extend(entrypoint_paths);
let target = entrypoint_labels.join(",");
for source in source_files {
let source = crate::codebase::ts_resolver::normalize_path(&source);
if reachable.contains(&source) {
continue;
}
let file = relative_slash_path(root, &source);
findings.push(RuleFinding {
rule: RULE_ID.to_string(),
file: file.clone(),
line: 1,
message: format!(
"{file} is not runtime-reachable from configured entrypoints: {target}"
),
import: None,
target: Some(target.clone()),
});
}
}
findings
}

fn resolve_entrypoint(root: &Path, configured: &str) -> Option<PathBuf> {
let configured = Path::new(configured.trim_start_matches("./"));
let path = if configured.is_absolute() {
configured.to_path_buf()
} else {
root.join(configured)
};
let path = crate::codebase::ts_resolver::normalize_path(&path);
path.starts_with(root).then_some(path)
}

fn runtime_edge_kinds() -> HashSet<EdgeKind> {
[
EdgeKind::Import,
EdgeKind::DynamicImport,
EdgeKind::Require,
Comment thread
jonathanong marked this conversation as resolved.
EdgeKind::WorkspaceImport,
Comment thread
jonathanong marked this conversation as resolved.
]
.into_iter()
.collect()
}

fn config_finding(message: &str, target: Option<String>) -> RuleFinding {
RuleFinding {
rule: RULE_ID.to_string(),
file: ".no-mistakes.yml".to_string(),
line: 1,
message: format!("{RULE_ID}: {message}"),
import: None,
target,
}
}

#[cfg(test)]
mod tests;
Loading
Loading