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

Adds support for default values to params and flags #4770

Merged
merged 1 commit into from
Mar 7, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Default values
  • Loading branch information
sophiajt committed Mar 7, 2022
commit 37b9a5f0745e06f90a14dcf1467f0de040b87ed2
11 changes: 11 additions & 0 deletions crates/nu-engine/src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ fn eval_call(
if let Some(arg) = call.positional.get(param_idx) {
let result = eval_expression(engine_state, caller_stack, arg)?;
callee_stack.add_var(var_id, result);
} else if let Some(arg) = &param.default_value {
let result = eval_expression(engine_state, caller_stack, arg)?;
callee_stack.add_var(var_id, result);
} else {
callee_stack.add_var(var_id, Value::nothing(call.head));
}
Expand Down Expand Up @@ -103,6 +106,10 @@ fn eval_call(
if let Some(arg) = &call_named.1 {
let result = eval_expression(engine_state, caller_stack, arg)?;

callee_stack.add_var(var_id, result);
} else if let Some(arg) = &named.default_value {
let result = eval_expression(engine_state, caller_stack, arg)?;

callee_stack.add_var(var_id, result);
} else {
callee_stack.add_var(
Expand All @@ -126,6 +133,10 @@ fn eval_call(
span: call.head,
},
)
} else if let Some(arg) = &named.default_value {
let result = eval_expression(engine_state, caller_stack, arg)?;

callee_stack.add_var(var_id, result);
} else {
callee_stack.add_var(var_id, Value::Nothing { span: call.head })
}
Expand Down
8 changes: 8 additions & 0 deletions crates/nu-parser/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ pub enum ParseError {
#[diagnostic(code(nu::parser::extra_positional), url(docsrs), help("Usage: {0}"))]
ExtraPositional(String, #[label = "extra positional argument"] Span),

#[error("Require positional parameter after optional parameter")]
#[diagnostic(code(nu::parser::required_after_optional), url(docsrs))]
RequiredAfterOptional(
String,
#[label = "required parameter {0} after optional parameter"] Span,
),

#[error("Unexpected end of code.")]
#[diagnostic(code(nu::parser::unexpected_eof), url(docsrs))]
UnexpectedEof(String, #[label("expected closing {0}")] Span),
Expand Down Expand Up @@ -246,6 +253,7 @@ impl ParseError {
ParseError::UnknownCommand(s) => *s,
ParseError::NonUtf8(s) => *s,
ParseError::UnknownFlag(_, _, s) => *s,
ParseError::RequiredAfterOptional(_, s) => *s,
ParseError::UnknownType(s) => *s,
ParseError::MissingFlagParam(_, s) => *s,
ParseError::ShortFlagBatchCantTakeArg(s) => *s,
Expand Down
1 change: 1 addition & 0 deletions crates/nu-parser/src/parse_keywords.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ pub fn parse_for(
desc: String::new(),
shape: SyntaxShape::Any,
var_id: Some(*var_id),
default_value: None,
},
);
}
Expand Down
133 changes: 131 additions & 2 deletions crates/nu-parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2677,6 +2677,7 @@ pub fn parse_row_condition(
desc: "row condition".into(),
shape: SyntaxShape::Any,
var_id: Some(var_id),
default_value: None,
});

working_set.add_block(block)
Expand Down Expand Up @@ -2742,11 +2743,14 @@ pub fn parse_signature_helper(
working_set: &mut StateWorkingSet,
span: Span,
) -> (Box<Signature>, Option<ParseError>) {
#[allow(clippy::enum_variant_names)]
enum ParseMode {
ArgMode,
TypeMode,
DefaultValueMode,
}

#[derive(Debug)]
enum Arg {
Positional(PositionalArg, bool), // bool - required
RestPositional(PositionalArg),
Expand All @@ -2756,7 +2760,13 @@ pub fn parse_signature_helper(
let mut error = None;
let source = working_set.get_span_contents(span);

let (output, err) = lex(source, span.start, &[b'\n', b'\r', b','], &[b':'], false);
let (output, err) = lex(
source,
span.start,
&[b'\n', b'\r', b','],
&[b':', b'='],
false,
);
error = error.or(err);

let mut args: Vec<Arg> = vec![];
Expand All @@ -2776,12 +2786,24 @@ pub fn parse_signature_helper(
ParseMode::ArgMode => {
parse_mode = ParseMode::TypeMode;
}
ParseMode::TypeMode => {
ParseMode::TypeMode | ParseMode::DefaultValueMode => {
// We're seeing two types for the same thing for some reason, error
error =
error.or_else(|| Some(ParseError::Expected("type".into(), span)));
}
}
} else if contents == b"=" {
match parse_mode {
ParseMode::ArgMode | ParseMode::TypeMode => {
parse_mode = ParseMode::DefaultValueMode;
}
ParseMode::DefaultValueMode => {
// We're seeing two default values for some reason, error
error = error.or_else(|| {
Some(ParseError::Expected("default value".into(), span))
});
}
}
} else {
match parse_mode {
ParseMode::ArgMode => {
Expand All @@ -2802,6 +2824,7 @@ pub fn parse_signature_helper(
short: None,
required: false,
var_id: Some(var_id),
default_value: None,
}));
} else {
let short_flag = &flags[1];
Expand Down Expand Up @@ -2832,6 +2855,7 @@ pub fn parse_signature_helper(
short: Some(chars[0]),
required: false,
var_id: Some(var_id),
default_value: None,
}));
} else {
error = error.or_else(|| {
Expand Down Expand Up @@ -2864,6 +2888,7 @@ pub fn parse_signature_helper(
short: Some(chars[0]),
required: false,
var_id: Some(var_id),
default_value: None,
}));
} else if contents.starts_with(b"(-") {
let short_flag = &contents[2..];
Expand Down Expand Up @@ -2921,6 +2946,7 @@ pub fn parse_signature_helper(
name,
shape: SyntaxShape::Any,
var_id: Some(var_id),
default_value: None,
},
false,
))
Expand All @@ -2935,6 +2961,7 @@ pub fn parse_signature_helper(
name,
shape: SyntaxShape::Any,
var_id: Some(var_id),
default_value: None,
}));
} else {
let name = String::from_utf8_lossy(contents).to_string();
Expand All @@ -2949,6 +2976,7 @@ pub fn parse_signature_helper(
name,
shape: SyntaxShape::Any,
var_id: Some(var_id),
default_value: None,
},
true,
))
Expand Down Expand Up @@ -2982,6 +3010,97 @@ pub fn parse_signature_helper(
}
parse_mode = ParseMode::ArgMode;
}
ParseMode::DefaultValueMode => {
if let Some(last) = args.last_mut() {
let (expression, err) =
parse_value(working_set, span, &SyntaxShape::Any);
error = error.or(err);

//TODO check if we're replacing a custom parameter already
match last {
Arg::Positional(
PositionalArg {
shape,
var_id,
default_value,
..
},
required,
) => {
let var_id = var_id.expect("internal error: all custom parameters must have var_ids");
let var_type = working_set.get_variable(var_id);
match var_type {
Type::Unknown => {
working_set.set_variable_type(
var_id,
expression.ty.clone(),
);
}
t => {
if t != &expression.ty {
error = error.or_else(|| {
Some(ParseError::AssignmentMismatch(
"Default value wrong type".into(),
format!("default value not {}", t),
expression.span,
))
})
}
}
}
*shape = expression.ty.to_shape();
*default_value = Some(expression);
*required = false;
}
Arg::RestPositional(..) => {
error = error.or_else(|| {
Some(ParseError::AssignmentMismatch(
"Rest parameter given default value".into(),
"can't have default value".into(),
expression.span,
))
})
}
Arg::Flag(Flag {
arg,
var_id,
default_value,
..
}) => {
let var_id = var_id.expect("internal error: all custom parameters must have var_ids");
let var_type = working_set.get_variable(var_id);

let expression_ty = expression.ty.clone();
let expression_span = expression.span;

*default_value = Some(expression);

// Flags with a boolean type are just present/not-present switches
if var_type != &Type::Bool {
match var_type {
Type::Unknown => {
*arg = Some(expression_ty.to_shape());
working_set
.set_variable_type(var_id, expression_ty);
}
t => {
if t != &expression_ty {
error = error.or_else(|| {
Some(ParseError::AssignmentMismatch(
"Default value wrong type".into(),
format!("default value not {}", t),
expression_span,
))
})
}
}
}
}
}
}
}
parse_mode = ParseMode::ArgMode;
}
}
}
}
Expand Down Expand Up @@ -3030,6 +3149,14 @@ pub fn parse_signature_helper(
match arg {
Arg::Positional(positional, required) => {
if required {
if !sig.optional_positional.is_empty() {
error = error.or_else(|| {
Some(ParseError::RequiredAfterOptional(
positional.name.clone(),
span,
))
})
}
sig.required_positional.push(positional)
} else {
sig.optional_positional.push(positional)
Expand Down Expand Up @@ -3379,6 +3506,7 @@ pub fn parse_block_expression(
name: "$it".into(),
desc: String::new(),
shape: SyntaxShape::Any,
default_value: None,
});
output.signature = Box::new(signature);
}
Expand Down Expand Up @@ -4503,6 +4631,7 @@ fn wrap_expr_with_collect(working_set: &mut StateWorkingSet, expr: &Expression)
name: "$in".into(),
desc: String::new(),
shape: SyntaxShape::Any,
default_value: None,
});

