Skip to content

Allow clippy public API related complaints #317

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Allow clippy public API related complaints #317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77d8a63
make UDT types' names Rust case conformant
wprzytula Jun 6, 2025
97e856a
clippy.toml: disable `avoid-breaking-exported-api`
wprzytula Jun 6, 2025
79eac67
PR template: mention Makefile for test filters
wprzytula Jun 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,4 @@
- [ ] All commit messages clearly explain what they change and why.
- [ ] PR description sums up the changes and reasons why they should be introduced.
- [ ] I have implemented Rust unit tests for the features/changes introduced.
- [ ] I have enabled appropriate tests in `.github/workflows/build.yml` in `gtest_filter`.
- [ ] I have enabled appropriate tests in `.github/workflows/cassandra.yml` in `gtest_filter`.
- [ ] I have enabled appropriate tests in `Makefile` in `{SCYLLA,CASSANDRA}_(NO_VALGRIND_)TEST_FILTER`.
3 changes: 2 additions & 1 deletion scylla-rust-wrapper/clippy.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ disallowed-methods = [
"std::rc::Rc::into_raw",

"const_ptr::as_ref",
"mut_ptr::as_mut"
"mut_ptr::as_mut",
]
avoid-breaking-exported-api = false
56 changes: 28 additions & 28 deletions scylla-rust-wrapper/src/cass_types.rs
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit: make UDT types' names Rust case conformant

I don't agree with removing DataType suffix from UDTDataType - I think changing to UdtDataType is enough.
Why?

  • The prefix is not really redundant. DataType is a concept from cpp-driver, so a separate thing from defined type (which DT in UDT expands to).
  • Udt may suggest it holds the contents of UDT, while it really only represents the type.
  • It introduces inconsistency with other variants of CassDataTypeInner.

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub(crate) use crate::cass_data_types::CassValueType;

