From 4239e01f8cc8efff90fe1d0e161765b877b88dd4 Mon Sep 17 00:00:00 2001 From: Chad Brokaw Date: Tue, 14 Nov 2023 14:52:55 -0500 Subject: [PATCH] Re-enable clippy and fix lints Found a workaround for the clippy crash so this re-enables it in CI and fixes issues that snuck in while it was disabled. --- .github/workflows/rust.yml | 27 +++++++++++++-------------- fauntlet/src/compare_glyphs.rs | 1 + fauntlet/src/font/freetype.rs | 2 +- fauntlet/src/main.rs | 15 ++++++--------- skrifa/src/scale/cff/hint.rs | 4 ++-- skrifa/src/scale/glyf/deltas.rs | 4 +--- skrifa/src/scale/glyf/scaler.rs | 7 +++---- skrifa/src/scale/mod.rs | 7 +++---- 8 files changed, 30 insertions(+), 37 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 9236056b2..ad5fccf7d 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -30,20 +30,19 @@ jobs: - name: check no println! or eprintln! statements run: resources/scripts/check_no_println.sh - # disabled temporarily pending fixing a crash in clippy - #clippy-lint: - #name: Clippy lints - #runs-on: ubuntu-latest - #steps: - #- uses: actions/checkout@v3 - #- name: install stable toolchain - #uses: dtolnay/rust-toolchain@stable - - #- name: cargo clippy --all-features - #run: cargo clippy --all-features --all-targets -- -D warnings - - #- name: cargo clippy --no-default-features - #run: cargo clippy --all-targets --no-default-features -- -D warnings + clippy-lint: + name: Clippy lints + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: install stable toolchain + uses: dtolnay/rust-toolchain@stable + + - name: cargo clippy --all-features + run: cargo clippy --all-features --all-targets -- -D warnings + + - name: cargo clippy --no-default-features + run: cargo clippy --all-targets --no-default-features -- -D warnings test-stable: name: cargo test stable diff --git a/fauntlet/src/compare_glyphs.rs b/fauntlet/src/compare_glyphs.rs index d99b72b3a..69fd48886 100644 --- a/fauntlet/src/compare_glyphs.rs +++ b/fauntlet/src/compare_glyphs.rs @@ -2,6 +2,7 @@ use super::{FreeTypeInstance, InstanceOptions, RecordingPen, RegularizingPen, Sk use skrifa::GlyphId; use std::{io::Write, path::Path}; +#[allow(clippy::explicit_write)] pub fn compare_glyphs( path: &Path, options: &InstanceOptions, diff --git a/fauntlet/src/font/freetype.rs b/fauntlet/src/font/freetype.rs index e07c3c255..81c5383f3 100644 --- a/fauntlet/src/font/freetype.rs +++ b/fauntlet/src/font/freetype.rs @@ -1,6 +1,6 @@ use freetype::{ face::LoadFlag, - ffi::{FT_Error, FT_Face, FT_Fixed, FT_Int32, FT_Long, FT_UInt, FT_Vector}, + ffi::{FT_Long, FT_Vector}, Face, Library, }; use skrifa::{raw::FileRef, scale::Pen, GlyphId}; diff --git a/fauntlet/src/main.rs b/fauntlet/src/main.rs index 353efb2c7..7cddc66d3 100644 --- a/fauntlet/src/main.rs +++ b/fauntlet/src/main.rs @@ -26,12 +26,13 @@ enum Command { }, } +#[allow(clippy::explicit_write)] fn main() { // Pixels per em sizes. A size of 0 means an explicit unscaled comparison let ppem_sizes = [0, 8, 16, 50, 72, 113, 144]; // Locations in normalized variation space - let var_locations = [-1.0, -0.32, 0.0, 0.42, 1.0].map(|c| F2Dot14::from_f32(c)); + let var_locations = [-1.0, -0.32, 0.0, 0.42, 1.0].map(F2Dot14::from_f32); use clap::Parser as _; let args = Args::parse_from(wild::args()); @@ -48,7 +49,7 @@ fn main() { if print_paths { writeln!(std::io::stdout(), "[{font_path:?}]").unwrap(); } - if let Some(mut font_data) = fauntlet::Font::new(&font_path) { + if let Some(mut font_data) = fauntlet::Font::new(font_path) { for font_ix in 0..font_data.count() { for ppem in &ppem_sizes { let axis_count = font_data.axis_count(font_ix) as usize; @@ -61,7 +62,7 @@ fn main() { fauntlet::InstanceOptions::new(font_ix, *ppem, &coords); if let Some(instances) = font_data.instantiate(&options) { if !compare_glyphs( - &font_path, + font_path, &options, instances, exit_on_fail, @@ -73,12 +74,8 @@ fn main() { } else { let options = InstanceOptions::new(font_ix, *ppem, &[]); if let Some(instances) = font_data.instantiate(&options) { - if !compare_glyphs( - &font_path, - &options, - instances, - exit_on_fail, - ) { + if !compare_glyphs(font_path, &options, instances, exit_on_fail) + { ok.store(false, Ordering::Release); } } diff --git a/skrifa/src/scale/cff/hint.rs b/skrifa/src/scale/cff/hint.rs index 0ff35ded5..0ff859984 100644 --- a/skrifa/src/scale/cff/hint.rs +++ b/skrifa/src/scale/cff/hint.rs @@ -1020,7 +1020,7 @@ impl<'a, S: CommandSink> CommandSink for HintingSink<'a, S> { fn hint_mask(&mut self, mask: &[u8]) { // For invalid hint masks, FreeType assumes all hints are active. // See - let mask = HintMask::new(mask).unwrap_or_else(|| HintMask::all()); + let mask = HintMask::new(mask).unwrap_or_else(HintMask::all); if mask != self.mask { self.mask = mask; self.map.is_valid = false; @@ -1033,7 +1033,7 @@ impl<'a, S: CommandSink> CommandSink for HintingSink<'a, S> { // mask." Building the map modifies the stem hint array as a // side effect. // See - let mask = HintMask::new(mask).unwrap_or_else(|| HintMask::all()); + let mask = HintMask::new(mask).unwrap_or_else(HintMask::all); let mut map = HintMap::new(self.state.scale); map.build( self.state, diff --git a/skrifa/src/scale/glyf/deltas.rs b/skrifa/src/scale/glyf/deltas.rs index 62ef41137..b4d5bbfa6 100644 --- a/skrifa/src/scale/glyf/deltas.rs +++ b/skrifa/src/scale/glyf/deltas.rs @@ -238,13 +238,11 @@ impl<'a> Jiggler<'a> { if range.is_empty() { return Some(()); } - let RefPoints(ref1_ix, ref2_ix) = ref_points; // FreeType uses pointer tricks to handle x and y coords with a single piece of code. // Try a macro instead. macro_rules! interp_coord { ($coord:ident) => { - let mut ref1_ix = ref1_ix; - let mut ref2_ix = ref2_ix; + let RefPoints(mut ref1_ix, mut ref2_ix) = ref_points; if self.points.get(ref1_ix)?.$coord > self.points.get(ref2_ix)?.$coord { core::mem::swap(&mut ref1_ix, &mut ref2_ix); } diff --git a/skrifa/src/scale/glyf/scaler.rs b/skrifa/src/scale/glyf/scaler.rs index b919caa56..e605ddfe6 100644 --- a/skrifa/src/scale/glyf/scaler.rs +++ b/skrifa/src/scale/glyf/scaler.rs @@ -752,11 +752,10 @@ mod tests { assert_eq!( expected_gids_with_overlap, (0..glyph_count) - .filter_map(|gid| scaler - .glyph(GlyphId::new(gid), false) + .filter(|gid| scaler + .glyph(GlyphId::new(*gid), false) .unwrap() - .has_overlaps - .then_some(gid)) + .has_overlaps) .collect::>() ); } diff --git a/skrifa/src/scale/mod.rs b/skrifa/src/scale/mod.rs index 18ffb828d..77e3bda68 100644 --- a/skrifa/src/scale/mod.rs +++ b/skrifa/src/scale/mod.rs @@ -256,11 +256,10 @@ mod tests { assert_eq!( expected_gids_with_overlap, (0..glyph_count) - .filter_map(|gid| scaler - .outline(GlyphId::new(gid), &mut path) + .filter(|gid| scaler + .outline(GlyphId::new(*gid), &mut path) .unwrap() - .has_overlaps - .then_some(gid)) + .has_overlaps) .collect::>() ); }