Enable (selected) new cargo clippy lints

Cranky.toml

@@ -84,25 +84,30 @@ warn = [
   "clippy::needless_borrow",
   "clippy::needless_continue",
   "clippy::needless_for_each",
+  "clippy::needless_pass_by_ref_mut",
   "clippy::needless_pass_by_value",
   "clippy::negative_feature_names",
   "clippy::nonstandard_macro_braces",
   "clippy::option_option",
   "clippy::path_buf_push_overwrite",
   "clippy::ptr_as_ptr",
   "clippy::ptr_cast_constness",
+  "clippy::pub_without_shorthand",
   "clippy::rc_mutex",
+  "clippy::readonly_write_lock",
   "clippy::redundant_type_annotations",
   "clippy::ref_option_ref",
   "clippy::rest_pat_in_fully_bound_structs",
   "clippy::same_functions_in_if_condition",
   "clippy::semicolon_if_nothing_returned",
+  "clippy::should_panic_without_expect",
   "clippy::significant_drop_tightening",
   "clippy::single_match_else",
   "clippy::str_to_string",
   "clippy::string_add_assign",
   "clippy::string_add",
   "clippy::string_lit_as_bytes",
+  "clippy::string_lit_chars_any",
   "clippy::string_to_string",
   "clippy::suspicious_command_arg_space",
   "clippy::suspicious_xor_used_as_pow",
@@ -158,4 +163,7 @@ allow = [
   # Enabling this lint in 2023-04-27 yielded 556 hits.
   # Enabling this lint in 2023-09-23 yielded 352 hits
   "clippy::unwrap_used",
+
+  # It's nice to avoid ref pattern, but there are some situations that are hard (impossible?) to express without.
+  "clippy::ref_patterns",
 ]

crates/re_data_ui/src/item_ui.rs

@@ -168,7 +168,7 @@ fn entity_stats_ui(ui: &mut egui::Ui, timeline: &Timeline, stats: &re_arrow_stor
 
 /// Show a component path and make it selectable.
 pub fn component_path_button(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     component_path: &ComponentPath,
 ) -> egui::Response {
@@ -182,7 +182,7 @@ pub fn component_path_button(
 
 /// Show a component path and make it selectable.
 pub fn component_path_button_to(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     text: impl Into<egui::WidgetText>,
     component_path: &ComponentPath,
@@ -193,7 +193,7 @@ pub fn component_path_button_to(
 }
 
 pub fn data_blueprint_group_button_to(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     text: impl Into<egui::WidgetText>,
     space_view_id: SpaceViewId,
@@ -233,7 +233,7 @@ pub fn data_blueprint_button_to(
 }
 
 pub fn time_button(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     timeline: &Timeline,
     value: TimeInt,
@@ -261,15 +261,15 @@ pub fn time_button(
 }
 
 pub fn timeline_button(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     timeline: &Timeline,
 ) -> egui::Response {
     timeline_button_to(ctx, ui, timeline.name().to_string(), timeline)
 }
 
 pub fn timeline_button_to(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     text: impl Into<egui::WidgetText>,
     timeline: &Timeline,
@@ -289,7 +289,7 @@ pub fn timeline_button_to(
 
 // TODO(andreas): Move elsewhere, this is not directly part of the item_ui.
 pub fn cursor_interact_with_selectable(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     response: egui::Response,
     item: Item,
 ) -> egui::Response {
@@ -307,11 +307,7 @@ pub fn cursor_interact_with_selectable(
 }
 
 // TODO(andreas): Move elsewhere, this is not directly part of the item_ui.
-pub fn select_hovered_on_click(
-    ctx: &mut ViewerContext<'_>,
-    response: &egui::Response,
-    items: &[Item],
-) {
+pub fn select_hovered_on_click(ctx: &ViewerContext<'_>, response: &egui::Response, items: &[Item]) {
     re_tracing::profile_function!();
 
     if response.hovered() {

crates/re_query/benches/query_benchmark.rs

@@ -103,9 +103,9 @@ fn mono_points(c: &mut Criterion) {
     {
         let mut group = c.benchmark_group("arrow_mono_points2");
         group.throughput(criterion::Throughput::Elements(NUM_POINTS as _));
-        let mut store = insert_rows(msgs.iter());
+        let store = insert_rows(msgs.iter());
         group.bench_function("query", |b| {
-            b.iter(|| query_and_visit_points(&mut store, &paths));
+            b.iter(|| query_and_visit_points(&store, &paths));
         });
     }
 }
@@ -131,9 +131,9 @@ fn mono_strings(c: &mut Criterion) {
     {
         let mut group = c.benchmark_group("arrow_mono_strings2");
         group.throughput(criterion::Throughput::Elements(NUM_POINTS as _));
-        let mut store = insert_rows(msgs.iter());
+        let store = insert_rows(msgs.iter());
         group.bench_function("query", |b| {
-            b.iter(|| query_and_visit_strings(&mut store, &paths));
+            b.iter(|| query_and_visit_strings(&store, &paths));
         });
     }
 }
@@ -156,9 +156,9 @@ fn batch_points(c: &mut Criterion) {
     {
         let mut group = c.benchmark_group("arrow_batch_points2");
         group.throughput(criterion::Throughput::Elements(NUM_POINTS as _));
-        let mut store = insert_rows(msgs.iter());
+        let store = insert_rows(msgs.iter());
         group.bench_function("query", |b| {
-            b.iter(|| query_and_visit_points(&mut store, &paths));
+            b.iter(|| query_and_visit_points(&store, &paths));
         });
     }
 }
@@ -181,9 +181,9 @@ fn batch_strings(c: &mut Criterion) {
     {
         let mut group = c.benchmark_group("arrow_batch_strings2");
         group.throughput(criterion::Throughput::Elements(NUM_POINTS as _));
-        let mut store = insert_rows(msgs.iter());
+        let store = insert_rows(msgs.iter());
         group.bench_function("query", |b| {
-            b.iter(|| query_and_visit_strings(&mut store, &paths));
+            b.iter(|| query_and_visit_strings(&store, &paths));
         });
     }
 }
@@ -264,7 +264,7 @@ struct SavePoint {
     _color: Option<Color>,
 }
 
-fn query_and_visit_points(store: &mut DataStore, paths: &[EntityPath]) -> Vec<SavePoint> {
+fn query_and_visit_points(store: &DataStore, paths: &[EntityPath]) -> Vec<SavePoint> {
     let timeline_frame_nr = Timeline::new("frame_nr", TimeType::Sequence);
     let query = LatestAtQuery::new(timeline_frame_nr, (NUM_FRAMES_POINTS as i64 / 2).into());
 
@@ -292,7 +292,7 @@ struct SaveString {
     _label: Option<Text>,
 }
 
-fn query_and_visit_strings(store: &mut DataStore, paths: &[EntityPath]) -> Vec<SaveString> {
+fn query_and_visit_strings(store: &DataStore, paths: &[EntityPath]) -> Vec<SaveString> {
     let timeline_frame_nr = Timeline::new("frame_nr", TimeType::Sequence);
     let query = LatestAtQuery::new(timeline_frame_nr, (NUM_FRAMES_STRINGS as i64 / 2).into());
 

crates/re_renderer/examples/depth_cloud.rs

@@ -400,7 +400,7 @@ struct DepthTexture {
 }
 
 impl DepthTexture {
-    pub fn spiral(re_ctx: &mut re_renderer::RenderContext, dimensions: glam::UVec2) -> Self {
+    pub fn spiral(re_ctx: &re_renderer::RenderContext, dimensions: glam::UVec2) -> Self {
         let size = (dimensions.x * dimensions.y) as usize;
         let mut data = std::iter::repeat(0f32).take(size).collect_vec();
         spiral(dimensions).for_each(|(texcoords, d)| {
@@ -442,7 +442,7 @@ struct AlbedoTexture {
 }
 
 impl AlbedoTexture {
-    pub fn spiral(re_ctx: &mut re_renderer::RenderContext, dimensions: glam::UVec2) -> Self {
+    pub fn spiral(re_ctx: &re_renderer::RenderContext, dimensions: glam::UVec2) -> Self {
         let size = (dimensions.x * dimensions.y) as usize;
         let mut rgba8 = std::iter::repeat(0).take(size * 4).collect_vec();
         spiral(dimensions).for_each(|(texcoords, d)| {

crates/re_renderer/examples/framework.rs

@@ -342,7 +342,7 @@ impl<E: Example + 'static> Application<E> {
 }
 
 #[allow(dead_code)]
-pub fn load_rerun_mesh(re_ctx: &mut RenderContext) -> Vec<re_renderer::renderer::MeshInstance> {
+pub fn load_rerun_mesh(re_ctx: &RenderContext) -> Vec<re_renderer::renderer::MeshInstance> {
     let reader = std::io::Cursor::new(include_bytes!("rerun.obj.zip"));
     let mut zip = zip::ZipArchive::new(reader).unwrap();
     let mut zipped_obj = zip.by_name("rerun.obj").unwrap();

crates/re_renderer/src/file_resolver.rs

@@ -779,7 +779,8 @@ mod tests_file_resolver {
     }
 
     #[test]
-    #[should_panic] // TODO(cmc): check error contents
+    #[allow(clippy::should_panic_without_expect)] // TODO(cmc): check error contents
+    #[should_panic]
     fn cyclic_direct() {
         let fs = MemFileSystem::get();
         {
@@ -819,7 +820,8 @@ mod tests_file_resolver {
     }
 
     #[test]
-    #[should_panic] // TODO(cmc): check error contents
+    #[allow(clippy::should_panic_without_expect)] // TODO(cmc): check error contents
+    #[should_panic]
     fn cyclic_indirect() {
         let fs = MemFileSystem::get();
         {

crates/re_renderer/src/wgpu_resources/dynamic_resource_pool.rs

@@ -281,7 +281,7 @@ mod tests {
         let initial_resource_descs = [0, 0, 1, 2, 2, 3];
 
         // Alloc on a new pool always returns a new resource.
-        allocate_resources(&initial_resource_descs, &mut pool, true);
+        allocate_resources(&initial_resource_descs, &pool, true);
 
         // After frame maintenance we get used resources.
         // Still, no resources were dropped.
@@ -295,9 +295,9 @@ mod tests {
         }
 
         // Allocate the same resources again, this should *not* create any new resources.
-        allocate_resources(&initial_resource_descs, &mut pool, false);
+        allocate_resources(&initial_resource_descs, &pool, false);
         // Doing it again, it will again create resources.
-        allocate_resources(&initial_resource_descs, &mut pool, true);
+        allocate_resources(&initial_resource_descs, &pool, true);
 
         // Doing frame maintenance twice will drop all resources
         {
@@ -360,7 +360,7 @@ mod tests {
 
     fn allocate_resources(
         descs: &[u32],
-        pool: &mut DynamicResourcePool<ConcreteHandle, ConcreteResourceDesc, ConcreteResource>,
+        pool: &DynamicResourcePool<ConcreteHandle, ConcreteResourceDesc, ConcreteResource>,
         expect_allocation: bool,
     ) {
         let drop_counter_before = DROP_COUNTER.with(|c| c.get());

crates/re_space_view_spatial/src/ui.rs

@@ -113,7 +113,7 @@ impl SpatialSpaceViewState {
 
     pub fn selection_ui(
         &mut self,
-        ctx: &mut ViewerContext<'_>,
+        ctx: &ViewerContext<'_>,
         ui: &mut egui::Ui,
         space_origin: &EntityPath,
         spatial_kind: SpatialSpaceViewKind,

crates/re_time_panel/src/data_density_graph.rs

@@ -523,7 +523,7 @@ fn make_brighter(color: Color32) -> Color32 {
 
 fn show_row_ids_tooltip(
     ctx: &mut ViewerContext<'_>,
-    time_ctrl: &mut TimeControl,
+    time_ctrl: &TimeControl,
     egui_ctx: &egui::Context,
     item: &Item,
     time_range: TimeRange,

crates/re_time_panel/src/lib.rs

@@ -168,7 +168,7 @@ impl TimePanel {
     #[allow(clippy::unused_self)]
     fn collapsed_ui(
         &mut self,
-        ctx: &mut ViewerContext<'_>,
+        ctx: &ViewerContext<'_>,
         ui: &mut egui::Ui,
         time_ctrl: &mut TimeControl,
     ) {
@@ -691,7 +691,7 @@ impl TimePanel {
 
     fn top_row_ui(
         &mut self,
-        ctx: &mut ViewerContext<'_>,
+        ctx: &ViewerContext<'_>,
         ui: &mut egui::Ui,
         time_ctrl: &mut TimeControl,
     ) {
@@ -773,7 +773,7 @@ fn highlight_timeline_row(
 
 fn collapsed_time_marker_and_time(
     ui: &mut egui::Ui,
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     time_ctrl: &mut TimeControl,
 ) {
     let space_needed_for_current_time = match time_ctrl.timeline().typ() {

crates/re_time_panel/src/time_ranges_ui.rs

@@ -241,7 +241,7 @@ impl TimeRangesUi {
     }
 
     // Make sure playback time doesn't get stuck between non-continuous regions:
-    pub fn snap_time_control(&self, ctx: &mut ViewerContext<'_>) {
+    pub fn snap_time_control(&self, ctx: &ViewerContext<'_>) {
         let mut time_ctrl = ctx.rec_cfg.time_ctrl.write();
 
         if time_ctrl.play_state() != PlayState::Playing {

crates/re_ui/src/list_item.rs

@@ -391,7 +391,7 @@ impl<'a> ListItem<'a> {
             // Draw text next to the icon.
             let mut text_rect = rect;
             text_rect.min.x += collapse_extra + icon_extra;
-            if let Some(ref button_response) = button_response {
+            if let Some(button_response) = &button_response {
                 text_rect.max.x -= button_response.rect.width() + ReUi::text_to_icon_padding();
             }
 

crates/re_ui/src/toggle_switch.rs

@@ -37,13 +37,14 @@ fn toggle_switch_ui(ui: &mut egui::Ui, on: &mut bool) -> egui::Response {
     response
 }
 
-// A wrapper that allows the more idiomatic usage pattern: `ui.add(toggle_switch(&mut my_bool))`
+/// A wrapper that allows the more idiomatic usage pattern: `ui.add(toggle_switch(&mut my_bool))`
 /// iOS-style toggle switch.
 ///
 /// ## Example:
 /// ``` ignore
 /// ui.add(toggle_switch(&mut my_bool));
 /// ```
+#[allow(clippy::needless_pass_by_ref_mut)] // False positive, toggle_switch_ui needs &mut
 pub fn toggle_switch(on: &mut bool) -> impl egui::Widget + '_ {
     move |ui: &mut egui::Ui| toggle_switch_ui(ui, on)
 }

crates/re_viewer/src/app_state.rs

@@ -291,7 +291,7 @@ impl AppState {
         }
 
         // This must run after any ui code, or other code that tells egui to open an url:
-        check_for_clicked_hyperlinks(&re_ui.egui_ctx, &mut rec_cfg.selection_state);
+        check_for_clicked_hyperlinks(&re_ui.egui_ctx, &rec_cfg.selection_state);
     }
 
     pub fn recording_config_mut(&mut self, rec_id: &StoreId) -> Option<&mut RecordingConfig> {
@@ -350,7 +350,7 @@ fn recording_config_entry<'cfgs>(
 /// Detect and handle that here.
 ///
 /// Must run after any ui code, or other code that tells egui to open an url.
-fn check_for_clicked_hyperlinks(egui_ctx: &egui::Context, selection_state: &mut SelectionState) {
+fn check_for_clicked_hyperlinks(egui_ctx: &egui::Context, selection_state: &SelectionState) {
     let recording_scheme = "recording://";
 
     let mut path = None;

crates/re_viewer/src/ui/selection_panel.rs

@@ -156,7 +156,7 @@ fn has_data_section(item: &Item) -> bool {
 }
 
 fn space_view_button(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     space_view: &re_viewport::SpaceViewBlueprint,
 ) -> egui::Response {

crates/re_viewer/src/ui/visible_history.rs

@@ -71,7 +71,7 @@ fn has_visible_history(
 }
 
 pub fn visible_history_ui(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     space_view_class: &SpaceViewClassName,
     is_space_view: bool,
@@ -238,7 +238,7 @@ pub fn visible_history_ui(
 }
 
 fn current_range_ui(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut Ui,
     current_time: i64,
     is_sequence_timeline: bool,
@@ -278,7 +278,7 @@ fn current_range_ui(
 
 #[allow(clippy::too_many_arguments)]
 fn resolved_visible_history_boundary_ui(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     visible_history_boundary: &VisibleHistoryBoundary,
     is_sequence_timeline: bool,
@@ -388,7 +388,7 @@ fn visible_history_boundary_combo_label(
 
 #[allow(clippy::too_many_arguments)]
 fn visible_history_boundary_ui(
-    ctx: &mut ViewerContext<'_>,
+    ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     visible_history_boundary: &mut VisibleHistoryBoundary,
     is_sequence_timeline: bool,