Add clippy to CI + address clippy problems

.travis.yml

@@ -11,14 +11,23 @@ branches:
  - master
 
 matrix:
+ allow_failures:
+ - name: "Clippy: stable"
  include:
  - rust: stable
+ name: "Format: stable"
  script:
  - rustup component add rustfmt
  - cargo fmt -- --check
+ - rust: stable
+ name: "Clippy: stable"
+ script:
+ - rustup component add clippy
+ - cargo clippy
 
 cache: cargo
 
 script:
  - cargo test
  - cargo test --all-features
+

examples/transcode.rs

@@ -25,7 +25,7 @@ fn main() {
  )
  "#;
 
- let value = Value::from_str(data).expect("Failed to deserialize");
+ let value: Value = data.parse().expect("Failed to deserialize");
  let mut ser = serde_json::Serializer::pretty(std::io::stdout());
  value.serialize(&mut ser).expect("Failed to serialize");
 }

src/de/mod.rs

@@ -26,6 +26,8 @@ pub struct Deserializer<'de> {
 }
 
 impl<'de> Deserializer<'de> {
+ // Cannot implement trait here since output is tied to input lifetime 'de.
+ #[allow(clippy::should_implement_trait)]
  pub fn from_str(input: &'de str) -> Result<Self> {
  Deserializer::from_bytes(input.as_bytes())
  }
@@ -102,18 +104,18 @@ impl<'de> Deserializer<'de> {
  // Create a working copy
  let mut bytes = self.bytes;
 
- match bytes.consume("(") {
- true => {
- bytes.skip_ws()?;
+ if bytes.consume("(") {
+ bytes.skip_ws()?;
 
-  match bytes.check_tuple_struct()? {
-  // first argument is technically incorrect, but ignored anyway
-  true => self.deserialize_tuple(0, visitor),
-  // first two arguments are technically incorrect, but ignored anyway
-  false => self.deserialize_struct("", &[], visitor),
- }
+ if bytes.check_tuple_struct()? {
+ // first argument is technically incorrect, but ignored anyway
+ self.deserialize_tuple(0, visitor)
+ } else {
+ // first two arguments are technically incorrect, but ignored anyway
+ self.deserialize_struct("", &[], visitor)
  }
- false => visitor.visit_unit(),
+ } else {
+ visitor.visit_unit()
  }
  }
 }

src/de/value.rs

@@ -10,9 +10,11 @@ use crate::{
  value::{Map, Number, Value},
 };
 
-impl Value {
+impl std::str::FromStr for Value {
+ type Err = de::Error;
+
  /// Creates a value from a string reference.
- pub fn from_str(s: &str) -> de::Result<Self> {
+ fn from_str(s: &str) -> de::Result<Self> {
  let mut de = super::Deserializer::from_str(s)?;
 
  let val = Value::deserialize(&mut de)?;
@@ -166,9 +168,10 @@ impl<'de> Visitor<'de> for ValueVisitor {
 #[cfg(test)]
 mod tests {
  use super::*;
+ use std::str::FromStr;
 
  fn eval(s: &str) -> Value {
- Value::from_str(s).expect("Failed to parse")
+ s.parse().expect("Failed to parse")
  }
 
  #[test]

src/parse.rs

@@ -155,6 +155,9 @@ impl<'a> Bytes<'a> {
  }
 
  pub fn any_num(&mut self) -> Result<AnyNum> {
+ // We are not doing float comparisons here in the traditional sense.
+ // Instead, this code checks if a f64 fits inside an f32.
+ #[allow(clippy::float_cmp)]
  fn any_float(f: f64) -> Result<AnyNum> {
  if f == f as f32 as f64 {
  Ok(AnyNum::F32(f as f32))
@@ -378,10 +381,8 @@ impl<'a> Bytes<'a> {
  }
 
  pub fn expect_byte(&mut self, byte: u8, error: ParseError) -> Result<()> {
- self.eat_byte().and_then(|b| match b == byte {
- true => Ok(()),
- false => self.err(error),
- })
+ self.eat_byte()
+ .and_then(|b| if b == byte { Ok(()) } else { self.err(error) })
  }
 
  /// Returns the extensions bit mask.
@@ -424,9 +425,10 @@ impl<'a> Bytes<'a> {
 
  self.skip_ws()?;
 
- match self.consume_all(&[")", "]"])? {
- true => Ok(extensions),
- false => Err(self.error(ParseError::ExpectedAttributeEnd)),
+ if self.consume_all(&[")", "]"])? {
+ Ok(extensions)
+ } else {
+ Err(self.error(ParseError::ExpectedAttributeEnd))
  }
  }
 
@@ -714,9 +716,8 @@ impl<'a> Bytes<'a> {
  }
 
  self.expect_byte(b'}', ParseError::InvalidEscape("No } at the end"))?;
- let character = char_from_u32(bytes)
- .ok_or_else(|| self.error(ParseError::InvalidEscape("Not a valid char")))?;
- character
+ char_from_u32(bytes)
+ .ok_or_else(|| self.error(ParseError::InvalidEscape("Not a valid char")))?
  }
  _ => {
  return self.err(ParseError::InvalidEscape("Unknown escape character"));

src/value.rs

@@ -153,10 +153,7 @@ impl Number {
 /// underlying `f64` values itself.
 impl PartialEq for Number {
  fn eq(&self, other: &Self) -> bool {
- if self.0.is_nan() && other.0.is_nan() {
- return true;
- }
- return self.0 == other.0;
+ self.0.is_nan() && other.0.is_nan() || self.0 == other.0
  }
 }
 

tests/value.rs

@@ -3,61 +3,55 @@ use serde::Serialize;
 
 #[test]
 fn bool() {
- assert_eq!(Value::from_str("true"), Ok(Value::Bool(true)));
- assert_eq!(Value::from_str("false"), Ok(Value::Bool(false)));
+ assert_eq!("true".parse(), Ok(Value::Bool(true)));
+ assert_eq!("false".parse(), Ok(Value::Bool(false)));
 }
 
 #[test]
 fn char() {
- assert_eq!(Value::from_str("'a'"), Ok(Value::Char('a')));
+ assert_eq!("'a'".parse(), Ok(Value::Char('a')));
 }
 
 #[test]
 fn map() {
  let mut map = Map::new();
  map.insert(Value::Char('a'), Value::Number(Number::new(1f64)));
  map.insert(Value::Char('b'), Value::Number(Number::new(2f64)));
- assert_eq!(Value::from_str("{ 'a': 1, 'b': 2 }"), Ok(Value::Map(map)));
+ assert_eq!("{ 'a': 1, 'b': 2 }".parse(), Ok(Value::Map(map)));
 }
 
 #[test]
 fn number() {
- assert_eq!(Value::from_str("42"), Ok(Value::Number(Number::new(42f64))));
- assert_eq!(
- Value::from_str("3.1415"),
- Ok(Value::Number(Number::new(3.1415f64)))
- );
+ assert_eq!("42".parse(), Ok(Value::Number(Number::new(42f64))));
+ assert_eq!("3.1415".parse(), Ok(Value::Number(Number::new(3.1415f64))));
 }
 
 #[test]
 fn option() {
  let opt = Some(Box::new(Value::Char('c')));
- assert_eq!(Value::from_str("Some('c')"), Ok(Value::Option(opt)));
+ assert_eq!("Some('c')".parse(), Ok(Value::Option(opt)));
 }
 
 #[test]
 fn string() {
  let normal = "\"String\"";
- assert_eq!(Value::from_str(normal), Ok(Value::String("String".into())));
+ assert_eq!(normal.parse(), Ok(Value::String("String".into())));
 
  let raw = "r\"Raw String\"";
- assert_eq!(Value::from_str(raw), Ok(Value::String("Raw String".into())));
+ assert_eq!(raw.parse(), Ok(Value::String("Raw String".into())));
 
  let raw_hashes = "r#\"Raw String\"#";
- assert_eq!(
- Value::from_str(raw_hashes),
- Ok(Value::String("Raw String".into()))
- );
+ assert_eq!(raw_hashes.parse(), Ok(Value::String("Raw String".into())));
 
  let raw_escaped = "r##\"Contains \"#\"##";
  assert_eq!(
- Value::from_str(raw_escaped),
+ raw_escaped.parse(),
  Ok(Value::String("Contains \"#".into()))
  );
 
  let raw_multi_line = "r\"Multi\nLine\"";
  assert_eq!(
- Value::from_str(raw_multi_line),
+ raw_multi_line.parse(),
  Ok(Value::String("Multi\nLine".into()))
  );
 }
@@ -68,18 +62,18 @@ fn seq() {
  Value::Number(Number::new(1f64)),
  Value::Number(Number::new(2f64)),
  ];
- assert_eq!(Value::from_str("[1, 2]"), Ok(Value::Seq(seq)));
+ assert_eq!("[1, 2]".parse(), Ok(Value::Seq(seq)));
 }
 
 #[test]
 fn unit() {
  use ron::de::{Error, ParseError, Position};
 
- assert_eq!(Value::from_str("()"), Ok(Value::Unit));
- assert_eq!(Value::from_str("Foo"), Ok(Value::Unit));
+ assert_eq!("()".parse(), Ok(Value::Unit));
+ assert_eq!("Foo".parse(), Ok(Value::Unit));
 
  assert_eq!(
- Value::from_str(""),
+ "".parse::<Value>(),
  Err(Error::Parser(ParseError::Eof, Position { col: 1, line: 1 }))
  );
 }