Skip to content

Commit

Permalink
Refactor parameter syntax and expansion implementation (#386)
Browse files Browse the repository at this point in the history
  • Loading branch information
magicant committed Jul 15, 2024
2 parents cd69bbb + 0aee68c commit 78ee47f
Show file tree
Hide file tree
Showing 18 changed files with 802 additions and 538 deletions.
4 changes: 4 additions & 0 deletions yash-cli/CHANGELOG-bin.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- The shell now rejects an invalid parameter as a syntax error. Specifically,
if a parameter starts with a digit but is not a valid number, the shell now
reports a syntax error instead of treating it as a variable. For example,
`${1abc}` and `${0_1}` are now syntax errors.
- Improved error messages for some parameter expansion errors.

## [0.1.0-beta.2] - 2024-07-13
Expand Down
10 changes: 6 additions & 4 deletions yash-semantics/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`MessageBase::footers` method.
- The `AssignReadOnlyError` struct now has a `vacancy: Option<Vacancy>`
field.
- The `initial::VacantError` struct now has a `name: String` field.
- The `initial::VacantError` struct now has a `param: Param` field.
- The `initial::NonassignableErrorCause` enum is a successor to the previous
`NonassignableError` enum. The new `NotVariable` variant has a `name:
String` field.
`NonassignableError` enum. The new `NotVariable` variant has a `param:
Param` field.

### Changed

- Error types in the `expansion` module (some of which are reexported in the
`assign` module) have been extended for more informative error messages:
- The `ErrorCause::UnsetParameter` variant now has a `name: String` field.
- The `ErrorCause::UnsetParameter` variant now has a `param: Param` field.
- The `message` and `label` methods of `ErrorCause` return more informative
messages for the `UnsetParameter` and `VacantExpansion` variants.
- The `expansion::initial::NonassignableError` enum has been replaced with a
struct of the same name so that it can have a `Vacancy` field.
- The `MessageBase::additional_annotations` method implementation for the
`Error` struct has been extended to produce more annotations for errors
with `Vacancy` information.
- External dependency versions:
- yash-syntax 0.10.0 → 0.11.0

## [0.3.0] - 2024-07-13

Expand Down
25 changes: 17 additions & 8 deletions yash-semantics/src/expansion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ use yash_syntax::source::pretty::AnnotationType;
use yash_syntax::source::pretty::Footer;
use yash_syntax::source::pretty::MessageBase;
use yash_syntax::source::Location;
use yash_syntax::syntax::Param;
use yash_syntax::syntax::Text;
use yash_syntax::syntax::Word;

Expand Down Expand Up @@ -139,8 +140,8 @@ pub enum ErrorCause {
AssignReadOnly(#[from] AssignReadOnlyError),

/// Expansion of an unset parameter with the `nounset` option
#[error("unset parameter {name:?}")]
UnsetParameter { name: String },
#[error("unset parameter `{param}`")]
UnsetParameter { param: Param },

/// Expansion of an empty value with an error switch
#[error(transparent)]
Expand Down Expand Up @@ -173,13 +174,21 @@ impl ErrorCause {
// TODO Localize
use ErrorCause::*;
match self {
CommandSubstError(e) => e.to_string().into(),
ArithError(e) => e.to_string().into(),
AssignReadOnly(e) => e.to_string().into(),
UnsetParameter { name } => format!("parameter {name:?} is not set").into(),
VacantExpansion(e) => format!("{}: {}", e.name, e.vacancy).into(),
NonassignableParameter(e) => e.to_string().into(),
CommandSubstError(e) => e.to_string(),
ArithError(e) => e.to_string(),
AssignReadOnly(e) => e.to_string(),
UnsetParameter { param } => format!("parameter `{param}` is not set"),
VacantExpansion(e) => match e.vacancy {
Vacancy::Unset => format!("parameter `{}` is not set", e.param),
Vacancy::EmptyScalar => format!("parameter `{}` is an empty string", e.param),
Vacancy::ValuelessArray => format!("parameter `{}` is an empty array", e.param),
Vacancy::EmptyValueArray => {
format!("parameter `{}` is an array of an empty string", e.param)
}
},
NonassignableParameter(e) => e.to_string(),
}
.into()
}

/// Returns a location related with the error cause and a message describing
Expand Down
11 changes: 6 additions & 5 deletions yash-semantics/src/expansion/initial/arith.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use yash_env::variable::Scope::Global;
use yash_syntax::source::Code;
use yash_syntax::source::Location;
use yash_syntax::source::Source;
use yash_syntax::syntax::Param;
use yash_syntax::syntax::Text;

/// Types of errors that may occur in arithmetic expansion
Expand Down Expand Up @@ -135,7 +136,7 @@ impl ArithError {
/// Error expanding an unset variable
#[derive(Clone, Debug, Eq, PartialEq)]
struct UnsetVariable {
name: String,
param: Param,
}

/// Converts `yash_arith::ErrorCause` into `initial::ErrorCause`.
Expand Down Expand Up @@ -192,8 +193,8 @@ fn convert_error_cause(
}
yash_arith::EvalError::ReverseShifting => ErrorCause::ArithError(ReverseShifting),
yash_arith::EvalError::AssignmentToValue => ErrorCause::ArithError(AssignmentToValue),
yash_arith::EvalError::GetVariableError(UnsetVariable { name }) => {
ErrorCause::UnsetParameter { name }
yash_arith::EvalError::GetVariableError(UnsetVariable { param }) => {
ErrorCause::UnsetParameter { param }
}
yash_arith::EvalError::AssignVariableError(e) => ErrorCause::AssignReadOnly(e),
},
Expand All @@ -217,7 +218,7 @@ impl<'a> yash_arith::Env for VarEnv<'a> {
// TODO If the variable exists but is not scalar, UnsetVariable
// does not seem to be the right error.
Off => Err(UnsetVariable {
name: name.to_owned(),
param: Param::variable(name),
}),
On => Ok(None),
},
Expand Down Expand Up @@ -363,7 +364,7 @@ mod tests {
assert_eq!(
result,
Err(UnsetVariable {
name: "v".to_string()
param: Param::variable("v")
})
);
}
Expand Down
72 changes: 36 additions & 36 deletions yash-semantics/src/expansion/initial/param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,31 +25,32 @@ use super::Env;
use super::Expand;
use yash_env::option::Option::Unset;
use yash_env::option::State::Off;
use yash_env::variable::Expansion;
use yash_env::variable::Value;
use yash_syntax::source::Location;
use yash_syntax::syntax::BracedParam;
use yash_syntax::syntax::Modifier;
use yash_syntax::syntax::Param;
use yash_syntax::syntax::ParamType;
use yash_syntax::syntax::SpecialParam;

/// Reference to a parameter expansion
pub struct ParamRef<'a> {
pub name: &'a str,
pub param: &'a Param,
pub modifier: &'a Modifier,
pub location: &'a Location,
}

impl<'a> From<&'a Param> for ParamRef<'a> {
fn from(param: &'a Param) -> Self {
impl<'a> From<&'a BracedParam> for ParamRef<'a> {
fn from(bp: &'a BracedParam) -> Self {
ParamRef {
name: &param.name,
modifier: &param.modifier,
location: &param.location,
param: &bp.param,
modifier: &bp.modifier,
location: &bp.location,
}
}
}

// TODO Consider exporting these modules
mod name;
mod resolve;
mod switch;
mod trim;
Expand All @@ -64,11 +65,7 @@ impl Expand for ParamRef<'_> {
// TODO Expand and parse Index

// Lookup //
let name = self.name.try_into().ok();
let resolve = match name {
Some(name) => resolve::resolve(name, env.inner, self.location),
None => Expansion::Unset,
};
let resolve = resolve::resolve(env.inner, self.param, self.location);

// TODO Apply Index

Expand All @@ -77,7 +74,7 @@ impl Expand for ParamRef<'_> {
// Switch //
if let Modifier::Switch(switch) = self.modifier {
if let Some(result) =
switch::apply(env, switch, self.name, name, value.as_ref(), self.location).await
switch::apply(env, switch, self.param, value.as_ref(), self.location).await
{
return result;
}
Expand All @@ -86,7 +83,7 @@ impl Expand for ParamRef<'_> {
if value.is_none() && env.inner.options.get(Unset) == Off {
return Err(Error {
cause: ErrorCause::UnsetParameter {
name: self.name.to_owned(),
param: self.param.clone(),
},
location: self.location.clone(),
});
Expand Down Expand Up @@ -116,7 +113,7 @@ impl Expand for ParamRef<'_> {
}

let mut phrase = into_phrase(value);
if !env.will_split && self.name == "*" {
if !env.will_split && self.param.r#type == ParamType::Special(SpecialParam::Asterisk) {
phrase = Phrase::Field(phrase.ifs_join(&env.inner.variables));
}
Ok(phrase)
Expand Down Expand Up @@ -169,14 +166,18 @@ pub mod tests {
env
}

pub fn param<N: ToString>(name: N) -> Param {
Param {
name: name.to_string(),
pub fn braced_param<P: Into<Param>>(param: P) -> BracedParam {
BracedParam {
param: param.into(),
modifier: Modifier::None,
location: Location::dummy(""),
}
}

pub fn braced_variable<I: Into<String>>(id: I) -> BracedParam {
braced_param(Param::variable(id))
}

#[test]
fn basic_expansion() {
let mut env = yash_env::Env::new_virtual();
Expand All @@ -185,7 +186,7 @@ pub mod tests {
.assign("a1\u{30A4}", None)
.unwrap();
let mut env = Env::new(&mut env);
let param = param("foo");
let param = braced_variable("foo");
let param = ParamRef::from(&param);

let phrase = param.expand(&mut env).now_or_never().unwrap().unwrap();
Expand All @@ -196,7 +197,7 @@ pub mod tests {
fn length_of_unset() {
let mut env = yash_env::Env::new_virtual();
let mut env = Env::new(&mut env);
let mut param = param("foo");
let mut param = braced_variable("foo");
param.modifier = Modifier::Length;
let param = ParamRef::from(&param);

Expand All @@ -212,7 +213,7 @@ pub mod tests {
.assign("a1\u{30A4}", None)
.unwrap();
let mut env = Env::new(&mut env);
let mut param = param("foo");
let mut param = braced_variable("foo");
param.modifier = Modifier::Length;
let param = ParamRef::from(&param);

Expand All @@ -228,7 +229,7 @@ pub mod tests {
.assign(Value::array(["", "foo", "1", "bar"]), None)
.unwrap();
let mut env = Env::new(&mut env);
let mut param = param("foo");
let mut param = braced_variable("foo");
param.modifier = Modifier::Length;
let param = ParamRef::from(&param);

Expand All @@ -253,7 +254,7 @@ pub mod tests {
.get_or_new("foo", Scope::Global)
.assign("", None)
.unwrap();
let mut param = param("foo");
let mut param = braced_variable("foo");
param.modifier = Modifier::Switch(Switch {
r#type: SwitchType::Alter,
condition: SwitchCondition::Unset,
Expand All @@ -276,7 +277,7 @@ pub mod tests {
.assign("abc", None)
.unwrap();
let mut env = Env::new(&mut env);
let mut param = param("foo");
let mut param = braced_variable("foo");
param.modifier = Modifier::Trim(Trim {
side: TrimSide::Prefix,
length: TrimLength::Shortest,
Expand All @@ -294,7 +295,7 @@ pub mod tests {

let mut env = yash_env::Env::new_virtual();
let mut env = Env::new(&mut env);
let mut param = param("foo");
let mut param = braced_variable("foo");
param.modifier = Modifier::Trim(Trim {
side: TrimSide::Prefix,
length: TrimLength::Shortest,
Expand All @@ -310,7 +311,7 @@ pub mod tests {
fn unset_option() {
let mut env = yash_env::Env::new_virtual();
let mut env = Env::new(&mut env);
let param = param("foo");
let param = braced_variable("foo");
let param = ParamRef::from(&param);

let phrase = param.expand(&mut env).now_or_never().unwrap().unwrap();
Expand All @@ -322,12 +323,11 @@ pub mod tests {
let mut env = yash_env::Env::new_virtual();
env.options.set(Unset, Off);
let mut env = Env::new(&mut env);
let name = "foo".to_owned();
let param = param(&name);
let param = ParamRef::from(&param);
let param = braced_variable("foo");
let pr = ParamRef::from(&param);

let e = param.expand(&mut env).now_or_never().unwrap().unwrap_err();
assert_eq!(e.cause, ErrorCause::UnsetParameter { name });
let e = pr.expand(&mut env).now_or_never().unwrap().unwrap_err();
assert_eq!(e.cause, ErrorCause::UnsetParameter { param: param.param });
assert_eq!(e.location, Location::dummy(""));
}

Expand All @@ -336,7 +336,7 @@ pub mod tests {
let mut env = yash_env::Env::new_virtual();
env.options.set(Unset, Off);
let mut env = Env::new(&mut env);
let mut param = param("foo");
let mut param = braced_variable("foo");
param.modifier = Modifier::Switch(Switch {
r#type: SwitchType::Alter,
condition: SwitchCondition::Unset,
Expand All @@ -351,7 +351,7 @@ pub mod tests {
#[test]
fn expand_at_no_join_in_non_splitting_context() {
let mut env = env_with_positional_params_and_ifs();
let param = param("@");
let param = braced_param(SpecialParam::At);
let param = ParamRef::from(&param);
let mut env = Env::new(&mut env);
env.will_split = false;
Expand All @@ -363,7 +363,7 @@ pub mod tests {
#[test]
fn expand_asterisk_no_join_in_splitting_context() {
let mut env = env_with_positional_params_and_ifs();
let param = param("*");
let param = braced_param(SpecialParam::Asterisk);
let param = ParamRef::from(&param);
let mut env = Env::new(&mut env);

Expand All @@ -374,7 +374,7 @@ pub mod tests {
#[test]
fn expand_asterisk_ifs_join_in_non_splitting_context() {
let mut env = env_with_positional_params_and_ifs();
let param = param("*");
let param = braced_param(SpecialParam::Asterisk);
let param = ParamRef::from(&param);
let mut env = Env::new(&mut env);
env.will_split = false;
Expand Down
Loading

0 comments on commit 78ee47f

Please sign in to comment.