Skip to content

Commit

Permalink
Add Linebender lint set v2. (#46)
Browse files Browse the repository at this point in the history
Actually satisfying a good chunk of these lints is deferred and they are
in various `allow` blocks.

Still, I did satisfy some of the lints.
  • Loading branch information
xStrom authored Nov 27, 2024
1 parent b450094 commit 2d6cd95
Show file tree
Hide file tree
Showing 16 changed files with 177 additions and 38 deletions.
10 changes: 10 additions & 0 deletions .clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# LINEBENDER LINT SET - .clippy.toml - v1
# See https://linebender.org/wiki/canonical-lints/

# The default Clippy value is capped at 8 bytes, which was chosen to improve performance on 32-bit.
# Given that we are building for the future and even low-end mobile phones have 64-bit CPUs,
# it makes sense to optimize for 64-bit and accept the performance hits on 32-bit.
# 16 bytes is the number of bytes that fits into two 64-bit CPU registers.
trivial-copy-size-limit = 16

# END LINEBENDER LINT SET
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ env:
# version like 1.70. Note that we only specify MAJOR.MINOR and not PATCH so that bugfixes still
# come automatically. If the version specified here is no longer the latest stable version,
# then please feel free to submit a PR that adjusts it along with the potential clippy fixes.
RUST_STABLE_VER: "1.76" # In quotes because otherwise (e.g.) 1.70 would be interpreted as 1.7
RUST_STABLE_VER: "1.82" # In quotes because otherwise (e.g.) 1.70 would be interpreted as 1.7
# The purpose of checking with the minimum supported Rust toolchain is to detect its staleness.
# If the compilation fails, then the version specified here needs to be bumped up to reality.
# Be sure to also update the rust-version property in the workspace Cargo.toml file,
Expand Down
56 changes: 56 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,64 @@ default-target = "x86_64-unknown-linux-gnu"
targets = []

[workspace.lints]
# LINEBENDER LINT SET - Cargo.toml - v2
# See https://linebender.org/wiki/canonical-lints/
rust.keyword_idents_2024 = "forbid"
rust.non_ascii_idents = "forbid"
rust.non_local_definitions = "forbid"
rust.unsafe_op_in_unsafe_fn = "forbid"

rust.elided_lifetimes_in_paths = "warn"
rust.let_underscore_drop = "warn"
rust.missing_debug_implementations = "warn"
rust.missing_docs = "warn"
rust.single_use_lifetimes = "warn"
rust.trivial_numeric_casts = "warn"
rust.unexpected_cfgs = "warn"
rust.unit_bindings = "warn"
rust.unnameable_types = "warn"
rust.unreachable_pub = "warn"
rust.unused_import_braces = "warn"
rust.unused_lifetimes = "warn"
rust.unused_macro_rules = "warn"
rust.unused_qualifications = "warn"
rust.variant_size_differences = "warn"

clippy.allow_attributes = "warn"
clippy.allow_attributes_without_reason = "warn"
clippy.cast_possible_truncation = "warn"
clippy.collection_is_never_read = "warn"
clippy.dbg_macro = "warn"
clippy.debug_assert_with_mut_call = "warn"
clippy.doc_markdown = "warn"
clippy.exhaustive_enums = "warn"
clippy.fn_to_numeric_cast_any = "warn" # Can't be forbid due to 'scenes' requiring an exception.
clippy.infinite_loop = "warn"
clippy.large_include_file = "warn"
clippy.large_stack_arrays = "warn"
clippy.match_same_arms = "warn"
clippy.mismatching_type_param_order = "warn"
clippy.missing_assert_message = "warn"
clippy.missing_errors_doc = "warn"
clippy.missing_fields_in_debug = "warn"
clippy.missing_panics_doc = "warn"
clippy.partial_pub_fields = "warn"
clippy.return_self_not_must_use = "warn"
clippy.same_functions_in_if_condition = "warn"
clippy.semicolon_if_nothing_returned = "warn"
clippy.shadow_unrelated = "warn"
clippy.should_panic_without_expect = "warn"
clippy.todo = "warn"
clippy.trivially_copy_pass_by_ref = "warn"
clippy.unseparated_literal_suffix = "warn"
clippy.use_self = "warn"
clippy.wildcard_imports = "warn"

clippy.cargo_common_metadata = "warn"
clippy.negative_feature_names = "warn"
clippy.redundant_feature_names = "warn"
clippy.wildcard_dependencies = "warn"
# END LINEBENDER LINT SET

[workspace.dependencies]
# NOTE: Make sure to keep this in sync with the version badge in README.md
Expand Down
2 changes: 2 additions & 0 deletions examples/run_wasm/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright 2022 the Velato Authors
// SPDX-License-Identifier: Apache-2.0 OR MIT

//! Wasm
/// Use [cargo-run-wasm](https://github.com/rukai/cargo-run-wasm) to build an example for web
///
/// Usage:
Expand Down
5 changes: 3 additions & 2 deletions examples/scenes/src/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use inquire::Confirm;
use std::io::Read;
mod default_downloads;

#[allow(clippy::partial_pub_fields)]
#[derive(Args, Debug)]
pub(crate) struct Download {
#[clap(long)]
Expand Down Expand Up @@ -64,7 +65,7 @@ impl Download {
println!(
"{} ({}) under license {} from {}",
download.name,
byte_unit::Byte::from_bytes(builtin.expected_size.into())
Byte::from_bytes(builtin.expected_size.into())
.get_appropriate_unit(false),
builtin.license,
builtin.info
Expand Down Expand Up @@ -151,7 +152,7 @@ impl Download {
fn download_prompt(total_bytes: u64) -> Result<bool> {
let prompt = format!(
"Would you like to download a set of default lottie files, as explained above? ({})",
byte_unit::Byte::from_bytes(total_bytes.into()).get_appropriate_unit(false)
Byte::from_bytes(total_bytes.into()).get_appropriate_unit(false)
);
let accepted = Confirm::new(&prompt).with_default(false).prompt()?;
Ok(accepted)
Expand Down
29 changes: 25 additions & 4 deletions examples/scenes/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@
// Copyright 2022 the Velato Authors
// SPDX-License-Identifier: Apache-2.0 OR MIT

//! Scenes
// The following lints are part of the Linebender standard set,
// but resolving them has been deferred for now.
// Feel free to send a PR that solves one or more of these.
#![allow(
missing_debug_implementations,
unreachable_pub,
missing_docs,
unused_macro_rules,
clippy::shadow_unrelated,
clippy::use_self,
clippy::missing_assert_message,
clippy::missing_errors_doc,
clippy::missing_panics_doc,
clippy::allow_attributes,
clippy::allow_attributes_without_reason,
clippy::wildcard_imports
)]

#[cfg(not(target_arch = "wasm32"))]
pub mod download;
mod lottie;
Expand Down Expand Up @@ -29,7 +49,7 @@ pub struct SceneParams<'a> {
pub interactive: bool,
pub text: &'a mut RobotoText,
pub resolution: Option<Vec2>,
pub base_color: Option<vello::peniko::Color>,
pub base_color: Option<Color>,
pub complexity: usize,
}

Expand All @@ -45,11 +65,11 @@ pub struct ExampleScene {
}

pub trait TestScene {
fn render(&mut self, scene: &mut Scene, params: &mut SceneParams);
fn render(&mut self, scene: &mut Scene, params: &mut SceneParams<'_>);
}

impl<F: FnMut(&mut Scene, &mut SceneParams)> TestScene for F {
fn render(&mut self, scene: &mut Scene, params: &mut SceneParams) {
impl<F: FnMut(&mut Scene, &mut SceneParams<'_>)> TestScene for F {
fn render(&mut self, scene: &mut Scene, params: &mut SceneParams<'_>) {
self(scene, params);
}
}
Expand All @@ -58,6 +78,7 @@ pub struct SceneSet {
pub scenes: Vec<ExampleScene>,
}

#[allow(clippy::partial_pub_fields)]
#[derive(Args, Debug)]
/// Shared config for scene selection
pub struct Arguments {
Expand Down
10 changes: 5 additions & 5 deletions examples/scenes/src/lottie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,29 +91,29 @@ fn example_scene_of(file: PathBuf) -> ExampleScene {
pub fn lottie_function_of<R: AsRef<str>>(
name: String,
contents: impl FnOnce() -> R + Send + 'static,
) -> impl FnMut(&mut Scene, &mut SceneParams) {
) -> impl FnMut(&mut Scene, &mut SceneParams<'_>) {
let start = Instant::now();
let lottie = Arc::new(
velato::Composition::from_slice(contents().as_ref())
Composition::from_slice(contents().as_ref())
.unwrap_or_else(|e| panic!("failed to parse lottie file {name}: {e}")),
);
eprintln!("Parsed lottie {name} in {:?}", start.elapsed());
fn render_lottie_contents(
renderer: &mut velato::Renderer,
lottie: &Composition,
start: &Instant,
start: Instant,
) -> Scene {
let frame = ((start.elapsed().as_secs_f64() * lottie.frame_rate)
% (lottie.frames.end - lottie.frames.start))
+ lottie.frames.start;
renderer.render(lottie, frame, Affine::IDENTITY, 1.0)
}
let started = instant::Instant::now();
let started = Instant::now();
let mut renderer = velato::Renderer::new();
let lottie = lottie.clone();
let resolution = Vec2::new(lottie.width as f64, lottie.height as f64);
move |scene, params| {
params.resolution = Some(resolution);
*scene = render_lottie_contents(&mut renderer, &lottie, &started);
*scene = render_lottie_contents(&mut renderer, &lottie, started);
}
}
4 changes: 2 additions & 2 deletions examples/scenes/src/simple_text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ impl RobotoText {
let metrics = font_ref.metrics(font_size, &var_loc);
let line_height = metrics.ascent - metrics.descent + metrics.leading;
let glyph_metrics = font_ref.glyph_metrics(font_size, &var_loc);
let mut pen_x = 0f32;
let mut pen_y = 0f32;
let mut pen_x = 0_f32;
let mut pen_y = 0_f32;
scene
.draw_glyphs(font)
.font_size(size)
Expand Down
4 changes: 2 additions & 2 deletions examples/scenes/src/test_scenes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub fn test_scenes() -> SceneSet {
}

// Scenes
fn splash_screen(scene: &mut Scene, params: &mut SceneParams) {
fn splash_screen(scene: &mut Scene, params: &mut SceneParams<'_>) {
let strings = [
"Velato Demo",
#[cfg(not(target_arch = "wasm32"))]
Expand All @@ -58,7 +58,7 @@ fn splash_screen(scene: &mut Scene, params: &mut SceneParams) {
}
}

fn splash_with_tiger() -> impl FnMut(&mut Scene, &mut SceneParams) {
fn splash_with_tiger() -> impl FnMut(&mut Scene, &mut SceneParams<'_>) {
let contents = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/../assets/google_fonts/Tiger.json"
Expand Down
26 changes: 22 additions & 4 deletions examples/with_winit/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,22 @@
// Copyright 2022 the Velato Authors
// SPDX-License-Identifier: Apache-2.0 OR MIT

//! With Winit
// The following lints are part of the Linebender standard set,
// but resolving them has been deferred for now.
// Feel free to send a PR that solves one or more of these.
#![allow(
missing_docs,
clippy::wildcard_imports,
clippy::use_self,
clippy::missing_errors_doc,
clippy::shadow_unrelated,
clippy::cast_possible_truncation,
clippy::allow_attributes,
clippy::allow_attributes_without_reason
)]

use instant::Instant;
use std::collections::HashSet;
use std::num::NonZeroUsize;
Expand Down Expand Up @@ -63,7 +79,7 @@ fn run(
args: Args,
mut scenes: SceneSet,
render_cx: RenderContext,
#[cfg(target_arch = "wasm32")] render_state: RenderState,
#[cfg(target_arch = "wasm32")] render_state: RenderState<'_>,
) {
use winit::event::*;
use winit::event_loop::ControlFlow;
Expand All @@ -72,7 +88,7 @@ fn run(
#[cfg(not(target_arch = "wasm32"))]
let mut render_cx = render_cx;
#[cfg(not(target_arch = "wasm32"))]
let mut render_state = None::<RenderState>;
let mut render_state = None::<RenderState<'_>>;
let use_cpu = args.use_cpu;
// The design of `RenderContext` forces delayed renderer initialisation to
// not work on wasm, as WASM futures effectively must be 'static.
Expand Down Expand Up @@ -477,7 +493,7 @@ fn run(
{}
#[cfg(not(target_arch = "wasm32"))]
{
let Option::None = render_state else { return };
let None = render_state else { return };
let window = cached_window
.take()
.unwrap_or_else(|| create_window(event_loop));
Expand Down Expand Up @@ -560,6 +576,8 @@ fn display_error_message() -> Option<()> {
Some(())
}

/// # Panics
/// Can panic.
pub fn main() -> Result<()> {
// TODO: initializing both env_logger and console_logger fails on wasm.
// Figure out a more principled approach.
Expand Down Expand Up @@ -596,7 +614,7 @@ pub fn main() -> Result<()> {
.and_then(|body| body.append_child(canvas.as_ref()).ok())
.expect("couldn't append canvas to document body");
// Best effort to start with the canvas focused, taking input
_ = web_sys::HtmlElement::from(canvas).focus();
drop(web_sys::HtmlElement::from(canvas).focus());
wasm_bindgen_futures::spawn_local(async move {
let (width, height, scale_factor) = web_sys::window()
.map(|w| {
Expand Down
2 changes: 2 additions & 0 deletions examples/with_winit/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright 2022 the Velato Authors
// SPDX-License-Identifier: Apache-2.0 OR MIT

//! With Winit
use anyhow::Result;

fn main() -> Result<()> {
Expand Down
10 changes: 5 additions & 5 deletions examples/with_winit/src/multi_touch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use winit::event::{Touch, TouchPhase};

/// All you probably need to know about a multi-touch gesture.
#[derive(Clone, Copy, Debug, PartialEq)]
pub struct MultiTouchInfo {
pub(crate) struct MultiTouchInfo {
/// Number of touches (fingers) on the surface. Value is ≥ 2 since for a single touch no
/// [`MultiTouchInfo`] is created.
pub num_touches: usize,
Expand Down Expand Up @@ -93,15 +93,15 @@ struct ActiveTouch {
}

impl TouchState {
pub fn new() -> Self {
pub(crate) fn new() -> Self {
Self {
active_touches: Default::default(),
gesture_state: None,
added_or_removed_touches: false,
}
}

pub fn add_event(&mut self, event: &Touch) {
pub(crate) fn add_event(&mut self, event: &Touch) {
let pos = Point::new(event.location.x, event.location.y);
match event.phase {
TouchPhase::Started => {
Expand All @@ -120,7 +120,7 @@ impl TouchState {
}
}

pub fn end_frame(&mut self) {
pub(crate) fn end_frame(&mut self) {
// This needs to be called each frame, even if there are no new touch events.
// Otherwise, we would send the same old delta information multiple times:
self.update_gesture();
Expand All @@ -135,7 +135,7 @@ impl TouchState {
self.added_or_removed_touches = false;
}

pub fn info(&self) -> Option<MultiTouchInfo> {
pub(crate) fn info(&self) -> Option<MultiTouchInfo> {
self.gesture_state.as_ref().map(|state| {
// state.previous can be `None` when the number of simultaneous touches has just
// changed. In this case, we take `current` as `previous`, pretending that there
Expand Down
Loading

0 comments on commit 2d6cd95

Please sign in to comment.