Skip to content

Commit

Permalink
feat: boundaries (#9634)
Browse files Browse the repository at this point in the history
### Description

This is a first pass at a boundaries implementation. We add a top level
command `turbo boundaries` that checks your files for invalid imports.
We currently check for:

- Importing a file outside of the package's directory
- Importing a type without declaring the import as a type
- Importing a package that is not specified in dependencies

### Testing Instructions

Added tests via `query` for the three scenarios listed above.

---------

Co-authored-by: Chris Olszewski <[email protected]>
  • Loading branch information
NicholasLYang and chris-olszewski authored Jan 22, 2025
1 parent 8a09c59 commit 7828e2c
Show file tree
Hide file tree
Showing 41 changed files with 874 additions and 79 deletions.
19 changes: 11 additions & 8 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ dunce = "1.0.3"
either = "1.9.0"
futures = "0.3.26"
futures-retry = "0.6.0"
git2 = { version = "0.19.0", default-features = false }
hex = "0.4.3"
httpmock = { version = "0.6.8", default-features = false }
indexmap = "1.9.2"
Expand All @@ -117,6 +118,7 @@ notify = "6.1.1"
num_cpus = "1.15.0"
once_cell = "1.17.1"
owo-colors = "3.5.0"
oxc_resolver = { version = "2.1.0" }
parking_lot = "0.12.1"
path-clean = "1.0.1"
pathdiff = "0.2.1"
Expand Down
2 changes: 1 addition & 1 deletion crates/turbo-trace/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ clap = { version = "4.5.17", features = ["derive"] }
futures = { workspace = true }
globwalk = { version = "0.1.0", path = "../turborepo-globwalk" }
miette = { workspace = true, features = ["fancy"] }
oxc_resolver = { version = "2.1.0" }
oxc_resolver = { workspace = true }
swc_common = { workspace = true, features = ["concurrent", "tty-emitter"] }
swc_ecma_ast = { workspace = true }
swc_ecma_parser = { workspace = true }
Expand Down
51 changes: 38 additions & 13 deletions crates/turbo-trace/src/import_finder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,41 +2,63 @@ use swc_common::{Span, Spanned};
use swc_ecma_ast::{Decl, ModuleDecl, Stmt};
use swc_ecma_visit::{Visit, VisitWith};

use crate::tracer::ImportType;
use crate::tracer::ImportTraceType;

/// The type of import that we find.
///
/// Either an import with a `type` keyword (indicating that it is importing only
/// types) or an import without the `type` keyword (indicating that it is
/// importing values and possibly types).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ImportType {
Type,
Value,
}

pub struct ImportFinder {
import_type: ImportType,
imports: Vec<(String, Span)>,
import_type: ImportTraceType,
imports: Vec<(String, Span, ImportType)>,
}

impl Default for ImportFinder {
fn default() -> Self {
Self::new(ImportTraceType::All)
}
}

