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

Fix most of the clippy warnings #74

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
3 changes: 1 addition & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ cache:
- target

rust:
- 1.16.0
- 1.17.0
- 1.46.0
- stable
- nightly
- beta
Expand Down
24 changes: 12 additions & 12 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl MapBuilder {
let MapBuilder { mut data } = self;
let value = to_data(value)?;
data.insert(key.into(), value);
Ok(MapBuilder { data: data })
Ok(MapBuilder { data })
}

/// Add a `String` to the `MapBuilder`.
Expand All @@ -56,7 +56,7 @@ impl MapBuilder {
{
let MapBuilder { mut data } = self;
data.insert(key.into(), Data::String(value.into()));
MapBuilder { data: data }
MapBuilder { data }
}

/// Add a `bool` to the `MapBuilder`.
Expand All @@ -74,7 +74,7 @@ impl MapBuilder {
{
let MapBuilder { mut data } = self;
data.insert(key.into(), Data::Bool(value));
MapBuilder { data: data }
MapBuilder { data }
}

/// Add a `Vec` to the `MapBuilder`.
Expand All @@ -97,7 +97,7 @@ impl MapBuilder {
let MapBuilder { mut data } = self;
let builder = f(VecBuilder::new());
data.insert(key.into(), builder.build());
MapBuilder { data: data }
MapBuilder { data }
}

/// Add a `Map` to the `MapBuilder`.
Expand Down Expand Up @@ -126,7 +126,7 @@ impl MapBuilder {
let MapBuilder { mut data } = self;
let builder = f(MapBuilder::new());
data.insert(key.into(), builder.build());
MapBuilder { data: data }
MapBuilder { data }
}

/// Add a function to the `MapBuilder`.
Expand All @@ -147,7 +147,7 @@ impl MapBuilder {
{
let MapBuilder { mut data } = self;
data.insert(key.to_string(), Data::Fun(RefCell::new(Box::new(f))));
MapBuilder { data: data }
MapBuilder { data }
}

/// Return the built `Data`.
Expand Down Expand Up @@ -183,7 +183,7 @@ impl VecBuilder {
let VecBuilder { mut data } = self;
let value = to_data(value)?;
data.push(value);
Ok(VecBuilder { data: data })
Ok(VecBuilder { data })
}

/// Add a `String` to the `VecBuilder`.
Expand All @@ -199,7 +199,7 @@ impl VecBuilder {
pub fn push_str<T: ToString>(self, value: T) -> VecBuilder {
let VecBuilder { mut data } = self;
data.push(Data::String(value.to_string()));
VecBuilder { data: data }
VecBuilder { data }
}

/// Add a `bool` to the `VecBuilder`.
Expand All @@ -215,7 +215,7 @@ impl VecBuilder {
pub fn push_bool(self, value: bool) -> VecBuilder {
let VecBuilder { mut data } = self;
data.push(Data::Bool(value));
VecBuilder { data: data }
VecBuilder { data }
}

/// Add a `Vec` to the `MapBuilder`.
Expand All @@ -237,7 +237,7 @@ impl VecBuilder {
let VecBuilder { mut data } = self;
let builder = f(VecBuilder::new());
data.push(builder.build());
VecBuilder { data: data }
VecBuilder { data }
}

/// Add a `Map` to the `VecBuilder`.
Expand All @@ -264,7 +264,7 @@ impl VecBuilder {
let VecBuilder { mut data } = self;
let builder = f(MapBuilder::new());
data.push(builder.build());
VecBuilder { data: data }
VecBuilder { data }
}

/// Add a function to the `VecBuilder`.
Expand All @@ -285,7 +285,7 @@ impl VecBuilder {
{
let VecBuilder { mut data } = self;
data.push(Data::Fun(RefCell::new(Box::new(f))));
VecBuilder { data: data }
VecBuilder { data }
}

#[inline]
Expand Down
18 changes: 6 additions & 12 deletions src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ impl<T: Iterator<Item = char>> Compiler<T> {
/// Construct a default compiler.
pub fn new(ctx: Context, reader: T) -> Compiler<T> {
Compiler {
ctx: ctx,
reader: reader,
ctx,
reader,
partials: HashMap::new(),
otag: "{{".to_string(),
ctag: "}}".to_string(),
Expand All @@ -38,20 +38,14 @@ impl<T: Iterator<Item = char>> Compiler<T> {
otag: String,
ctag: String)
-> Compiler<T> {
Compiler {
ctx: ctx,
reader: reader,
partials: partials,
otag: otag,
ctag: ctag,
}
Compiler { ctx, reader, partials, otag, ctag }
}

/// Compiles a template into a series of tokens.
pub fn compile(mut self) -> Result<(Vec<Token>, PartialsMap)> {
let (tokens, partials) = {
let parser = Parser::new(&mut self.reader, &self.otag, &self.ctag);
try!(parser.parse())
parser.parse()?
};

// Compile the partials if we haven't done so already.
Expand All @@ -66,7 +60,7 @@ impl<T: Iterator<Item = char>> Compiler<T> {
match File::open(&path) {
Ok(mut file) => {
let mut string = String::new();
try!(file.read_to_string(&mut string));
file.read_to_string(&mut string)?;

let compiler = Compiler {
ctx: self.ctx.clone(),
Expand All @@ -76,7 +70,7 @@ impl<T: Iterator<Item = char>> Compiler<T> {
ctag: "}}".to_string(),
};

let (tokens, subpartials) = try!(compiler.compile());
let (tokens, subpartials) = compiler.compile()?;

// Include subpartials
self.partials.extend(subpartials.into_iter());
Expand Down
8 changes: 4 additions & 4 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl Context {
/// Compiles a template from a string
pub fn compile<IT: Iterator<Item = char>>(&self, reader: IT) -> Result<Template> {
let compiler = compiler::Compiler::new(self.clone(), reader);
let (tokens, partials) = try!(compiler.compile());
let (tokens, partials) = compiler.compile()?;

Ok(template::new(self.clone(), tokens, partials))
}
Expand All @@ -49,8 +49,8 @@ impl Context {
let mut path = self.template_path.join(path.as_ref());
path.set_extension(&self.template_extension);
let mut s = vec![];
let mut file = try!(File::open(&path));
try!(file.read_to_end(&mut s));
let mut file = File::open(&path)?;
file.read_to_end(&mut s)?;

// TODO: maybe allow UTF-16 as well?
let template = match str::from_utf8(&*s) {
Expand All @@ -62,4 +62,4 @@ impl Context {

self.compile(template.chars())
}
}
}
12 changes: 9 additions & 3 deletions src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ use std::collections::HashMap;
use std::cell::RefCell;
use std::fmt;

// for bug!
use log::{log, error};

pub enum Data {
Null,
String(String),
Bool(bool),
Vec(Vec<Data>),
Map(HashMap<String, Data>),
Fun(RefCell<Box<FnMut(String) -> String + Send>>),
Fun(RefCell<Box<dyn FnMut(String) -> String + Send>>),
}

impl PartialEq for Data {
Expand All @@ -20,7 +23,10 @@ impl PartialEq for Data {
(&Data::Bool(ref v0), &Data::Bool(ref v1)) => v0 == v1,
(&Data::Vec(ref v0), &Data::Vec(ref v1)) => v0 == v1,
(&Data::Map(ref v0), &Data::Map(ref v1)) => v0 == v1,
(&Data::Fun(_), &Data::Fun(_)) => bug!("Cannot compare closures"),
(&Data::Fun(_), &Data::Fun(_)) => {
bug!("Cannot compare closures");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a bug, or simply always false, i.e. no two closures are equal, even to themselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wasn't really sure, so I left it as is. I can see an argument where closures aren't reflexive (and they don't implement PartialEq, which is a hint that the Rust developers agree), but I can also see how users might reasonably assert that some closures are be equal.

I think we run into the halting problem, and overall asserting that they're not equal is the right thing to do, but logging a message might tell the middleware developer there's a problem and that's why the template isn't firing. Since bug! is now a wrapper around error! I think it's okay as is.

false
},
(_, _) => false,
}
}
Expand All @@ -37,4 +43,4 @@ impl fmt::Debug for Data {
Data::Fun(_) => write!(f, "Fun(...)"),
}
}
}
}
35 changes: 13 additions & 22 deletions src/encoder.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::collections::HashMap;
use std::error;
use std::error::Error as StdError;
use std::fmt::{self, Display};
use std::result;
use std::result::Result as StdResult;

use serde::{self, Serialize, ser};

Expand All @@ -12,16 +12,14 @@ use super::{Data, to_data};
/// This type is not intended to be matched exhaustively as new variants
/// may be added in future without a version bump.
#[derive(Debug)]
#[non_exhaustive]
pub enum Error {
NestedOptions,
UnsupportedType,
MissingElements,
KeyIsNotString,
NoDataToEncode,
Message(String),

#[doc(hidden)]
__Nonexhaustive,
}

impl serde::ser::Error for Error {
Expand All @@ -31,29 +29,23 @@ impl serde::ser::Error for Error {
}

/// Alias for a `Result` with the error type `mustache::encoder::Error`.
pub type Result<T> = result::Result<T, Error>;
pub type Result<T> = StdResult<T, Error>;

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use std::error::Error;
self.description().fmt(f)
}
}

impl error::Error for Error {
fn description(&self) -> &str {
match *self {
write!(f, "{}", match *self {
Error::NestedOptions => "nested Option types are not supported",
Error::UnsupportedType => "unsupported type",
Error::MissingElements => "no elements in value",
Error::KeyIsNotString => "key is not a string",
Error::NoDataToEncode => "the encodable type created no data",
Error::Message(ref s) => s,
Error::__Nonexhaustive => unreachable!(),
}
})
}
}

impl StdError for Error { }

#[derive(Default)]
pub struct Encoder;

Expand Down Expand Up @@ -328,7 +320,7 @@ impl ser::SerializeTupleVariant for SerializeTupleVariant {
fn serialize_field<T: ?Sized>(&mut self, value: &T) -> Result<()>
where T: Serialize
{
self.vec.push(try!(to_data(&value)));
self.vec.push(to_data(&value)?);
Ok(())
}

Expand Down Expand Up @@ -362,11 +354,10 @@ impl ser::SerializeMap for SerializeMap {
where
T: Serialize
{
let key = self.next_key.take();
// Panic because this indicates a bug in the program rather than an
// expected failure.
let key = key.expect("serialize_value called before serialize_key");
self.map.insert(key, try!(to_data(&value)));
// Taking the key should only fail if this gets called before
// serialize_key, which is a bug in the library.
let key = self.next_key.take().ok_or(Error::MissingElements)?;
self.map.insert(key, to_data(&value)?);
Ok(())
}

Expand Down
32 changes: 12 additions & 20 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::error::Error as StdError;
use std::fmt;
use std::io::Error as StdIoError;
use std::result;
use std::result::Result as StdResult;

use parser;
use encoder;
Expand All @@ -11,35 +10,28 @@ use encoder;
/// This type is not intended to be matched exhaustively as new variants
/// may be added in future without a version bump.
#[derive(Debug)]
#[non_exhaustive]
pub enum Error {
InvalidStr,
NoFilename,
IncompleteSection,
Io(StdIoError),
Parser(parser::Error),
Encoder(encoder::Error),

#[doc(hidden)]
__Nonexhaustive,
}

pub type Result<T> = result::Result<T, Error>;
pub type Result<T> = StdResult<T, Error>;

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
self.description().fmt(f)
}
}

impl StdError for Error {
fn description(&self) -> &str {
match *self {
Error::InvalidStr => "invalid str",
Error::NoFilename => "a filename must be provided",
Error::Io(ref err) => err.description(),
Error::Parser(ref err) => err.description(),
Error::Encoder(ref err) => err.description(),
Error::__Nonexhaustive => unreachable!(),
}
write!(f, "{}", match *self {
Error::InvalidStr => "invalid str".to_string(),
Error::NoFilename => "a filename must be provided".to_string(),
Error::IncompleteSection => "a section wasn't completed".to_string(), // Is there a better way to put this?
Error::Io(ref err) => err.to_string(),
Error::Parser(ref err) => err.to_string(),
Error::Encoder(ref err) => err.to_string(),
})
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub fn compile_path<U: AsRef<Path>>(path: U) -> Result<Template> {

match path.file_name() {
Some(filename) => {
let template_dir = path.parent().unwrap_or(Path::new("."));
let template_dir = path.parent().unwrap_or_else(|| Path::new("."));
// FIXME: Should work with OsStrings, this will not use provided extension if
// the extension is not utf8 :(
let extension = path.extension().and_then(|ext| ext.to_str()).unwrap_or("mustache");
Expand Down
4 changes: 1 addition & 3 deletions src/macros.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
/// This should be the only place to panic inside non-test code.
// TODO: ensure no panics elsewhere via clippy
macro_rules! bug {
($msg:expr) => ({
bug!("{}", $msg)
});
($fmt:expr, $($arg:tt)+) => ({
panic!(
error!(
concat!("bug: ",
$fmt,
". Please report this issue on GitHub if you find \
Expand Down
Loading