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.

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

Add redundant-clones lint rule, remove stuff it finds #164

Merged
merged 1 commit into from
Dec 11, 2024
Merged

Add redundant-clones lint rule, remove stuff it finds #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion crates/ndc-sqlserver/src/error/convert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions crates/query-engine/sql/src/sql/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -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],
},
Expand Down Expand Up @@ -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())],
},
Expand Down
12 changes: 6 additions & 6 deletions crates/query-engine/sql/src/sql/rewrites/constant_folding.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,31 +216,31 @@ 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());
}

#[test]
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());
}

#[test]
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());
}

#[test]
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());
}

Expand Down Expand Up @@ -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);
}

Expand Down
5 changes: 1 addition & 4 deletions crates/query-engine/translation/src/translation/mutation/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
};
Expand Down
8 changes: 4 additions & 4 deletions crates/query-engine/translation/src/translation/query/filtering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)),
)];

Expand All @@ -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,
},
};

Expand Down Expand Up @@ -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)),
)];

Expand Down
6 changes: 3 additions & 3 deletions crates/query-engine/translation/src/translation/query/root.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
))
}
Expand All @@ -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,
Expand Down Expand Up @@ -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))
}
Expand Down
15 changes: 6 additions & 9 deletions crates/query-engine/translation/src/translation/query/sorting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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)],
},
};

Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down
Loading