Clippy pass, cosmetic fixes, and `rust-version.workspace=true` for web examples (#689)

Phew, fixing (future) clippy-lints is grunt-work (I enabled
`unnameable_types` for this), this is by far not complete, mostly
xilem-side, but at least a small step towards enabling more of our lint
set.

There's a few "drive-by fixes" like hiding `View::ViewState` types in
Xilem (I don't think we generally want to expose these), or exposing
event types in xilem_web.

I would suggest disabling the `allow_attributes_without_reason` lint for
now, so that we can update the rust version. I feel like adding comments
next to these is just unnecessary extra-work. I think adding more
reasons should probably be a separate pass, after a rust version update,
so that `#[allow(..., reason="")]` is supported.
This commit is contained in:
Philipp Mildenberger 2024-10-20 17:57:22 +02:00 committed by GitHub
parent 9ccc0f9e59
commit 4817f24e86
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
36 changed files with 93 additions and 59 deletions

View File

@ -9,7 +9,6 @@ use crate::event::PointerButton;
// TODO - TextCursor changed, ImeChanged, EnterKey, MouseEnter
#[non_exhaustive]
#[allow(missing_docs)]
/// Events from UI elements.
///
/// Note: Actions are still a WIP feature.

View File

@ -19,6 +19,7 @@ pub trait AppDriver {
fn on_action(&mut self, ctx: &mut DriverCtx<'_>, widget_id: WidgetId, action: Action);
#[allow(unused_variables)]
// reason: otherwise `state` would need to be named `_state` which behaves badly when using rust-analyzer to implement the trait
/// A hook which will be executed when the application starts, to allow initial configuration of the `MasonryState`.
///
/// Use cases include loading fonts.

View File

@ -1236,6 +1236,7 @@ impl<'a, Ctx: IsContext, W> Drop for RawWrapperMut<'a, Ctx, W> {
}
mod private {
#[allow(unnameable_types)] // reason: see https://predr.ag/blog/definitive-guide-to-sealed-traits-in-rust/
pub trait Sealed {}
}
@ -1243,7 +1244,7 @@ mod private {
// We're exporting a trait with a method that returns a private type.
// It's mostly fine because the trait is sealed anyway, but it's not great for documentation.
#[allow(private_interfaces)]
#[allow(private_interfaces)] // reason: see https://predr.ag/blog/definitive-guide-to-sealed-traits-in-rust/
pub trait IsContext: private::Sealed {
fn get_widget_state(&mut self) -> &mut WidgetState;
}
@ -1252,7 +1253,7 @@ macro_rules! impl_context_trait {
($SomeCtx:tt) => {
impl private::Sealed for $SomeCtx<'_> {}
#[allow(private_interfaces)]
#[allow(private_interfaces)] // reason: see https://predr.ag/blog/definitive-guide-to-sealed-traits-in-rust/
impl IsContext for $SomeCtx<'_> {
fn get_widget_state(&mut self) -> &mut WidgetState {
self.widget_state

View File

@ -146,7 +146,7 @@ impl crate::core::one_of::PhantomElementCtx for ViewCtx {
type PhantomElement = Pod<Box<dyn Widget>>;
}
#[allow(unnameable_types)] // Public because of trait visibility rules, but has no public API.
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub enum OneOfWidget<A, B, C, D, E, F, G, H, I> {
A(WidgetPod<A>),
B(WidgetPod<B>),
@ -175,7 +175,7 @@ impl<
fn on_text_event(&mut self, _ctx: &mut EventCtx, _event: &TextEvent) {}
fn on_access_event(&mut self, _ctx: &mut EventCtx, _event: &AccessEvent) {}
#[allow(missing_docs)]
#[allow(missing_docs)] // reason: Doesn't do anything and is not available publicly
fn on_status_change(&mut self, _: &mut UpdateCtx, _: &StatusChange) {
// Intentionally do nothing
}

View File

@ -167,9 +167,7 @@ where
}
}
#[allow(clippy::large_enum_variant)]
pub enum FlexElement {
// Avoid making the enum massive for the spacer cases by boxing
Child(Pod<Box<dyn Widget>>, FlexParams),
FixedSpacer(f64),
FlexSpacer(f64),
@ -595,21 +593,28 @@ where
}
}
#[doc(hidden)] // Implementation detail, public because of trait visibility rules
pub struct AnyFlexChildState<State: 'static, Action: 'static> {
/// Just the optional view state of the flex item view
#[allow(clippy::type_complexity)]
inner: Option<
<FlexItem<Box<AnyWidgetView<State, Action>>, State, Action> as View<
State,
Action,
ViewCtx,
>>::ViewState,
>,
/// The generational id handling is essentially very similar to that of the `Option<impl ViewSequence>`,
/// where `None` would represent a Spacer, and `Some` a view
generation: u64,
mod hidden {
use super::FlexItem;
use crate::{core::View, AnyWidgetView, ViewCtx};
#[doc(hidden)] // Implementation detail, public because of trait visibility rules
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct AnyFlexChildState<State: 'static, Action: 'static> {
/// Just the optional view state of the flex item view
#[allow(clippy::type_complexity)]
// reason: There's no reasonable other way to avoid this.
pub(crate) inner: Option<
<FlexItem<Box<AnyWidgetView<State, Action>>, State, Action> as View<
State,
Action,
ViewCtx,
>>::ViewState,
>,
/// The generational id handling is essentially very similar to that of the `Option<impl ViewSequence>`,
/// where `None` would represent a Spacer, and `Some` a view
pub(crate) generation: u64,
}
}
use hidden::AnyFlexChildState;
impl<State, Action> ViewMarker for AnyFlexChild<State, Action> {}
impl<State, Action> View<State, Action, ViewCtx> for AnyFlexChild<State, Action>

View File

@ -165,6 +165,7 @@ where
/// The state used by [`AnyView`].
#[doc(hidden)]
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct AnyViewState {
inner_state: Box<dyn Any>,
/// The generation is the value which is shown

View File

@ -196,7 +196,7 @@ where
}
/// The state used to implement `ViewSequence` for `Option<impl ViewSequence>`
#[allow(unnameable_types)] // Public because of trait visibility rules, but has no public API.
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct OptionSeqState<InnerState> {
/// The current state.
///
@ -339,7 +339,8 @@ where
/// to the index, and the other half used for the generation.
///
// This is managed in [`create_vector_view_id`] and [`view_id_to_index_generation`]
#[doc(hidden)] // Implementation detail, public because of trait visibility rules
#[doc(hidden)]
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct VecViewState<InnerState> {
inner_states: Vec<InnerState>,

View File

@ -204,6 +204,7 @@ where
}
}
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct ArcState<ViewState> {
view_state: ViewState,
dirty: bool,

View File

@ -21,7 +21,6 @@ pub struct Adapt<
> {
proxy_fn: ProxyFn,
child: ChildView,
#[allow(clippy::type_complexity)]
phantom: PhantomData<
fn() -> (
ParentState,

View File

@ -58,6 +58,7 @@ where
}
}
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct MemoizeState<V, VState> {
view: V,
view_state: VState,

View File

@ -524,7 +524,7 @@ mod hidden {
use super::PhantomElementCtx;
use crate::{View, ViewMarker};
#[allow(unreachable_pub)]
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub enum Never {}
impl ViewMarker for Never {}
@ -564,7 +564,7 @@ mod hidden {
}
}
/// The state used to implement `View` for `OneOfN`
#[allow(unreachable_pub)]
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct OneOfState<A, B, C, D, E, F, G, H, I> {
/// The current state of the inner view or view sequence.
pub(super) inner_state: super::OneOf<A, B, C, D, E, F, G, H, I>,

View File

@ -73,7 +73,7 @@ where
impl<State, Fragment: DomFragment<State>, InitFragment: FnMut(&mut State) -> Fragment>
AppInner<State, Fragment, InitFragment>
{
pub fn new(root: web_sys::Node, data: State, app_logic: InitFragment) -> Self {
fn new(root: web_sys::Node, data: State, app_logic: InitFragment) -> Self {
let ctx = ViewCtx::default();
AppInner {
data,

View File

@ -57,6 +57,7 @@ where
}
}
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct IntervalState {
// Closures are retained so they can be called by environment
interval_fn: Closure<dyn FnMut()>,

View File

@ -101,6 +101,7 @@ where
}
#[derive(Default)]
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct MemoizedAwaitState {
generation: u64,
schedule_update: bool,

View File

@ -102,6 +102,7 @@ pub struct Task<F, H, M> {
message: PhantomData<fn() -> M>,
}
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct TaskState {
abort_handle: Option<AbortHandle>,
}

View File

@ -153,14 +153,15 @@ impl<'a, 'b, 'c, 'd> DomChildrenSplice<'a, 'b, 'c, 'd> {
impl<'a, 'b, 'c, 'd> ElementSplice<AnyPod> for DomChildrenSplice<'a, 'b, 'c, 'd> {
fn with_scratch<R>(&mut self, f: impl FnOnce(&mut AppendVec<AnyPod>) -> R) -> R {
let ret = f(self.scratch);
#[allow(unused_assignments, unused_mut)]
let mut add_dom_children_to_parent = true;
#[cfg(feature = "hydration")]
{
add_dom_children_to_parent = !self.in_hydration;
}
if !self.scratch.is_empty() {
#[allow(unused_assignments, unused_mut)]
// reason: when the feature "hydration" is enabled/disabled, avoid warnings
let mut add_dom_children_to_parent = true;
#[cfg(feature = "hydration")]
{
add_dom_children_to_parent = !self.in_hydration;
}
for element in self.scratch.drain() {
if add_dom_children_to_parent {
self.fragment

View File

@ -107,13 +107,18 @@ fn remove_event_listener(
.unwrap_throw();
}
/// State for the `OnEvent` view.
pub struct OnEventState<S> {
#[allow(unused)]
child_state: S,
callback: Closure<dyn FnMut(web_sys::Event)>,
mod hidden {
use wasm_bindgen::prelude::Closure;
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
/// State for the `OnEvent` view.
pub struct OnEventState<S> {
pub(crate) child_state: S,
pub(crate) callback: Closure<dyn FnMut(web_sys::Event)>,
}
}
use hidden::OnEventState;
// These (boilerplatey) functions are there to reduce the boilerplate created by the macro-expansion below.
fn build_event_listener<State, Action, V, Event>(
@ -143,7 +148,7 @@ where
})
}
#[allow(clippy::too_many_arguments)]
#[allow(clippy::too_many_arguments)] // reason: This is only used to avoid more boilerplate in macros, also so that rust-analyzer can be of help here.
fn rebuild_event_listener<State, Action, V, Event>(
element_view: &V,
prev_element_view: &V,
@ -519,7 +524,7 @@ pub struct OnResize<V, State, Action, Callback> {
pub struct OnResizeState<VState> {
child_state: VState,
// Closures are retained so they can be called by environment
// reason: Closures are retained so they can be called by environment
#[allow(unused)]
callback: Closure<dyn FnMut(js_sys::Array)>,
observer: web_sys::ResizeObserver,

View File

@ -35,7 +35,6 @@ mod class;
mod context;
mod dom_helpers;
mod element_props;
mod events;
mod message;
mod one_of;
mod optional_action;
@ -49,6 +48,7 @@ mod vecmap;
pub mod concurrent;
pub mod elements;
pub mod events;
pub mod interfaces;
pub mod svg;

View File

@ -51,7 +51,7 @@ impl dyn Message {
/// If the message contained within `self` is not of type `T`, returns `self`
/// (so that e.g. a different type can be used)
pub fn downcast<T: Message>(self: Box<Self>) -> Result<Box<T>, Box<Self>> {
// The panic is unreachable
// reason: The panic is unreachable
#![allow(clippy::missing_panics_doc)]
if self.deref().as_any().is::<T>() {
Ok(self

View File

@ -183,6 +183,7 @@ where
}
}
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub enum Noop {}
impl PhantomElementCtx for ViewCtx {

View File

@ -13,6 +13,7 @@ pub trait OptionalAction<A>: sealed::Sealed {
fn action(self) -> Option<A>;
}
mod sealed {
#[allow(unnameable_types)] // reason: see https://predr.ag/blog/definitive-guide-to-sealed-traits-in-rust/
pub trait Sealed {}
}

View File

@ -19,8 +19,9 @@ pub struct Pointer<V, T, A, F> {
phantom: PhantomData<fn() -> (T, A)>,
}
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct PointerState<S> {
// Closures are retained so they can be called by environment
// reason: Closures are retained so they can be called by environment
#[allow(unused)]
down_closure: Closure<dyn FnMut(PointerEvent)>,
#[allow(unused)]

View File

@ -142,6 +142,7 @@ where
}
}
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct StrokeState<ChildState> {
brush_svg_repr: Option<AttributeValue>,
stroke_dash_pattern_svg_repr: Option<AttributeValue>,

View File

@ -11,6 +11,7 @@ use wasm_bindgen::UnwrapThrowExt;
/// This view creates an internally cached deep-clone of the underlying DOM node. When the inner view is created again, this will be done more efficiently.
pub struct Templated<E>(Rc<E>);
#[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules
pub struct TemplatedState<ViewState> {
view_state: ViewState,
dirty: bool,

View File

@ -140,7 +140,6 @@ impl<K, V> VecMap<K, V> {
/// assert_eq!((*first_key, *first_value), (1, "a"));
/// ```
pub fn iter(&self) -> impl Iterator<Item = (&K, &V)> {
#[allow(clippy::map_identity)]
self.0.iter().map(|(k, v)| (k, v))
}

View File

@ -4,6 +4,7 @@ version = "0.1.0"
publish = false
license.workspace = true
edition.workspace = true
rust-version.workspace = true
[lints]
workspace = true

View File

@ -4,6 +4,7 @@ version = "0.1.0"
publish = false
license.workspace = true
edition.workspace = true
rust-version.workspace = true
[lints]
workspace = true

View File

@ -4,6 +4,7 @@ version = "0.0.0" # not versioned
publish = false
license.workspace = true
edition.workspace = true
rust-version.workspace = true
[lints]
workspace = true

View File

@ -4,6 +4,7 @@ version = "0.1.0"
publish = false
license.workspace = true
edition.workspace = true
rust-version.workspace = true
[lints]
workspace = true

View File

@ -4,6 +4,7 @@ version = "0.1.0"
publish = false
license.workspace = true
edition.workspace = true
rust-version.workspace = true
[lints]
workspace = true

View File

@ -4,6 +4,7 @@ version = "0.1.0"
publish = false
license.workspace = true
edition.workspace = true
rust-version.workspace = true
[dependencies]
console_error_panic_hook = "0.1"

View File

@ -4,6 +4,7 @@ version = "0.0.0" # not versioned
publish = false
license.workspace = true
edition.workspace = true
rust-version.workspace = true
[lints]
workspace = true

View File

@ -35,7 +35,10 @@ async fn create_ping_task(proxy: TaskProxy, shutdown_signal: ShutdownSignal) {
log::debug!("Start ping task");
let mut abort = shutdown_signal.into_future().fuse();
#[allow(clippy::infinite_loop)]
#[allow(
clippy::infinite_loop,
// reason = "False-Positive of clippy, not recognizing that the loop will be aborted"
)]
loop {
let mut timeout = TimeoutFuture::new(1_000).fuse();

View File

@ -4,6 +4,7 @@ version = "0.1.0"
publish = false
license.workspace = true
edition.workspace = true
rust-version.workspace = true
[lints]
workspace = true

View File

@ -4,6 +4,7 @@ version = "0.1.0"
publish = false
license.workspace = true
edition.workspace = true
rust-version.workspace = true
[lints]
workspace = true

View File

@ -7,7 +7,7 @@ use wasm_bindgen::UnwrapThrowExt;
const KEY: &str = "todomvc_persist";
#[derive(Default, Debug, Serialize, Deserialize)]
pub struct AppState {
pub(crate) struct AppState {
#[serde(skip)]
pub new_todo: String,
pub todos: Vec<Todo>,
@ -21,7 +21,7 @@ pub struct AppState {
}
impl AppState {
pub fn create_todo(&mut self) {
pub(crate) fn create_todo(&mut self) {
if self.new_todo.is_empty() {
return;
}
@ -39,13 +39,13 @@ impl AppState {
}
/// Are all the todos complete?
pub fn are_all_complete(&self) -> bool {
pub(crate) fn are_all_complete(&self) -> bool {
self.todos.iter().all(|todo| todo.completed)
}
/// If all TODOs are complete, then mark them all not complete,
/// else mark them all complete.
pub fn toggle_all_complete(&mut self) {
pub(crate) fn toggle_all_complete(&mut self) {
if self.are_all_complete() {
for todo in self.todos.iter_mut() {
todo.completed = false;
@ -58,7 +58,7 @@ impl AppState {
self.save();
}
pub fn visible_todos(&mut self) -> impl Iterator<Item = (usize, &mut Todo)> {
pub(crate) fn visible_todos(&mut self) -> impl Iterator<Item = (usize, &mut Todo)> {
self.todos
.iter_mut()
.enumerate()
@ -69,12 +69,12 @@ impl AppState {
})
}
pub fn update_new_todo(&mut self, new_text: &str) {
pub(crate) fn update_new_todo(&mut self, new_text: &str) {
self.new_todo.clear();
self.new_todo.push_str(new_text);
}
pub fn start_editing(&mut self, id: u64) {
pub(crate) fn start_editing(&mut self, id: u64) {
if let Some(ref mut todo) = self.todos.iter_mut().find(|todo| todo.id == id) {
todo.title_editing.clear();
todo.title_editing.push_str(&todo.title);
@ -83,7 +83,7 @@ impl AppState {
}
/// Load the current state from local storage, or use the default.
pub fn load() -> Self {
pub(crate) fn load() -> Self {
let Some(raw) = storage().get_item(KEY).unwrap_throw() else {
return Default::default();
};
@ -97,14 +97,14 @@ impl AppState {
}
/// Save the current state to local storage
pub fn save(&self) {
pub(crate) fn save(&self) {
let raw = serde_json::to_string(self).unwrap_throw();
storage().set_item(KEY, &raw).unwrap_throw();
}
}
#[derive(Debug, Serialize, Deserialize)]
pub struct Todo {
pub(crate) struct Todo {
pub id: u64,
pub title: String,
#[serde(skip)]
@ -113,7 +113,7 @@ pub struct Todo {
}
impl Todo {
pub fn new(title: String, id: u64) -> Self {
pub(crate) fn new(title: String, id: u64) -> Self {
let title_editing = title.clone();
Self {
id,
@ -123,14 +123,14 @@ impl Todo {
}
}
pub fn save_editing(&mut self) {
pub(crate) fn save_editing(&mut self) {
self.title.clear();
self.title.push_str(&self.title_editing);
}
}
#[derive(Debug, Default, PartialEq, Copy, Clone)]
pub enum Filter {
pub(crate) enum Filter {
#[default]
All,
Active,