diff --git a/Cargo.lock b/Cargo.lock index d06ce10e339b2..d425bff560d61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "Inflector" @@ -6543,6 +6543,7 @@ dependencies = [ "human_format", "humantime", "ignore", + "indicatif", "insta", "itertools 0.10.5", "jsonc-parser 0.21.0", diff --git a/crates/turborepo-lib/Cargo.toml b/crates/turborepo-lib/Cargo.toml index a600ea843317f..410a2b0457d62 100644 --- a/crates/turborepo-lib/Cargo.toml +++ b/crates/turborepo-lib/Cargo.toml @@ -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 } diff --git a/crates/turborepo-lib/src/boundaries/imports.rs b/crates/turborepo-lib/src/boundaries/imports.rs index d1d629158f1e2..439f0e77cc09b 100644 --- a/crates/turborepo-lib/src/boundaries/imports.rs +++ b/crates/turborepo-lib/src/boundaries/imports.rs @@ -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(), @@ -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()), @@ -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()), @@ -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()), diff --git a/crates/turborepo-lib/src/boundaries/mod.rs b/crates/turborepo-lib/src/boundaries/mod.rs index e07810aa449a1..25ff4f7278e68 100644 --- a/crates/turborepo-lib/src/boundaries/mod.rs +++ b/crates/turborepo-lib/src/boundaries/mod.rs @@ -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::{ @@ -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}; @@ -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, @@ -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, @@ -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, @@ -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 = LazyLock::new(|| { @@ -211,13 +230,21 @@ impl BoundariesResult { } impl Run { - pub async fn check_boundaries(&self) -> Result { + pub async fn check_boundaries(&self, show_progress: bool) -> Result { 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) { @@ -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::>(); + + 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(()) + } } diff --git a/crates/turborepo-lib/src/cli/error.rs b/crates/turborepo-lib/src/cli/error.rs index a60e8c9496363..56bf15f13d981 100644 --- a/crates/turborepo-lib/src/cli/error.rs +++ b/crates/turborepo-lib/src/cli/error.rs @@ -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; diff --git a/crates/turborepo-lib/src/cli/mod.rs b/crates/turborepo-lib/src/cli/mod.rs index 695ab92e6e8ed..d38ca2d57873e 100644 --- a/crates/turborepo-lib/src/cli/mod.rs +++ b/crates/turborepo-lib/src/cli/mod.rs @@ -587,6 +587,10 @@ pub enum Command { Boundaries { #[clap(short = 'F', long, group = "scope-filter-group")] filter: Vec, + #[clap(long, value_enum, default_missing_value = "prompt", num_args = 0..=1, require_equals = true)] + ignore: Option, + #[clap(long, requires = "ignore")] + reason: Option, }, #[clap(hide = true)] Clone { @@ -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 @@ -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, @@ -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); + } + } } diff --git a/crates/turborepo-lib/src/commands/boundaries.rs b/crates/turborepo-lib/src/commands/boundaries.rs index bd3f67b43a6b6..ea60f230f0cae 100644 --- a/crates/turborepo-lib/src/commands/boundaries.rs +++ b/crates/turborepo-lib/src/commands/boundaries.rs @@ -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 { +pub async fn run( + base: CommandBase, + telemetry: CommandEventBuilder, + ignore: Option, + reason: Option, +) -> Result { let signal = get_signal()?; let handler = SignalHandler::new(signal); @@ -12,9 +23,68 @@ pub async fn run(base: CommandBase, telemetry: CommandEventBuilder) -> Result> = 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 { + 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) diff --git a/crates/turborepo-lib/src/opts.rs b/crates/turborepo-lib/src/opts.rs index 28af416109c93..11c8a25c79556 100644 --- a/crates/turborepo-lib/src/opts.rs +++ b/crates/turborepo-lib/src/opts.rs @@ -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() diff --git a/crates/turborepo-lib/src/query/boundaries.rs b/crates/turborepo-lib/src/query/boundaries.rs index 77c4c57ee73ef..df5100f82f523 100644 --- a/crates/turborepo-lib/src/query/boundaries.rs +++ b/crates/turborepo-lib/src/query/boundaries.rs @@ -5,27 +5,41 @@ impl From for Diagnostic { fn from(diagnostic: BoundariesDiagnostic) -> Self { let message = diagnostic.to_string(); match diagnostic { - BoundariesDiagnostic::NotTypeOnlyImport { import, span, text } => Diagnostic { + BoundariesDiagnostic::NotTypeOnlyImport { + import, + span, + text: _, + path, + } => Diagnostic { message, - path: Some(text.name().to_string()), + path: Some(path.to_string()), start: Some(span.offset()), end: Some(span.offset() + span.len()), import: Some(import), reason: None, }, - BoundariesDiagnostic::PackageNotFound { name, span, text } => Diagnostic { + BoundariesDiagnostic::PackageNotFound { + name, + span, + text: _, + path, + } => Diagnostic { message, - path: Some(text.name().to_string()), + path: Some(path.to_string()), start: Some(span.offset()), end: Some(span.offset() + span.len()), import: Some(name.to_string()), reason: None, }, BoundariesDiagnostic::ImportLeavesPackage { - import, span, text, .. + import, + span, + text: _, + path, + .. } => Diagnostic { message, - path: Some(text.name().to_string()), + path: Some(path.to_string()), start: Some(span.offset()), end: Some(span.offset() + span.len()), import: Some(import), diff --git a/crates/turborepo-lib/src/query/mod.rs b/crates/turborepo-lib/src/query/mod.rs index a8c2da0abe846..d24e29a359990 100644 --- a/crates/turborepo-lib/src/query/mod.rs +++ b/crates/turborepo-lib/src/query/mod.rs @@ -571,7 +571,7 @@ impl RepositoryQuery { /// Check boundaries for all packages. async fn boundaries(&self) -> Result, Error> { - match self.run.check_boundaries().await { + match self.run.check_boundaries(false).await { Ok(result) => Ok(result .diagnostics .into_iter() diff --git a/turborepo-tests/integration/tests/command-boundaries.t b/turborepo-tests/integration/tests/command-boundaries.t new file mode 100644 index 0000000000000..a93dcbc13c810 --- /dev/null +++ b/turborepo-tests/integration/tests/command-boundaries.t @@ -0,0 +1,41 @@ +Setup + $ . ${TESTDIR}/../../helpers/setup_integration_test.sh boundaries + +Ignore all errors + $ ${TURBO} boundaries --ignore=all + Checking packages... + patching apps(\\|/)my-app(\\|/)(index|types).ts (re) + patching apps(\\|/)my-app(\\|/)(index|types).ts (re) + [1] + + $ git diff + diff --git a/apps/my-app/index.ts b/apps/my-app/index.ts + index 6baec29..d4c7af6 100644 + --- a/apps/my-app/index.ts + +++ b/apps/my-app/index.ts + @@ -1,9 +1,13 @@ + // Import directly from a file instead of via package name + +// @boundaries-ignore automatically added by `turbo boundaries --ignore=all` + import { blackbeard } from "../../packages/another/index.jsx"; + // Import type without "type" specifier in import + +// @boundaries-ignore automatically added by `turbo boundaries --ignore=all` + import { Ship } from "ship"; + +// @boundaries-ignore automatically added by `turbo boundaries --ignore=all` + import { Ship } from "@types/ship"; + // Import package that is not specified + +// @boundaries-ignore automatically added by `turbo boundaries --ignore=all` + import { walkThePlank } from "module-package"; + + // Import from a package that is not specified, but we have `@boundaries-ignore` on it + diff --git a/apps/my-app/types.ts b/apps/my-app/types.ts + index ce28692..3615d9c 100644 + --- a/apps/my-app/types.ts + +++ b/apps/my-app/types.ts + @@ -1,4 +1,6 @@ + +// @boundaries-ignore automatically added by `turbo boundaries --ignore=all` + import { blackbeard } from "@/../../packages/another/index.jsx"; + +// @boundaries-ignore automatically added by `turbo boundaries --ignore=all` + import { blackbead } from "!"; + + export interface Pirate { +