Improve parameter expansion error message (#385)

check-release.sh

@@ -20,9 +20,12 @@ ls "$package" | grep -q '^LICENSE' || {
     printf 'error: no LICENSE file found\n' >&2
 }
 
-grep -Fqx "## [$version] - $today" "$package/CHANGELOG.md" || {
-    success=false
-    printf 'error: release date for version %s is not set to %s in CHANGELOG.md\n' "$version" "$today" >&2
-}
+for changelog in "$package"/CHANGELOG*.md; do
+    grep -Fqx "## [$version] - $today" "$changelog" || {
+        success=false
+        printf 'error: release date for version %s is not set to %s in %s\n' \
+            "$version" "$today" "$changelog" >&2
+    }
+done
 
 "$success" # return the final exit status

yash-builtin/src/typeset.rs

@@ -533,6 +533,7 @@ impl From<AssignReadOnlyError> for yash_semantics::expansion::AssignReadOnlyErro
             name: e.name,
             new_value: e.new_value,
             read_only_location: e.read_only_location,
+            vacancy: None,
         }
     }
 }

yash-cli/CHANGELOG-bin.md

@@ -0,0 +1,48 @@
+# Changelog
+
+All notable changes to the shell are documented in this file.
+
+This file lists changes to the shell binary as a whole. Changes to the
+implementing library crate are in [CHANGELOG-lib.md](CHANGELOG-lib.md).
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
+and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [0.1.0-beta.3] - Unreleased
+
+### Changed
+
+- Improved error messages for some parameter expansion errors.
+
+## [0.1.0-beta.2] - 2024-07-13
+
+### Changed
+
+- The shell now shows the prompt before reading the input in the interactive mode.
+
+### Fixed
+
+- The break and continue built-ins no longer allow exiting a trap.
+- The read built-in now shows a prompt when reading a continued line.
+- The source built-in now echoes the input when the verbose shell option is set.
+- The set built-in no longer sets the `SIGTTIN`, `SIGTTOU`, and `SIGTSTP` signals
+  to be ignored when invoked with the `-m` option in a subshell of an
+  interactive shell.
+
+## [0.1.0-beta.1] - 2024-06-09
+
+### Changed
+
+- The shell now enables blocking reads on the standard input if it is a terminal
+  or a pipe as [required by POSIX](https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/utilities/sh.html#tag_20_117_06).
+
+## [0.1.0-alpha.1] - 2024-04-13
+
+### Added
+
+- Initial release of the shell
+
+[0.1.0-beta.3]: https://github.com/magicant/yash-rs/releases/tag/yash-cli-0.1.0-beta.3
+[0.1.0-beta.2]: https://github.com/magicant/yash-rs/releases/tag/yash-cli-0.1.0-beta.2
+[0.1.0-beta.1]: https://github.com/magicant/yash-rs/releases/tag/yash-cli-0.1.0-beta.1
+[0.1.0-alpha.1]: https://github.com/magicant/yash-rs/releases/tag/yash-cli-0.1.0-alpha.1

yash-cli/CHANGELOG.md → yash-cli/CHANGELOG-lib.md

@@ -1,6 +1,10 @@
 # Changelog
 
-All notable changes to `yash-cli` will be documented in this file.
+All notable changes to the `yash-cli` library crate are documented in this file.
+
+This file lists changes to the library crate, which is unlikely to be of interest
+to users of the shell.
+For changes to the shell binary as a whole, see [CHANGELOG-bin.md](CHANGELOG-bin.md).
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
@@ -35,15 +39,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
       is now implemented in `yash_env::input::Echo`, which is included in
       the `startup::SourceInput::input` field.
 
-### Fixed
-
-- The break and continue built-ins no longer allow exiting a trap.
-- The read built-in now shows a prompt when reading a continued line.
-- The source built-in now echoes the input when the verbose shell option is set.
-- The set built-in no longer sets the `SIGTTIN`, `SIGTTOU`, and `SIGTSTP` signals
-  to be ignored when invoked with the `-m` option in a subshell of an
-  interactive shell.
-
 ## [0.1.0-beta.1] - 2024-06-09
 
 ### Changed

yash-cli/README.md

@@ -8,7 +8,8 @@ programs.
 [![yash-cli at docs.rs](https://docs.rs/yash-cli/badge.svg)](https://docs.rs/yash-cli)
 [![Build status](https://github.com/magicant/yash-rs/actions/workflows/rust.yml/badge.svg)](https://github.com/magicant/yash-rs/actions/workflows/rust.yml)
 
-- [Changelog](CHANGELOG.md)
+- [Changelog for the entire shell binary](CHANGELOG-bin.md)
+- [Changelog for the library crate](CHANGELOG-lib.md)
 
 ## License
 

yash-semantics/CHANGELOG.md

@@ -5,6 +5,35 @@ All notable changes to `yash-semantics` will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.4.0] - Unreleased
+
+### Added
+
+- 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::footer` method has been added.
+    - The `Error` struct now has non-default implementation of the
+      `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::NonassignableErrorCause` enum is a successor to the previous
+      `NonassignableError` enum. The new `NotVariable` variant has a `name:
+      String` 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 `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.
+
 ## [0.3.0] - 2024-07-13
 
 ### Added
@@ -86,6 +115,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Initial implementation of the `yash-semantics` crate
 
+[0.4.0]: https://github.com/magicant/yash-rs/releases/tag/yash-semantics-0.4.0
 [0.3.0]: https://github.com/magicant/yash-rs/releases/tag/yash-semantics-0.3.0
 [0.2.0]: https://github.com/magicant/yash-rs/releases/tag/yash-semantics-0.2.0
 [0.1.0]: https://github.com/magicant/yash-rs/releases/tag/yash-semantics-0.1.0

yash-semantics/src/assign.rs

@@ -67,6 +67,7 @@ pub async fn perform_assignment(
                 name: assign.name.clone(),
                 new_value: e.new_value,
                 read_only_location: e.read_only_location,
+                vacancy: None,
             }),
             location: e.assigned_location.unwrap(),
         })?;
@@ -179,6 +180,7 @@ mod tests {
             assert_eq!(error.name, "v");
             assert_eq!(error.new_value, Value::scalar("new"));
             assert_eq!(error.read_only_location, location);
+            assert_eq!(error.vacancy, None);
         });
         assert_eq!(e.location, Location::dummy("v=new"));
     }

