Skip to content
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

feature(turborepo): fancy package.json errors #8299

Merged
merged 11 commits into from
Jul 19, 2024
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
1 change: 0 additions & 1 deletion crates/turborepo-errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ impl<T> Deref for Spanned<T> {
&self.value
}
}

pub trait WithMetadata {
fn add_text(&mut self, text: Arc<str>);
fn add_path(&mut self, path: Arc<str>);
Expand Down
1 change: 1 addition & 0 deletions crates/turborepo-lib/src/cli/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub enum Error {
#[error(transparent)]
ChromeTracing(#[from] crate::tracing::Error),
#[error(transparent)]
#[diagnostic(transparent)]
BuildPackageGraph(#[from] package_graph::builder::Error),
#[error(transparent)]
Rewrite(#[from] RewriteError),
Expand Down
7 changes: 5 additions & 2 deletions crates/turborepo-lib/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,8 +586,11 @@ mod test {
let scripts = if had_build {
BTreeMap::from_iter(
[
("build".to_string(), "echo built!".to_string()),
("dev".to_string(), "echo running dev!".to_string()),
("build".to_string(), Spanned::new("echo built!".to_string())),
(
"dev".to_string(),
Spanned::new("echo running dev!".to_string()),
),
]
.into_iter(),
)
Expand Down
2 changes: 2 additions & 0 deletions crates/turborepo-lib/src/run/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub enum Error {
#[diagnostic(transparent)]
PackageJson(#[from] turborepo_repository::package_json::Error),
#[error(transparent)]
#[diagnostic(transparent)]
PackageManager(#[from] turborepo_repository::package_manager::Error),
#[error(transparent)]
#[diagnostic(transparent)]
Expand All @@ -49,6 +50,7 @@ pub enum Error {
#[error(transparent)]
TaskHash(#[from] task_hash::Error),
#[error(transparent)]
#[diagnostic(transparent)]
Visitor(#[from] task_graph::VisitorError),
#[error("error registering signal handler: {0}")]
SignalHandler(std::io::Error),
Expand Down
1 change: 1 addition & 0 deletions crates/turborepo-lib/src/run/summary/task_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ impl<'a> TaskSummaryFactory<'a> {
.package_json
.scripts
.get(task_id.task())
.map(|script| script.as_inner())
.cloned()
.unwrap_or_else(|| "<NONEXISTENT>".to_string());

Expand Down
15 changes: 13 additions & 2 deletions crates/turborepo-lib/src/task_graph/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use console::{Style, StyledObject};
use either::Either;
use futures::{stream::FuturesUnordered, StreamExt};
use itertools::Itertools;
use miette::{Diagnostic, NamedSource, SourceSpan};
use regex::Regex;
use tokio::sync::{mpsc, oneshot};
use tracing::{debug, error, Instrument, Span};
Expand Down Expand Up @@ -67,7 +68,7 @@ pub struct Visitor<'a> {
is_watch: bool,
}

#[derive(Debug, thiserror::Error)]
#[derive(Debug, thiserror::Error, Diagnostic)]
pub enum Error {
#[error("cannot find package {package_name} for task {task_id}")]
MissingPackage {
Expand All @@ -77,7 +78,14 @@ pub enum Error {
#[error(
"root task {task_name} ({command}) looks like it invokes turbo and might cause a loop"
)]
RecursiveTurbo { task_name: String, command: String },
RecursiveTurbo {
task_name: String,
command: String,
#[label("task found here")]
span: Option<SourceSpan>,
#[source_code]
text: NamedSource,
},
#[error("Could not find definition for task")]
MissingDefinition,
#[error("error while executing engine: {0}")]
Expand Down Expand Up @@ -186,9 +194,12 @@ impl<'a> Visitor<'a> {
match command {
Some(cmd) if info.package() == ROOT_PKG_NAME && turbo_regex().is_match(&cmd) => {
package_task_event.track_error(TrackedErrors::RecursiveError);
let (span, text) = cmd.span_and_text("package.json");
return Err(Error::RecursiveTurbo {
task_name: info.to_string(),
command: cmd.to_string(),
span,
text,
});
}
_ => (),
Expand Down
4 changes: 2 additions & 2 deletions crates/turborepo-lib/src/turbo_json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ mod tests {
#[test_case(
None,
PackageJson {
scripts: [("build".to_string(), "echo build".to_string())].into_iter().collect(),
scripts: [("build".to_string(), Spanned::new("echo build".to_string()))].into_iter().collect(),
..PackageJson::default()
},
TurboJson {
Expand All @@ -818,7 +818,7 @@ mod tests {
}
}"#),
PackageJson {
scripts: [("test".to_string(), "echo test".to_string())].into_iter().collect(),
scripts: [("test".to_string(), Spanned::new("echo test".to_string()))].into_iter().collect(),
..PackageJson::default()
},
TurboJson {
Expand Down
13 changes: 4 additions & 9 deletions crates/turborepo-repository/src/package_graph/builder.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use std::{
backtrace::Backtrace,
collections::{BTreeMap, HashMap, HashSet},
};
use std::collections::{BTreeMap, HashMap, HashSet};

use miette::Diagnostic;
use petgraph::graph::{Graph, NodeIndex};
Expand Down Expand Up @@ -36,11 +33,9 @@ pub struct PackageGraphBuilder<'a, T> {

#[derive(Debug, Diagnostic, thiserror::Error)]
pub enum Error {
#[error("could not resolve workspaces: {0}")]
PackageManager(
#[from] crate::package_manager::Error,
#[backtrace] Backtrace,
),
#[error("could not resolve workspaces")]
#[diagnostic(transparent)]
PackageManager(#[from] crate::package_manager::Error),
#[error(
"Failed to add workspace \"{name}\" from \"{path}\", it already exists at \
\"{existing_path}\""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl<'a> DependencyVersion<'a> {
_ => {
// If we got this far, then we need to check the workspace package version to
// see it satisfies the dependencies range to determin whether
// or not its an internal or external dependency.
// or not it's an internal or external dependency.
let constraint = node_semver::Range::parse(self.version);
let version = node_semver::Version::parse(package_version);

Expand Down
48 changes: 37 additions & 11 deletions crates/turborepo-repository/src/package_json.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::collections::{BTreeMap, HashMap};
use std::{
collections::{BTreeMap, HashMap},
sync::Arc,
};

use anyhow::Result;
use biome_deserialize::{json::deserialize_from_json_str, Text};
Expand All @@ -8,7 +11,7 @@ use biome_json_parser::JsonParserOptions;
use miette::Diagnostic;
use serde::Serialize;
use turbopath::{AbsoluteSystemPath, RelativeUnixPathBuf};
use turborepo_errors::ParseDiagnostic;
use turborepo_errors::{ParseDiagnostic, Spanned, WithMetadata};
use turborepo_unescape::UnescapedString;

#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)]
Expand All @@ -19,7 +22,7 @@ pub struct PackageJson {
#[serde(skip_serializing_if = "Option::is_none")]
pub version: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub package_manager: Option<String>,
pub package_manager: Option<Spanned<String>>,
#[serde(skip_serializing_if = "Option::is_none")]
pub dependencies: Option<BTreeMap<String, String>>,
#[serde(skip_serializing_if = "Option::is_none")]
Expand All @@ -29,7 +32,7 @@ pub struct PackageJson {
#[serde(skip_serializing_if = "Option::is_none")]
pub peer_dependencies: Option<BTreeMap<String, String>>,
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
pub scripts: BTreeMap<String, String>,
pub scripts: BTreeMap<String, Spanned<String>>,
#[serde(skip_serializing_if = "Option::is_none")]
pub resolutions: Option<BTreeMap<String, String>>,
#[serde(skip_serializing_if = "Option::is_none")]
Expand All @@ -53,12 +56,12 @@ pub struct PnpmConfig {
pub struct RawPackageJson {
pub name: Option<UnescapedString>,
pub version: Option<UnescapedString>,
pub package_manager: Option<UnescapedString>,
pub package_manager: Option<Spanned<UnescapedString>>,
pub dependencies: Option<BTreeMap<String, UnescapedString>>,
pub dev_dependencies: Option<BTreeMap<String, UnescapedString>>,
pub optional_dependencies: Option<BTreeMap<String, UnescapedString>>,
pub peer_dependencies: Option<BTreeMap<String, UnescapedString>>,
pub scripts: BTreeMap<String, UnescapedString>,
pub scripts: BTreeMap<String, Spanned<UnescapedString>>,
pub resolutions: Option<BTreeMap<String, UnescapedString>>,
pub pnpm: Option<RawPnpmConfig>,
// Unstructured fields kept for round trip capabilities
Expand All @@ -85,12 +88,32 @@ pub enum Error {
Parse(#[related] Vec<ParseDiagnostic>),
}

impl WithMetadata for RawPackageJson {
fn add_text(&mut self, text: Arc<str>) {
if let Some(ref mut package_manager) = self.package_manager {
package_manager.add_text(text.clone());
}
self.scripts
.iter_mut()
.for_each(|(_, v)| v.add_text(text.clone()));
}

fn add_path(&mut self, path: Arc<str>) {
if let Some(ref mut package_manager) = self.package_manager {
package_manager.add_path(path.clone());
}
self.scripts
.iter_mut()
.for_each(|(_, v)| v.add_path(path.clone()));
}
}

impl From<RawPackageJson> for PackageJson {
fn from(raw: RawPackageJson) -> Self {
Self {
name: raw.name.map(|s| s.into()),
version: raw.version.map(|s| s.into()),
package_manager: raw.package_manager.map(|s| s.into()),
package_manager: raw.package_manager.map(|s| s.map(|s| s.into())),
dependencies: raw
.dependencies
.map(|m| m.into_iter().map(|(k, v)| (k, v.into())).collect()),
Expand All @@ -106,7 +129,7 @@ impl From<RawPackageJson> for PackageJson {
scripts: raw
.scripts
.into_iter()
.map(|(k, v)| (k, v.into()))
.map(|(k, v)| (k, v.map(|v| v.into())))
.collect(),
resolutions: raw
.resolutions
Expand Down Expand Up @@ -158,9 +181,12 @@ impl PackageJson {
}

// We expect a result if there are no errors
Ok(result
.expect("no parse errors produced but no result")
.into())
let mut package_json = result.expect("no parse errors produced but no result");

package_json.add_path(path.into());
package_json.add_text(contents.into());

Ok(package_json.into())
}

// Utility method for easy construction of package.json during testing
Expand Down
Loading
Loading