From a758208c20e6daec4fc4826a5c49a80d5e1b4e3d Mon Sep 17 00:00:00 2001 From: Daniel Harvey Date: Wed, 11 Dec 2024 17:43:58 +0000 Subject: [PATCH] Add lint rule, remove stuff it finds --- Cargo.toml | 2 ++ crates/ndc-sqlserver/src/error/convert.rs | 2 +- crates/query-engine/sql/src/sql/helpers.rs | 6 +++--- .../sql/src/sql/rewrites/constant_folding.rs | 12 ++++++------ .../translation/src/translation/mutation/mod.rs | 5 +---- .../src/translation/query/filtering.rs | 8 ++++---- .../translation/src/translation/query/root.rs | 6 +++--- .../translation/src/translation/query/sorting.rs | 15 ++++++--------- 8 files changed, 26 insertions(+), 30 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 45ea79f4..b0795a88 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,8 @@ members = [ [workspace.lints.clippy] all = { level = "warn", priority = -1 } pedantic = { level = "warn", priority = -1 } +# extra pedanticness +redundant_clone = "warn" # disable certain pedantic warnings doc_markdown = "allow" missing_errors_doc = "allow" diff --git a/crates/ndc-sqlserver/src/error/convert.rs b/crates/ndc-sqlserver/src/error/convert.rs index 35cbfbe4..ebbb6e2a 100644 --- a/crates/ndc-sqlserver/src/error/convert.rs +++ b/crates/ndc-sqlserver/src/error/convert.rs @@ -7,7 +7,7 @@ pub fn execution_error_to_response(error: query_engine_execution::error::Error) use query_engine_execution::error::*; match error { Error::Query(query_error) => { - connector::QueryError::new_invalid_request(&query_error.to_string()).into() + connector::QueryError::new_invalid_request(&query_error).into() } Error::Mutation(mutation_error) => { connector::MutationError::new_invalid_request(&mutation_error.to_string()).into() diff --git a/crates/query-engine/sql/src/sql/helpers.rs b/crates/query-engine/sql/src/sql/helpers.rs index 7ee19d40..fc3eea94 100644 --- a/crates/query-engine/sql/src/sql/helpers.rs +++ b/crates/query-engine/sql/src/sql/helpers.rs @@ -161,7 +161,7 @@ pub fn select_rowset( let mut final_row_select = simple_select(rows_row); final_row_select.from = Some(From::Select { - alias: row_table_alias.clone(), + alias: row_table_alias, select: Box::new(row_select), alias_path: AliasPath { elements: vec![row_column_alias], @@ -264,7 +264,7 @@ pub fn select_rowset( final_select.joins = vec![Join::CrossJoin(CrossJoin { select: Box::new(aggregate_select_star), - alias: aggregate_table_alias.clone(), + alias: aggregate_table_alias, alias_path: AliasPath { elements: vec![aggregate_column_alias], }, @@ -478,7 +478,7 @@ pub fn select_mutation_rowset( final_select.joins = vec![Join::CrossJoin(CrossJoin { select: Box::new(aggregate_select_star), - alias: aggregate_table_alias.clone(), + alias: aggregate_table_alias, alias_path: AliasPath { elements: vec![make_column_alias("aggregates".to_string())], }, diff --git a/crates/query-engine/sql/src/sql/rewrites/constant_folding.rs b/crates/query-engine/sql/src/sql/rewrites/constant_folding.rs index 6fba712d..356f1c6e 100644 --- a/crates/query-engine/sql/src/sql/rewrites/constant_folding.rs +++ b/crates/query-engine/sql/src/sql/rewrites/constant_folding.rs @@ -216,7 +216,7 @@ mod tests { fn true_and_true_is_true() { let left_side = expr_true(); let right_side = expr_true(); - let expr = expr_and(left_side, right_side.clone()); + let expr = expr_and(left_side, right_side); assert_eq!(normalize_expr(expr), expr_true()); } @@ -224,7 +224,7 @@ mod tests { fn false_or_false_is_false() { let left_side = expr_false(); let right_side = expr_false(); - let expr = expr_or(left_side, right_side.clone()); + let expr = expr_or(left_side, right_side); assert_eq!(normalize_expr(expr), expr_false()); } @@ -232,7 +232,7 @@ mod tests { fn true_and_false_is_false() { let left_side = expr_true(); let right_side = expr_false(); - let expr = expr_and(left_side, right_side.clone()); + let expr = expr_and(left_side, right_side); assert_eq!(normalize_expr(expr), expr_false()); } @@ -240,7 +240,7 @@ mod tests { fn false_or_true_is_true() { let left_side = expr_false(); let right_side = expr_true(); - let expr = expr_or(left_side, right_side.clone()); + let expr = expr_or(left_side, right_side); assert_eq!(normalize_expr(expr), expr_true()); } @@ -273,8 +273,8 @@ mod tests { fn eq_expr_is_not_removed() { let eq_expr = expr_eq(expr_seven(), expr_seven()); let left_side = expr_seven(); - let right_side = expr_and(eq_expr.clone(), eq_expr.clone()); - let expr = expr_and(left_side, right_side.clone()); + let right_side = expr_and(eq_expr.clone(), eq_expr); + let expr = expr_and(left_side, right_side); assert_eq!(normalize_expr(expr.clone()), expr); } diff --git a/crates/query-engine/translation/src/translation/mutation/mod.rs b/crates/query-engine/translation/src/translation/mutation/mod.rs index 114ad5ce..5c0c34b2 100644 --- a/crates/query-engine/translation/src/translation/mutation/mod.rs +++ b/crates/query-engine/translation/src/translation/mutation/mod.rs @@ -70,10 +70,7 @@ fn translate_mutation_operation( }) } ProcedureInfo::StoredProcedure { name, info } => { - let stored_procedure_info = super::helpers::StoredProcedureInfo { - name: name.to_string(), - info, - }; + let stored_procedure_info = super::helpers::StoredProcedureInfo { name, info }; MutationOperationKind::StoredProcedure(stored_procedure_info) } }; diff --git a/crates/query-engine/translation/src/translation/query/filtering.rs b/crates/query-engine/translation/src/translation/query/filtering.rs index 587a80ee..040c79c7 100644 --- a/crates/query-engine/translation/src/translation/query/filtering.rs +++ b/crates/query-engine/translation/src/translation/query/filtering.rs @@ -350,7 +350,7 @@ fn translate_comparison_target( Ok(( sql::ast::Expression::ColumnReference(sql::ast::ColumnReference::TableColumn { - table: table_ref.reference.clone(), + table: table_ref.reference, name, }), joins, @@ -443,7 +443,7 @@ pub fn translate_exists_in_collection( let column_alias = sql::helpers::make_column_alias("one".to_string()); let select_cols = vec![( - column_alias.clone(), + column_alias, sql::ast::Expression::Value(sql::ast::Value::Int8(1)), )]; @@ -455,7 +455,7 @@ pub fn translate_exists_in_collection( root_table: root_and_current_tables.root_table.clone(), current_table: TableNameAndReference { reference: table.reference.clone(), - name: table.name.clone(), + name: table.name, }, }; @@ -502,7 +502,7 @@ pub fn translate_exists_in_collection( let column_alias = sql::helpers::make_column_alias("one".to_string()); let select_cols = vec![( - column_alias.clone(), + column_alias, sql::ast::Expression::Value(sql::ast::Value::Int8(1)), )]; diff --git a/crates/query-engine/translation/src/translation/query/root.rs b/crates/query-engine/translation/src/translation/query/root.rs index 59f7a7e4..320d85d7 100644 --- a/crates/query-engine/translation/src/translation/query/root.rs +++ b/crates/query-engine/translation/src/translation/query/root.rs @@ -187,7 +187,7 @@ pub fn translate_rows_query( let column_info = collection_info.lookup_column(&column)?; Ok(sql::helpers::make_column( current_table.reference.clone(), - column_info.name.clone(), + column_info.name, sql::helpers::make_column_alias(alias.to_string()), )) } @@ -202,7 +202,7 @@ pub fn translate_rows_query( let json_column_alias = sql::helpers::make_json_column_alias(); let column_name = sql::ast::ColumnReference::AliasedColumn { table: sql::ast::TableReference::AliasedTable(table_alias.clone()), - column: json_column_alias.clone(), + column: json_column_alias, }; join_fields.push(relationships::JoinFieldInfo { table_alias, @@ -362,7 +362,7 @@ pub fn make_from_clause_and_reference( let current_table = TableNameAndReference { name: collection_name.to_string(), - reference: collection_alias_name.clone(), + reference: collection_alias_name, }; Ok((current_table, from_clause)) } diff --git a/crates/query-engine/translation/src/translation/query/sorting.rs b/crates/query-engine/translation/src/translation/query/sorting.rs index 7c4743f0..2f345c20 100644 --- a/crates/query-engine/translation/src/translation/query/sorting.rs +++ b/crates/query-engine/translation/src/translation/query/sorting.rs @@ -352,7 +352,7 @@ fn translate_order_by_target_for_column( table: root_and_current_tables.current_table.reference.clone(), // we are going to deliberately use the table column name and not an alias we get from // the query request because this is internal to the sorting mechanism. - column: sql::helpers::make_column_alias(selected_column.name.0.clone()), + column: sql::helpers::make_column_alias(selected_column.name.0), }; Ok(ColumnOrSelect::Column(selected_column_name)) } @@ -367,18 +367,16 @@ fn translate_order_by_target_for_column( table: last_table.reference, // we are going to deliberately use the table column name and not an alias we get from // the query request because this is internal to the sorting mechanism. - column: sql::helpers::make_column_alias(selected_column.name.0.clone()), + column: sql::helpers::make_column_alias(selected_column.name.0), }; // if we got a function, we wrap the required column with // a function call. let selected_column_expr = match function { - None => sql::ast::Expression::ColumnReference(selected_column_name.clone()), + None => sql::ast::Expression::ColumnReference(selected_column_name), Some(func) => sql::ast::Expression::FunctionCall { function: sql::ast::Function::Unknown(func.to_string()), - args: vec![sql::ast::Expression::ColumnReference( - selected_column_name.clone(), - )], + args: vec![sql::ast::Expression::ColumnReference(selected_column_name)], }, }; @@ -467,7 +465,7 @@ fn process_path_element_for_order_by_target_for_column( // we are going to deliberately use the table column name and not an alias we get from // the query request because this is internal to the sorting mechanism. let selected_column_alias = - sql::helpers::make_column_alias(selected_column.name.0.clone()); + sql::helpers::make_column_alias(selected_column.name.0); // we use the real name of the column as an alias as well. Ok(( selected_column_alias.clone(), @@ -486,8 +484,7 @@ fn process_path_element_for_order_by_target_for_column( let selected_column = target_collection.lookup_column(&target_column_name.into())?; // we are going to deliberately use the table column name and not an alias we get from // the query request because this is internal to the sorting mechanism. - let selected_column_alias = - sql::helpers::make_column_alias(selected_column.name.0.clone()); + let selected_column_alias = sql::helpers::make_column_alias(selected_column.name.0); // we use the real name of the column as an alias as well. Ok(vec![( selected_column_alias.clone(),