impl ImportFinder {
pub fn new(import_type: ImportType) -> Self {
pub fn new(import_type: ImportTraceType) -> Self {
Self {
import_type,
imports: Vec::new(),
}
}

pub fn imports(&self) -> &[(String, Span)] {
pub fn imports(&self) -> &[(String, Span, ImportType)] {
&self.imports
}
}

impl Visit for ImportFinder {
fn visit_module_decl(&mut self, decl: &ModuleDecl) {
if let ModuleDecl::Import(import) = decl {
let import_type = if import.type_only {
ImportType::Type
} else {
ImportType::Value
};
match self.import_type {
ImportType::All => {
ImportTraceType::All => {
self.imports
.push((import.src.value.to_string(), import.span));
.push((import.src.value.to_string(), import.span, import_type));
}
ImportType::Types if import.type_only => {
ImportTraceType::Types if import.type_only => {
self.imports
.push((import.src.value.to_string(), import.span));
.push((import.src.value.to_string(), import.span, import_type));
}
ImportType::Values if !import.type_only => {
ImportTraceType::Values if !import.type_only => {
self.imports
.push((import.src.value.to_string(), import.span));
.push((import.src.value.to_string(), import.span, import_type));
}
_ => {}
}
Expand All @@ -56,8 +78,11 @@ impl Visit for ImportFinder {
lit_str,
)) = &*arg.expr
{
self.imports
.push((lit_str.value.to_string(), expr.span()));
self.imports.push((
lit_str.value.to_string(),
expr.span(),
ImportType::Value,
));
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion crates/turbo-trace/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
mod import_finder;
mod tracer;

pub use tracer::{ImportType, TraceError, TraceResult, Tracer};
pub use import_finder::{ImportFinder, ImportType};
pub use tracer::{ImportTraceType, TraceError, TraceResult, Tracer};
35 changes: 22 additions & 13 deletions crates/turbo-trace/src/tracer.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{collections::HashMap, sync::Arc};
use std::{collections::HashMap, fmt, sync::Arc};

use camino::{Utf8Path, Utf8PathBuf};
use globwalk::WalkType;
Expand Down Expand Up @@ -38,7 +38,7 @@ pub struct Tracer {
source_map: Arc<SourceMap>,
cwd: AbsoluteSystemPathBuf,
errors: Vec<TraceError>,
import_type: ImportType,
import_type: ImportTraceType,
}

#[derive(Clone, Debug, Error, Diagnostic)]
Expand Down Expand Up @@ -95,10 +95,19 @@ pub struct TraceResult {
pub files: HashMap<AbsoluteSystemPathBuf, SeenFile>,
}

impl fmt::Debug for TraceResult {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("TraceResult")
.field("files", &self.files)
.field("errors", &self.errors)
.finish()
}
}

/// The type of imports to trace.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[allow(dead_code)]
pub enum ImportType {
pub enum ImportTraceType {
/// Trace all imports.
All,
/// Trace only `import type` imports
Expand All @@ -122,14 +131,14 @@ impl Tracer {
files,
ts_config,
cwd,
import_type: ImportType::All,
import_type: ImportTraceType::All,
errors: Vec::new(),
source_map: Arc::new(SourceMap::default()),
}
}

#[allow(dead_code)]
pub fn set_import_type(&mut self, import_type: ImportType) {
pub fn set_import_type(&mut self, import_type: ImportTraceType) {
self.import_type = import_type;
}

Expand All @@ -139,7 +148,7 @@ impl Tracer {
errors: &mut Vec<TraceError>,
resolver: &Resolver,
file_path: &AbsoluteSystemPath,
import_type: ImportType,
import_type: ImportTraceType,
) -> Option<(Vec<AbsoluteSystemPathBuf>, SeenFile)> {
// Read the file content
let Ok(file_content) = tokio::fs::read_to_string(&file_path).await else {
Expand Down Expand Up @@ -191,7 +200,7 @@ impl Tracer {
// Convert found imports/requires to absolute paths and add them to files to
// visit
let mut files = Vec::new();
for (import, span) in finder.imports() {
for (import, span, _) in finder.imports() {
debug!("processing {} in {}", import, file_path);
let Some(file_dir) = file_path.parent() else {
errors.push(TraceError::RootFile(file_path.to_owned()));
Expand Down Expand Up @@ -344,7 +353,7 @@ impl Tracer {
}
}

pub fn create_resolver(&mut self) -> Resolver {
pub fn create_resolver(ts_config: Option<&AbsoluteSystemPath>) -> Resolver {
let mut options = ResolveOptions::default()
.with_builtin_modules(true)
.with_force_extension(EnforceExtension::Disabled)
Expand All @@ -362,9 +371,9 @@ impl Tracer {
// We add a bunch so oxc_resolver can resolve all kinds of imports.
.with_condition_names(&["import", "require", "node", "types", "default"]);

if let Some(ts_config) = self.ts_config.take() {
if let Some(ts_config) = ts_config {
options.tsconfig = Some(TsconfigOptions {
config_file: ts_config.into(),
config_file: ts_config.as_std_path().into(),
references: TsconfigReferences::Auto,
});
}
Expand All @@ -374,7 +383,7 @@ impl Tracer {

pub async fn trace(mut self, max_depth: Option<usize>) -> TraceResult {
let mut seen: HashMap<AbsoluteSystemPathBuf, SeenFile> = HashMap::new();
let resolver = self.create_resolver();
let resolver = Self::create_resolver(self.ts_config.as_deref());

while let Some((file_path, file_depth)) = self.files.pop() {
if let Some(max_depth) = max_depth {
Expand All @@ -393,7 +402,7 @@ impl Tracer {
}
}

pub async fn reverse_trace(mut self) -> TraceResult {
pub async fn reverse_trace(self) -> TraceResult {
let files = match globwalk::globwalk(
&self.cwd,
&[
Expand All @@ -420,7 +429,7 @@ impl Tracer {

let mut futures = JoinSet::new();

let resolver = Arc::new(self.create_resolver());
let resolver = Arc::new(Self::create_resolver(self.ts_config.as_deref()));
let source_map = self.source_map.clone();
let shared_self = Arc::new(self);

Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-filewatch/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ version = "1.0.4"
version = "0.2.4"

[dev-dependencies]
git2 = { version = "0.16.1", default-features = false }
git2 = { workspace = true, default-features = false }
tempfile = { workspace = true }
tokio-scoped = "0.2.0"
tracing-test = "0.2.4"
Expand Down
3 changes: 3 additions & 0 deletions crates/turborepo-lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ dunce = { workspace = true }
either = { workspace = true }
futures = "0.3.30"
futures-core = "0.3.30"
git2 = { workspace = true, default-features = false }
globwalk = { version = "0.1.0", path = "../turborepo-globwalk" }
globwatch = { path = "../turborepo-globwatch" }
go-parse-duration = "0.1.1"
Expand All @@ -78,6 +79,7 @@ nix = "0.26.2"
notify = { workspace = true }
num_cpus = { workspace = true }
owo-colors = { workspace = true }
oxc_resolver = { workspace = true }
path-clean = "1.0.1"
petgraph = { workspace = true }
pidlock = { path = "../turborepo-pidlock" }
Expand All @@ -103,6 +105,7 @@ svix-ksuid = { version = "0.7.0", features = ["serde"] }
swc_common = { workspace = true }
swc_ecma_ast = { workspace = true, features = ["serde-impl"] }
swc_ecma_parser = { workspace = true }
swc_ecma_visit = { workspace = true }
sysinfo = "0.27.7"
tabwriter = "1.3.0"
thiserror = "1.0.38"
Expand Down
Loading

0 comments on commit 7828e2c

Please sign in to comment.