let mut expr = expr.clone();
Expand Down
2 changes: 2 additions & 0 deletions crates/nu-plugin/src/serializers/capnp/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ fn deserialize_argument(reader: argument::Reader) -> Result<PositionalArg, Shell
desc: desc.to_string(),
shape,
var_id: None,
default_value: None,
})
}

Expand Down Expand Up @@ -262,6 +263,7 @@ fn deserialize_flag(reader: flag::Reader) -> Result<Flag, ShellError> {
required,
desc: desc.to_string(),
var_id: None,
default_value: None,
})
}

Expand Down
4 changes: 3 additions & 1 deletion crates/nu-protocol/src/ast/call.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use serde::{Deserialize, Serialize};

use super::Expression;
use crate::{DeclId, Span, Spanned};

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Call {
/// identifier of the declaration to call
pub decl_id: DeclId,
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-protocol/src/ast/cell_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl CellPath {
}
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct FullCellPath {
pub head: Expression,
pub tail: Vec<PathMember>,
Expand Down
3 changes: 2 additions & 1 deletion crates/nu-protocol/src/ast/expr.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use chrono::FixedOffset;
use serde::{Deserialize, Serialize};

use super::{Call, CellPath, Expression, FullCellPath, Operator, RangeOperator};
use crate::{ast::ImportPattern, BlockId, Signature, Span, Spanned, Unit, VarId};

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub enum Expr {
Bool(bool),
Int(i64),
Expand Down
4 changes: 3 additions & 1 deletion crates/nu-protocol/src/ast/expression.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use serde::{Deserialize, Serialize};

use super::{Expr, Operator};
use crate::ast::ImportPattern;
use crate::{engine::StateWorkingSet, BlockId, Signature, Span, Type, VarId, IN_VARIABLE_ID};

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Expression {
pub expr: Expr,
pub span: Span,
Expand Down
8 changes: 5 additions & 3 deletions crates/nu-protocol/src/ast/import_pattern.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
use serde::{Deserialize, Serialize};

use crate::{span, OverlayId, Span};
use std::collections::HashSet;

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub enum ImportPatternMember {
Glob { span: Span },
Name { name: Vec<u8>, span: Span },
List { names: Vec<(Vec<u8>, Span)> },
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct ImportPatternHead {
pub name: Vec<u8>,
pub id: Option<OverlayId>,
pub span: Span,
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct ImportPattern {
pub head: ImportPatternHead,
pub members: Vec<ImportPatternMember>,
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-protocol/src/ast/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub enum RangeInclusion {
RightExclusive,
}

#[derive(Debug, Copy, Clone)]
#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)]
pub struct RangeOperator {
pub inclusion: RangeInclusion,
pub span: Span,
Expand Down