#[derive(Clone, Debug)]
#[cfg_attr(test, derive(PartialEq, Eq))]
pub struct UDTDataType {
pub struct Udt {
// Vec to preserve the order of types
pub field_types: Vec<(String, Arc<CassDataType>)>,

Expand All @@ -25,18 +25,18 @@ pub struct UDTDataType {
pub frozen: bool,
}

impl UDTDataType {
pub fn new() -> UDTDataType {
UDTDataType {
impl Udt {
pub fn new() -> Udt {
Udt {
field_types: Vec::new(),
keyspace: "".to_string(),
name: "".to_string(),
frozen: false,
}
}

pub fn with_capacity(capacity: usize) -> UDTDataType {
UDTDataType {
pub fn with_capacity(capacity: usize) -> Udt {
Udt {
field_types: Vec::with_capacity(capacity),
keyspace: "".to_string(),
name: "".to_string(),
Expand All @@ -59,7 +59,7 @@ impl UDTDataType {
self.field_types.get(index).map(|(_, b)| b)
}

fn typecheck_equals(&self, other: &UDTDataType) -> bool {
fn typecheck_equals(&self, other: &Udt) -> bool {
// See: https://github.com/scylladb/cpp-driver/blob/master/src/data_type.hpp#L354-L386

if !any_string_empty_or_both_equal(&self.keyspace, &other.keyspace) {
Expand Down Expand Up @@ -101,7 +101,7 @@ fn any_string_empty_or_both_equal(s1: &str, s2: &str) -> bool {
s1.is_empty() || s2.is_empty() || s1 == s2
}

impl Default for UDTDataType {
impl Default for Udt {
fn default() -> Self {
Self::new()
}
Expand All @@ -125,7 +125,7 @@ pub struct CassColumnSpec {
#[cfg_attr(test, derive(PartialEq, Eq))]
pub enum CassDataTypeInner {
Value(CassValueType),
UDT(UDTDataType),
Udt(Udt),
List {
// None stands for untyped list.
typ: Option<Arc<CassDataType>>,
Expand Down Expand Up @@ -156,8 +156,8 @@ impl CassDataTypeInner {
pub fn typecheck_equals(&self, other: &CassDataTypeInner) -> bool {
match self {
CassDataTypeInner::Value(t) => *t == other.get_value_type(),
CassDataTypeInner::UDT(udt) => match other {
CassDataTypeInner::UDT(other_udt) => udt.typecheck_equals(other_udt),
CassDataTypeInner::Udt(udt) => match other {
CassDataTypeInner::Udt(other_udt) => udt.typecheck_equals(other_udt),
_ => false,
},
CassDataTypeInner::List { typ, .. } | CassDataTypeInner::Set { typ, .. } => match other
Expand Down Expand Up @@ -298,7 +298,7 @@ fn native_type_to_cass_value_type(native_type: &NativeType) -> CassValueType {
impl CassDataTypeInner {
fn get_sub_data_type(&self, index: usize) -> Option<&Arc<CassDataType>> {
match self {
CassDataTypeInner::UDT(udt_data_type) => {
CassDataTypeInner::Udt(udt_data_type) => {
udt_data_type.field_types.get(index).map(|(_, b)| b)
}
CassDataTypeInner::List { typ, .. } | CassDataTypeInner::Set { typ, .. } => {
Expand Down Expand Up @@ -366,17 +366,17 @@ impl CassDataTypeInner {
}
}

pub fn get_udt_type(&self) -> &UDTDataType {
pub fn get_udt_type(&self) -> &Udt {
match self {
CassDataTypeInner::UDT(udt) => udt,
CassDataTypeInner::Udt(udt) => udt,
_ => panic!("Can get UDT out of non-UDT data type"),
}
}

pub fn get_value_type(&self) -> CassValueType {
match &self {
CassDataTypeInner::Value(value_data_type) => *value_data_type,
CassDataTypeInner::UDT { .. } => CassValueType::CASS_VALUE_TYPE_UDT,
CassDataTypeInner::Udt { .. } => CassValueType::CASS_VALUE_TYPE_UDT,
CassDataTypeInner::List { .. } => CassValueType::CASS_VALUE_TYPE_LIST,
CassDataTypeInner::Set { .. } => CassValueType::CASS_VALUE_TYPE_SET,
CassDataTypeInner::Map { .. } => CassValueType::CASS_VALUE_TYPE_MAP,
Expand Down Expand Up @@ -415,7 +415,7 @@ pub fn get_column_type(column_type: &ColumnType) -> CassDataType {
typ: Some(Arc::new(get_column_type(boxed_type.as_ref()))),
frozen: *frozen,
},
UserDefinedType { definition, frozen } => CassDataTypeInner::UDT(UDTDataType {
UserDefinedType { definition, frozen } => CassDataTypeInner::Udt(Udt {
field_types: definition
.field_types
.iter()
Expand Down Expand Up @@ -461,7 +461,7 @@ pub unsafe extern "C" fn cass_data_type_new(
typ: MapDataType::Untyped,
frozen: false,
},
CassValueType::CASS_VALUE_TYPE_UDT => CassDataTypeInner::UDT(UDTDataType::new()),
CassValueType::CASS_VALUE_TYPE_UDT => CassDataTypeInner::Udt(Udt::new()),
CassValueType::CASS_VALUE_TYPE_CUSTOM => CassDataTypeInner::Custom("".to_string()),
CassValueType::CASS_VALUE_TYPE_UNKNOWN => return ArcFFI::null(),
t if t < CassValueType::CASS_VALUE_TYPE_LAST_ENTRY => CassDataTypeInner::Value(t),
Expand Down Expand Up @@ -497,8 +497,8 @@ pub unsafe extern "C" fn cass_data_type_new_tuple(
pub unsafe extern "C" fn cass_data_type_new_udt(
field_count: size_t,
) -> CassOwnedSharedPtr<CassDataType, CMut> {
ArcFFI::into_ptr(CassDataType::new_arced(CassDataTypeInner::UDT(
UDTDataType::with_capacity(field_count as usize),
ArcFFI::into_ptr(CassDataType::new_arced(CassDataTypeInner::Udt(
Udt::with_capacity(field_count as usize),
)))
}

Expand Down Expand Up @@ -529,7 +529,7 @@ pub unsafe extern "C" fn cass_data_type_is_frozen(
};

let is_frozen = match unsafe { data_type.get_unchecked() } {
CassDataTypeInner::UDT(udt) => udt.frozen,
CassDataTypeInner::Udt(udt) => udt.frozen,
CassDataTypeInner::List { frozen, .. } => *frozen,
CassDataTypeInner::Set { frozen, .. } => *frozen,
CassDataTypeInner::Map { frozen, .. } => *frozen,
Expand All @@ -551,7 +551,7 @@ pub unsafe extern "C" fn cass_data_type_type_name(
};

match unsafe { data_type.get_unchecked() } {
CassDataTypeInner::UDT(UDTDataType { name, .. }) => {
CassDataTypeInner::Udt(Udt { name, .. }) => {
unsafe { write_str_to_c(name, type_name, type_name_length) };
CassError::CASS_OK
}
Expand Down Expand Up @@ -583,7 +583,7 @@ pub unsafe extern "C" fn cass_data_type_set_type_name_n(
.to_string();

match unsafe { data_type.get_mut_unchecked() } {
CassDataTypeInner::UDT(udt_data_type) => {
CassDataTypeInner::Udt(udt_data_type) => {
udt_data_type.name = type_name_string;
CassError::CASS_OK
}
Expand All @@ -603,7 +603,7 @@ pub unsafe extern "C" fn cass_data_type_keyspace(
};

match unsafe { data_type.get_unchecked() } {
CassDataTypeInner::UDT(UDTDataType { name, .. }) => {
CassDataTypeInner::Udt(Udt { name, .. }) => {
unsafe { write_str_to_c(name, keyspace, keyspace_length) };
CassError::CASS_OK
}
Expand Down Expand Up @@ -635,7 +635,7 @@ pub unsafe extern "C" fn cass_data_type_set_keyspace_n(
.to_string();

match unsafe { data_type.get_mut_unchecked() } {
CassDataTypeInner::UDT(udt_data_type) => {
CassDataTypeInner::Udt(udt_data_type) => {
udt_data_type.keyspace = keyspace_string;
CassError::CASS_OK
}
Expand Down Expand Up @@ -705,7 +705,7 @@ pub unsafe extern "C" fn cass_data_type_sub_type_count(

match unsafe { data_type.get_unchecked() } {
CassDataTypeInner::Value(..) => 0,
CassDataTypeInner::UDT(udt_data_type) => udt_data_type.field_types.len() as size_t,
CassDataTypeInner::Udt(udt_data_type) => udt_data_type.field_types.len() as size_t,
CassDataTypeInner::List { typ, .. } | CassDataTypeInner::Set { typ, .. } => {
typ.is_some() as size_t
}
Expand Down Expand Up @@ -769,7 +769,7 @@ pub unsafe extern "C" fn cass_data_type_sub_data_type_by_name_n(

let name_str = unsafe { ptr_to_cstr_n(name, name_length) }.unwrap();
match unsafe { data_type.get_unchecked() } {
CassDataTypeInner::UDT(udt) => match udt.get_field_by_name(name_str) {
CassDataTypeInner::Udt(udt) => match udt.get_field_by_name(name_str) {
None => ArcFFI::null(),
Some(t) => ArcFFI::as_ptr(t),
},
Expand All @@ -790,7 +790,7 @@ pub unsafe extern "C" fn cass_data_type_sub_type_name(
};

match unsafe { data_type.get_unchecked() } {
CassDataTypeInner::UDT(udt) => match udt.field_types.get(index as usize) {
CassDataTypeInner::Udt(udt) => match udt.field_types.get(index as usize) {
None => CassError::CASS_ERROR_LIB_INDEX_OUT_OF_BOUNDS,
Some((field_name, _)) => {
unsafe { write_str_to_c(field_name, name, name_length) };
Expand Down Expand Up @@ -855,7 +855,7 @@ pub unsafe extern "C" fn cass_data_type_add_sub_type_by_name_n(
.to_string();

match unsafe { data_type.get_mut_unchecked() } {
CassDataTypeInner::UDT(udt_data_type) => {
CassDataTypeInner::Udt(udt_data_type) => {
// The Cpp Driver does not check whether field_types size
// exceeded field_count.
udt_data_type.field_types.push((name_string, sub_data_type));
Expand Down
2 changes: 1 addition & 1 deletion scylla-rust-wrapper/src/iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ impl<'result> CassUdtIterator<'result> {

// SAFETY: `CassDataType` is obtained from `CassResultMetadata`, which is immutable.
let metadata = match unsafe { value.value_type.get_unchecked() } {
CassDataTypeInner::UDT(udt) => udt.field_types.as_slice(),
CassDataTypeInner::Udt(udt) => udt.field_types.as_slice(),
_ => panic!("Expected UDT type. Typecheck should have prevented such scenario!"),
};

Expand Down
2 changes: 1 addition & 1 deletion scylla-rust-wrapper/src/user_type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub unsafe extern "C" fn cass_user_type_new_from_data_type(
};

match unsafe { data_type.get_unchecked() } {
CassDataTypeInner::UDT(udt_data_type) => {
CassDataTypeInner::Udt(udt_data_type) => {
let field_values = vec![None; udt_data_type.field_types.len()];
BoxFFI::into_ptr(Box::new(CassUserType {
data_type,
Expand Down
21 changes: 10 additions & 11 deletions scylla-rust-wrapper/src/value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ mod tests {
use scylla::value::{CqlDate, CqlDecimal, CqlDuration};

use crate::{
cass_types::{CassDataType, CassDataTypeInner, CassValueType, MapDataType, UDTDataType},
cass_types::{CassDataType, CassDataTypeInner, CassValueType, MapDataType, Udt},
value::{CassCqlValue, is_type_compatible},
};

Expand Down Expand Up @@ -631,7 +631,7 @@ mod tests {
let user_udt_name = "user".to_owned();
let empty_str = "".to_owned();

let data_type_udt_simple = CassDataType::new_arced(CassDataTypeInner::UDT(UDTDataType {
let data_type_udt_simple = CassDataType::new_arced(CassDataTypeInner::Udt(Udt {
field_types: simple_fields.clone(),
keyspace: ks_keyspace_name.clone(),
name: user_udt_name.clone(),
Expand Down Expand Up @@ -770,14 +770,14 @@ mod tests {
// UDT
{
let data_type_udt_simple_empty_keyspace =
CassDataType::new_arced(CassDataTypeInner::UDT(UDTDataType {
CassDataType::new_arced(CassDataTypeInner::Udt(Udt {
field_types: simple_fields.clone(),
keyspace: empty_str.to_owned(),
name: user_udt_name.clone(),
frozen: false,
}));
let data_type_udt_simple_empty_name =
CassDataType::new_arced(CassDataTypeInner::UDT(UDTDataType {
CassDataType::new_arced(CassDataTypeInner::Udt(Udt {
field_types: simple_fields.clone(),
keyspace: ks_keyspace_name.clone(),
name: empty_str.clone(),
Expand All @@ -789,13 +789,12 @@ mod tests {
("foo".to_owned(), data_type_float.clone()),
("bar".to_owned(), data_type_bool.clone()),
];
let data_type_udt_small =
CassDataType::new_arced(CassDataTypeInner::UDT(UDTDataType {
field_types: small_fields.clone(),
keyspace: ks_keyspace_name.clone(),
name: user_udt_name.clone(),
frozen: false,
}));
let data_type_udt_small = CassDataType::new_arced(CassDataTypeInner::Udt(Udt {
field_types: small_fields.clone(),
keyspace: ks_keyspace_name.clone(),
name: user_udt_name.clone(),
frozen: false,
}));

let test_cases = &[TestCase {
value: CassCqlValue::UserDefinedType {
Expand Down