yash-semantics/src/command/compound_command/for_loop.rs

@@ -94,6 +94,7 @@ pub async fn execute(
                     name: name.value,
                     new_value: error.new_value,
                     read_only_location: error.read_only_location,
+                    vacancy: None,
                 });
                 let location = name.origin;
                 let error = Error { cause, location };

yash-semantics/src/expansion.rs

@@ -83,6 +83,7 @@ use self::initial::ArithError;
 use self::initial::Expand;
 use self::initial::Expand as _;
 use self::initial::NonassignableError;
+use self::initial::Vacancy;
 use self::initial::VacantError;
 use self::quote_removal::skip_quotes;
 use self::split::Ifs;
@@ -94,6 +95,7 @@ use yash_env::variable::Value;
 use yash_env::variable::IFS;
 use yash_syntax::source::pretty::Annotation;
 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::Text;
@@ -108,10 +110,17 @@ pub use yash_env::semantics::Field;
 pub struct AssignReadOnlyError {
     /// Name of the read-only variable
     pub name: String,
-    /// Value that was being assigned.
+    /// Value that was being assigned
     pub new_value: Value,
-    /// Location where the variable was made read-only.
+    /// Location where the variable was made read-only
     pub read_only_location: Location,
+    /// State of the variable before the assignment
+    ///
+    /// If this assignment error occurred in a parameter expansion as in
+    /// `${foo=bar}` or `${foo:=bar}`, this field is `Some`, and the value is
+    /// the state of the variable before the assignment. In other cases, this
+    /// field is `None`.
+    pub vacancy: Option<Vacancy>,
 }
 
 /// Types of errors that may occur in the word expansion.
