Skip to content

Commit

Permalink
Clean up string module
Browse files Browse the repository at this point in the history
- Rename `string::escape_unicode` to
  `string::format_unicode_debug_into`.
- Rename writer type parameters to `W` for int and unicode debug
  formatters.
- Remove `&mut` from writer parameter.
- Remove the `impl From<fmt::Error> for WriteError` conversion.
- Sync docs for `format_unicode_debug_into` from BurntSushi/bstr#37.

While fixing up the use of `escape_unicode` in `artichoke-frontend`, I
got a bit distracted and let the scope of this change creep.

- Unify Ruby CLI errors on `Exception`. Imports `IOError` and
  `LoadError` to manually construct some errors from strings.
- Do not use `eprintln!` in `artichoke` binary to suppress broken pipe
  errors instead of panicking.
- Set exit code to 1 on error in `artichoke` binary.
  • Loading branch information
lopopolo committed Feb 22, 2020
1 parent 43351cb commit e191001
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 101 deletions.
5 changes: 3 additions & 2 deletions artichoke-backend/src/exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,11 @@ impl CaughtException {
}

impl fmt::Display for CaughtException {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fn fmt(&self, mut f: &mut fmt::Formatter) -> fmt::Result {
let classname = self.name();
write!(f, "{} (", classname)?;
string::escape_unicode(f, self.message()).map_err(string::WriteError::into_inner)?;
string::format_unicode_debug_into(&mut f, self.message())
.map_err(string::WriteError::into_inner)?;
write!(f, ")")
}
}
Expand Down
4 changes: 2 additions & 2 deletions artichoke-backend/src/extn/core/exception/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,10 +350,10 @@ macro_rules! ruby_exception_impl {
}

