Skip to content

Commit

Permalink
Fix a lot of Clippy warnings (#1808)
Browse files Browse the repository at this point in the history
* fix a lot of clippy warnings

* fix more clippy warnings

* fix yet more clippy warnings

* bump msrv to 1.70.0 to silence warnings

* fix a lot of clippy warnings

* fix more clippy warnings

* fix yet more clippy warnings

* fix a few more warnings

* fix a clippy warning

* remove a commented out line

* silense too many arguments error

* fix more clippy warnings

* prefix underscore to unused vars/functions to fix warnings

* use filter instead of map

* move raw-rs-tests feature flat to module level to fix unused imports warnings

* fix a couple of unused result warnings

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>
  • Loading branch information
imor and Keavon authored Jul 9, 2024
1 parent f7ada70 commit 62f73df
Show file tree
Hide file tree
Showing 49 changed files with 264 additions and 260 deletions.
2 changes: 1 addition & 1 deletion editor/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ mod test {
for message in messages {
block_on(crate::node_graph_executor::run_node_graph());
let mut res = VecDeque::new();
editor.poll_node_graph_evaluation(&mut res);
editor.poll_node_graph_evaluation(&mut res).expect("poll_node_graph_evaluation failed");

let res = editor.handle_message(message);
responses.push(res);
Expand Down
5 changes: 3 additions & 2 deletions editor/src/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl Dispatcher {
if let Some(document) = self.message_handlers.portfolio_message_handler.active_document() {
let data = ToolMessageData {
document_id: self.message_handlers.portfolio_message_handler.active_document_id().unwrap(),
document: document,
document,
input: &self.message_handlers.input_preprocessor_message_handler,
persistent_data: &self.message_handlers.portfolio_message_handler.persistent_data,
node_graph: &self.message_handlers.portfolio_message_handler.executor,
Expand Down Expand Up @@ -451,7 +451,8 @@ mod test {
let portfolio = &mut editor.dispatcher.message_handlers.portfolio_message_handler;
portfolio
.executor
.submit_node_graph_evaluation(portfolio.documents.get_mut(&portfolio.active_document_id.unwrap()).unwrap(), glam::UVec2::ONE);
.submit_node_graph_evaluation(portfolio.documents.get_mut(&portfolio.active_document_id.unwrap()).unwrap(), glam::UVec2::ONE)
.expect("submit_node_graph_evaluation failed");
crate::node_graph_executor::run_node_graph().await;
let mut messages = VecDeque::new();
editor.poll_node_graph_evaluation(&mut messages).expect("Graph should render");
Expand Down
11 changes: 3 additions & 8 deletions editor/src/messages/dialog/simple_dialogs/demo_artwork_dialog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,11 @@ impl LayoutHolder for DemoArtworkDialog {
fn layout(&self) -> Layout {
let mut rows_of_images_with_buttons: Vec<_> = ARTWORK
.chunks(3)
.into_iter()
.map(|chunk| {
let images = chunk
.into_iter()
.map(|(_, thumbnail, _)| ImageLabel::new(*thumbnail).width(Some("256px".into())).widget_holder())
.collect();
.flat_map(|chunk| {
let images = chunk.iter().map(|(_, thumbnail, _)| ImageLabel::new(*thumbnail).width(Some("256px".into())).widget_holder()).collect();

let buttons = chunk
.into_iter()
.iter()
.map(|(name, _, filename)| {
TextButton::new(*name)
.min_width(256)
Expand All @@ -56,7 +52,6 @@ impl LayoutHolder for DemoArtworkDialog {

vec![LayoutGroup::Row { widgets: images }, LayoutGroup::Row { widgets: buttons }, LayoutGroup::Row { widgets: vec![] }]
})
.flatten()
.collect();
let _ = rows_of_images_with_buttons.pop();

Expand Down
4 changes: 2 additions & 2 deletions editor/src/messages/input_mapper/utility_types/input_mouse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ impl ScrollDelta {
}

pub fn as_dvec2(&self) -> DVec2 {
DVec2::new(self.x as f64, self.y as f64)
DVec2::new(self.x, self.y)
}

pub fn scroll_delta(&self) -> f64 {
let (dx, dy) = (self.x, self.y);
dy.signum() as f64 * ((dy * dy + f64::min(dy.abs(), dx.abs()).powi(2)) as f64).sqrt()
dy.signum() * (dy * dy + f64::min(dy.abs(), dx.abs()).powi(2)).sqrt()
}
}

Expand Down
6 changes: 6 additions & 0 deletions editor/src/messages/input_mapper/utility_types/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ impl KeyMappingEntries {
}
}

impl Default for KeyMappingEntries {
fn default() -> Self {
Self::new()
}
}

#[derive(PartialEq, Clone, Debug)]
pub struct MappingEntry {
/// Serves two purposes:
Expand Down
4 changes: 2 additions & 2 deletions editor/src/messages/layout/layout_message_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,11 @@ impl LayoutMessageHandler {
Widget::DropdownInput(dropdown_input) => {
let callback_message = match action {
WidgetValueAction::Commit => {
let update_value = value.as_u64().expect(&format!("DropdownInput commit was not of type `u64`, found {value:?}"));
let update_value = value.as_u64().unwrap_or_else(|| panic!("DropdownInput commit was not of type `u64`, found {value:?}"));
(dropdown_input.entries.iter().flatten().nth(update_value as usize).unwrap().on_commit.callback)(&())
}
WidgetValueAction::Update => {
let update_value = value.as_u64().expect(&format!("DropdownInput update was not of type `u64`, found {value:?}"));
let update_value = value.as_u64().unwrap_or_else(|| panic!("DropdownInput update was not of type `u64`, found {value:?}"));
dropdown_input.selected_index = Some(update_value as u32);
(dropdown_input.entries.iter().flatten().nth(update_value as usize).unwrap().on_update.callback)(&())
}
Expand Down
10 changes: 5 additions & 5 deletions editor/src/messages/layout/utility_types/layout_widget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl WidgetLayout {
// TODO: Diff insersion and deletion of items
if self.layout.len() != new.layout.len() {
// Update the layout to the new layout
self.layout = new.layout.clone();
self.layout.clone_from(&new.layout);

// Push an update sublayout to the diff
let new = DiffUpdate::SubLayout(new.layout);
Expand Down Expand Up @@ -342,7 +342,7 @@ impl LayoutGroup {
Widget::InvisibleStandinInput(_) | Widget::PivotInput(_) | Widget::RadioInput(_) | Widget::Separator(_) | Widget::WorkingColorsInput(_) => continue,
};
if val.is_empty() {
*val = tooltip.clone();
val.clone_from(&tooltip);
}
}
if is_col {
Expand All @@ -361,7 +361,7 @@ impl LayoutGroup {
// TODO: Diff insersion and deletion of items
if current_widgets.len() != new_widgets.len() {
// Update to the new value
*current_widgets = new_widgets.clone();
current_widgets.clone_from(&new_widgets);

// Push back a LayoutGroup update to the diff
let new_value = DiffUpdate::LayoutGroup(if is_column { Self::Column { widgets: new_widgets } } else { Self::Row { widgets: new_widgets } });
Expand Down Expand Up @@ -394,10 +394,10 @@ impl LayoutGroup {
// TODO: Diff insersion and deletion of items
if current_layout.len() != new_layout.len() || *current_name != new_name || *current_visible != new_visible || *current_id != new_id {
// Update self to reflect new changes
*current_name = new_name.clone();
current_name.clone_from(&new_name);
*current_visible = new_visible;
*current_id = new_id;
*current_layout = new_layout.clone();
current_layout.clone_from(&new_layout);

// Push an update layout group to the diff
let new_value = DiffUpdate::LayoutGroup(Self::Section {
Expand Down
39 changes: 19 additions & 20 deletions editor/src/messages/portfolio/document/document_message_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
node_graph_ptz: &mut self.node_graph_ptz,
graph_view_overlay_open: self.graph_view_overlay_open,
node_graph_handler: &self.node_graph_handler,
node_graph_to_viewport: &self.node_graph_to_viewport.entry(self.node_graph_handler.network.clone()).or_insert(DAffine2::IDENTITY),
node_graph_to_viewport: self.node_graph_to_viewport.entry(self.node_graph_handler.network.clone()).or_insert(DAffine2::IDENTITY),
};

self.navigation_handler.process_message(message, responses, data);
Expand Down Expand Up @@ -221,7 +221,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
collapsed: &mut self.collapsed,
ipp,
graph_view_overlay_open: self.graph_view_overlay_open,
node_graph_to_viewport: &self.node_graph_to_viewport.entry(self.node_graph_handler.network.clone()).or_insert(DAffine2::IDENTITY),
node_graph_to_viewport: self.node_graph_to_viewport.entry(self.node_graph_handler.network.clone()).or_insert(DAffine2::IDENTITY),
},
);
}
Expand Down Expand Up @@ -384,7 +384,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag

// TODO: Update click targets when node graph is closed so that this is not necessary
if self.graph_view_overlay_open {
self.node_graph_handler.update_all_click_targets(&mut self.network, self.node_graph_handler.network.clone())
self.node_graph_handler.update_all_click_targets(&self.network, self.node_graph_handler.network.clone())
}

responses.add(FrontendMessage::TriggerGraphViewOverlay { open });
Expand Down Expand Up @@ -606,7 +606,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
// Reconnect layer_to_move to new parent at insert index.
responses.add(GraphOperationMessage::InsertNodeAtStackIndex {
node_id: layer_to_move.to_node(),
parent: parent,
parent,
insert_index,
});
}
Expand Down Expand Up @@ -1047,13 +1047,13 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
});

// Get the node that feeds into the primary input for the folder (if it exists)
if let Some(NodeInput::Node { node_id, .. }) = self.network.nodes.get(&folder.to_node()).expect("Folder should always exist").inputs.get(0) {
if let Some(NodeInput::Node { node_id, .. }) = self.network.nodes.get(&folder.to_node()).expect("Folder should always exist").inputs.first() {
let upstream_sibling_id = *node_id;

// Get the node at the bottom of the first layer node stack
let mut last_child_node_id = child_layer.to_node();
loop {
let Some(NodeInput::Node { node_id, .. }) = self.network.nodes.get(&last_child_node_id).expect("Child node should always exist").inputs.get(0) else {
let Some(NodeInput::Node { node_id, .. }) = self.network.nodes.get(&last_child_node_id).expect("Child node should always exist").inputs.first() else {
break;
};
last_child_node_id = *node_id;
Expand Down Expand Up @@ -1276,7 +1276,7 @@ impl DocumentMessageHandler {
self.selected_nodes
.selected_nodes(network)
.filter_map(|node| {
let Some(node_metadata) = self.node_graph_handler.node_metadata.get(&node) else {
let Some(node_metadata) = self.node_graph_handler.node_metadata.get(node) else {
log::debug!("Could not get click target for node {node}");
return None;
};
Expand Down Expand Up @@ -1437,7 +1437,7 @@ impl DocumentMessageHandler {
// Push the UpdateOpenDocumentsList message to the bus in order to update the save status of the open documents
responses.add(PortfolioMessage::UpdateOpenDocumentsList);
// If there is no history return and don't broadcast SelectionChanged
let Some(network) = self.document_undo_history.pop_back() else { return None };
let network = self.document_undo_history.pop_back()?;

responses.add(BroadcastEvent::SelectionChanged);

Expand All @@ -1460,7 +1460,7 @@ impl DocumentMessageHandler {
// Push the UpdateOpenDocumentsList message to the bus in order to update the save status of the open documents
responses.add(PortfolioMessage::UpdateOpenDocumentsList);
// If there is no history return and don't broadcast SelectionChanged
let Some(network) = self.document_redo_history.pop_back() else { return None };
let network = self.document_redo_history.pop_back()?;

responses.add(BroadcastEvent::SelectionChanged);

Expand All @@ -1480,9 +1480,9 @@ impl DocumentMessageHandler {
};

for (node_id, current_node) in &network.nodes {
if let Some(previous_node) = previous_nested_network.nodes.get(&node_id) {
if let Some(previous_node) = previous_nested_network.nodes.get(node_id) {
if previous_node.alias == current_node.alias
&& previous_node.inputs.iter().map(|node_input| node_input.is_exposed()).count() == current_node.inputs.iter().map(|node_input| node_input.is_exposed()).count()
&& previous_node.inputs.iter().filter(|node_input| node_input.is_exposed()).count() == current_node.inputs.iter().filter(|node_input| node_input.is_exposed()).count()
&& previous_node.is_layer == current_node.is_layer
&& previous_node.metadata.position == current_node.metadata.position
{
Expand Down Expand Up @@ -1542,9 +1542,7 @@ impl DocumentMessageHandler {
};

// Downstream layer should always exist
let Some((downstream_layer_node_id, downstream_layer_is_parent)) = downstream_layer else {
return None;
};
let (downstream_layer_node_id, downstream_layer_is_parent) = downstream_layer?;

// Horizontal traversal if layer_to_move is the top of its layer stack, primary traversal if not
let flow_type = if downstream_layer_is_parent { FlowType::HorizontalFlow } else { FlowType::PrimaryFlow };
Expand Down Expand Up @@ -2105,11 +2103,12 @@ impl DocumentMessageHandler {

fn root_network() -> NodeNetwork {
{
let mut network = NodeNetwork::default();
network.exports = vec![NodeInput::Value {
tagged_value: TaggedValue::ArtboardGroup(graphene_core::ArtboardGroup::EMPTY),
exposed: true,
}];
network
NodeNetwork {
exports: vec![NodeInput::Value {
tagged_value: TaggedValue::ArtboardGroup(graphene_core::ArtboardGroup::EMPTY),
exposed: true,
}],
..Default::default()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
}),
(Some(downstream_node), None) => responses.add(GraphOperationMessage::SetNodeInput {
node_id: downstream_node,
input_index: input_index,
input_index,
input: NodeInput::node(*new_layer_id, 0),
}),
(None, Some(upstream_node)) => responses.add(GraphOperationMessage::InsertNodeBetween {
Expand Down Expand Up @@ -215,20 +215,20 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
};

responses.add(GraphOperationMessage::ShiftUpstream {
node_id: node_id,
node_id,
shift: offset_to_post_node,
shift_self: true,
});

match (post_node_id, pre_node_id) {
(Some(post_node_id), Some(pre_node_id)) => responses.add(GraphOperationMessage::InsertNodeBetween {
post_node_id: post_node_id,
post_node_input_index: post_node_input_index,
post_node_id,
post_node_input_index,
insert_node_output_index: 0,
insert_node_id: node_id,
insert_node_input_index: 0,
pre_node_output_index: 0,
pre_node_id: pre_node_id,
pre_node_id,
}),
(None, Some(pre_node_id)) => responses.add(GraphOperationMessage::InsertNodeBetween {
post_node_id: document_network.exports_metadata.0,
Expand All @@ -237,7 +237,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
insert_node_id: node_id,
insert_node_input_index: 0,
pre_node_output_index: 0,
pre_node_id: pre_node_id,
pre_node_id,
}),
(Some(post_node_id), None) => responses.add(GraphOperationMessage::SetNodeInput {
node_id: post_node_id,
Expand All @@ -253,13 +253,13 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr

// Shift stack down, starting at the moved node.
responses.add(GraphOperationMessage::ShiftUpstream {
node_id: node_id,
node_id,
shift: IVec2::new(0, 3),
shift_self: true,
});
}
GraphOperationMessage::InsertBooleanOperation { operation } => {
let mut selected_layers = selected_nodes.selected_layers(&document_metadata);
let mut selected_layers = selected_nodes.selected_layers(document_metadata);

let upper_layer = selected_layers.next();
let lower_layer = selected_layers.next();
Expand Down Expand Up @@ -466,7 +466,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
}
}
GraphOperationMessage::MoveSelectedSiblingsToChild { new_parent } => {
let Some(group_parent) = new_parent.parent(&document_metadata) else {
let Some(group_parent) = new_parent.parent(document_metadata) else {
log::error!("Could not find parent for layer {:?}", new_parent);
return;
};
Expand All @@ -475,7 +475,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
let mut selected_siblings = Vec::new();

// Skip over horizontal non layer node chain that feeds into parent
let Some(mut current_stack_node_id) = group_parent.first_child(&document_metadata).and_then(|current_stack_node| Some(current_stack_node.to_node())) else {
let Some(mut current_stack_node_id) = group_parent.first_child(document_metadata).map(|current_stack_node| current_stack_node.to_node()) else {
log::error!("Folder should always have child");
return;
};
Expand All @@ -486,14 +486,14 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr

// Check if the current stack node is a selected layer
if selected_nodes
.selected_layers(&document_metadata)
.selected_layers(document_metadata)
.any(|selected_node_id| selected_node_id.to_node() == *current_stack_node_id)
{
selected_siblings.push(*current_stack_node_id);

// Push all non layer sibling nodes directly upstream of the selected layer
loop {
let Some(NodeInput::Node { node_id, .. }) = current_stack_node.inputs.get(0) else { break };
let Some(NodeInput::Node { node_id, .. }) = current_stack_node.inputs.first() else { break };

let next_node = document_network.nodes.get(node_id).expect("Stack node id should always be a node");

Expand All @@ -510,7 +510,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
}

// Get next node
let Some(NodeInput::Node { node_id, .. }) = current_stack_node.inputs.get(0) else { break };
let Some(NodeInput::Node { node_id, .. }) = current_stack_node.inputs.first() else { break };
*current_stack_node_id = *node_id;
}

Expand Down Expand Up @@ -567,7 +567,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
let new_ids: HashMap<_, _> = nodes.iter().map(|(&id, _)| (id, NodeId(generate_uuid()))).collect();

if let Some(node) = modify_inputs.document_network.nodes.get_mut(&id) {
node.alias = alias.clone();
node.alias.clone_from(&alias);
}

let shift = nodes
Expand Down Expand Up @@ -703,9 +703,9 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
responses.add(DocumentMessage::StartTransaction);

// If any of the selected nodes are hidden, show them all. Otherwise, hide them all.
let visible = !selected_nodes.selected_layers(&document_metadata).all(|layer| document_metadata.node_is_visible(layer.to_node()));
let visible = !selected_nodes.selected_layers(document_metadata).all(|layer| document_metadata.node_is_visible(layer.to_node()));

for layer in selected_nodes.selected_layers(&document_metadata) {
for layer in selected_nodes.selected_layers(document_metadata) {
responses.add(GraphOperationMessage::SetVisibility { node_id: layer.to_node(), visible });
}
}
Expand Down Expand Up @@ -738,9 +738,9 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
responses.add(DocumentMessage::StartTransaction);

// If any of the selected nodes are locked, show them all. Otherwise, hide them all.
let locked = !selected_nodes.selected_layers(&document_metadata).all(|layer| document_metadata.node_is_locked(layer.to_node()));
let locked = !selected_nodes.selected_layers(document_metadata).all(|layer| document_metadata.node_is_locked(layer.to_node()));

for layer in selected_nodes.selected_layers(&document_metadata) {
for layer in selected_nodes.selected_layers(document_metadata) {
responses.add(GraphOperationMessage::SetLocked { node_id: layer.to_node(), locked });
}
}
Expand Down
Loading

0 comments on commit 62f73df

Please sign in to comment.