@@ -130,8 +139,8 @@ pub enum ErrorCause {
     AssignReadOnly(#[from] AssignReadOnlyError),
 
     /// Expansion of an unset parameter with the `nounset` option
-    #[error("unset parameter")]
-    UnsetParameter,
+    #[error("unset parameter {name:?}")]
+    UnsetParameter { name: String },
 
     /// Expansion of an empty value with an error switch
     #[error(transparent)]
@@ -152,7 +161,7 @@ impl ErrorCause {
             CommandSubstError(_) => "error performing the command substitution",
             ArithError(_) => "error evaluating the arithmetic expansion",
             AssignReadOnly(_) => "error assigning to variable",
-            UnsetParameter => "unset parameter",
+            UnsetParameter { .. } => "cannot expand unset parameter",
             VacantExpansion(error) => error.message_or_default(),
             NonassignableParameter(_) => "cannot assign to parameter",
         }
@@ -167,8 +176,8 @@ impl ErrorCause {
             CommandSubstError(e) => e.to_string().into(),
             ArithError(e) => e.to_string().into(),
             AssignReadOnly(e) => e.to_string().into(),
-            UnsetParameter => "unset parameter disallowed by the nounset option".into(),
-            VacantExpansion(e) => e.vacancy.description().into(),
+            UnsetParameter { name } => format!("parameter {name:?} is not set").into(),
+            VacantExpansion(e) => format!("{}: {}", e.name, e.vacancy).into(),
             NonassignableParameter(e) => e.to_string().into(),
         }
     }
@@ -186,11 +195,26 @@ impl ErrorCause {
                 &e.read_only_location,
                 "the variable was made read-only here",
             )),
-            UnsetParameter => None,
+            UnsetParameter { .. } => None,
             VacantExpansion(_) => None,
             NonassignableParameter(_) => None,
         }
     }
+
+    /// Returns a footer message for the error.
+    #[must_use]
+    pub fn footer(&self) -> Option<&'static str> {
+        use ErrorCause::*;
+        match self {
+            CommandSubstError(_)
+            | ArithError(_)
+            | AssignReadOnly(_)
+            | VacantExpansion(_)
+            | NonassignableParameter(_) => None,
+
+            UnsetParameter { .. } => Some("unset parameters are disallowed by the nounset option"),
+        }
+    }
 }
 
 /// Explanation of an expansion failure.
@@ -219,6 +243,47 @@ impl MessageBase for Error {
                 location,
             )))
         }
+
+        // Report the vacancy that caused the assignment that led to the error.
+        let vacancy = match &self.cause {
+            ErrorCause::CommandSubstError(_) => None,
+            ErrorCause::ArithError(_) => None,
+            ErrorCause::AssignReadOnly(e) => e.vacancy,
+            ErrorCause::UnsetParameter { .. } => None,
+            ErrorCause::VacantExpansion(_) => None,
+            ErrorCause::NonassignableParameter(e) => Some(e.vacancy),
+        };
+        if let Some(vacancy) = vacancy {
+            let message = match vacancy {
+                Vacancy::Unset => "assignment was attempted because the parameter was not set",
+                Vacancy::EmptyScalar => {
+                    "assignment was attempted because the parameter was an empty string"
+                }
+                Vacancy::ValuelessArray => {
+                    "assignment was attempted because the parameter was an empty array"
+                }
+                Vacancy::EmptyValueArray => {
+                    "assignment was attempted because the parameter was an array of an empty string"
+                }
+            };
+            // TODO Use Extend::extend_one
+            results.extend(std::iter::once(Annotation::new(
+                AnnotationType::Info,
+                message.into(),
+                &self.location,
+            )));
+        }
+    }
+
+    fn footers(&self) -> Vec<Footer> {
+        self.cause
+            .footer()
+            .into_iter()
+            .map(|label| Footer {
+                r#type: AnnotationType::Info,
+                label: label.into(),
+            })
+            .collect()
     }
 }
 
@@ -377,6 +442,7 @@ mod tests {
                 name: "foo".into(),
                 new_value: "value".into(),
                 read_only_location: Location::dummy("ROL"),
+                vacancy: None,
             }),
             location: Location {
                 range: 2..4,
@@ -401,6 +467,33 @@ mod tests {
         assert_eq!(message.annotations[1].location, &Location::dummy("ROL"));
     }
 
+    #[test]
+    fn from_error_for_message_with_vacancy() {
+        let error = Error {
+            cause: ErrorCause::AssignReadOnly(AssignReadOnlyError {
+                name: "foo".into(),
+                new_value: "value".into(),
+                read_only_location: Location::dummy("ROL"),
+                vacancy: Some(Vacancy::EmptyScalar),
+            }),
+            location: Location {
+                range: 2..4,
+                ..Location::dummy("hello")
+            },
+        };
+
+        let message = Message::from(&error);
+        assert!(
+            message.annotations.iter().any(|a| {
+                a.r#type == AnnotationType::Info
+                    && a.label
+                        == "assignment was attempted because the parameter was an empty string"
+                    && a.location == &error.location
+            }),
+            "{message:?}"
+        );
+    }
+
     #[test]
     fn expand_words_performs_initial_expansion() {
         in_virtual_system(|mut env, _state| async move {