这是indexloc提供的服务，不要输入任何密码
Skip to content

feat(boundaries): implicit dependencies #10117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 7, 2025
Merged

feat(boundaries): implicit dependencies #10117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
92a64f0
Add implicit boundaries config
NicholasLYang Mar 6, 2025
38f9dd8
Refactor boundaries code to use new TurboJsonLoader API
NicholasLYang Mar 6, 2025
653da22
Implement implicitDependencies in boundaries
NicholasLYang Mar 6, 2025
34dd52b
Extend with global implicit dependencies
NicholasLYang Mar 6, 2025
013a58c
Clean up tests with serde directive
NicholasLYang Mar 7, 2025
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
6 changes: 5 additions & 1 deletion crates/turborepo-lib/src/boundaries/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ use struct_iterable::Iterable;
use turborepo_errors::Spanned;

#[derive(Serialize, Default, Debug, Clone, Iterable, Deserializable, PartialEq)]
pub struct RootBoundariesConfig {
pub struct BoundariesConfig {
#[serde(skip_serializing_if = "Option::is_none")]
pub tags: Option<Spanned<RulesMap>>,
#[serde(skip_serializing_if = "Option::is_none")]
pub implicit_dependencies: Option<Spanned<Vec<Spanned<String>>>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed in the snapshots we're serializing "implicitDependencies": null, maybe #[serde(skip_serializing_if = "Option::is_none")]?

}

pub type RulesMap = HashMap<String, Spanned<Rule>>;

#[derive(Serialize, Default, Debug, Clone, Iterable, Deserializable, PartialEq)]
Expand Down
128 changes: 66 additions & 62 deletions crates/turborepo-lib/src/boundaries/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::{
collections::{BTreeMap, HashSet},
collections::{BTreeMap, HashMap, HashSet},
sync::Arc,
};

Expand All @@ -10,8 +10,9 @@ use oxc_resolver::{ResolveError, Resolver, TsConfig};
use swc_common::{comments::SingleThreadedComments, SourceFile, Span};
use turbo_trace::ImportType;
use turbopath::{AbsoluteSystemPath, AnchoredSystemPathBuf, PathRelation, RelativeUnixPath};
use turborepo_errors::Spanned;
use turborepo_repository::{
package_graph::{PackageInfo, PackageName, PackageNode},
package_graph::{PackageName, PackageNode},
package_json::PackageJson,
};

Expand All @@ -20,6 +21,64 @@ use crate::{
run::Run,
};

/// All the places a dependency can be declared
#[derive(Clone, Copy)]
pub struct DependencyLocations<'a> {
pub(crate) internal_dependencies: &'a HashSet<&'a PackageNode>,
pub(crate) package_json: &'a PackageJson,
pub(crate) unresolved_external_dependencies: Option<&'a BTreeMap<String, String>>,
pub(crate) implicit_dependencies: &'a HashMap<String, Spanned<()>>,
pub(crate) global_implicit_dependencies: &'a HashMap<String, Spanned<()>>,
}

impl<'a> DependencyLocations<'a> {
/// Go through all the possible places a package could be declared to see if
/// it's a valid import. We don't use `oxc_resolver` because there are some
/// cases where you can resolve a package that isn't declared properly.
fn is_dependency(&self, package_name: &PackageNode) -> bool {
self.internal_dependencies.contains(package_name)
|| self
.unresolved_external_dependencies
.is_some_and(|external_dependencies| {
external_dependencies.contains_key(package_name.as_package_name().as_str())
})
|| self
.package_json
.dependencies
.as_ref()
.is_some_and(|dependencies| {
dependencies.contains_key(package_name.as_package_name().as_str())
})
|| self
.package_json
.dev_dependencies
.as_ref()
.is_some_and(|dev_dependencies| {
dev_dependencies.contains_key(package_name.as_package_name().as_str())
})
|| self
.package_json
.peer_dependencies
.as_ref()
.is_some_and(|peer_dependencies| {
peer_dependencies.contains_key(package_name.as_package_name().as_str())
})
|| self
.package_json
.optional_dependencies
.as_ref()
.is_some_and(|optional_dependencies| {
optional_dependencies.contains_key(package_name.as_package_name().as_str())
})
|| self
.implicit_dependencies
.contains_key(package_name.as_package_name().as_str())
|| self
.global_implicit_dependencies
.contains_key(package_name.as_package_name().as_str())
}
}

