Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Commit

Permalink
Improve errors here and there
Browse files Browse the repository at this point in the history
  • Loading branch information
cassaundra committed Mar 26, 2024
1 parent 49fc0b3 commit 416c7a0
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 28 deletions.
8 changes: 7 additions & 1 deletion crates/quake_core/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,17 @@ impl Metadata {
})
}

pub fn register_task(&mut self, name: String, task: impl Into<Arc<Task>>) -> Result<TaskId> {
pub fn register_task(
&mut self,
name: String,
task: impl Into<Arc<Task>>,
span: Span,
) -> Result<TaskId> {
if let Ok(existing) = self.find_task(&name, None) {
return Err(errors::TaskDuplicateDefinition {
name,
existing_span: existing.name.span,
span,
}
.into());
}
Expand Down
1 change: 1 addition & 0 deletions crates/quake_engine/src/nu/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ impl Command for Subtask {
decl_body: None,
run_body: Some(closure.block_id),
}),
name.span,
)
.into_shell_result()?;

Expand Down
6 changes: 4 additions & 2 deletions crates/quake_engine/src/nu/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ fn parse_def_task(
}

// note: errors when task has already been defined
let name_span = name.span;
if let Err(err) = metadata.register_task(
name.item.clone(),
Arc::new(Task {
Expand All @@ -198,6 +199,7 @@ fn parse_def_task(
decl_body,
run_body,
}),
name_span,
) {
working_set.error(err.into_parse_error());

Expand Down Expand Up @@ -232,9 +234,9 @@ fn transform_depends(
.find_task(&dep_id.item, Some(dep_id.span))
.into_shell_result()?
.depends_decl_id
.ok_or(errors::TaskCannotDepend {
.ok_or(errors::TaskNotFound {
name: dep_id.item,
span: name_span,
span: Some(name_span),
})
.into_diagnostic()
.into_shell_result()?;
Expand Down
2 changes: 1 addition & 1 deletion crates/quake_engine/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ fn get_scope_id(stack: &Stack, span: Span) -> ShellResult<ScopeId> {
}
}

Err(errors::UnknownScope { span })
Err(errors::InvalidScope { span })
.into_diagnostic()
.into_shell_result()
}
Expand Down
34 changes: 11 additions & 23 deletions crates/quake_errors/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
use miette::Diagnostic;
use nu_protocol::Span;
use thiserror::Error;

pub const QUAKE_OTHER_ERROR_CODE: &str = "quake::other";

pub(crate) trait QuakeDiagnostic: Diagnostic {}

macro_rules! make_error {
($name:ident, $item:item) => {
#[derive(Debug, Clone, Error, Diagnostic)]
#[derive(Debug, Clone, ::thiserror::Error, ::miette::Diagnostic)]
$item

impl QuakeDiagnostic for $name {}
impl $crate::QuakeDiagnostic for $name {}
}
}

Expand Down Expand Up @@ -42,59 +38,53 @@ make_errors! {

#[error("Build script not found")]
#[diagnostic(
code(quake::missing_build_script),
code(quake::build_script_not_found),
help("Add a `build.quake` file to the project root")
)]
pub struct BuildScriptNotFound;

// TODO add "did you mean?" or list available tasks
#[error("Task not found: {name}")]
#[diagnostic(code(quake::task::not_found))]
#[diagnostic(code(quake::task_not_found))]
pub struct TaskNotFound {
pub name: String,
#[label("task referenced here")]
pub span: Option<Span>,
}

#[error("Task already defined: {name}")]
#[diagnostic(code(quake::task::duplicate_definition))]
#[diagnostic(code(quake::duplicate_task_definition))]
pub struct TaskDuplicateDefinition {
pub name: String,
#[label("first defined here")]
pub existing_span: Span,
}

#[error("Task cannot be depended upon")]
#[diagnostic(code(quake::task::cannot_depend))]
pub struct TaskCannotDepend {
pub name: String,
#[label("task referenced here")]
#[label("defined again here")]
pub span: Span,
}

#[error("Declarative task has extra body")]
#[diagnostic(
code(quake::task::cannot_depend),
code(quake::decl_task_has_extra_body),
help("Remove the `--decl` flag or remove the extra block")
)]
pub struct DeclTaskHasExtraBody {
#[label("extra block")]
pub span: Span,
}

#[error("Unknown scope")]
#[error("Invalid scope for command")]
#[diagnostic(
code(quake::task::unknown_scope),
code(quake::invalid_scope),
help("Did you mean to evaluate this command inside of a special scope block? (e.g. def-task)")
)]
pub struct UnknownScope {
pub struct InvalidScope {
#[label("command used here")]
pub span: Span,
}

#[error("Attempt to define nested task scopes")]
#[diagnostic(
code(quake::scope::no_nested_scopes),
code(quake::nested_scope),
help("Define this task in the outer scope instead, or use `subtask`")
)]
pub struct NestedScopes {
Expand All @@ -107,8 +97,6 @@ make_errors! {
mod tests {
use anstream::adapter::strip_str;

use super::*;

#[test]
fn test_make_errors_macro() {
macro_rules! err {
Expand Down
2 changes: 2 additions & 0 deletions crates/quake_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ mod macros;
pub type Result<T> = miette::Result<T>;

pub type Error = miette::Error;

pub(crate) trait QuakeDiagnostic: miette::Diagnostic {}
2 changes: 1 addition & 1 deletion crates/quake_errors/src/nu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use nu_protocol::Span;

pub use nu_protocol::{ParseError, ShellError};

use crate::errors::QuakeDiagnostic;
use crate::QuakeDiagnostic;

pub type ParseResult<T> = Result<T, ParseError>;
pub type ShellResult<T> = Result<T, ShellError>;
Expand Down

0 comments on commit 416c7a0

Please sign in to comment.