Fix clippy lints and run clippy in ci

.github/workflows/rust.yml

@@ -24,41 +24,35 @@ jobs:
           command: fmt
           args: --all -- --check
 
-  test:
-    name: Test Suite
-    runs-on: macOS-10.14
+  test-stable:
+    runs-on: ubuntu-latest
+    name: cargo test stable
     steps:
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v2
 
-      - name: install cairo
-        run: brew install cairo
+      - name: install libgtk-dev
+        run: |
+          sudo apt update
+          sudo apt install libgtk-3-dev
 
-      - uses: actions-rs/toolchain@v1
+      - name: install stable toolchain
+        uses: actions-rs/toolchain@v1
         with:
           toolchain: stable
+          components: clippy
+          profile: minimal
           override: true
 
+      - name: cargo clippy
+        uses: actions-rs/cargo@v1
+        with:
+          command: clippy
+          args: -- -D warnings
+
       - run: git submodule update --init --recursive
 
-      - uses: actions-rs/cargo@v1
+      - name: cargo test
+        uses: actions-rs/cargo@v1
         with:
           command: test
           args: --all-features
-
-  doc:
-    name: Docs
-    runs-on: macOS-10.14
-    steps:
-      - uses: actions/checkout@v1
-
-      - name: install cairo
-        run: brew install cairo
-
-      - uses: actions-rs/toolchain@v1
-        with:
-          toolchain: nightly
-          override: true
-      - uses: actions-rs/cargo@v1
-        with:
-          command: doc
-          args: --document-private-items

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "norad"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Colin Rofls <[email protected]>", "Nikolaus Waxweiler <[email protected]>"]
 license = "MIT/Apache-2.0"
 edition = "2018"

src/glyph/builder.rs