impl Run {
/// Checks if the given import can be resolved as a tsconfig path alias,
/// e.g. `@/types/foo` -> `./src/foo`, and if so, checks the resolved paths.
Expand Down Expand Up @@ -79,9 +138,7 @@ impl Run {
span: &Span,
file_path: &AbsoluteSystemPath,
file_content: &str,
package_info: &PackageInfo,
internal_dependencies: &HashSet<&PackageNode>,
unresolved_external_dependencies: Option<&BTreeMap<String, String>>,
dependency_locations: DependencyLocations<'_>,
resolver: &Resolver,
) -> Result<(), Error> {
// If the import is prefixed with `@boundaries-ignore`, we ignore it, but print
Expand Down Expand Up @@ -154,9 +211,7 @@ impl Run {
span,
file_path,
file_content,
&package_info.package_json,
internal_dependencies,
unresolved_external_dependencies,
dependency_locations,
resolver,
)
} else {
Expand Down Expand Up @@ -207,45 +262,6 @@ impl Run {
}
}

/// Go through all the possible places a package could be declared to see if
/// it's a valid import. We don't use `oxc_resolver` because there are some
/// cases where you can resolve a package that isn't declared properly.
fn is_dependency(
internal_dependencies: &HashSet<&PackageNode>,
package_json: &PackageJson,
unresolved_external_dependencies: Option<&BTreeMap<String, String>>,
package_name: &PackageNode,
) -> bool {
internal_dependencies.contains(&package_name)
|| unresolved_external_dependencies.is_some_and(|external_dependencies| {
external_dependencies.contains_key(package_name.as_package_name().as_str())
})
|| package_json
.dependencies
.as_ref()
.is_some_and(|dependencies| {
dependencies.contains_key(package_name.as_package_name().as_str())
})
|| package_json
.dev_dependencies
.as_ref()
.is_some_and(|dev_dependencies| {
dev_dependencies.contains_key(package_name.as_package_name().as_str())
})
|| package_json
.peer_dependencies
.as_ref()
.is_some_and(|peer_dependencies| {
peer_dependencies.contains_key(package_name.as_package_name().as_str())
})
|| package_json
.optional_dependencies
.as_ref()
.is_some_and(|optional_dependencies| {
optional_dependencies.contains_key(package_name.as_package_name().as_str())
})
}

fn get_package_name(import: &str) -> String {
if import.starts_with("@") {
import.split('/').take(2).join("/")
Expand All @@ -266,9 +282,7 @@ impl Run {
span: SourceSpan,
file_path: &AbsoluteSystemPath,
file_content: &str,
package_json: &PackageJson,
internal_dependencies: &HashSet<&PackageNode>,
unresolved_external_dependencies: Option<&BTreeMap<String, String>>,
dependency_locations: DependencyLocations<'_>,
resolver: &Resolver,
) -> Option<BoundariesDiagnostic> {
let package_name = Self::get_package_name(import);
Expand All @@ -282,12 +296,7 @@ impl Run {
}
let package_name = PackageNode::Workspace(PackageName::Other(package_name));
let folder = file_path.parent().expect("file_path should have a parent");
let is_valid_dependency = Self::is_dependency(
internal_dependencies,
package_json,
unresolved_external_dependencies,
&package_name,
);
let is_valid_dependency = dependency_locations.is_dependency(&package_name);

if !is_valid_dependency
&& !matches!(
Expand All @@ -300,12 +309,7 @@ impl Run {
"@types/{}",
package_name.as_package_name().as_str()
)));
let is_types_dependency = Self::is_dependency(
internal_dependencies,
package_json,
unresolved_external_dependencies,
&types_package_name,
);
let is_types_dependency = dependency_locations.is_dependency(&types_package_name);

if is_types_dependency {
return match import_type {
Expand Down
61 changes: 46 additions & 15 deletions crates/turborepo-lib/src/boundaries/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::{
sync::{Arc, LazyLock, Mutex},
};

pub use config::{Permissions, RootBoundariesConfig, Rule};
pub use config::{BoundariesConfig, Permissions, Rule};
use git2::Repository;
use globwalk::Settings;
use miette::{Diagnostic, NamedSource, Report, SourceSpan};
Expand All @@ -31,8 +31,9 @@ use turborepo_repository::package_graph::{PackageInfo, PackageName, PackageNode}
use turborepo_ui::{color, ColorConfig, BOLD_GREEN, BOLD_RED};

use crate::{
boundaries::{tags::ProcessedRulesMap, tsconfig::TsConfigLoader},
boundaries::{imports::DependencyLocations, tags::ProcessedRulesMap, tsconfig::TsConfigLoader},
run::Run,
turbo_json::TurboJson,
};

#[derive(Clone, Debug, Error, Diagnostic)]
Expand Down Expand Up @@ -211,12 +212,11 @@ impl BoundariesResult {

impl Run {
pub async fn check_boundaries(&self) -> Result<BoundariesResult, Error> {
let package_tags = self.get_package_tags();
let rules_map = self.get_processed_rules_map();
let packages: Vec<_> = self.pkg_dep_graph().packages().collect();
let repo = Repository::discover(self.repo_root()).ok().map(Mutex::new);
let mut result = BoundariesResult::default();

let global_implicit_dependencies = self.get_implicit_dependencies(&PackageName::Root);
for (package_name, package_info) in packages {
if !self.filtered_pkgs().contains(package_name)
|| matches!(package_name, PackageName::Root)
Expand All @@ -228,8 +228,8 @@ impl Run {
&repo,
package_name,
package_info,
&package_tags,
&rules_map,
&global_implicit_dependencies,
&mut result,
)
.await?;
Expand All @@ -251,26 +251,49 @@ impl Run {
None
}

pub fn get_implicit_dependencies(&self, pkg: &PackageName) -> HashMap<String, Spanned<()>> {
self.turbo_json_loader()
.load(pkg)
.ok()
.and_then(|turbo_json| turbo_json.boundaries.as_ref())
.and_then(|boundaries| boundaries.implicit_dependencies.as_ref())
.into_iter()
.flatten()
.flatten()
.map(|dep| dep.clone().split())
.collect::<HashMap<_, _>>()
}

/// Either returns a list of errors and number of files checked or a single,
/// fatal error
async fn check_package(
&self,
repo: &Option<Mutex<Repository>>,
package_name: &PackageName,
package_info: &PackageInfo,
all_package_tags: &HashMap<PackageName, Spanned<Vec<Spanned<String>>>>,
tag_rules: &Option<ProcessedRulesMap>,
global_implicit_dependencies: &HashMap<String, Spanned<()>>,
result: &mut BoundariesResult,
) -> Result<(), Error> {
self.check_package_files(repo, package_name, package_info, result)
.await?;

if let Some(current_package_tags) = all_package_tags.get(package_name) {
let implicit_dependencies = self.get_implicit_dependencies(package_name);
self.check_package_files(
repo,
package_name,
package_info,
implicit_dependencies,
global_implicit_dependencies,
result,
)
.await?;

if let Ok(TurboJson {
tags: Some(tags), ..
}) = self.turbo_json_loader().load(package_name)
{
if let Some(tag_rules) = tag_rules {
result.diagnostics.extend(self.check_package_tags(
PackageNode::Workspace(package_name.clone()),
current_package_tags,
all_package_tags,
tags,
tag_rules,
)?);
} else {
Expand All @@ -297,6 +320,8 @@ impl Run {
repo: &Option<Mutex<Repository>>,
package_name: &PackageName,
package_info: &PackageInfo,
implicit_dependencies: HashMap<String, Spanned<()>>,
global_implicit_dependencies: &HashMap<String, Spanned<()>>,
result: &mut BoundariesResult,
) -> Result<(), Error> {
let package_root = self.repo_root().resolve(package_info.package_path());
Expand Down Expand Up @@ -393,6 +418,14 @@ impl Run {
// Visit the AST and find imports
let mut finder = ImportFinder::default();
module.visit_with(&mut finder);
let dependency_locations = DependencyLocations {
internal_dependencies: &internal_dependencies,
package_json: &package_info.package_json,
implicit_dependencies: &implicit_dependencies,
global_implicit_dependencies,
unresolved_external_dependencies,
};

for (import, span, import_type) in finder.imports() {
self.check_import(
&comments,
Expand All @@ -406,9 +439,7 @@ impl Run {
span,
file_path,
&file_content,
package_info,
&internal_dependencies,
unresolved_external_dependencies,
dependency_locations,
&resolver,
)?;
}
Expand Down
21 changes: 15 additions & 6 deletions crates/turborepo-lib/src/boundaries/tags.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,25 @@ impl From<Permissions> for ProcessedPermissions {
}

impl Run {
pub fn load_package_tags(&self, pkg: &PackageName) -> Option<&Spanned<Vec<Spanned<String>>>> {
self.turbo_json_loader()
.load(pkg)
.ok()
.and_then(|turbo_json| turbo_json.tags.as_ref())
}

pub(crate) fn get_package_tags(&self) -> HashMap<PackageName, Spanned<Vec<Spanned<String>>>> {
let mut package_tags = HashMap::new();
let turbo_json_loader = self.turbo_json_loader();
for (package, _) in self.pkg_dep_graph().packages() {
if let Ok(TurboJson {
tags: Some(tags),
boundaries,
..
}) = turbo_json_loader.load(package)
}) = self.turbo_json_loader().load(package)
{
if boundaries.is_some() && !matches!(package, PackageName::Root) {
if boundaries.as_ref().is_some_and(|b| b.tags.is_some())
&& !matches!(package, PackageName::Root)
{
warn!(
"Boundaries rules can only be defined in the root turbo.json. Any rules \
defined in a package's turbo.json will be ignored."
Expand Down Expand Up @@ -163,7 +171,6 @@ impl Run {
&self,
pkg: PackageNode,
current_package_tags: &Spanned<Vec<Spanned<String>>>,
all_package_tags: &HashMap<PackageName, Spanned<Vec<Spanned<String>>>>,
tags_rules: &ProcessedRulesMap,
) -> Result<Vec<BoundariesDiagnostic>, Error> {
let mut diagnostics = Vec::new();
Expand All @@ -174,7 +181,9 @@ impl Run {
if matches!(dependency, PackageNode::Root) {
continue;
}
let dependency_tags = all_package_tags.get(dependency.as_package_name());

let dependency_tags = self.load_package_tags(dependency.as_package_name());

diagnostics.extend(self.validate_relation(
pkg.as_package_name(),
dependency.as_package_name(),
Expand All @@ -190,7 +199,7 @@ impl Run {
if matches!(dependent, PackageNode::Root) {
continue;
}
let dependent_tags = all_package_tags.get(dependent.as_package_name());
let dependent_tags = self.load_package_tags(dependent.as_package_name());
diagnostics.extend(self.validate_relation(
pkg.as_package_name(),
dependent.as_package_name(),
Expand Down
Loading
Loading