diff --git a/crates/turborepo-lib/src/boundaries/config.rs b/crates/turborepo-lib/src/boundaries/config.rs index 454258dfd2827..1f0e511d3aa86 100644 --- a/crates/turborepo-lib/src/boundaries/config.rs +++ b/crates/turborepo-lib/src/boundaries/config.rs @@ -11,13 +11,21 @@ pub struct BoundariesConfig { pub tags: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub implicit_dependencies: Option>>>, + /// If in a package `turbo.json`, the following two keys define + /// boundaries rules for that package + #[serde(skip_serializing_if = "Option::is_none")] + pub dependencies: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub dependents: Option>, } pub type RulesMap = HashMap>; #[derive(Serialize, Default, Debug, Clone, Iterable, Deserializable, PartialEq)] pub struct Rule { + #[serde(skip_serializing_if = "Option::is_none")] pub dependencies: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub dependents: Option>, } diff --git a/crates/turborepo-lib/src/boundaries/mod.rs b/crates/turborepo-lib/src/boundaries/mod.rs index df332762bdcfa..7c785094d8e0b 100644 --- a/crates/turborepo-lib/src/boundaries/mod.rs +++ b/crates/turborepo-lib/src/boundaries/mod.rs @@ -36,7 +36,6 @@ use turborepo_ui::{color, ColorConfig, BOLD_GREEN, BOLD_RED}; use crate::{ boundaries::{imports::DependencyLocations, tags::ProcessedRulesMap, tsconfig::TsConfigLoader}, run::Run, - turbo_json::TurboJson, }; #[derive(Clone, Debug, Error, Diagnostic)] @@ -67,6 +66,13 @@ pub enum SecondaryDiagnostic { #[derive(Clone, Debug, Error, Diagnostic)] pub enum BoundariesDiagnostic { + #[error("Package boundaries rules cannot have `tags` key")] + PackageBoundariesHasTags { + #[label("tags defined here")] + span: Option, + #[source_code] + text: NamedSource, + }, #[error("Tag `{tag}` cannot share the same name as package `{package}`")] TagSharesPackageName { tag: String, @@ -332,25 +338,13 @@ impl Run { ) .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()), - &package_info.package_json, - tags, - tag_rules, - )?); - } else { - // NOTE: if we use tags for something other than boundaries, we should remove - // this warning - warn!( - "No boundaries rules found, but package {} has tags", - package_name - ); - } + if let Ok(turbo_json) = self.turbo_json_loader().load(package_name) { + result.diagnostics.extend(self.check_package_tags( + PackageNode::Workspace(package_name.clone()), + &package_info.package_json, + turbo_json.tags.as_ref(), + tag_rules.as_ref(), + )?); } result.packages_checked += 1; diff --git a/crates/turborepo-lib/src/boundaries/tags.rs b/crates/turborepo-lib/src/boundaries/tags.rs index 98b19e7cfd7dd..2fa6e44393323 100644 --- a/crates/turborepo-lib/src/boundaries/tags.rs +++ b/crates/turborepo-lib/src/boundaries/tags.rs @@ -177,73 +177,132 @@ impl Run { Ok(None) } + pub(crate) fn check_tag( + &self, + diagnostics: &mut Vec, + dependencies: Option<&ProcessedPermissions>, + dependents: Option<&ProcessedPermissions>, + pkg: &PackageNode, + package_json: &PackageJson, + ) -> Result<(), Error> { + if let Some(dependency_permissions) = dependencies { + for dependency in self.pkg_dep_graph().dependencies(pkg) { + if matches!(dependency, PackageNode::Root) { + continue; + } + + let dependency_tags = self.load_package_tags(dependency.as_package_name()); + + diagnostics.extend(self.validate_relation( + pkg.as_package_name(), + package_json, + dependency.as_package_name(), + dependency_tags, + dependency_permissions.allow.as_ref(), + dependency_permissions.deny.as_ref(), + )?); + } + } + + if let Some(dependent_permissions) = dependents { + for dependent in self.pkg_dep_graph().ancestors(pkg) { + if matches!(dependent, PackageNode::Root) { + continue; + } + let dependent_tags = self.load_package_tags(dependent.as_package_name()); + diagnostics.extend(self.validate_relation( + pkg.as_package_name(), + package_json, + dependent.as_package_name(), + dependent_tags, + dependent_permissions.allow.as_ref(), + dependent_permissions.deny.as_ref(), + )?) + } + } + + Ok(()) + } + + fn check_if_package_name_is_tag( + tags_rules: &ProcessedRulesMap, + pkg: &PackageNode, + package_json: &PackageJson, + ) -> Option { + let rule = tags_rules.get(pkg.as_package_name().as_str())?; + let (tag_span, tag_text) = rule.span.span_and_text("turbo.json"); + let (package_span, package_text) = package_json + .name + .as_ref() + .map(|name| name.span_and_text("package.json")) + .unwrap_or_else(|| (None, NamedSource::new("package.json", "".into()))); + Some(BoundariesDiagnostic::TagSharesPackageName { + tag: pkg.as_package_name().to_string(), + package: pkg.as_package_name().to_string(), + tag_span, + tag_text, + secondary: [SecondaryDiagnostic::PackageDefinedHere { + package: pkg.as_package_name().to_string(), + package_span, + package_text, + }], + }) + } + pub(crate) fn check_package_tags( &self, pkg: PackageNode, package_json: &PackageJson, - current_package_tags: &Spanned>>, - tags_rules: &ProcessedRulesMap, + current_package_tags: Option<&Spanned>>>, + tags_rules: Option<&ProcessedRulesMap>, ) -> Result, Error> { let mut diagnostics = Vec::new(); + let package_turbo_json = self.turbo_json_loader().load(pkg.as_package_name()); + let package_boundaries = package_turbo_json + .ok() + .and_then(|turbo_json| turbo_json.boundaries.as_ref()); - // We don't allow tags to share the same name as the package because - // we allow package names to be used as a tag - if let Some(rule) = tags_rules.get(pkg.as_package_name().as_str()) { - let (tag_span, tag_text) = rule.span.span_and_text("turbo.json"); - let (package_span, package_text) = package_json - .name - .as_ref() - .map(|name| name.span_and_text("package.json")) - .unwrap_or_else(|| (None, NamedSource::new("package.json", "".into()))); - diagnostics.push(BoundariesDiagnostic::TagSharesPackageName { - tag: pkg.as_package_name().to_string(), - package: pkg.as_package_name().to_string(), - tag_span, - tag_text, - secondary: [SecondaryDiagnostic::PackageDefinedHere { - package: pkg.as_package_name().to_string(), - package_span, - package_text, - }], - }); + if let Some(boundaries) = package_boundaries { + if let Some(tags) = &boundaries.tags { + let (span, text) = tags.span_and_text("turbo.json"); + diagnostics.push(BoundariesDiagnostic::PackageBoundariesHasTags { span, text }); + } + let dependencies = boundaries + .dependencies + .clone() + .map(|deps| deps.into_inner().into()); + let dependents = boundaries + .dependents + .clone() + .map(|deps| deps.into_inner().into()); + + self.check_tag( + &mut diagnostics, + dependencies.as_ref(), + dependents.as_ref(), + &pkg, + package_json, + )?; } - for tag in current_package_tags.iter() { - if let Some(rule) = tags_rules.get(tag.as_inner()) { - if let Some(dependency_permissions) = &rule.dependencies { - for dependency in self.pkg_dep_graph().dependencies(&pkg) { - if matches!(dependency, PackageNode::Root) { - continue; - } - - let dependency_tags = self.load_package_tags(dependency.as_package_name()); - - diagnostics.extend(self.validate_relation( - pkg.as_package_name(), - package_json, - dependency.as_package_name(), - dependency_tags, - dependency_permissions.allow.as_ref(), - dependency_permissions.deny.as_ref(), - )?); - } - } + if let Some(tags_rules) = tags_rules { + // We don't allow tags to share the same name as the package + // because we allow package names to be used as a tag + diagnostics.extend(Self::check_if_package_name_is_tag( + tags_rules, + &pkg, + package_json, + )); - if let Some(dependent_permissions) = &rule.dependents { - for dependent in self.pkg_dep_graph().ancestors(&pkg) { - if matches!(dependent, PackageNode::Root) { - continue; - } - let dependent_tags = self.load_package_tags(dependent.as_package_name()); - diagnostics.extend(self.validate_relation( - pkg.as_package_name(), - package_json, - dependent.as_package_name(), - dependent_tags, - dependent_permissions.allow.as_ref(), - dependent_permissions.deny.as_ref(), - )?) - } + for tag in current_package_tags.into_iter().flatten().flatten() { + if let Some(rule) = tags_rules.get(tag.as_inner()) { + self.check_tag( + &mut diagnostics, + rule.dependencies.as_ref(), + rule.dependents.as_ref(), + &pkg, + package_json, + )?; } } } diff --git a/crates/turborepo-lib/src/query/boundaries.rs b/crates/turborepo-lib/src/query/boundaries.rs index 0642ade33a727..9a7d1ffa30f0f 100644 --- a/crates/turborepo-lib/src/query/boundaries.rs +++ b/crates/turborepo-lib/src/query/boundaries.rs @@ -100,6 +100,14 @@ impl From for Diagnostic { import: None, reason: Some(tag), }, + BoundariesDiagnostic::PackageBoundariesHasTags { span, text: _ } => Diagnostic { + message, + path: None, + start: span.map(|span| span.offset()), + end: span.map(|span| span.offset() + span.len()), + import: None, + reason: None, + }, } } } diff --git a/crates/turborepo-lib/src/turbo_json/mod.rs b/crates/turborepo-lib/src/turbo_json/mod.rs index 17a9ae753f045..7b863f33b8670 100644 --- a/crates/turborepo-lib/src/turbo_json/mod.rs +++ b/crates/turborepo-lib/src/turbo_json/mod.rs @@ -908,6 +908,14 @@ mod tests { }"#, "implicit dependencies and tags" )] + #[test_case( + r#"{ + "dependencies": { + "allow": ["my-package"] + } + }"#, + "package rule" + )] fn test_deserialize_boundaries(json: &str, name: &str) { let deserialized_result = deserialize_from_json_str( json, diff --git a/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__implicit_dependencies_and_tags.snap b/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__implicit_dependencies_and_tags.snap index bc368549fb0b3..63798ee762794 100644 --- a/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__implicit_dependencies_and_tags.snap +++ b/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__implicit_dependencies_and_tags.snap @@ -5,7 +5,6 @@ expression: raw_boundaries_config { "tags": { "my-tag": { - "dependencies": null, "dependents": { "allow": [ "my-package" diff --git a/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__package_rule.snap b/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__package_rule.snap new file mode 100644 index 0000000000000..916a08b05388f --- /dev/null +++ b/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__package_rule.snap @@ -0,0 +1,12 @@ +--- +source: crates/turborepo-lib/src/turbo_json/mod.rs +expression: raw_boundaries_config +--- +{ + "dependencies": { + "allow": [ + "my-package" + ], + "deny": null + } +} diff --git a/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependencies.snap b/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependencies.snap index c40788bc0790d..0bc9b31481351 100644 --- a/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependencies.snap +++ b/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependencies.snap @@ -10,8 +10,7 @@ expression: raw_boundaries_config "my-package" ], "deny": null - }, - "dependents": null + } } } } diff --git a/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependencies_2.snap b/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependencies_2.snap index 65403aefa91ec..774fe8990cb08 100644 --- a/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependencies_2.snap +++ b/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependencies_2.snap @@ -12,8 +12,7 @@ expression: raw_boundaries_config "deny": [ "my-other-package" ] - }, - "dependents": null + } } } } diff --git a/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependents.snap b/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependents.snap index 83ae0c0da9076..3d775946347bd 100644 --- a/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependents.snap +++ b/crates/turborepo-lib/src/turbo_json/snapshots/turborepo_lib__turbo_json__tests__tags_and_dependents.snap @@ -5,7 +5,6 @@ expression: raw_boundaries_config { "tags": { "my-tag": { - "dependencies": null, "dependents": { "allow": [ "my-package" diff --git a/crates/turborepo/tests/snapshots/boundaries__boundaries_tags_get_boundaries_lints_(npm@10.5.0).snap b/crates/turborepo/tests/snapshots/boundaries__boundaries_tags_get_boundaries_lints_(npm@10.5.0).snap index b9302e766ca9f..ce19e9426c67a 100644 --- a/crates/turborepo/tests/snapshots/boundaries__boundaries_tags_get_boundaries_lints_(npm@10.5.0).snap +++ b/crates/turborepo/tests/snapshots/boundaries__boundaries_tags_get_boundaries_lints_(npm@10.5.0).snap @@ -10,6 +10,10 @@ expression: query_output "message": "Package `@vercel/allowed-and-denied-tag` found with tag listed in denylist for `@vercel/my-app`: `unsafe`", "import": "@vercel/allowed-and-denied-tag" }, + { + "message": "Package `@vercel/not-allowed-dependency` found with tag listed in denylist for `@vercel/package-with-boundaries-rules`: `@vercel/not-allowed-dependency`", + "import": "@vercel/not-allowed-dependency" + }, { "message": "Package `@vercel/not-allowed-dependent` found without any tag listed in allowlist for `@vercel/allowed-and-denied-tag`", "import": "@vercel/not-allowed-dependent" diff --git a/turborepo-tests/integration/fixtures/boundaries_tags/package-lock.json b/turborepo-tests/integration/fixtures/boundaries_tags/package-lock.json new file mode 100644 index 0000000000000..fae180d8b22c3 --- /dev/null +++ b/turborepo-tests/integration/fixtures/boundaries_tags/package-lock.json @@ -0,0 +1,77 @@ +{ + "name": "monorepo", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "monorepo", + "workspaces": [ + "apps/*", + "packages/*" + ] + }, + "apps/my-app": { + "name": "@vercel/my-app", + "dependencies": { + "@vercel/allowed-and-denied-tag": "*", + "@vercel/allowed-tag": "*", + "@vercel/package-as-tag": "*" + } + }, + "node_modules/@vercel/allowed-and-denied-tag": { + "resolved": "packages/allowed-and-denied-tag", + "link": true + }, + "node_modules/@vercel/allowed-tag": { + "resolved": "packages/allowed-tag", + "link": true + }, + "node_modules/@vercel/my-app": { + "resolved": "apps/my-app", + "link": true + }, + "node_modules/@vercel/not-allowed-dependency": { + "resolved": "packages/not-allowed-dependency", + "link": true + }, + "node_modules/@vercel/not-allowed-dependent": { + "resolved": "packages/not-allowed-dependent", + "link": true + }, + "node_modules/@vercel/package-as-tag": { + "resolved": "packages/package-as-tag", + "link": true + }, + "node_modules/@vercel/package-with-boundaries-rules": { + "resolved": "packages/package-with-boundaries-rules", + "link": true + }, + "packages/allowed-and-denied-tag": { + "name": "@vercel/allowed-and-denied-tag" + }, + "packages/allowed-tag": { + "name": "@vercel/allowed-tag", + "dependencies": { + "@vercel/package-as-tag": "*" + } + }, + "packages/not-allowed-dependency": { + "name": "@vercel/not-allowed-dependency" + }, + "packages/not-allowed-dependent": { + "name": "@vercel/not-allowed-dependent", + "dependencies": { + "@vercel/my-app": "*" + } + }, + "packages/package-as-tag": { + "name": "@vercel/package-as-tag" + }, + "packages/package-with-boundaries-rules": { + "name": "@vercel/package-with-boundaries-rules", + "dependencies": { + "@vercel/not-allowed-dependency": "*" + } + } + } +} diff --git a/turborepo-tests/integration/fixtures/boundaries_tags/package.json b/turborepo-tests/integration/fixtures/boundaries_tags/package.json index 404d3338e68cf..a4e7dba046e09 100644 --- a/turborepo-tests/integration/fixtures/boundaries_tags/package.json +++ b/turborepo-tests/integration/fixtures/boundaries_tags/package.json @@ -3,9 +3,6 @@ "scripts": { "something": "turbo run build" }, - "dependencies": { - "module-package": "*" - }, "packageManager": "npm@10.5.0", "workspaces": [ "apps/*", diff --git a/turborepo-tests/integration/fixtures/boundaries_tags/packages/allowed-and-denied-tag/package.json b/turborepo-tests/integration/fixtures/boundaries_tags/packages/allowed-and-denied-tag/package.json index 57c9c9dca9557..b4aab2d58d5c6 100644 --- a/turborepo-tests/integration/fixtures/boundaries_tags/packages/allowed-and-denied-tag/package.json +++ b/turborepo-tests/integration/fixtures/boundaries_tags/packages/allowed-and-denied-tag/package.json @@ -2,8 +2,5 @@ "name": "@vercel/allowed-and-denied-tag", "scripts": { "dev": "echo building" - }, - "dependencies": { - "utils": "*" } } diff --git a/turborepo-tests/integration/fixtures/boundaries_tags/packages/allowed-tag/package.json b/turborepo-tests/integration/fixtures/boundaries_tags/packages/allowed-tag/package.json index 8840761734b20..48606941b9a34 100644 --- a/turborepo-tests/integration/fixtures/boundaries_tags/packages/allowed-tag/package.json +++ b/turborepo-tests/integration/fixtures/boundaries_tags/packages/allowed-tag/package.json @@ -2,7 +2,6 @@ "name": "@vercel/allowed-tag", "module": "my-module.mjs", "dependencies": { - "@vercel/no-allowlist": "*", "@vercel/package-as-tag": "*" } } \ No newline at end of file diff --git a/turborepo-tests/integration/fixtures/boundaries_tags/packages/not-allowed-dependency/package.json b/turborepo-tests/integration/fixtures/boundaries_tags/packages/not-allowed-dependency/package.json new file mode 100644 index 0000000000000..1e0e367efe13d --- /dev/null +++ b/turborepo-tests/integration/fixtures/boundaries_tags/packages/not-allowed-dependency/package.json @@ -0,0 +1,3 @@ +{ + "name": "@vercel/not-allowed-dependency" +} \ No newline at end of file diff --git a/turborepo-tests/integration/fixtures/boundaries_tags/packages/package-with-boundaries-rules/package.json b/turborepo-tests/integration/fixtures/boundaries_tags/packages/package-with-boundaries-rules/package.json new file mode 100644 index 0000000000000..14cc41ef716dc --- /dev/null +++ b/turborepo-tests/integration/fixtures/boundaries_tags/packages/package-with-boundaries-rules/package.json @@ -0,0 +1,6 @@ +{ + "name": "@vercel/package-with-boundaries-rules", + "dependencies": { + "@vercel/not-allowed-dependency": "*" + } +} \ No newline at end of file diff --git a/turborepo-tests/integration/fixtures/boundaries_tags/packages/package-with-boundaries-rules/turbo.json b/turborepo-tests/integration/fixtures/boundaries_tags/packages/package-with-boundaries-rules/turbo.json new file mode 100644 index 0000000000000..ee95f9e738866 --- /dev/null +++ b/turborepo-tests/integration/fixtures/boundaries_tags/packages/package-with-boundaries-rules/turbo.json @@ -0,0 +1,9 @@ +{ + "boundaries": { + "dependencies": { + "deny": [ + "@vercel/not-allowed-dependency" + ] + } + } +} \ No newline at end of file