extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2025-08-15 23:48:25 +02:00
Code Issues Packages Projects Releases Wiki Activity

Document and critically review ShellError variants - Ep. 2 (#8326)

Browse Source 
Continuation of #8229 

# Description

The `ShellError` enum at the moment is kind of messy. 

Many variants are basic tuple structs where you always have to reference
the implementation with its macro invocation to know which field serves
which purpose.
Furthermore we have both variants that are kind of redundant or either
overly broad to be useful for the user to match on or overly specific
with few uses.

So I set out to start fixing the lacking documentation and naming to
make it feasible to critically review the individual usages and fix
those.
Furthermore we can decide to join or split up variants that don't seem
to be fit for purpose.

**Everyone:** Feel free to add review comments if you spot inconsistent
use of `ShellError` variants.

- Name fields of `SE::IncorrectValue`
- Merge and name fields on `SE::TypeMismatch`
- Name fields on `SE::UnsupportedOperator`
- Name fields on `AssignmentRequires*` and fix doc
- Name fields on `SE::UnknownOperator`
- Name fields on `SE::MissingParameter`
- Name fields on `SE::DelimiterError`
- Name fields on `SE::IncompatibleParametersSingle`

# User-Facing Changes

(None now, end goal more explicit and consistent error messages)

# Tests + Formatting

(No additional tests needed so far)
This commit is contained in:
Stefan Holderbach
2023-03-06 11:31:07 +01:00
committed by GitHub
parent 4ae1b1cc26
commit f7b8f97873
60 changed files with 477 additions and 375 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
crates/nu-protocol/src/pipeline_data.rs
View File

@ -290,10 +290,14 @@ impl PipelineData {
match self {
PipelineData::Empty => Ok((String::new(), span, None)),
PipelineData::Value(Value::String { val, span }, metadata) => Ok((val, span, metadata)),
PipelineData::Value(val, _) => {
Err(ShellError::TypeMismatch("string".into(), val.span()?))
}
PipelineData::ListStream(_, _) => Err(ShellError::TypeMismatch("string".into(), span)),
PipelineData::Value(val, _) => Err(ShellError::TypeMismatch {
err_message: "string".into(),
span: val.span()?,
}),
PipelineData::ListStream(_, _) => Err(ShellError::TypeMismatch {
err_message: "string".into(),
span,
}),
PipelineData::ExternalStream {
stdout: None,
metadata,

68
crates/nu-protocol/src/shell_error.rs
View File

@ -98,17 +98,7 @@ pub enum ShellError {
/// Convert the argument type before passing it in, or change the command to accept the type.
#[error("Type mismatch.")]
#[diagnostic(code(nu::shell::type_mismatch))]
TypeMismatch(String, #[label = "{0}"] Span),
// TODO: merge with `TypeMismatch` as they are currently identical in capability
/// A command received an argument of the wrong type.
///
/// ## Resolution
///
/// Convert the argument type before passing it in, or change the command to accept the type.
#[error("Type mismatch.")]
#[diagnostic(code(nu::shell::type_mismatch))]
TypeMismatchGenericMessage {
TypeMismatch {
err_message: String,
#[label = "{err_message}"]
span: Span,
@ -121,7 +111,11 @@ pub enum ShellError {
/// Correct the argument value before passing it in or change the command.
#[error("Incorrect value.")]
#[diagnostic(code(nu::shell::incorrect_value))]
IncorrectValue(String, #[label = "{0}"] Span),
IncorrectValue {
msg: String,
#[label = "{msg}"]
span: Span,
},
/// This value cannot be used with this operator.
///
@ -129,45 +123,63 @@ pub enum ShellError {
///
/// Not all values, for example custom values, can be used with all operators. Either
/// implement support for the operator on this type, or convert the type to a supported one.
#[error("Unsupported operator: {0}.")]
#[error("Unsupported operator: {operator}.")]
#[diagnostic(code(nu::shell::unsupported_operator))]
UnsupportedOperator(Operator, #[label = "unsupported operator"] Span),
UnsupportedOperator {
operator: Operator,
#[label = "unsupported operator"]
span: Span,
},
/// This value cannot be used with this operator.
/// Invalid assignment left-hand side
///
/// ## Resolution
///
/// Assignment requires that you assign to a variable or variable cell path.
#[error("Assignment operations require a variable.")]
#[diagnostic(code(nu::shell::assignment_requires_variable))]
AssignmentRequiresVar(#[label = "needs to be a variable"] Span),
AssignmentRequiresVar {
#[label = "needs to be a variable"]
lhs_span: Span,
},
/// This value cannot be used with this operator.
/// Invalid assignment left-hand side
///
/// ## Resolution
///
/// Assignment requires that you assign to a mutable variable or cell path.
#[error("Assignment to an immutable variable.")]
#[diagnostic(code(nu::shell::assignment_requires_mutable_variable))]
AssignmentRequiresMutableVar(#[label = "needs to be a mutable variable"] Span),
AssignmentRequiresMutableVar {
#[label = "needs to be a mutable variable"]
lhs_span: Span,
},
/// An operator was not recognized during evaluation.
///
/// ## Resolution
///
/// Did you write the correct operator?
#[error("Unknown operator: {0}.")]
#[error("Unknown operator: {op_token}.")]
#[diagnostic(code(nu::shell::unknown_operator))]
UnknownOperator(String, #[label = "unknown operator"] Span),
UnknownOperator {
op_token: String,
#[label = "unknown operator"]
span: Span,
},
/// An expected command parameter is missing.
///
/// ## Resolution
///
/// Add the expected parameter and try again.
#[error("Missing parameter: {0}.")]
#[error("Missing parameter: {param_name}.")]
#[diagnostic(code(nu::shell::missing_parameter))]
MissingParameter(String, #[label = "missing parameter: {0}"] Span),
MissingParameter {
param_name: String,
#[label = "missing parameter: {param_name}"]
span: Span,
},
/// Two parameters conflict with each other or are otherwise mutually exclusive.
///
@ -193,7 +205,11 @@ pub enum ShellError {
/// Check your syntax for mismatched braces, RegExp syntax errors, etc, based on the specific error message.
#[error("Delimiter error")]
#[diagnostic(code(nu::shell::delimiter_error))]
DelimiterError(String, #[label("{0}")] Span),
DelimiterError {
msg: String,
#[label("{msg}")]
span: Span,
},
/// An operation received parameters with some sort of incompatibility
/// (for example, different number of rows in a table, incompatible column names, etc).
@ -204,7 +220,11 @@ pub enum ShellError {
/// inputs to make sure they match that way.
#[error("Incompatible parameters.")]
#[diagnostic(code(nu::shell::incompatible_parameters))]
IncompatibleParametersSingle(String, #[label = "{0}"] Span),
IncompatibleParametersSingle {
msg: String,
#[label = "{msg}"]
span: Span,
},
/// This build of nushell implements this feature, but it has not been enabled.
///

2
crates/nu-protocol/src/value/custom_value.rs
View File

@ -55,6 +55,6 @@ pub trait CustomValue: fmt::Debug + Send + Sync {
op: Span,
_right: &Value,
) -> Result<Value, ShellError> {
Err(ShellError::UnsupportedOperator(operator, op))
Err(ShellError::UnsupportedOperator { operator, span: op })
}
}

22
crates/nu-protocol/src/value/mod.rs
View File

@ -763,7 +763,7 @@ impl Value {
// Records (and tables) are the only built-in which support column names,
// so only use this message for them.
Value::Record { .. } => {
err_or_null!(ShellError::TypeMismatchGenericMessage {
err_or_null!(ShellError::TypeMismatch {
err_message: "Can't access record values with a row index. Try specifying a column name instead".into(),
span: *origin_span, }, *origin_span)
}
@ -2573,7 +2573,10 @@ impl Value {
&& (self.get_type() != Type::Any)
&& (rhs.get_type() != Type::Any)
{
return Err(ShellError::TypeMismatch("compatible type".to_string(), op));
return Err(ShellError::TypeMismatch {
err_message: "compatible type".to_string(),
span: op,
});
}
if let Some(ordering) = self.partial_cmp(rhs) {
@ -2606,7 +2609,10 @@ impl Value {
&& (self.get_type() != Type::Any)
&& (rhs.get_type() != Type::Any)
{
return Err(ShellError::TypeMismatch("compatible type".to_string(), op));
return Err(ShellError::TypeMismatch {
err_message: "compatible type".to_string(),
span: op,
});
}
self.partial_cmp(rhs)
@ -2637,7 +2643,10 @@ impl Value {
&& (self.get_type() != Type::Any)
&& (rhs.get_type() != Type::Any)
{
return Err(ShellError::TypeMismatch("compatible type".to_string(), op));
return Err(ShellError::TypeMismatch {
err_message: "compatible type".to_string(),
span: op,
});
}
self.partial_cmp(rhs)
@ -2668,7 +2677,10 @@ impl Value {
&& (self.get_type() != Type::Any)
&& (rhs.get_type() != Type::Any)
{
return Err(ShellError::TypeMismatch("compatible type".to_string(), op));
return Err(ShellError::TypeMismatch {
err_message: "compatible type".to_string(),
span: op,
});
}
match self.partial_cmp(rhs) {