Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run clippy on public API too #2596

Merged
merged 12 commits into from
Jul 4, 2023
2 changes: 2 additions & 0 deletions Cranky.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ warn = [
"clippy::iter_on_empty_collections",
"clippy::iter_on_single_items",
"clippy::large_digit_groups",
"clippy::large_include_file",
"clippy::large_stack_arrays",
"clippy::large_types_passed_by_value",
"clippy::let_unit_value",
Expand Down Expand Up @@ -99,6 +100,7 @@ warn = [
"clippy::string_to_string",
"clippy::suspicious_xor_used_as_pow",
"clippy::todo",
"clippy::too_many_lines",
"clippy::trailing_empty_array",
"clippy::trait_duplication_in_bounds",
"clippy::unchecked_duration_subtraction",
Expand Down
16 changes: 16 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -1,9 +1,25 @@
# There is also a scripts/clippy_wasm/clippy.toml which forbids some methods that are not available in wasm.

# -----------------------------------------------------------------------------
# Section identical to the main scripts/clippy_wasm/clippy.toml:

msrv = "1.69"

allow-unwrap-in-tests = true

# https://doc.rust-lang.org/nightly/clippy/lint_configuration.html#avoid-breaking-exported-api
# We want suggestions, even if it changes public API.
avoid-breaking-exported-api = false

max-fn-params-bools = 2 # TODO(emilk): decrease this to 1

# https://rust-lang.github.io/rust-clippy/master/index.html#/large_include_file
max-include-file-size = 1000000

too-many-lines-threshold = 600 # TODO(emilk): decrease this

# -----------------------------------------------------------------------------

# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros
disallowed-macros = [
'dbg',
Expand Down
6 changes: 3 additions & 3 deletions crates/re_arrow_store/src/store_arrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ fn serialize_control_columns(
// NOTE: Optional column, so make sure it's actually there:
if !col_insert_id.is_empty() {
let (insert_id_field, insert_id_column) =
DataTable::serialize_primitive_column(COLUMN_INSERT_ID, col_insert_id, None)?;
DataTable::serialize_primitive_column(COLUMN_INSERT_ID, col_insert_id, None);
schema.fields.push(insert_id_field);
columns.push(insert_id_column);
}
Expand All @@ -158,13 +158,13 @@ fn serialize_control_columns(
timeline.name(),
col_time,
timeline.datatype().into(),
)?;
);
schema.fields.push(time_field);
columns.push(time_column);
}

let (num_instances_field, num_instances_column) =
DataTable::serialize_primitive_column(COLUMN_NUM_INSTANCES, col_num_instances, None)?;
DataTable::serialize_primitive_column(COLUMN_NUM_INSTANCES, col_num_instances, None);
schema.fields.push(num_instances_field);
columns.push(num_instances_column);

Expand Down
3 changes: 1 addition & 2 deletions crates/re_arrow_store/src/store_polars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,7 @@ impl IndexedBucket {
self.timeline.name(),
col_time,
self.timeline.datatype().into(),
)
.unwrap();
);
let num_rows = times.len();

let insert_ids = config
Expand Down
2 changes: 1 addition & 1 deletion crates/re_components/src/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl ColorRGBA {
}

#[inline]
pub fn to_array(&self) -> [u8; 4] {
pub fn to_array(self) -> [u8; 4] {
[
(self.0 >> 24) as u8,
(self.0 >> 16) as u8,
Expand Down
2 changes: 2 additions & 0 deletions crates/re_components/src/coordinates.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::wrong_self_convention)] // TODO(emilk): re-enable

use arrow2::datatypes::DataType;
use arrow2_convert::{
deserialize::ArrowDeserialize,
Expand Down
2 changes: 2 additions & 0 deletions crates/re_components/src/rect.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::upper_case_acronyms)]

use arrow2_convert::{ArrowDeserialize, ArrowField, ArrowSerialize};

use super::Vec4D;
Expand Down
1 change: 1 addition & 0 deletions crates/re_components/src/tensor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ impl ArrowDeserialize for TensorId {
/// ```
#[derive(Clone, PartialEq, ArrowField, ArrowSerialize, ArrowDeserialize)]
#[arrow_field(type = "dense")]
#[allow(clippy::upper_case_acronyms)] // TODO(emilk): Rename to `Jpeg`.
pub enum TensorData {
U8(Buffer<u8>),
U16(Buffer<u16>),
Expand Down
2 changes: 1 addition & 1 deletion crates/re_components/src/transform3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ impl Transform3D {
#[cfg(feature = "glam")]
impl Transform3D {
#[inline]
pub fn to_parent_from_child_transform(&self) -> glam::Affine3A {
pub fn to_parent_from_child_transform(self) -> glam::Affine3A {
let transform: glam::Affine3A = self.transform.into();
if self.from_parent {
transform.inverse()
Expand Down
3 changes: 2 additions & 1 deletion crates/re_log_encoding/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl EncodingOptions {
}
}

pub fn to_bytes(&self) -> [u8; 4] {
pub fn to_bytes(self) -> [u8; 4] {
[
self.compression as u8,
self.serializer as u8,
Expand All @@ -93,6 +93,7 @@ impl EncodingOptions {

/// On failure to decode [`EncodingOptions`]
#[derive(thiserror::Error, Debug)]
#[allow(clippy::enum_variant_names)]
pub enum OptionsError {
#[error("Reserved bytes not zero")]
UnknownReservedBytes,
Expand Down
4 changes: 2 additions & 2 deletions crates/re_log_types/src/data_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ impl DataCell {
///
/// Fails if the array is not a valid list of components.
#[inline]
#[allow(clippy::unnecessary_wraps)] // TODO(cmc): check that it is indeed a component datatype
pub fn try_from_arrow(
name: ComponentName,
values: Box<dyn arrow2::array::Array>,
Expand Down Expand Up @@ -267,12 +268,11 @@ impl DataCell {
///
/// Fails if the datatype is not a valid component type.
#[inline]
#[allow(clippy::unnecessary_wraps)] // TODO(cmc): check that it is indeed a component datatype
pub fn try_from_arrow_empty(
name: ComponentName,
datatype: arrow2::datatypes::DataType,
) -> DataCellResult<Self> {
// TODO(cmc): check that it is indeed a component datatype

let mut inner = DataCellInner {
name,
size_bytes: 0,
Expand Down
6 changes: 3 additions & 3 deletions crates/re_log_types/src/data_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ impl DataTable {
COLUMN_NUM_INSTANCES,
col_num_instances.as_slice(),
None,
)?;
);
schema.fields.push(num_instances_field);
columns.push(num_instances_column);

Expand Down Expand Up @@ -730,7 +730,7 @@ impl DataTable {
name: &str,
values: &[T],
datatype: Option<DataType>,
) -> DataTableResult<(Field, Box<dyn Array>)> {
) -> (Field, Box<dyn Array>) {
re_tracing::profile_function!();

let data = PrimitiveArray::from_slice(values);
Expand All @@ -747,7 +747,7 @@ impl DataTable {
.extend([("ARROW:extension:name".to_owned(), name)]);
}

Ok((field, data))
(field, data)
}

/// Serializes all data columns into an arrow payload and schema.
Expand Down
2 changes: 1 addition & 1 deletion crates/re_log_types/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl Time {
}

/// Returns the absolute datetime if applicable.
pub fn to_datetime(&self) -> Option<OffsetDateTime> {
pub fn to_datetime(self) -> Option<OffsetDateTime> {
let ns_since_epoch = self.nanos_since_epoch();
if self.is_absolute_date() {
OffsetDateTime::from_unix_timestamp_nanos(ns_since_epoch as i128).ok()
Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/examples/2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ impl framework::Example for Render2D {
);
}

let line_strip_draw_data = line_strip_builder.to_draw_data(re_ctx).unwrap();
let point_draw_data = point_cloud_builder.to_draw_data(re_ctx).unwrap();
let line_strip_draw_data = line_strip_builder.into_draw_data(re_ctx).unwrap();
let point_draw_data = point_cloud_builder.into_draw_data(re_ctx);

let image_scale = 4.0;
let rectangle_draw_data = RectangleDrawData::new(
Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/examples/depth_cloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl RenderDepthClouds {
std::iter::empty::<re_renderer::PickingLayerInstanceId>(),
);

builder.to_draw_data(re_ctx).unwrap()
builder.into_draw_data(re_ctx)
};

let mut view_builder = ViewBuilder::new(
Expand Down Expand Up @@ -305,7 +305,7 @@ impl framework::Example for RenderDepthClouds {
glam::Vec3::ONE * 0.5,
));
}
builder.to_draw_data(re_ctx).unwrap()
builder.into_draw_data(re_ctx).unwrap()
};

let image_draw_data = RectangleDrawData::new(
Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/examples/multiview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ fn build_lines(re_ctx: &mut RenderContext, seconds_since_startup: f32) -> LineDr
.radius(Size::new_scene(0.1))
.flags(LineStripFlags::FLAG_CAP_END_TRIANGLE);

builder.to_draw_data(re_ctx).unwrap()
builder.into_draw_data(re_ctx).unwrap()
}

enum CameraControl {
Expand Down Expand Up @@ -334,7 +334,7 @@ impl Example for Multiview {
std::iter::empty::<re_renderer::PickingLayerInstanceId>(),
);

let point_cloud = builder.to_draw_data(re_ctx).unwrap();
let point_cloud = builder.into_draw_data(re_ctx);
let meshes = build_mesh_instances(
re_ctx,
&self.model_mesh_instances,
Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/examples/picking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl framework::Example for Picking {
point_set.picking_ids.iter().cloned(),
);
}
view_builder.queue_draw(point_builder.to_draw_data(re_ctx).unwrap());
view_builder.queue_draw(point_builder.into_draw_data(re_ctx));

let instances = self
.model_mesh_instances
Expand Down
1 change: 1 addition & 0 deletions crates/re_renderer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ impl DeviceCaps {
}

/// Required features for the given device tier.
#[allow(clippy::unused_self)]
pub fn features(&self) -> wgpu::Features {
wgpu::Features::empty()
}
Expand Down
1 change: 1 addition & 0 deletions crates/re_renderer/src/file_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ mod file_server_impl {
f(&mut Self)
}

#[allow(clippy::unused_self)]
pub fn collect<Fs: FileSystem>(
&mut self,
_resolver: &mut FileResolver<Fs>,
Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/src/line_strip_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl LineStripSeriesBuilder {
}

/// Finalizes the builder and returns a line draw data with all the lines added so far.
pub fn to_draw_data(
pub fn into_draw_data(
self,
ctx: &mut crate::context::RenderContext,
) -> Result<LineDrawData, LineDrawDataError> {
Expand Down
10 changes: 2 additions & 8 deletions crates/re_renderer/src/point_cloud_builder.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use crate::{
allocator::CpuWriteGpuReadBuffer,
draw_phases::PickingLayerObjectId,
renderer::{
PointCloudBatchFlags, PointCloudBatchInfo, PointCloudDrawData, PointCloudDrawDataError,
PointCloudVertex,
},
renderer::{PointCloudBatchFlags, PointCloudBatchInfo, PointCloudDrawData, PointCloudVertex},
Color32, DebugLabel, DepthOffset, OutlineMaskPreference, PickingLayerInstanceId, RenderContext,
Size,
};
Expand Down Expand Up @@ -100,10 +97,7 @@ impl PointCloudBuilder {
}

/// Finalizes the builder and returns a point cloud draw data with all the points added so far.
pub fn to_draw_data(
self,
ctx: &mut crate::context::RenderContext,
) -> Result<PointCloudDrawData, PointCloudDrawDataError> {
pub fn into_draw_data(self, ctx: &mut crate::context::RenderContext) -> PointCloudDrawData {
PointCloudDrawData::new(ctx, self)
}
}
Expand Down
13 changes: 5 additions & 8 deletions crates/re_renderer/src/renderer/point_cloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,7 @@ impl PointCloudDrawData {
/// Number of vertices and colors has to be equal.
///
/// If no batches are passed, all points are assumed to be in a single batch with identity transform.
pub fn new(
ctx: &mut RenderContext,
mut builder: PointCloudBuilder,
) -> Result<Self, PointCloudDrawDataError> {
pub fn new(ctx: &mut RenderContext, mut builder: PointCloudBuilder) -> Self {
re_tracing::profile_function!();

let mut renderers = ctx.renderers.write();
Expand All @@ -201,11 +198,11 @@ impl PointCloudDrawData {
let batches = builder.batches.as_slice();

if vertices.is_empty() {
return Ok(PointCloudDrawData {
return PointCloudDrawData {
bind_group_all_points: None,
bind_group_all_points_outline_mask: None,
batches: Vec::new(),
});
};
}

let fallback_batches = [PointCloudBatchInfo {
Expand Down Expand Up @@ -491,11 +488,11 @@ impl PointCloudDrawData {
}
}

Ok(PointCloudDrawData {
PointCloudDrawData {
bind_group_all_points: Some(bind_group_all_points),
bind_group_all_points_outline_mask: Some(bind_group_all_points_outline_mask),
batches: batches_internal,
})
}
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/re_sdk/src/msg_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ impl MsgSender {

/// Consumes, packs, sanity checks and finally sends the message to the currently configured
/// target of the SDK.
#[allow(clippy::unnecessary_wraps)] // We might want to return errors in the future.
pub fn send(self, rec_stream: &RecordingStream) -> Result<(), DataTableError> {
if !rec_stream.is_enabled() {
return Ok(()); // silently drop the message
Expand Down
1 change: 1 addition & 0 deletions crates/re_sdk/src/recording_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ impl RecordingStreamBuilder {
self
}

#[allow(clippy::wrong_self_convention)]
#[doc(hidden)]
pub fn is_official_example(mut self, is_official_example: bool) -> Self {
self.is_official_example = is_official_example;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,19 @@ impl SharedRenderBuilders {
) -> Vec<re_renderer::QueueableDrawData> {
let mut result = Vec::new();
result.extend(self.lines.take().and_then(
|l| match l.into_inner().to_draw_data(render_ctx) {
|l| match l.into_inner().into_draw_data(render_ctx) {
Ok(d) => Some(d.into()),
Err(err) => {
re_log::error_once!("Failed to build line strip draw data: {err}");
None
}
},
));
result.extend(self.points.take().and_then(
|l| match l.into_inner().to_draw_data(render_ctx) {
Ok(d) => Some(d.into()),
Err(err) => {
re_log::error_once!("Failed to build point draw data: {err}");
None
}
},
));
result.extend(
self.points
.take()
.map(|l| l.into_inner().into_draw_data(render_ctx).into()),
);
result
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/re_space_view_spatial/src/ui_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ pub fn view_3d(
}

// Commit ui induced lines.
match line_builder.to_draw_data(ctx.render_ctx) {
match line_builder.into_draw_data(ctx.render_ctx) {
Ok(line_draw_data) => {
view_builder.queue_draw(line_draw_data);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/re_types/src/components/color_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ impl Color {
}

#[inline]
pub fn to_array(&self) -> [u8; 4] {
pub fn to_array(self) -> [u8; 4] {
[
(self.0 >> 24) as u8,
(self.0 >> 16) as u8,
Expand Down
Loading
Loading