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

feat(boundaries): auto ignore #10147

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.

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 9 commits into from
Mar 12, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion Cargo.lock

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

1 change: 1 addition & 0 deletions crates/turborepo-lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ human-panic = "1.2.1"
human_format = "1.1.0"
humantime = "2.1.0"
ignore = "0.4.22"
indicatif = { workspace = true }
itertools = { workspace = true }
jsonc-parser = { version = "0.21.0" }
lazy_static = { workspace = true }
Expand Down
4 changes: 4 additions & 0 deletions crates/turborepo-lib/src/boundaries/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ impl Run {
.to_string();

Ok(Some(BoundariesDiagnostic::ImportLeavesPackage {
path: file_path.to_owned(),
import: import.to_string(),
resolved_import_path,
package_name: package_name.to_owned(),
Expand Down Expand Up @@ -289,6 +290,7 @@ impl Run {

if package_name.starts_with("@types/") && matches!(import_type, ImportType::Value) {
return Some(BoundariesDiagnostic::NotTypeOnlyImport {
path: file_path.to_owned(),
import: import.to_string(),
span,
text: NamedSource::new(file_path.as_str(), file_content.to_string()),
Expand All @@ -315,6 +317,7 @@ impl Run {
return match import_type {
ImportType::Type => None,
ImportType::Value => Some(BoundariesDiagnostic::NotTypeOnlyImport {
path: file_path.to_owned(),
import: import.to_string(),
span,
text: NamedSource::new(file_path.as_str(), file_content.to_string()),
Expand All @@ -323,6 +326,7 @@ impl Run {
}

return Some(BoundariesDiagnostic::PackageNotFound {
path: file_path.to_owned(),
name: package_name.to_string(),
span,
text: NamedSource::new(file_path.as_str(), file_content.to_string()),
Expand Down
88 changes: 84 additions & 4 deletions crates/turborepo-lib/src/boundaries/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ mod tags;
mod tsconfig;

use std::{
collections::{HashMap, HashSet},
collections::{BTreeMap, HashMap, HashSet},
fs::OpenOptions,
io::Write,
sync::{Arc, LazyLock, Mutex},
};

pub use config::{BoundariesConfig, Permissions, Rule};
use git2::Repository;
use globwalk::Settings;
use indicatif::{ProgressBar, ProgressIterator};
use miette::{Diagnostic, NamedSource, Report, SourceSpan};
use regex::Regex;
use swc_common::{
Expand All @@ -25,7 +28,7 @@ use swc_ecma_visit::VisitWith;
use thiserror::Error;
use tracing::log::warn;
use turbo_trace::{ImportFinder, Tracer};
use turbopath::AbsoluteSystemPathBuf;
use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf};
use turborepo_errors::Spanned;
use turborepo_repository::package_graph::{PackageInfo, PackageName, PackageNode};
use turborepo_ui::{color, ColorConfig, BOLD_GREEN, BOLD_RED};
Expand Down Expand Up @@ -97,6 +100,7 @@ pub enum BoundariesDiagnostic {
)]
#[help("add `type` to the import declaration")]
NotTypeOnlyImport {
path: AbsoluteSystemPathBuf,
import: String,
#[label("package imported here")]
span: SourceSpan,
Expand All @@ -105,6 +109,7 @@ pub enum BoundariesDiagnostic {
},
#[error("cannot import package `{name}` because it is not a dependency")]
PackageNotFound {
path: AbsoluteSystemPathBuf,
name: String,
#[label("package imported here")]
span: SourceSpan,
Expand All @@ -116,6 +121,7 @@ pub enum BoundariesDiagnostic {
"`{import}` resolves to path `{resolved_import_path}` which is outside of `{package_name}`"
))]
ImportLeavesPackage {
path: AbsoluteSystemPathBuf,
import: String,
resolved_import_path: String,
package_name: PackageName,
Expand All @@ -142,6 +148,19 @@ pub enum Error {
GlobWalk(#[from] globwalk::WalkError),
#[error("failed to read file: {0}")]
FileNotFound(AbsoluteSystemPathBuf),
#[error("failed to write to file: {0}")]
FileWrite(AbsoluteSystemPathBuf),
}

impl BoundariesDiagnostic {
pub fn path_and_span(&self) -> Option<(&AbsoluteSystemPath, SourceSpan)> {
match self {
Self::ImportLeavesPackage { path, span, .. } => Some((path, *span)),
Self::PackageNotFound { path, span, .. } => Some((path, *span)),
Self::NotTypeOnlyImport { path, span, .. } => Some((path, *span)),
_ => None,
}
}
}

static PACKAGE_NAME_REGEX: LazyLock<Regex> = LazyLock::new(|| {
Expand Down Expand Up @@ -211,13 +230,21 @@ impl BoundariesResult {
}

impl Run {
pub async fn check_boundaries(&self) -> Result<BoundariesResult, Error> {
pub async fn check_boundaries(&self, show_progress: bool) -> Result<BoundariesResult, Error> {
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 {

let progress = if show_progress {
println!("Checking packages...");
ProgressBar::new(packages.len() as u64)
} else {
ProgressBar::hidden()
};

for (package_name, package_info) in packages.into_iter().progress_with(progress) {
if !self.filtered_pkgs().contains(package_name)
|| matches!(package_name, PackageName::Root)
{
Expand Down Expand Up @@ -456,4 +483,57 @@ impl Run {

Ok(())
}

pub fn patch_file(
&self,
file_path: &AbsoluteSystemPath,
file_patches: Vec<(SourceSpan, String)>,
) -> Result<(), Error> {
// Deduplicate and sort by offset
let file_patches = file_patches
.into_iter()
.map(|(span, patch)| (span.offset(), patch))
.collect::<BTreeMap<usize, String>>();

let contents = file_path
.read_to_string()
.map_err(|_| Error::FileNotFound(file_path.to_owned()))?;

let mut options = OpenOptions::new();
options.read(true).write(true).truncate(true);
let mut file = file_path
.open_with_options(options)
.map_err(|_| Error::FileNotFound(file_path.to_owned()))?;

let mut last_idx = 0;
for (idx, reason) in file_patches {
let contents_before_span = &contents[last_idx..idx];

// Find the last newline before the span (note this is the index into the slice,
// not the full file)
let newline_idx = contents_before_span.rfind('\n');

// If newline exists, we write all the contents before newline
if let Some(newline_idx) = newline_idx {
file.write_all(contents[last_idx..(last_idx + newline_idx)].as_bytes())
.map_err(|_| Error::FileWrite(file_path.to_owned()))?;
file.write_all(b"\n")
.map_err(|_| Error::FileWrite(file_path.to_owned()))?;
}

file.write_all(b"// @boundaries-ignore ")
.map_err(|_| Error::FileWrite(file_path.to_owned()))?;
file.write_all(reason.as_bytes())
.map_err(|_| Error::FileWrite(file_path.to_owned()))?;
file.write_all(b"\n")
.map_err(|_| Error::FileWrite(file_path.to_owned()))?;

last_idx = idx;
}

file.write_all(contents[last_idx..].as_bytes())
.map_err(|_| Error::FileWrite(file_path.to_owned()))?;

Ok(())
}
}
2 changes: 2 additions & 0 deletions crates/turborepo-lib/src/cli/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ pub enum Error {
Opts(#[from] crate::opts::Error),
#[error(transparent)]
SignalListener(#[from] turborepo_signals::listeners::Error),
#[error(transparent)]
Dialoguer(#[from] dialoguer::Error),
}

const MAX_CHARS_PER_TASK_LINE: usize = 100;
Expand Down
35 changes: 33 additions & 2 deletions crates/turborepo-lib/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,10 @@ pub enum Command {
Boundaries {
#[clap(short = 'F', long, group = "scope-filter-group")]
filter: Vec<String>,
#[clap(long, value_enum, default_missing_value = "prompt", num_args = 0..=1, require_equals = true)]
ignore: Option<BoundariesIgnore>,
#[clap(long, requires = "ignore")]
reason: Option<String>,
},
#[clap(hide = true)]
Clone {
Expand Down Expand Up @@ -761,6 +765,15 @@ pub enum Command {
},
}

#[derive(Copy, Clone, Debug, Default, ValueEnum, Serialize, Eq, PartialEq)]
pub enum BoundariesIgnore {
/// Adds a `@boundaries-ignore` comment everywhere possible
All,
/// Prompts user if they want to add `@boundaries-ignore` comment
#[default]
Prompt,
}

#[derive(Parser, Clone, Debug, Default, Serialize, PartialEq)]
pub struct GenerateWorkspaceArgs {
/// Name for the new workspace
Expand Down Expand Up @@ -1391,13 +1404,15 @@ pub async fn run(

Ok(0)
}
Command::Boundaries { .. } => {
Command::Boundaries { ignore, reason, .. } => {
let event = CommandEventBuilder::new("boundaries").with_parent(&root_telemetry);
let ignore = *ignore;
let reason = reason.clone();

event.track_call();
let base = CommandBase::new(cli_args.clone(), repo_root, version, color_config)?;

Ok(boundaries::run(base, event).await?)
Ok(boundaries::run(base, event, ignore, reason).await?)
}
Command::Clone {
url,
Expand Down Expand Up @@ -3329,4 +3344,20 @@ mod test {
assert_snapshot!(args.join("-").as_str(), err);
}
}

#[test_case::test_case(&["turbo", "boundaries"], true; "empty")]
#[test_case::test_case(&["turbo", "boundaries", "--ignore"], true; "with ignore")]
#[test_case::test_case(&["turbo", "boundaries", "--ignore=all"], true; "with ignore all")]
#[test_case::test_case(&["turbo", "boundaries", "--ignore=prompt"], true; "with ignore prompt")]
#[test_case::test_case(&["turbo", "boundaries", "--filter", "ui"], true; "with filter")]
fn test_boundaries(args: &[&str], is_okay: bool) {
let os_args = args.iter().map(|s| OsString::from(*s)).collect();
let cli = Args::parse(os_args);
if is_okay {
cli.unwrap();
} else {
let err = cli.unwrap_err();
assert_snapshot!(args.join("-").as_str(), err);
}
}
}
78 changes: 74 additions & 4 deletions crates/turborepo-lib/src/commands/boundaries.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
use std::collections::HashMap;

use dialoguer::{Confirm, Input};
use miette::{Report, SourceSpan};
use turbopath::AbsoluteSystemPath;
use turborepo_signals::{listeners::get_signal, SignalHandler};
use turborepo_telemetry::events::command::CommandEventBuilder;
use turborepo_ui::{color, BOLD_GREEN};

use crate::{cli, commands::CommandBase, run::builder::RunBuilder};
use crate::{cli, cli::BoundariesIgnore, commands::CommandBase, run::builder::RunBuilder};

pub async fn run(base: CommandBase, telemetry: CommandEventBuilder) -> Result<i32, cli::Error> {
pub async fn run(
base: CommandBase,
telemetry: CommandEventBuilder,
ignore: Option<BoundariesIgnore>,
reason: Option<String>,
) -> Result<i32, cli::Error> {
let signal = get_signal()?;
let handler = SignalHandler::new(signal);

Expand All @@ -12,9 +23,68 @@ pub async fn run(base: CommandBase, telemetry: CommandEventBuilder) -> Result<i3
.build(&handler, telemetry)
.await?;

let result = run.check_boundaries().await?;
let result = run.check_boundaries(true).await?;

if let Some(ignore) = ignore {
let mut patches: HashMap<&AbsoluteSystemPath, Vec<(SourceSpan, String)>> = HashMap::new();
for diagnostic in &result.diagnostics {
let Some((path, span)) = diagnostic.path_and_span() else {
continue;
};

let reason = match ignore {
BoundariesIgnore::All => Some(reason.clone().unwrap_or_else(|| {
"automatically added by `turbo boundaries --ignore=all`".to_string()
})),
BoundariesIgnore::Prompt => {
print!("{esc}c", esc = 27 as char);
println!();
println!();
println!("{:?}", Report::new(diagnostic.clone()));
let prompt = format!(
"Ignore this error by adding a {} comment?",
color!(run.color_config(), BOLD_GREEN, "@boundaries-ignore"),
);
if Confirm::new()
.with_prompt(prompt)
.default(false)
.interact()?
{
if let Some(reason) = reason.clone() {
Some(reason)
} else {
Some(
Input::new()
.with_prompt("Reason for ignoring this error")
.interact_text()?,
)
}
} else {
None
}
}
};

result.emit(run.color_config());
if let Some(reason) = reason {
patches.entry(path).or_default().push((span, reason));
}
}

for (path, file_patches) in patches {
Copy link
Member

Choose a reason for hiding this comment

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

Unstable iteration order is causing issues for snapshots

let short_path = match run.repo_root().anchor(path) {
Ok(path) => path.to_string(),
Err(_) => path.to_string(),
};
println!(
"{} {}",
color!(run.color_config(), BOLD_GREEN, "patching"),
short_path
);
run.patch_file(path, file_patches)?;
}
} else {
result.emit(run.color_config());
}

if result.is_ok() {
Ok(0)
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ impl Opts {

(&Box::new(execution_args), &Box::default())
}
Some(Command::Boundaries { filter }) => {
Some(Command::Boundaries { filter, .. }) => {
let execution_args = ExecutionArgs {
filter: filter.clone(),
..Default::default()
Expand Down
Loading
Loading