Skip to content

Commit

Permalink
chore: resolve clippy::must_use_candidate and `clippy::range_plus_o…
Browse files Browse the repository at this point in the history
…ne` lints in `proof-of-sql` (#212)

# Rationale for this change

We have cargo clippy running in our CI in order to enforce code quality.
In order to increase our standards, we should enable the
clippy::pedantic lint group.

# What changes are included in this PR?

This PR fixes `clippy::must_use_candidate` and `clippy::range_plus_one`
warnings for `proof-of-sql` lib.

# Are these changes tested?

Yes. In order to test individual lints, we can use the following
command:

`cargo clippy --lib -p proof-of-sql -- -W clippy::lint_name`
  • Loading branch information
iajoiner authored Oct 3, 2024
2 parents 03d88c8 + 42ccf90 commit c845139
Show file tree
Hide file tree
Showing 21 changed files with 75 additions and 6 deletions.
1 change: 1 addition & 0 deletions crates/proof-of-sql/src/base/commitment/column_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ impl ColumnBounds {
/// Construct a [`ColumnBounds`] from a column by reference.
///
/// If the column variant has order, only the minimum and maximum value will be copied.
#[must_use]
pub fn from_column(column: &CommittableColumn) -> ColumnBounds {
match column {
CommittableColumn::SmallInt(ints) => ColumnBounds::SmallInt(Bounds::from_iter(*ints)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ impl ColumnCommitmentMetadata {
}

/// Construct a [`ColumnCommitmentMetadata`] with widest possible bounds for the column type.
#[must_use]
pub fn from_column_type_with_max_bounds(column_type: ColumnType) -> Self {
let bounds = match column_type {
ColumnType::SmallInt => ColumnBounds::SmallInt(super::Bounds::Bounded(
Expand Down Expand Up @@ -100,16 +101,19 @@ impl ColumnCommitmentMetadata {
}

/// Immutable reference to this column's type.
#[must_use]
pub fn column_type(&self) -> &ColumnType {
&self.column_type
}

/// Immutable reference to this column's bounds.
#[must_use]
pub fn bounds(&self) -> &ColumnBounds {
&self.bounds
}

/// Contruct a [`ColumnCommitmentMetadata`] by analyzing a column.
#[must_use]
pub fn from_column(column: &CommittableColumn) -> ColumnCommitmentMetadata {
ColumnCommitmentMetadata {
column_type: column.column_type(),
Expand Down
6 changes: 6 additions & 0 deletions crates/proof-of-sql/src/base/commitment/column_commitments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,33 +75,39 @@ impl<C: Commitment> ColumnCommitments<C> {
}

/// Returns a reference to the stored commitments.
#[must_use]
pub fn commitments(&self) -> &Vec<C> {
&self.commitments
}

/// Returns a reference to the stored column metadata.
#[must_use]
pub fn column_metadata(&self) -> &ColumnCommitmentMetadataMap {
&self.column_metadata
}

/// Returns the number of columns.
#[must_use]
pub fn len(&self) -> usize {
self.column_metadata.len()
}

/// Returns true if there are no columns.
#[must_use]
pub fn is_empty(&self) -> bool {
self.column_metadata.is_empty()
}

/// Returns the commitment with the given identifier.
#[must_use]
pub fn get_commitment(&self, identifier: &Identifier) -> Option<C> {
self.column_metadata
.get_index_of(identifier)
.map(|index| self.commitments[index].clone())
}

/// Returns the metadata for the commitment with the given identifier.
#[must_use]
pub fn get_metadata(&self, identifier: &Identifier) -> Option<&ColumnCommitmentMetadata> {
self.column_metadata.get(identifier)
}
Expand Down
3 changes: 3 additions & 0 deletions crates/proof-of-sql/src/base/commitment/committable_column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub enum CommittableColumn<'a> {

impl<'a> CommittableColumn<'a> {
/// Returns the length of the column.
#[must_use]
pub fn len(&self) -> usize {
match self {
CommittableColumn::SmallInt(col) => col.len(),
Expand All @@ -64,11 +65,13 @@ impl<'a> CommittableColumn<'a> {
}

/// Returns true if the column is empty.
#[must_use]
pub fn is_empty(&self) -> bool {
self.len() == 0
}

/// Returns the type of the column.
#[must_use]
pub fn column_type(&self) -> ColumnType {
self.into()
}
Expand Down
4 changes: 4 additions & 0 deletions crates/proof-of-sql/src/base/commitment/table_commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,21 +170,25 @@ impl<C: Commitment> TableCommitment<C> {
}

/// Returns a reference to this type's internal [`ColumnCommitments`].
#[must_use]
pub fn column_commitments(&self) -> &ColumnCommitments<C> {
&self.column_commitments
}

/// Returns a reference to the range of rows this type commits to.
#[must_use]
pub fn range(&self) -> &Range<usize> {
&self.range
}

/// Returns the number of columns in the committed table.
#[must_use]
pub fn num_columns(&self) -> usize {
self.column_commitments.len()
}

/// Returns the number of rows that have been committed to.
#[must_use]
pub fn num_rows(&self) -> usize {
self.range.len()
}
Expand Down
18 changes: 18 additions & 0 deletions crates/proof-of-sql/src/base/database/column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub enum Column<'a, S: Scalar> {

impl<'a, S: Scalar> Column<'a, S> {
/// Provides the column type associated with the column
#[must_use]
pub fn column_type(&self) -> ColumnType {
match self {
Self::Boolean(_) => ColumnType::Boolean,
Expand All @@ -72,6 +73,7 @@ impl<'a, S: Scalar> Column<'a, S> {
}
}
/// Returns the length of the column.
#[must_use]
pub fn len(&self) -> usize {
match self {
Self::Boolean(col) => col.len(),
Expand All @@ -89,6 +91,7 @@ impl<'a, S: Scalar> Column<'a, S> {
}
}
/// Returns `true` if the column has no elements.
#[must_use]
pub fn is_empty(&self) -> bool {
self.len() == 0
}
Expand Down Expand Up @@ -238,6 +241,7 @@ pub enum ColumnType {

impl ColumnType {
/// Returns true if this column is numeric and false otherwise
#[must_use]
pub fn is_numeric(&self) -> bool {
matches!(
self,
Expand All @@ -251,6 +255,7 @@ impl ColumnType {
}

/// Returns true if this column is an integer and false otherwise
#[must_use]
pub fn is_integer(&self) -> bool {
matches!(
self,
Expand Down Expand Up @@ -285,6 +290,7 @@ impl ColumnType {
/// Returns the larger integer type of two ColumnTypes if they are both integers.
///
/// If either of the columns is not an integer, return None.
#[must_use]
pub fn max_integer_type(&self, other: &Self) -> Option<Self> {
// If either of the columns is not an integer, return None
if !self.is_integer() || !other.is_integer() {
Expand All @@ -298,6 +304,7 @@ impl ColumnType {
}

/// Returns the precision of a ColumnType if it is converted to a decimal wrapped in Some(). If it can not be converted to a decimal, return None.
#[must_use]
pub fn precision_value(&self) -> Option<u8> {
match self {
Self::SmallInt => Some(5_u8),
Expand All @@ -313,6 +320,7 @@ impl ColumnType {
}
}
/// Returns scale of a ColumnType if it is convertible to a decimal wrapped in Some(). Otherwise return None.
#[must_use]
pub fn scale(&self) -> Option<i8> {
match self {
Self::Decimal75(_, scale) => Some(*scale),
Expand All @@ -328,6 +336,7 @@ impl ColumnType {
}

/// Returns the byte size of the column type.
#[must_use]
pub fn byte_size(&self) -> usize {
match self {
Self::Boolean => size_of::<bool>(),
Expand All @@ -340,11 +349,13 @@ impl ColumnType {
}

/// Returns the bit size of the column type.
#[must_use]
pub fn bit_size(&self) -> u32 {
self.byte_size() as u32 * 8
}

/// Returns if the column type supports signed values.
#[must_use]
pub const fn is_signed(&self) -> bool {
match self {
Self::SmallInt | Self::Int | Self::BigInt | Self::Int128 | Self::TimestampTZ(_, _) => {
Expand Down Expand Up @@ -452,6 +463,7 @@ pub struct ColumnRef {

impl ColumnRef {
/// Create a new `ColumnRef` from a table, column identifier and column type
#[must_use]
pub fn new(table_ref: TableRef, column_id: Identifier, column_type: ColumnType) -> Self {
Self {
column_id,
Expand All @@ -461,16 +473,19 @@ impl ColumnRef {
}

/// Returns the table reference of this column
#[must_use]
pub fn table_ref(&self) -> TableRef {
self.table_ref
}

/// Returns the column identifier of this column
#[must_use]
pub fn column_id(&self) -> Identifier {
self.column_id
}

/// Returns the column type of this column
#[must_use]
pub fn column_type(&self) -> &ColumnType {
&self.column_type
}
Expand All @@ -488,16 +503,19 @@ pub struct ColumnField {

impl ColumnField {
/// Create a new `ColumnField` from a name and a type
#[must_use]
pub fn new(name: Identifier, data_type: ColumnType) -> ColumnField {
ColumnField { name, data_type }
}

/// Returns the name of the column
#[must_use]
pub fn name(&self) -> Identifier {
self.name
}

/// Returns the type of the column
#[must_use]
pub fn data_type(&self) -> ColumnType {
self.data_type
}
Expand Down
4 changes: 4 additions & 0 deletions crates/proof-of-sql/src/base/database/owned_column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ pub enum OwnedColumn<S: Scalar> {

impl<S: Scalar> OwnedColumn<S> {
/// Returns the length of the column.
#[must_use]
pub fn len(&self) -> usize {
match self {
OwnedColumn::Boolean(col) => col.len(),
Expand Down Expand Up @@ -80,6 +81,7 @@ impl<S: Scalar> OwnedColumn<S> {
}

/// Returns the sliced column.
#[must_use]
pub fn slice(&self, start: usize, end: usize) -> Self {
match self {
OwnedColumn::Boolean(col) => OwnedColumn::Boolean(col[start..end].to_vec()),
Expand All @@ -99,6 +101,7 @@ impl<S: Scalar> OwnedColumn<S> {
}

/// Returns true if the column is empty.
#[must_use]
pub fn is_empty(&self) -> bool {
match self {
OwnedColumn::Boolean(col) => col.is_empty(),
Expand All @@ -113,6 +116,7 @@ impl<S: Scalar> OwnedColumn<S> {
}
}
/// Returns the type of the column.
#[must_use]
pub fn column_type(&self) -> ColumnType {
match self {
OwnedColumn::Boolean(_) => ColumnType::Boolean,
Expand Down
5 changes: 5 additions & 0 deletions crates/proof-of-sql/src/base/database/owned_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ impl<S: Scalar> OwnedTable<S> {
Self::try_new(IndexMap::from_iter(iter))
}
/// Number of columns in the table.
#[must_use]
pub fn num_columns(&self) -> usize {
self.table.len()
}
/// Number of rows in the table.
#[must_use]
pub fn num_rows(&self) -> usize {
if self.table.is_empty() {
0
Expand All @@ -51,14 +53,17 @@ impl<S: Scalar> OwnedTable<S> {
}
}
/// Whether the table has no columns.
#[must_use]
pub fn is_empty(&self) -> bool {
self.table.is_empty()
}
/// Returns the columns of this table as an IndexMap
#[must_use]
pub fn into_inner(self) -> IndexMap<Identifier, OwnedColumn<S>> {
self.table
}
/// Returns the columns of this table as an IndexMap
#[must_use]
pub fn inner_table(&self) -> &IndexMap<Identifier, OwnedColumn<S>> {
&self.table
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub fn convert_scalar_to_i256<S: Scalar>(val: &S) -> i256 {

/// Converts an arrow i256 into limbed representation and then
/// into a type implementing [Scalar]
#[must_use]
pub fn convert_i256_to_scalar<S: Scalar>(value: &i256) -> Option<S> {
// Check if value is within the bounds
if value < &MIN_SUPPORTED_I256 || value > &MAX_SUPPORTED_I256 {
Expand Down
4 changes: 4 additions & 0 deletions crates/proof-of-sql/src/base/database/table_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,25 @@ pub struct TableRef {

impl TableRef {
/// Creates a new table reference from a resource id
#[must_use]
pub fn new(resource_id: ResourceId) -> Self {
Self { resource_id }
}

/// Returns the identifier of the schema
#[must_use]
pub fn schema_id(&self) -> Identifier {
self.resource_id.schema()
}

/// Returns the identifier of the table
#[must_use]
pub fn table_id(&self) -> Identifier {
self.resource_id.object_name()
}

/// Returns the underlying resource id of the table
#[must_use]
pub fn resource_id(&self) -> ResourceId {
self.resource_id
}
Expand Down
1 change: 1 addition & 0 deletions crates/proof-of-sql/src/base/math/decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ impl Precision {
}

/// Gets the precision as a u8 for this decimal
#[must_use]
pub fn value(&self) -> u8 {
self.0
}
Expand Down
1 change: 1 addition & 0 deletions crates/proof-of-sql/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![doc = include_str!("../README.md")]
#![cfg_attr(not(feature = "std"), no_std)]
#![allow(clippy::missing_panics_doc)] // Fixed in Issue #163
extern crate alloc;

pub mod base;
Expand Down
Loading

0 comments on commit c845139

Please sign in to comment.