@@ -149,7 +149,7 @@ impl GlyphBuilder {
     ///
     /// Errors when format version 1 is set.
     pub fn guideline(&mut self, guideline: Guideline) -> Result<&mut Self, ErrorKind> {
-        if &self.glyph.format == &GlifVersion::V1 {
+        if self.glyph.format == GlifVersion::V1 {
             return Err(ErrorKind::UnexpectedTag);
         }
         insert_identifier(&mut self.identifiers, guideline.identifier().cloned())?;
@@ -161,7 +161,7 @@ impl GlyphBuilder {
     ///
     /// Errors when format version 1 is set or the optional identifier is not unique within the glyph.
     pub fn anchor(&mut self, anchor: Anchor) -> Result<&mut Self, ErrorKind> {
-        if &self.glyph.format == &GlifVersion::V1 {
+        if self.glyph.format == GlifVersion::V1 {
             return Err(ErrorKind::UnexpectedTag);
         }
         insert_identifier(&mut self.identifiers, anchor.identifier.clone())?;
@@ -183,15 +183,15 @@ impl GlyphBuilder {
         if self.glyph.outline.is_some() {
             return Err(ErrorKind::UnexpectedDuplicate);
         }
-        if &self.glyph.format == &GlifVersion::V1 && !identifiers.is_empty() {
+        if self.glyph.format == GlifVersion::V1 && !identifiers.is_empty() {
             return Err(ErrorKind::UnexpectedAttribute);
         }
         if !self.identifiers.is_disjoint(&identifiers) {
             return Err(ErrorKind::DuplicateIdentifier);
         }
         self.identifiers.extend(identifiers);
 
-        if &self.glyph.format == &GlifVersion::V1 {
+        if self.glyph.format == GlifVersion::V1 {
             for c in &mut outline.contours {
                 if c.points.len() == 1
                     && c.points[0].typ == PointType::Move
@@ -211,14 +211,7 @@ impl GlyphBuilder {
             }
 
             // Clean up now empty contours.
-            let mut i = 0;
-            while i != outline.contours.len() {
-                if outline.contours[i].points.len() == 0 {
-                    outline.contours.remove(i);
-                } else {
-                    i += 1;
-                }
-            }
+            outline.contours.retain(|c| !c.points.is_empty());
         }
 
         self.glyph.outline.replace(outline);
@@ -229,7 +222,7 @@ impl GlyphBuilder {
     ///
     /// Errors when format version 1 is set or the function is called more than once.
     pub fn image(&mut self, image: Image) -> Result<&mut Self, ErrorKind> {
-        if &self.glyph.format == &GlifVersion::V1 {
+        if self.glyph.format == GlifVersion::V1 {
             return Err(ErrorKind::UnexpectedTag);
         }
         if self.glyph.image.is_some() {
@@ -271,7 +264,7 @@ impl GlyphBuilder {
 /// Primarily to be used in conjunction with [`GlyphBuilder`].
 ///
 /// [fontTools point pen]: https://fonttools.readthedocs.io/en/latest/pens/basePen.html
-#[derive(Debug)]
+#[derive(Debug, Default)]
 pub struct OutlineBuilder {
     identifiers: HashSet<Identifier>,
     outline: Outline,
@@ -281,12 +274,7 @@ pub struct OutlineBuilder {
 
 impl OutlineBuilder {
     pub fn new() -> Self {
-        Self {
-            identifiers: HashSet::new(),
-            outline: Outline::default(),
-            scratch_contour: None,
-            number_of_offcurves: 0,
-        }
+        Default::default()
     }
 
     /// Start a new path to be added to the glyph with `end_path()`.
@@ -397,7 +385,7 @@ impl OutlineBuilder {
                 }
                 self.number_of_offcurves = 0;
                 // Empty contours are allowed by the specification but make no sense, skip them.
-                if c.points.len() > 0 {
+                if !c.points.is_empty() {
                     self.outline.contours.push(c);
                 }
                 Ok(self)

src/glyph/mod.rs

@@ -639,7 +639,7 @@ impl From<druid::piet::Color> for Color {
         let g = ((rgba >> 16) & 0xff) as f32 / 255.0;
         let b = ((rgba >> 8) & 0xff) as f32 / 255.0;
         let a = (rgba & 0xff) as f32 / 255.0;
-        assert!(b >= 0.0 && b <= 1.0, "b: {}, raw {}", b, (rgba & (0xff << 8)));
+        assert!((0.0..=1.0).contains(&b), "b: {}, raw {}", b, (rgba & (0xff << 8)));
 
         Color {
             red: r.max(0.0).min(1.0),

src/glyph/serialize.rs

@@ -79,15 +79,15 @@ impl Glyph {
         // cloning and write out the original.
         let object_libs = self.dump_object_libs();
         if !object_libs.is_empty() {
-            let mut new_lib = self.lib.clone().unwrap_or_else(|| Plist::new());
+            let mut new_lib = self.lib.clone().unwrap_or_else(Plist::new);
             new_lib.insert(PUBLIC_OBJECT_LIBS_KEY.into(), plist::Value::Dictionary(object_libs));
             write_lib_section(&new_lib, &mut writer)?;
         } else if let Some(lib) = self.lib.as_ref().filter(|lib| !lib.is_empty()) {
             write_lib_section(lib, &mut writer)?;
         }
 
         writer.write_event(Event::End(BytesEnd::borrowed(b"glyph")))?;
-        writer.inner().write("\n".as_bytes())?;
+        writer.inner().write_all("\n".as_bytes())?;
         writer.inner().flush()?;
 
         Ok(writer.into_inner().into_inner())
@@ -120,7 +120,7 @@ fn write_lib_section<T: Write>(lib: &Plist, writer: &mut Writer<T>) -> Result<()
 
     writer.write_event(Event::Start(BytesStart::borrowed_name(b"lib")))?;
     for line in to_write.lines() {
-        writer.inner().write("\n\t\t".as_bytes())?;
+        writer.inner().write_all("\n\t\t".as_bytes())?;
         writer.inner().write_all(line.as_bytes())?;
     }
     writer.write_event(Event::End(BytesEnd::borrowed(b"lib")))?;

src/layer.rs

@@ -145,7 +145,7 @@ impl Layer {
     }
 
     /// Iterate over the glyphs in this layer.
-    pub fn iter_contents<'a>(&'a self) -> impl Iterator<Item = Arc<Glyph>> + 'a {
+    pub fn iter_contents(&self) -> impl Iterator<Item = Arc<Glyph>> + '_ {
         self.glyphs.values().map(Arc::clone)
     }
 
@@ -175,18 +175,16 @@ pub struct LayerInfo {
 impl LayerInfo {
     fn from_file(path: &PathBuf) -> Result<Self, Error> {
         let mut info_content = plist::Value::from_file(path)
-            .map_err(|e| Error::PlistError(e))?
+            .map_err(Error::PlistError)?
             .into_dictionary()
             .ok_or(Error::ExpectedPlistDictionaryError)?;
 
         let mut color = None;
         let color_str = info_content.remove("color");
         if let Some(v) = color_str {
             match v.into_string() {
-                Some(s) => {
-                    color.replace(Color::from_str(&s).map_err(|e| Error::InvalidDataError(e))?)
-                }
-                None => Err(Error::ExpectedPlistStringError)?,
+                Some(s) => color.replace(Color::from_str(&s).map_err(Error::InvalidDataError)?),
+                None => return Err(Error::ExpectedPlistStringError),
             };
         };
 
@@ -195,7 +193,7 @@ impl LayerInfo {
         if let Some(v) = lib_content {
             match v.into_dictionary() {
                 Some(d) => lib.replace(d),
-                None => Err(Error::ExpectedPlistDictionaryError)?,
+                None => return Err(Error::ExpectedPlistDictionaryError),
             };
         };
 

src/ufo.rs

@@ -292,9 +292,9 @@ impl Ufo {
         // existing lib and insert them there for serialization, otherwise write
         // out the original.
         let object_libs =
-            self.font_info.as_ref().map(|f| f.dump_object_libs()).unwrap_or_else(|| Plist::new());
+            self.font_info.as_ref().map(|f| f.dump_object_libs()).unwrap_or_else(Plist::new);
         if !object_libs.is_empty() {
-            let mut new_lib = self.lib.clone().unwrap_or_else(|| Plist::new());
+            let mut new_lib = self.lib.clone().unwrap_or_else(Plist::new);
             new_lib.insert(PUBLIC_OBJECT_LIBS_KEY.into(), plist::Value::Dictionary(object_libs));
             plist::Value::Dictionary(new_lib).to_file_xml(path.join(LIB_FILE))?;
         } else if let Some(lib) = self.lib.as_ref().filter(|lib| !lib.is_empty()) {