impl fmt::Display for $exception {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fn fmt(&self, mut f: &mut fmt::Formatter) -> fmt::Result {
let classname = self.name();
write!(f, "{} (", classname)?;
string::escape_unicode(f, self.message())
string::format_unicode_debug_into(&mut f, self.message())
.map_err(string::WriteError::into_inner)?;
write!(f, ")")
}
Expand Down
2 changes: 1 addition & 1 deletion artichoke-backend/src/extn/core/kernel/integer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ pub fn method(interp: &Artichoke, arg: Value, radix: Option<Value>) -> Result<Va

fn invalid_value_err(interp: &Artichoke, arg: &[u8]) -> Result<ArgumentError, Exception> {
let mut message = String::from(r#"invalid value for Integer(): ""#);
string::escape_unicode(&mut message, arg)?;
string::format_unicode_debug_into(&mut message, arg)?;
message.push('"');
Ok(ArgumentError::new(interp, message))
}
Expand Down
10 changes: 5 additions & 5 deletions artichoke-backend/src/extn/core/kernel/require.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub fn load(interp: &mut Artichoke, filename: Value) -> Result<Value, Exception>
}
let _ = interp.pop_context();
let mut logged_filename = String::new();
string::escape_unicode(&mut logged_filename, filename)?;
string::format_unicode_debug_into(&mut logged_filename, filename)?;
trace!(r#"Successful load of "{}" at {:?}"#, logged_filename, path,);
Ok(interp.convert(true))
}
Expand Down Expand Up @@ -131,7 +131,7 @@ pub fn require(
return Err(Exception::from(load_error(interp, b"internal error")?));
}
let mut logged_filename = String::new();
string::escape_unicode(&mut logged_filename, filename)?;
string::format_unicode_debug_into(&mut logged_filename, filename)?;
trace!(
r#"Successful require of "{}" at {:?}"#,
logged_filename,
Expand Down Expand Up @@ -187,7 +187,7 @@ pub fn require(
return Err(Exception::from(load_error(interp, b"internal error")?));
}
let mut logged_filename = String::new();
string::escape_unicode(&mut logged_filename, filename)?;
string::format_unicode_debug_into(&mut logged_filename, filename)?;
trace!(
r#"Successful require of "{}" at {:?}"#,
logged_filename,
Expand Down Expand Up @@ -243,7 +243,7 @@ pub fn require(
return Err(Exception::from(load_error(interp, b"internal error")?));
}
let mut logged_filename = String::new();
string::escape_unicode(&mut logged_filename, filename)?;
string::format_unicode_debug_into(&mut logged_filename, filename)?;
trace!(
r#"Successful require of "{}" at {:?}"#,
logged_filename,
Expand Down Expand Up @@ -273,6 +273,6 @@ pub fn require_relative(interp: &mut Artichoke, file: Value) -> Result<Value, Ex

fn load_error(interp: &Artichoke, filename: &[u8]) -> Result<LoadError, Exception> {
let mut message = String::from("cannot load such file -- ");
string::escape_unicode(&mut message, filename)?;
string::format_unicode_debug_into(&mut message, filename)?;
Ok(LoadError::new(interp, message))
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ pub fn method(
Ok(interp.convert_mut(group))
} else {
let mut message = String::from("undefined group name reference: \"");
string::escape_unicode(&mut message, name)?;
string::format_unicode_debug_into(&mut message, name)?;
message.push('"');
Err(Exception::from(IndexError::new(interp, message)))
}
Expand Down
60 changes: 31 additions & 29 deletions artichoke-backend/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,46 +15,54 @@ use crate::extn::core::exception::Fatal;
use crate::sys;
use crate::{Artichoke, ConvertMut};

/// Write a UTF-8 representation of a (potentially) binary `String`.
/// Write a UTF-8 debug representation of a byte slice into the given writer.
///
/// This function encodes a bytes slice into a UTF-8 valid representation by
/// This method encodes a bytes slice into a UTF-8 valid representation by
/// writing invalid sequences as `\xXX` escape codes.
///
/// This function uses `char::escape_debug` which means UTF-8 valid characters
/// like `\n` and `\t` are also escaped.
/// This method also escapes UTF-8 valid characters like `\n` and `\t`.
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// # use artichoke_backend::string::escape_unicode;
/// # use artichoke_backend::string::format_unicode_debug_into;
///
/// let mut message = String::from("cannot load such file -- ");
/// let filename = b"oh-no-\xFF";
/// escape_unicode(&mut message, &filename[..]);
/// assert_eq!(r"cannot load such file -- oh-no-\xFF", message);
/// let filename = b"utf8-invalid-name-\xFF";
/// format_unicode_debug_into(&mut message, &filename[..]);
/// assert_eq!(r"cannot load such file -- utf8-invalid-name-\xFF", message);
/// ```
pub fn escape_unicode<T>(f: &mut T, string: &[u8]) -> Result<(), WriteError>
///
/// # Errors
///
/// This method only returns an error when the given writer returns an
/// error.
pub fn format_unicode_debug_into<W>(mut f: W, string: &[u8]) -> Result<(), WriteError>
where
T: fmt::Write,
W: fmt::Write,
{
let buf = bstr::B(string);
for (start, end, ch) in buf.char_indices() {
for (start, end, ch) in string.char_indices() {
if ch == '\u{FFFD}' {
for byte in buf[start..end].as_bytes() {
write!(f, r"\x{:X}", byte)?;
if let Some(slice) = string.get(start..end) {
for byte in slice {
write!(f, r"\x{:X}", byte).map_err(WriteError)?;
}
}
} else {
write!(f, "{}", ch.escape_debug())?;
write!(f, "{}", ch.escape_debug()).map_err(WriteError)?;
}
}
Ok(())
}

pub fn format_int_into<T, I>(f: &mut T, value: I) -> Result<(), WriteError>
pub fn format_int_into<W, I>(f: W, value: I) -> Result<(), WriteError>
where
T: fmt::Write,
W: fmt::Write,
I: itoa::Integer,
{
itoa::fmt(f, value)?;
itoa::fmt(f, value).map_err(WriteError)?;
Ok(())
}

Expand All @@ -71,12 +79,6 @@ impl WriteError {
}
}

impl From<fmt::Error> for WriteError {
fn from(err: fmt::Error) -> Self {
Self(err)
}
}

impl fmt::Display for WriteError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "Unable to write escaped Unicode into destination")
Expand Down Expand Up @@ -144,33 +146,33 @@ impl From<Box<WriteError>> for Box<dyn RubyException> {

#[cfg(test)]
mod tests {
use super::escape_unicode;
use super::format_unicode_debug_into;

#[test]
fn invalid_utf8() {
let mut buf = String::new();
escape_unicode(&mut buf, &b"abc\xFF"[..]).unwrap();
format_unicode_debug_into(&mut buf, &b"abc\xFF"[..]).unwrap();
assert_eq!(r"abc\xFF", buf.as_str());
}

#[test]
fn ascii() {
let mut buf = String::new();
escape_unicode(&mut buf, &b"abc"[..]).unwrap();
format_unicode_debug_into(&mut buf, &b"abc"[..]).unwrap();
assert_eq!(r"abc", buf.as_str());
}

#[test]
fn emoji() {
let mut buf = String::new();
escape_unicode(&mut buf, "Ruby 💎".as_bytes()).unwrap();
format_unicode_debug_into(&mut buf, "Ruby 💎".as_bytes()).unwrap();
assert_eq!(r"Ruby 💎", buf.as_str());
}

#[test]
fn escaped() {
let mut buf = String::new();
escape_unicode(&mut buf, b"\n").unwrap();
format_unicode_debug_into(&mut buf, b"\n").unwrap();
assert_eq!(r"\n", buf.as_str());
}
}
11 changes: 6 additions & 5 deletions artichoke-frontend/src/bin/artichoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@
//! <programfile>
//! ```
use artichoke_frontend::ruby::{self, Error};
use artichoke_frontend::ruby;
use std::io::{self, Write};
use std::process;

fn main() {
match ruby::entrypoint() {
Ok(_) => {}
Err(Error::Ruby(err)) => eprintln!("{}", err),
Err(Error::Fail(err)) => eprintln!("{}", err),
if let Err(err) = ruby::entrypoint() {
let _ = writeln!(io::stderr(), "{}", err);
process::exit(1);
}
}
85 changes: 29 additions & 56 deletions artichoke-frontend/src/ruby.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
//! Exported as `ruby` and `artichoke` binaries.
use artichoke_backend::exception::Exception;
use artichoke_backend::extn::core::exception::{IOError, LoadError};
use artichoke_backend::ffi;
use artichoke_backend::state::parser::Context;
use artichoke_backend::string;
Expand Down Expand Up @@ -46,39 +47,12 @@ struct Opt {
programfile: Option<PathBuf>,
}

/// Error from Ruby CLI frontend
#[derive(Debug)]
pub enum Error {
/// Ruby `Exception` thrown during eval.
Ruby(Exception),
/// Fatal error from CLI internals.
Fail(String),
}

impl From<Exception> for Error {
fn from(err: Exception) -> Self {
Self::Ruby(err)
}
}

impl From<String> for Error {
fn from(err: String) -> Self {
Self::Fail(err)
}
}

impl From<&'static str> for Error {
fn from(err: &'static str) -> Self {
Self::Fail(err.to_owned())
}
}

/// Main entrypoint for Artichoke's version of the `ruby` CLI.
///
/// # Errors
///
/// If an exception is raised on the interpreter, then an error is returned.
pub fn entrypoint() -> Result<(), Error> {
pub fn entrypoint() -> Result<(), Exception> {
let opt = Opt::from_args();
if opt.copyright {
let mut interp = artichoke_backend::interpreter()?;
Expand All @@ -89,17 +63,17 @@ pub fn entrypoint() -> Result<(), Error> {
} else if let Some(programfile) = opt.programfile {
execute_program_file(programfile.as_path(), opt.fixture.as_ref().map(Path::new))
} else {
let mut interp = artichoke_backend::interpreter()?;
let mut program = vec![];
io::stdin()
.read_to_end(&mut program)
.map_err(|_| "Could not read program from STDIN")?;
let mut interp = artichoke_backend::interpreter()?;
.map_err(|_| IOError::new(&interp, "Could not read program from STDIN"))?;
let _ = interp.eval(program.as_slice())?;
Ok(())
}
}

fn execute_inline_eval(commands: Vec<OsString>, fixture: Option<&Path>) -> Result<(), Error> {
fn execute_inline_eval(commands: Vec<OsString>, fixture: Option<&Path>) -> Result<(), Exception> {
let mut interp = artichoke_backend::interpreter()?;
// safety:
//
Expand All @@ -111,10 +85,10 @@ fn execute_inline_eval(commands: Vec<OsString>, fixture: Option<&Path>) -> Resul
let data = if let Ok(data) = std::fs::read(fixture) {
data
} else {
return Err(Error::from(load_error(
fixture.as_os_str(),
"No such file or directory",
)?));
return Err(Exception::from(LoadError::new(
&interp,
load_error(fixture.as_os_str(), "No such file or directory")?,
)));
};
let sym = interp.intern_symbol(&b"$fixture"[..]);
let mrb = interp.0.borrow().mrb;
Expand All @@ -129,16 +103,16 @@ fn execute_inline_eval(commands: Vec<OsString>, fixture: Option<&Path>) -> Resul
Ok(())
}

fn execute_program_file(programfile: &Path, fixture: Option<&Path>) -> Result<(), Error> {
fn execute_program_file(programfile: &Path, fixture: Option<&Path>) -> Result<(), Exception> {
let mut interp = artichoke_backend::interpreter()?;
if let Some(ref fixture) = fixture {
let data = if let Ok(data) = std::fs::read(fixture) {
data
} else {
return Err(Error::from(load_error(
fixture.as_os_str(),
"No such file or directory",
)?));
return Err(Exception::from(LoadError::new(
&interp,
load_error(fixture.as_os_str(), "No such file or directory")?,
)));
};
let sym = interp.intern_symbol(&b"$fixture"[..]);
let mrb = interp.0.borrow().mrb;
Expand All @@ -151,30 +125,29 @@ fn execute_program_file(programfile: &Path, fixture: Option<&Path>) -> Result<()
Ok(programfile) => programfile,
Err(err) => {
return match err.kind() {
io::ErrorKind::NotFound => Err(Error::from(load_error(
programfile.as_os_str(),
"No such file or directory",
)?)),
io::ErrorKind::PermissionDenied => Err(Error::from(load_error(
programfile.as_os_str(),
"Permission denied",
)?)),
_ => Err(Error::from(load_error(
programfile.as_os_str(),
"Could not read file",
)?)),
io::ErrorKind::NotFound => Err(Exception::from(LoadError::new(
&interp,
load_error(programfile.as_os_str(), "No such file or directory")?,
))),
io::ErrorKind::PermissionDenied => Err(Exception::from(LoadError::new(
&interp,
load_error(programfile.as_os_str(), "Permission denied")?,
))),
_ => Err(Exception::from(LoadError::new(
&interp,
load_error(programfile.as_os_str(), "Could not read file")?,
))),
}
}
};
let _ = interp.eval(program.as_slice())?;
Ok(())
}

fn load_error(file: &OsStr, message: &str) -> Result<String, Error> {
fn load_error(file: &OsStr, message: &str) -> Result<String, Exception> {
let mut buf = String::from(message);
buf.push_str(" -- ");
let path = ffi::os_str_to_bytes(file).map_err(Exception::from)?;
string::escape_unicode(&mut buf, &path).map_err(Exception::from)?;
buf.push_str(" (LoadError)");
let path = ffi::os_str_to_bytes(file)?;
string::format_unicode_debug_into(&mut buf, &path)?;
Ok(buf)
}

0 comments on commit e191001

Please sign in to comment.