From da2e791274cf8131dc617f5766fa83ae5d6394cf Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Wed, 9 Oct 2024 21:57:12 -0700 Subject: [PATCH 1/4] Add ariadne --- Cargo.lock | 11 +++++++++++ Cargo.toml | 1 + 2 files changed, 12 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 38bbaec345..007f9bc974 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -84,6 +84,16 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "ariadne" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44055e597c674aef7cb903b2b9f6e4cba1277ed0d2d61dae7cd52d7ffa81f8e2" +dependencies = [ + "unicode-width 0.1.14", + "yansi", +] + [[package]] name = "arrayref" version = "0.3.9" @@ -524,6 +534,7 @@ name = "just" version = "1.37.0" dependencies = [ "ansi_term", + "ariadne", "blake3", "camino", "chrono", diff --git a/Cargo.toml b/Cargo.toml index f10743b9af..941ceaca4f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ members = [".", "crates/*"] [dependencies] ansi_term = "0.12.0" +ariadne = "0.4.1" blake3 = { version = "1.5.0", features = ["rayon", "mmap"] } camino = "1.0.4" chrono = "0.4.38" From 2411bc90ab10b396631f28fe4f109f10b68989ac Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Wed, 9 Oct 2024 23:13:09 -0700 Subject: [PATCH 2/4] Use ariadne https://github.com/casey/just/issues/1323 --- justfile | 3 +- src/compile_error.rs | 108 +++++++++++++++++++++++++++++++++++++++++++ src/error.rs | 7 +++ src/run.rs | 2 +- 4 files changed, 118 insertions(+), 2 deletions(-) diff --git a/justfile b/justfile index e975d506fb..8e94d44432 100755 --- a/justfile +++ b/justfile @@ -12,7 +12,7 @@ export JUST_LOG := log watch +args='test': cargo watch --clear --exec '{{ args }}' -[group: 'test'] +[group('test')] test: cargo test --all @@ -34,6 +34,7 @@ run: filter PATTERN: cargo test {{PATTERN}} +[group: 'misc'] [group: 'misc'] build: cargo build diff --git a/src/compile_error.rs b/src/compile_error.rs index 7fa2e0a3cb..6847900f06 100644 --- a/src/compile_error.rs +++ b/src/compile_error.rs @@ -19,6 +19,114 @@ impl<'src> CompileError<'src> { } } +pub(crate) fn render_compile_error(error: &CompileError) { + use ariadne::{Label, Report, ReportKind, Source}; + + let token = error.token; + let source = Source::from(token.src); + + let start = token.offset; + let end = token.offset + token.length; + + let path = format!("{}", token.path.display()); + let label = Label::new((&path, start..end)); + + let report = Report::build(ReportKind::Error, &path, start); + + let report = match &*error.kind { + CompileErrorKind::AttributeArgumentCountMismatch { + attribute, + found, + min, + max, + } => { + let label_msg = format!("Found {found} {}", Count("argument", *found)); + + let note = if min == max { + format!("`{attribute}` takes {min} {}", Count("argument", *min)) + } else { + format!("`{attribute}` takes between {min} and {max} arguments") + }; + + report + .with_code("E01") + .with_message("Attribute argument count mismatch") + .with_label(label.with_message(label_msg)) + .with_note(note) + .finish() + } + /* + CompileErrorKind::BacktickShebang => todo!(), + CompileErrorKind::CircularRecipeDependency { recipe, circle } => todo!(), + CompileErrorKind::CircularVariableDependency { variable, circle } => todo!(), + CompileErrorKind::DependencyArgumentCountMismatch { dependency, found, min, max } => todo!(), + CompileErrorKind::Redefinition { first, first_type, name, second_type } => todo!(), + */ + CompileErrorKind::DuplicateAttribute { attribute, first } => { + let original_label = source + .line(*first) + .map(|line| Label::new((&path, line.span())).with_message("original")); + + let mut report = report + .with_code("E02") + .with_message(format!("Duplicate attribute `{attribute}`")); + if let Some(original) = original_label { + report = report.with_label(original); + } + report.with_label(label.with_message("duplicate")).finish() + } + _ => { + let message = format!("{error}"); + report.with_message(message).with_label(label).finish() + } /* + CompileErrorKind::DuplicateParameter { recipe, parameter } => todo!(), + CompileErrorKind::DuplicateSet { setting, first } => todo!(), + CompileErrorKind::DuplicateVariable { variable } => todo!(), + CompileErrorKind::DuplicateUnexport { variable } => todo!(), + CompileErrorKind::ExpectedKeyword { expected, found } => todo!(), + CompileErrorKind::ExportUnexported { variable } => todo!(), + CompileErrorKind::ExtraLeadingWhitespace => todo!(), + CompileErrorKind::ExtraneousAttributes { count } => todo!(), + CompileErrorKind::FunctionArgumentCountMismatch { function, found, expected } => todo!(), + CompileErrorKind::Include => todo!(), + CompileErrorKind::InconsistentLeadingWhitespace { expected, found } => todo!(), + CompileErrorKind::Internal { message } => todo!(), + CompileErrorKind::InvalidAttribute { item_kind, item_name, attribute } => todo!(), + CompileErrorKind::InvalidEscapeSequence { character } => todo!(), + CompileErrorKind::MismatchedClosingDelimiter { close, open, open_line } => todo!(), + CompileErrorKind::MixedLeadingWhitespace { whitespace } => todo!(), + CompileErrorKind::ParameterFollowsVariadicParameter { parameter } => todo!(), + CompileErrorKind::ParsingRecursionDepthExceeded => todo!(), + CompileErrorKind::RequiredParameterFollowsDefaultParameter { parameter } => todo!(), + CompileErrorKind::ShebangAndScriptAttribute { recipe } => todo!(), + CompileErrorKind::ShellExpansion { err } => todo!(), + CompileErrorKind::UndefinedVariable { variable } => todo!(), + CompileErrorKind::UnexpectedCharacter { expected } => todo!(), + CompileErrorKind::UnexpectedClosingDelimiter { close } => todo!(), + CompileErrorKind::UnexpectedEndOfToken { expected } => todo!(), + CompileErrorKind::UnexpectedToken { expected, found } => todo!(), + CompileErrorKind::UnicodeEscapeCharacter { character } => todo!(), + CompileErrorKind::UnicodeEscapeDelimiter { character } => todo!(), + CompileErrorKind::UnicodeEscapeEmpty => todo!(), + CompileErrorKind::UnicodeEscapeLength { hex } => todo!(), + CompileErrorKind::UnicodeEscapeRange { hex } => todo!(), + CompileErrorKind::UnicodeEscapeUnterminated => todo!(), + CompileErrorKind::UnknownAliasTarget { alias, target } => todo!(), + CompileErrorKind::UnknownAttribute { attribute } => todo!(), + CompileErrorKind::UnknownDependency { recipe, unknown } => todo!(), + CompileErrorKind::UnknownFunction { function } => todo!(), + CompileErrorKind::UnknownSetting { setting } => todo!(), + CompileErrorKind::UnknownStartOfToken => todo!(), + CompileErrorKind::UnpairedCarriageReturn => todo!(), + CompileErrorKind::UnterminatedBacktick => todo!(), + CompileErrorKind::UnterminatedInterpolation => todo!(), + CompileErrorKind::UnterminatedString => todo!(), + */ + }; + + report.eprint((&path, source)).unwrap(); +} + fn capitalize(s: &str) -> String { let mut chars = s.chars(); match chars.next() { diff --git a/src/error.rs b/src/error.rs index a07c4efd62..93707105cd 100644 --- a/src/error.rs +++ b/src/error.rs @@ -504,6 +504,13 @@ impl ColorDisplay for Error<'_> { } } +pub(crate) fn render_error(error: &Error, color: Color) { + match error { + Error::Compile { compile_error } => compile_error::render_compile_error(compile_error), + _ => eprintln!("{}", error.color_display(color.stderr())), + } +} + fn format_cmd(binary: &OsString, arguments: &Vec) -> String { iter::once(binary) .chain(arguments) diff --git a/src/run.rs b/src/run.rs index a07b73bbc9..c17ba50236 100644 --- a/src/run.rs +++ b/src/run.rs @@ -29,7 +29,7 @@ pub fn run(args: impl Iterator + Clone>) -> Result<() }) .map_err(|error| { if !verbosity.quiet() && error.print_message() { - eprintln!("{}", error.color_display(color.stderr())); + crate::error::render_error(&error, color); } error.code().unwrap_or(EXIT_FAILURE) }) From e6d21f09362b54a837d83a21202fc7562a53c3e8 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Mon, 2 Dec 2024 02:57:32 -0800 Subject: [PATCH 3/4] Handle colors correctly --- justfile | 1 - src/compile_error.rs | 64 +++++--------------------------------------- src/error.rs | 2 +- tests/string.rs | 10 +++---- tests/subsequents.rs | 14 +++++----- 5 files changed, 19 insertions(+), 72 deletions(-) diff --git a/justfile b/justfile index 8e94d44432..759022e285 100755 --- a/justfile +++ b/justfile @@ -34,7 +34,6 @@ run: filter PATTERN: cargo test {{PATTERN}} -[group: 'misc'] [group: 'misc'] build: cargo build diff --git a/src/compile_error.rs b/src/compile_error.rs index 6847900f06..7c17c1c7bd 100644 --- a/src/compile_error.rs +++ b/src/compile_error.rs @@ -19,8 +19,8 @@ impl<'src> CompileError<'src> { } } -pub(crate) fn render_compile_error(error: &CompileError) { - use ariadne::{Label, Report, ReportKind, Source}; +pub(crate) fn render_compile_error(error: &CompileError, color: Color) { + use ariadne::{Config, Label, Report, ReportKind, Source}; let token = error.token; let source = Source::from(token.src); @@ -31,7 +31,8 @@ pub(crate) fn render_compile_error(error: &CompileError) { let path = format!("{}", token.path.display()); let label = Label::new((&path, start..end)); - let report = Report::build(ReportKind::Error, &path, start); + let config = Config::default().with_color(color.stderr().active()); + let report = Report::build(ReportKind::Error, &path, start).with_config(config); let report = match &*error.kind { CompileErrorKind::AttributeArgumentCountMismatch { @@ -49,27 +50,17 @@ pub(crate) fn render_compile_error(error: &CompileError) { }; report - .with_code("E01") .with_message("Attribute argument count mismatch") .with_label(label.with_message(label_msg)) .with_note(note) .finish() } - /* - CompileErrorKind::BacktickShebang => todo!(), - CompileErrorKind::CircularRecipeDependency { recipe, circle } => todo!(), - CompileErrorKind::CircularVariableDependency { variable, circle } => todo!(), - CompileErrorKind::DependencyArgumentCountMismatch { dependency, found, min, max } => todo!(), - CompileErrorKind::Redefinition { first, first_type, name, second_type } => todo!(), - */ CompileErrorKind::DuplicateAttribute { attribute, first } => { let original_label = source .line(*first) .map(|line| Label::new((&path, line.span())).with_message("original")); - let mut report = report - .with_code("E02") - .with_message(format!("Duplicate attribute `{attribute}`")); + let mut report = report.with_message(format!("Duplicate attribute `{attribute}`")); if let Some(original) = original_label { report = report.with_label(original); } @@ -78,50 +69,7 @@ pub(crate) fn render_compile_error(error: &CompileError) { _ => { let message = format!("{error}"); report.with_message(message).with_label(label).finish() - } /* - CompileErrorKind::DuplicateParameter { recipe, parameter } => todo!(), - CompileErrorKind::DuplicateSet { setting, first } => todo!(), - CompileErrorKind::DuplicateVariable { variable } => todo!(), - CompileErrorKind::DuplicateUnexport { variable } => todo!(), - CompileErrorKind::ExpectedKeyword { expected, found } => todo!(), - CompileErrorKind::ExportUnexported { variable } => todo!(), - CompileErrorKind::ExtraLeadingWhitespace => todo!(), - CompileErrorKind::ExtraneousAttributes { count } => todo!(), - CompileErrorKind::FunctionArgumentCountMismatch { function, found, expected } => todo!(), - CompileErrorKind::Include => todo!(), - CompileErrorKind::InconsistentLeadingWhitespace { expected, found } => todo!(), - CompileErrorKind::Internal { message } => todo!(), - CompileErrorKind::InvalidAttribute { item_kind, item_name, attribute } => todo!(), - CompileErrorKind::InvalidEscapeSequence { character } => todo!(), - CompileErrorKind::MismatchedClosingDelimiter { close, open, open_line } => todo!(), - CompileErrorKind::MixedLeadingWhitespace { whitespace } => todo!(), - CompileErrorKind::ParameterFollowsVariadicParameter { parameter } => todo!(), - CompileErrorKind::ParsingRecursionDepthExceeded => todo!(), - CompileErrorKind::RequiredParameterFollowsDefaultParameter { parameter } => todo!(), - CompileErrorKind::ShebangAndScriptAttribute { recipe } => todo!(), - CompileErrorKind::ShellExpansion { err } => todo!(), - CompileErrorKind::UndefinedVariable { variable } => todo!(), - CompileErrorKind::UnexpectedCharacter { expected } => todo!(), - CompileErrorKind::UnexpectedClosingDelimiter { close } => todo!(), - CompileErrorKind::UnexpectedEndOfToken { expected } => todo!(), - CompileErrorKind::UnexpectedToken { expected, found } => todo!(), - CompileErrorKind::UnicodeEscapeCharacter { character } => todo!(), - CompileErrorKind::UnicodeEscapeDelimiter { character } => todo!(), - CompileErrorKind::UnicodeEscapeEmpty => todo!(), - CompileErrorKind::UnicodeEscapeLength { hex } => todo!(), - CompileErrorKind::UnicodeEscapeRange { hex } => todo!(), - CompileErrorKind::UnicodeEscapeUnterminated => todo!(), - CompileErrorKind::UnknownAliasTarget { alias, target } => todo!(), - CompileErrorKind::UnknownAttribute { attribute } => todo!(), - CompileErrorKind::UnknownDependency { recipe, unknown } => todo!(), - CompileErrorKind::UnknownFunction { function } => todo!(), - CompileErrorKind::UnknownSetting { setting } => todo!(), - CompileErrorKind::UnknownStartOfToken => todo!(), - CompileErrorKind::UnpairedCarriageReturn => todo!(), - CompileErrorKind::UnterminatedBacktick => todo!(), - CompileErrorKind::UnterminatedInterpolation => todo!(), - CompileErrorKind::UnterminatedString => todo!(), - */ + } }; report.eprint((&path, source)).unwrap(); diff --git a/src/error.rs b/src/error.rs index 93707105cd..7b91b2fe03 100644 --- a/src/error.rs +++ b/src/error.rs @@ -506,7 +506,7 @@ impl ColorDisplay for Error<'_> { pub(crate) fn render_error(error: &Error, color: Color) { match error { - Error::Compile { compile_error } => compile_error::render_compile_error(compile_error), + Error::Compile { compile_error } => compile_error::render_compile_error(compile_error, color), _ => eprintln!("{}", error.color_display(color.stderr())), } } diff --git a/tests/string.rs b/tests/string.rs index 12bcbac0f7..3d28e1cedf 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -481,11 +481,11 @@ fn unicode_escape_non_hex() { .status(1) .stderr( r#" -error: expected hex digit [0-9A-Fa-f] but found `o` - ——▶ justfile:1:6 - │ -1 │ x := "\u{foo}" - │ ^^^^^^^^^ +Error: expected hex digit [0-9A-Fa-f] but found `o` + ╭─[justfile:1:6] + │ + 1 │ x := "\u{foo}" +───╯ "#, ) .run(); diff --git a/tests/subsequents.rs b/tests/subsequents.rs index 9ef4600067..c0a7ff9fe6 100644 --- a/tests/subsequents.rs +++ b/tests/subsequents.rs @@ -45,13 +45,13 @@ test! { justfile: " foo: && foo ", - stderr: " - error: Recipe `foo` depends on itself - ——▶ justfile:1:9 - │ - 1 │ foo: && foo - │ ^^^ - ", + stderr: +"Error: Recipe `foo` depends on itself + ╭─[justfile:1:9] + │ + 1 │ foo: && foo +───╯ +", status: EXIT_FAILURE, } From db1ff0374b75a21a0562caaa4af95db23fd26dfb Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Mon, 2 Dec 2024 21:26:43 -0800 Subject: [PATCH 4/4] Tests --- tests/string.rs | 54 +++++++++++++++++++------------------- tests/test.rs | 3 ++- tests/working_directory.rs | 12 ++++----- 3 files changed, 35 insertions(+), 34 deletions(-) diff --git a/tests/string.rs b/tests/string.rs index 3d28e1cedf..06a0d4a9a9 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -184,12 +184,12 @@ test! { args: ("a"), stdout: "", stderr: " - error: Unterminated string - ——▶ justfile:1:6 - │ - 1 │ a b= ': - │ ^ - ", +Error: Unterminated string + ╭─[justfile:1:6] + │ + 1 │ a b= ': +───╯ +", status: EXIT_FAILURE, } @@ -201,12 +201,12 @@ test! { args: ("a"), stdout: "", stderr: r#" - error: Unterminated string - ——▶ justfile:1:6 - │ - 1 │ a b= ": - │ ^ - "#, +Error: Unterminated string + ╭─[justfile:1:6] + │ + 1 │ a b= ": +───╯ +"#, status: EXIT_FAILURE, } @@ -234,11 +234,11 @@ test! { args: ("a"), stdout: "", stderr: " - error: Unterminated string - ——▶ justfile:1:6 - │ - 1 │ a b= ''': - │ ^^^ +Error: Unterminated string + ╭─[justfile:1:6] + │ + 1 │ a b= ''': +───╯ ", status: EXIT_FAILURE, } @@ -251,11 +251,11 @@ test! { args: ("a"), stdout: "", stderr: r#" - error: Unterminated string - ——▶ justfile:1:6 - │ - 1 │ a b= """: - │ ^^^ +Error: Unterminated string + ╭─[justfile:1:6] + │ + 1 │ a b= """: +───╯ "#, status: EXIT_FAILURE, } @@ -517,11 +517,11 @@ fn unicode_escape_too_long() { .status(1) .stderr( r#" -error: unicode escape sequence starting with `\u{FFFFFFF` longer than six hex digits - ——▶ justfile:1:6 - │ -1 │ x := "\u{FFFFFFFFFF}" - │ ^^^^^^^^^^^^^^^^ +Error: unicode escape sequence starting with `\u{FFFFFFF` longer than six hex digits + ╭─[justfile:1:6] + │ + 1 │ x := "\u{FFFFFFFFFF}" +───╯ "#, ) .run(); diff --git a/tests/test.rs b/tests/test.rs index e02b5a85b6..9bf68d3add 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -228,7 +228,8 @@ impl Test { fn compare_string(name: &str, have: &str, want: &str) -> bool { let equal = have == want; if !equal { - eprintln!("Bad {name}: {}", StrComparison::new(&have, &want)); + //eprintln!("Bad {name}: {}", StrComparison::new(&have, &want)); + eprintln!("Bad {name}:\n{}||\n-------------\n{}||\n=========", &have, &want); } equal } diff --git a/tests/working_directory.rs b/tests/working_directory.rs index bfae635f5c..9580b33ae1 100644 --- a/tests/working_directory.rs +++ b/tests/working_directory.rs @@ -381,12 +381,12 @@ fn attribute_with_nocd_is_forbidden() { ) .stderr( " - error: Recipe `bar` has both `[no-cd]` and `[working-directory]` attributes - ——▶ justfile:3:1 - │ - 3 │ bar: - │ ^^^ - ", +Error: Recipe `bar` has both `[no-cd]` and `[working-directory]` attributes + ╭─[justfile:3:1] + │ + 3 │ bar: +───╯ +" ) .status(EXIT_FAILURE) .run();