Skip to content

Commit 71b078b

Browse files
feat(boundaries): package name as tag punning (#10151)
### Description Allows a package name to be used as a tag ([tag punning](https://en.wikipedia.org/wiki/Type_punning)). We're adding this feature to help users define an explicit set of packages that their package can be imported by. Combined with the ability to define package rules in a package `turbo.json` (next PR after this, coming soon), and code owner restrictions, this will permit package owners to control who can import their package. ### Testing Instructions Added test in `boundaries_tags` fixture
1 parent 63ce544 commit 71b078b

File tree

19 files changed

+170
-53
lines changed

19 files changed

+170
-53
lines changed

crates/turborepo-errors/src/lib.rs

+6
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ impl<T: Deserializable> Deserializable for Spanned<T> {
113113
}
114114
}
115115

116+
impl Spanned<String> {
117+
pub fn as_str(&self) -> &str {
118+
self.value.as_str()
119+
}
120+
}
121+
116122
impl<T> Spanned<T> {
117123
pub fn new(t: T) -> Self {
118124
Self {

crates/turborepo-lib/src/boundaries/mod.rs

+20
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ use crate::{
4141

4242
#[derive(Clone, Debug, Error, Diagnostic)]
4343
pub enum SecondaryDiagnostic {
44+
#[error("package `{package} is defined here")]
45+
PackageDefinedHere {
46+
package: String,
47+
#[label]
48+
package_span: Option<SourceSpan>,
49+
#[source_code]
50+
package_text: NamedSource<String>,
51+
},
4452
#[error("consider adding one of the following tags listed here")]
4553
Allowlist {
4654
#[label]
@@ -59,6 +67,17 @@ pub enum SecondaryDiagnostic {
5967

6068
#[derive(Clone, Debug, Error, Diagnostic)]
6169
pub enum BoundariesDiagnostic {
70+
#[error("Tag `{tag}` cannot share the same name as package `{package}`")]
71+
TagSharesPackageName {
72+
tag: String,
73+
package: String,
74+
#[label("tag defined here")]
75+
tag_span: Option<SourceSpan>,
76+
#[source_code]
77+
tag_text: NamedSource<String>,
78+
#[related]
79+
secondary: [SecondaryDiagnostic; 1],
80+
},
6281
#[error("Path `{path}` is not valid UTF-8. Turborepo only supports UTF-8 paths.")]
6382
InvalidPath { path: String },
6483
#[error(
@@ -320,6 +339,7 @@ impl Run {
320339
if let Some(tag_rules) = tag_rules {
321340
result.diagnostics.extend(self.check_package_tags(
322341
PackageNode::Workspace(package_name.clone()),
342+
&package_info.package_json,
323343
tags,
324344
tag_rules,
325345
)?);

crates/turborepo-lib/src/boundaries/tags.rs

+68-32
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,30 @@
11
use std::collections::{HashMap, HashSet};
22

3-
use tracing::warn;
3+
use miette::NamedSource;
44
use turborepo_errors::Spanned;
5-
use turborepo_repository::package_graph::{PackageName, PackageNode};
5+
use turborepo_repository::{
6+
package_graph::{PackageName, PackageNode},
7+
package_json::PackageJson,
8+
};
69

710
use crate::{
811
boundaries::{config::Rule, BoundariesDiagnostic, Error, Permissions, SecondaryDiagnostic},
912
run::Run,
10-
turbo_json::TurboJson,
1113
};
1214

1315
pub type ProcessedRulesMap = HashMap<String, ProcessedRule>;
1416

1517
pub struct ProcessedRule {
18+
span: Spanned<()>,
1619
dependencies: Option<ProcessedPermissions>,
1720
dependents: Option<ProcessedPermissions>,
1821
}
1922

20-
impl From<Rule> for ProcessedRule {
21-
fn from(rule: Rule) -> Self {
23+
impl From<Spanned<Rule>> for ProcessedRule {
24+
fn from(rule: Spanned<Rule>) -> Self {
25+
let (rule, span) = rule.split();
2226
Self {
27+
span,
2328
dependencies: rule
2429
.dependencies
2530
.map(|dependencies| dependencies.into_inner().into()),
@@ -56,30 +61,6 @@ impl Run {
5661
.and_then(|turbo_json| turbo_json.tags.as_ref())
5762
}
5863

59-
pub(crate) fn get_package_tags(&self) -> HashMap<PackageName, Spanned<Vec<Spanned<String>>>> {
60-
let mut package_tags = HashMap::new();
61-
for (package, _) in self.pkg_dep_graph().packages() {
62-
if let Ok(TurboJson {
63-
tags: Some(tags),
64-
boundaries,
65-
..
66-
}) = self.turbo_json_loader().load(package)
67-
{
68-
if boundaries.as_ref().is_some_and(|b| b.tags.is_some())
69-
&& !matches!(package, PackageName::Root)
70-
{
71-
warn!(
72-
"Boundaries rules can only be defined in the root turbo.json. Any rules \
73-
defined in a package's turbo.json will be ignored."
74-
)
75-
}
76-
package_tags.insert(package.clone(), tags.clone());
77-
}
78-
}
79-
80-
package_tags
81-
}
82-
8364
pub(crate) fn get_processed_rules_map(&self) -> Option<ProcessedRulesMap> {
8465
self.root_turbo_json()
8566
.boundaries
@@ -88,7 +69,7 @@ impl Run {
8869
.map(|tags| {
8970
tags.as_inner()
9071
.iter()
91-
.map(|(k, v)| (k.clone(), v.as_inner().clone().into()))
72+
.map(|(k, v)| (k.clone(), v.clone().into()))
9273
.collect()
9374
})
9475
}
@@ -99,14 +80,43 @@ impl Run {
9980
fn validate_relation(
10081
&self,
10182
package_name: &PackageName,
83+
package_json: &PackageJson,
10284
relation_package_name: &PackageName,
10385
tags: Option<&Spanned<Vec<Spanned<String>>>>,
10486
allow_list: Option<&Spanned<HashSet<String>>>,
10587
deny_list: Option<&Spanned<HashSet<String>>>,
10688
) -> Result<Option<BoundariesDiagnostic>, Error> {
107-
// If there is no allow list, then we vacuously have a tag in the allow list
108-
let mut has_tag_in_allowlist = allow_list.is_none();
89+
// We allow "punning" the package name as a tag, so if the allow list contains
90+
// the package name, then we have a tag in the allow list
91+
// Likewise, if the allow list is empty, then we vacuously have a tag in the
92+
// allow list
93+
let mut has_tag_in_allowlist =
94+
allow_list.is_none_or(|allow_list| allow_list.contains(relation_package_name.as_str()));
10995
let tags_span = tags.map(|tags| tags.to(())).unwrap_or_default();
96+
if let Some(deny_list) = deny_list {
97+
if deny_list.contains(relation_package_name.as_str()) {
98+
let (span, text) = package_json
99+
.name
100+
.as_ref()
101+
.map(|name| name.span_and_text("turbo.json"))
102+
.unwrap_or_else(|| (None, NamedSource::new("package.json", String::new())));
103+
let deny_list_spanned = deny_list.to(());
104+
let (deny_list_span, deny_list_text) =
105+
deny_list_spanned.span_and_text("turbo.json");
106+
107+
return Ok(Some(BoundariesDiagnostic::DeniedTag {
108+
source_package_name: package_name.clone(),
109+
package_name: relation_package_name.clone(),
110+
tag: relation_package_name.to_string(),
111+
span,
112+
text,
113+
secondary: [SecondaryDiagnostic::Denylist {
114+
span: deny_list_span,
115+
text: deny_list_text,
116+
}],
117+
}));
118+
}
119+
}
110120

111121
for tag in tags.into_iter().flatten().flatten() {
112122
if let Some(allow_list) = allow_list {
@@ -170,10 +180,34 @@ impl Run {
170180
pub(crate) fn check_package_tags(
171181
&self,
172182
pkg: PackageNode,
183+
package_json: &PackageJson,
173184
current_package_tags: &Spanned<Vec<Spanned<String>>>,
174185
tags_rules: &ProcessedRulesMap,
175186
) -> Result<Vec<BoundariesDiagnostic>, Error> {
176187
let mut diagnostics = Vec::new();
188+
189+
// We don't allow tags to share the same name as the package because
190+
// we allow package names to be used as a tag
191+
if let Some(rule) = tags_rules.get(pkg.as_package_name().as_str()) {
192+
let (tag_span, tag_text) = rule.span.span_and_text("turbo.json");
193+
let (package_span, package_text) = package_json
194+
.name
195+
.as_ref()
196+
.map(|name| name.span_and_text("package.json"))
197+
.unwrap_or_else(|| (None, NamedSource::new("package.json", "".into())));
198+
diagnostics.push(BoundariesDiagnostic::TagSharesPackageName {
199+
tag: pkg.as_package_name().to_string(),
200+
package: pkg.as_package_name().to_string(),
201+
tag_span,
202+
tag_text,
203+
secondary: [SecondaryDiagnostic::PackageDefinedHere {
204+
package: pkg.as_package_name().to_string(),
205+
package_span,
206+
package_text,
207+
}],
208+
});
209+
}
210+
177211
for tag in current_package_tags.iter() {
178212
if let Some(rule) = tags_rules.get(tag.as_inner()) {
179213
if let Some(dependency_permissions) = &rule.dependencies {
@@ -186,6 +220,7 @@ impl Run {
186220

187221
diagnostics.extend(self.validate_relation(
188222
pkg.as_package_name(),
223+
package_json,
189224
dependency.as_package_name(),
190225
dependency_tags,
191226
dependency_permissions.allow.as_ref(),
@@ -202,6 +237,7 @@ impl Run {
202237
let dependent_tags = self.load_package_tags(dependent.as_package_name());
203238
diagnostics.extend(self.validate_relation(
204239
pkg.as_package_name(),
240+
package_json,
205241
dependent.as_package_name(),
206242
dependent_tags,
207243
dependent_permissions.allow.as_ref(),

crates/turborepo-lib/src/commands/prune.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,11 @@ impl<'a> Prune<'a> {
307307
};
308308
trace!(
309309
"target: {}",
310-
info.package_json.name.as_deref().unwrap_or_default()
310+
info.package_json
311+
.name
312+
.as_ref()
313+
.map(|name| name.as_str())
314+
.unwrap_or_default()
311315
);
312316
trace!("workspace package.json: {}", &info.package_json_path);
313317
trace!(

crates/turborepo-lib/src/engine/builder.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ mod test {
760760
$(
761761
let path = $root.join_components(&["packages", $name, "package.json"]);
762762
let dependencies = Some($deps.iter().map(|dep: &&str| (dep.to_string(), "workspace:*".to_string())).collect());
763-
let package_json = PackageJson { name: Some($name.to_string()), dependencies, ..Default::default() };
763+
let package_json = PackageJson { name: Some(Spanned::new($name.to_string())), dependencies, ..Default::default() };
764764
_map.insert(path, package_json);
765765
)+
766766
_map

crates/turborepo-lib/src/engine/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ mod test {
638638
};
639639

640640
let package = PackageJson {
641-
name: Some(name.to_string()),
641+
name: Some(Spanned::new(name.to_string())),
642642
scripts,
643643
..Default::default()
644644
};

crates/turborepo-lib/src/query/boundaries.rs

+8
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,14 @@ impl From<BoundariesDiagnostic> for Diagnostic {
9292
import: None,
9393
reason: None,
9494
},
95+
BoundariesDiagnostic::TagSharesPackageName { tag, tag_span, .. } => Diagnostic {
96+
message,
97+
path: None,
98+
start: tag_span.map(|span| span.offset()),
99+
end: tag_span.map(|span| span.offset() + span.len()),
100+
import: None,
101+
reason: Some(tag),
102+
},
95103
}
96104
}
97105
}

crates/turborepo-lib/src/run/scope/filter.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,7 @@ mod test {
713713
use tempfile::TempDir;
714714
use test_case::test_case;
715715
use turbopath::{AbsoluteSystemPathBuf, AnchoredSystemPathBuf, RelativeUnixPathBuf};
716+
use turborepo_errors::Spanned;
716717
use turborepo_repository::{
717718
change_mapper::PackageInclusionReason,
718719
discovery::PackageDiscovery,
@@ -806,7 +807,7 @@ mod test {
806807
RelativeUnixPathBuf::new(format!("{package_path}/package.json")).unwrap(),
807808
),
808809
PackageJson {
809-
name: Some(name.to_string()),
810+
name: Some(Spanned::new(name.to_string())),
810811
dependencies: dependencies.get(name).map(|v| {
811812
v.iter()
812813
.map(|name| (name.to_string(), "*".to_string()))

crates/turborepo-lsp/src/lib.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ impl LanguageServer for Backend {
325325
let package_json_name = if repo_root.contains(&wd.package_json) {
326326
Some("//")
327327
} else {
328-
package_json.name.as_deref()
328+
package_json.name.as_ref().map(|name| name.as_str())
329329
};
330330

331331
// todo: use jsonc_ast instead of text search
@@ -613,7 +613,7 @@ impl LanguageServer for Backend {
613613
.iter()
614614
.flat_map(|p| p.scripts.keys().map(move |k| (p.name.clone(), k)))
615615
.map(|(package, s)| CompletionItem {
616-
label: format!("{}#{}", package.unwrap_or_default(), s),
616+
label: format!("{}#{}", package.unwrap_or_default().into_inner(), s),
617617
kind: Some(CompletionItemKind::FIELD),
618618
..Default::default()
619619
});
@@ -708,7 +708,7 @@ impl Backend {
708708
{
709709
Some("//".to_string())
710710
} else {
711-
package_json.name
711+
package_json.name.map(|name| name.into_inner())
712712
};
713713
Some(
714714
package_json

crates/turborepo-repository/src/package_graph/builder.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,8 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedPackageManager, T> {
237237
let name = PackageName::Other(
238238
json.name
239239
.clone()
240-
.ok_or(Error::PackageJsonMissingName(package_json_path))?,
240+
.ok_or(Error::PackageJsonMissingName(package_json_path))?
241+
.into_inner(),
241242
);
242243
let entry = PackageInfo {
243244
package_json: json,
@@ -590,6 +591,8 @@ impl PackageInfo {
590591
mod test {
591592
use std::assert_matches::assert_matches;
592593

594+
use turborepo_errors::Spanned;
595+
593596
use super::*;
594597

595598
struct MockDiscovery;
@@ -617,7 +620,7 @@ mod test {
617620
let builder = PackageGraphBuilder::new(
618621
&root,
619622
PackageJson {
620-
name: Some("root".into()),
623+
name: Some(Spanned::new("root".into())),
621624
..Default::default()
622625
},
623626
)
@@ -627,14 +630,14 @@ mod test {
627630
map.insert(
628631
root.join_component("a"),
629632
PackageJson {
630-
name: Some("foo".into()),
633+
name: Some(Spanned::new("foo".into())),
631634
..Default::default()
632635
},
633636
);
634637
map.insert(
635638
root.join_component("b"),
636639
PackageJson {
637-
name: Some("foo".into()),
640+
name: Some(Spanned::new("foo".into())),
638641
..Default::default()
639642
},
640643
);

crates/turborepo-repository/src/package_graph/mod.rs

+7-3
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,10 @@ pub struct PackageInfo {
7070

7171
impl PackageInfo {
7272
pub fn package_name(&self) -> Option<String> {
73-
self.package_json.name.clone()
73+
self.package_json
74+
.name
75+
.as_ref()
76+
.map(|name| name.as_inner().clone())
7477
}
7578

7679
pub fn package_json_path(&self) -> &AnchoredSystemPath {
@@ -156,7 +159,7 @@ impl PackageGraph {
156159
if matches!(package_name, PackageName::Root) {
157160
continue;
158161
}
159-
let name = info.package_json.name.as_deref();
162+
let name = info.package_json.name.as_ref().map(|name| name.as_str());
160163
match name {
161164
Some("") | None => {
162165
let package_json_path = self.repo_root.resolve(info.package_json_path());
@@ -620,6 +623,7 @@ mod test {
620623
use std::assert_matches::assert_matches;
621624

622625
use serde_json::json;
626+
use turborepo_errors::Spanned;
623627

624628
use super::*;
625629
use crate::discovery::PackageDiscovery;
@@ -649,7 +653,7 @@ mod test {
649653
let pkg_graph = PackageGraph::builder(
650654
&root,
651655
PackageJson {
652-
name: Some("my-package".to_owned()),
656+
name: Some(Spanned::new("my-package".to_owned())),
653657
..Default::default()
654658
},
655659
)

0 commit comments

Comments
 (0)