Merge rust-lang#10440

10440: Fix Clippy warnings and replace some `if let`s with `match` r=Veykril a=arzg

I decided to try fixing a bunch of Clippy warnings. I am aware of this project’s opinion of Clippy (I have read both [rust-lang/clippy#5537](rust-lang/rust-clippy#5537) and [rust-analyzer/rowan#57 (comment)](rust-analyzer/rowan#57 (comment))), so I totally understand if part of or the entirety of this PR is rejected. In particular, I can see how the semicolons and `if let` vs `match` commits provide comparatively little benefit when compared to the ensuing churn.

I tried to separate each kind of change into its own commit to make it easier to discard certain changes. I also only applied Clippy suggestions where I thought they provided a definite improvement to the code (apart from semicolons, which is IMO more of a formatting/consistency question than a linting question). In the end I accumulated a list of 28 Clippy lints I ignored entirely.

Sidenote: I should really have asked about this on Zulip before going through all 1,555 `if let`s in the codebase to decide which ones definitely look better as `match` :P

Co-authored-by: Aramis Razzaghipour <[email protected]>

crates/flycheck/src/lib.rs

@@ -179,7 +179,7 @@ impl FlycheckActor {
  tracing::error!(
  "Flycheck failed to run the following command: {:?}",
  self.check_command()
- )
+ );
  }
  self.progress(Progress::DidFinish(res));
  }
@@ -253,14 +253,14 @@ impl FlycheckActor {
  }
 
  fn send(&self, check_task: Message) {
- (self.sender)(check_task)
+ (self.sender)(check_task);
  }
 }
 
 struct CargoHandle {
  child: JodChild,
  #[allow(unused)]
- thread: jod_thread::JoinHandle<io::Result<bool>>,
+ thread: jod_thread::JoinHandle<bool>,
  receiver: Receiver<CargoMessage>,
 }
 
@@ -279,7 +279,7 @@ impl CargoHandle {
  // It is okay to ignore the result, as it only errors if the process is already dead
  let _ = self.child.kill();
  let exit_status = self.child.wait()?;
- let read_at_least_one_message = self.thread.join()?;
+ let read_at_least_one_message = self.thread.join();
  if !exit_status.success() && !read_at_least_one_message {
  // FIXME: Read the stderr to display the reason, see `read2()` reference in PR comment:
  // https://github.com/rust-analyzer/rust-analyzer/pull/3632#discussion_r395605298
@@ -304,7 +304,7 @@ impl CargoActor {
  fn new(child_stdout: process::ChildStdout, sender: Sender<CargoMessage>) -> CargoActor {
  CargoActor { child_stdout, sender }
  }
- fn run(self) -> io::Result<bool> {
+ fn run(self) -> bool {
  // We manually read a line at a time, instead of using serde's
  // stream deserializers, because the deserializer cannot recover
  // from an error, resulting in it getting stuck, because we try to
@@ -334,20 +334,20 @@ impl CargoActor {
  // Skip certain kinds of messages to only spend time on what's useful
  JsonMessage::Cargo(message) => match message {
  cargo_metadata::Message::CompilerArtifact(artifact) if !artifact.fresh => {
- self.sender.send(CargoMessage::CompilerArtifact(artifact)).unwrap()
+ self.sender.send(CargoMessage::CompilerArtifact(artifact)).unwrap();
  }
  cargo_metadata::Message::CompilerMessage(msg) => {
- self.sender.send(CargoMessage::Diagnostic(msg.message)).unwrap()
+ self.sender.send(CargoMessage::Diagnostic(msg.message)).unwrap();
  }
  _ => (),
  },
  JsonMessage::Rustc(message) => {
- self.sender.send(CargoMessage::Diagnostic(message)).unwrap()
+ self.sender.send(CargoMessage::Diagnostic(message)).unwrap();
  }
  }
  }
  }
- Ok(read_at_least_one_message)
+ read_at_least_one_message
  }
 }
 

crates/hir/src/lib.rs

@@ -1119,7 +1119,7 @@ impl DefWithBody {
  if let ast::Expr::RecordExpr(record_expr) =
  &source_ptr.value.to_node(&root)
  {
- if let Some(_) = record_expr.record_expr_field_list() {
+ if record_expr.record_expr_field_list().is_some() {
  acc.push(
  MissingFields {
  file: source_ptr.file_id,
@@ -1143,7 +1143,7 @@ impl DefWithBody {
  if let Some(expr) = source_ptr.value.as_ref().left() {
  let root = source_ptr.file_syntax(db.upcast());
  if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) {
- if let Some(_) = record_pat.record_pat_field_list() {
+ if record_pat.record_pat_field_list().is_some() {
  acc.push(
  MissingFields {
  file: source_ptr.file_id,
@@ -2119,10 +2119,9 @@ impl Impl {
  };
 
  let fp = TyFingerprint::for_inherent_impl(&ty);
- let fp = if let Some(fp) = fp {
- fp
- } else {
- return Vec::new();
+ let fp = match fp {
+ Some(fp) => fp,
+ None => return Vec::new(),
  };
 
  let mut all = Vec::new();

crates/hir_def/src/body/lower.rs

@@ -474,10 +474,9 @@ impl ExprCollector<'_> {
  }
  ast::Expr::PrefixExpr(e) => {
  let expr = self.collect_expr_opt(e.expr());
- if let Some(op) = e.op_kind() {
- self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr)
- } else {
- self.alloc_expr(Expr::Missing, syntax_ptr)
+ match e.op_kind() {
+ Some(op) => self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr),
+ None => self.alloc_expr(Expr::Missing, syntax_ptr),
  }
  }
  ast::Expr::ClosureExpr(e) => {
@@ -624,10 +623,9 @@ impl ExprCollector<'_> {
  }
 
  fn collect_expr_opt(&mut self, expr: Option<ast::Expr>) -> ExprId {
- if let Some(expr) = expr {
- self.collect_expr(expr)
- } else {
- self.missing_expr()
+ match expr {
+ Some(expr) => self.collect_expr(expr),
+ None => self.missing_expr(),
  }
  }
 
@@ -724,10 +722,9 @@ impl ExprCollector<'_> {
  }
 
  fn collect_block_opt(&mut self, expr: Option<ast::BlockExpr>) -> ExprId {
- if let Some(block) = expr {
- self.collect_block(block)
- } else {
- self.missing_expr()
+ match expr {
+ Some(block) => self.collect_block(block),
+ None => self.missing_expr(),
  }
  }
 
@@ -890,10 +887,9 @@ impl ExprCollector<'_> {
  }
 
  fn collect_pat_opt(&mut self, pat: Option<ast::Pat>) -> PatId {
- if let Some(pat) = pat {
- self.collect_pat(pat)
- } else {
- self.missing_pat()
+ match pat {
+ Some(pat) => self.collect_pat(pat),
+ None => self.missing_pat(),
  }
  }
 

crates/hir_def/src/find_path.rs

@@ -209,10 +209,9 @@ fn find_path_inner(
  ) {
  path.push_segment(name);
 
- let new_path = if let Some(best_path) = best_path {
- select_best_path(best_path, path, prefer_no_std)
- } else {
- path
+ let new_path = match best_path {
+ Some(best_path) => select_best_path(best_path, path, prefer_no_std),
+ None => path,
  };
  best_path_len = new_path.len();
  best_path = Some(new_path);
@@ -243,10 +242,9 @@ fn find_path_inner(
  });
 
  for path in extern_paths {
- let new_path = if let Some(best_path) = best_path {
- select_best_path(best_path, path, prefer_no_std)
- } else {
- path
+ let new_path = match best_path {
+ Some(best_path) => select_best_path(best_path, path, prefer_no_std),
+ None => path,
  };
  best_path = Some(new_path);
  }
@@ -261,12 +259,11 @@ fn find_path_inner(
  }
  }
 
- if let Some(prefix) = prefixed.map(PrefixKind::prefix) {
- best_path.or_else(|| {
+ match prefixed.map(PrefixKind::prefix) {
+ Some(prefix) => best_path.or_else(|| {
  scope_name.map(|scope_name| ModPath::from_segments(prefix, vec![scope_name]))
- })
- } else {
- best_path
+ }),
+ None => best_path,
  }
 }
 
@@ -346,15 +343,13 @@ fn find_local_import_locations(
 
  if let Some((name, vis)) = data.scope.name_of(item) {
  if vis.is_visible_from(db, from) {
- let is_private = if let Visibility::Module(private_to) = vis {
- private_to.local_id == module.local_id
- } else {
- false
+ let is_private = match vis {
+ Visibility::Module(private_to) => private_to.local_id == module.local_id,
+ Visibility::Public => false,
  };
- let is_original_def = if let Some(module_def_id) = item.as_module_def_id() {
- data.scope.declarations().any(|it| it == module_def_id)
- } else {
- false
+ let is_original_def = match item.as_module_def_id() {
+ Some(module_def_id) => data.scope.declarations().any(|it| it == module_def_id),
+ None => false,
  };
 
  // Ignore private imports. these could be used if we are

crates/hir_def/src/item_tree.rs

@@ -475,10 +475,9 @@ macro_rules! mod_items {
  }
 
  fn id_from_mod_item(mod_item: ModItem) -> Option<FileItemTreeId<Self>> {
- if let ModItem::$typ(id) = mod_item {
- Some(id)
- } else {
- None
+ match mod_item {
+ ModItem::$typ(id) => Some(id),
+ _ => None,
  }
  }
 

crates/hir_def/src/nameres/path_resolution.rs

@@ -400,13 +400,10 @@ impl DefMap {
  };
  let from_scope_or_builtin = match shadow {
  BuiltinShadowMode::Module => from_scope.or(from_builtin),
- BuiltinShadowMode::Other => {
- if let Some(ModuleDefId::ModuleId(_)) = from_scope.take_types() {
- from_builtin.or(from_scope)
- } else {
- from_scope.or(from_builtin)
- }
- }
+ BuiltinShadowMode::Other => match from_scope.take_types() {
+ Some(ModuleDefId::ModuleId(_)) => from_builtin.or(from_scope),
+ Some(_) | None => from_scope.or(from_builtin),
+ },
  };
  let from_extern_prelude = self
  .extern_prelude

crates/hir_def/src/path/lower/lower_use.rs

@@ -18,10 +18,9 @@ pub(crate) fn convert_path(
  path: ast::Path,
  hygiene: &Hygiene,
 ) -> Option<ModPath> {
- let prefix = if let Some(qual) = path.qualifier() {
- Some(convert_path(db, prefix, qual, hygiene)?)
- } else {
- prefix
+ let prefix = match path.qualifier() {
+ Some(qual) => Some(convert_path(db, prefix, qual, hygiene)?),
+ None => prefix,
  };
 
  let segment = path.segment()?;

crates/hir_def/src/type_ref.rs

@@ -214,10 +214,9 @@ impl TypeRef {
  }
 
  pub(crate) fn from_ast_opt(ctx: &LowerCtx, node: Option<ast::Type>) -> Self {
- if let Some(node) = node {
- TypeRef::from_ast(ctx, node)
- } else {
- TypeRef::Error
+ match node {
+ Some(node) => TypeRef::from_ast(ctx, node),
+ None => TypeRef::Error,
  }
  }
 

crates/hir_expand/src/name.rs

@@ -48,10 +48,9 @@ impl Name {
 
  /// Resolve a name from the text of token.
  fn resolve(raw_text: &str) -> Name {
- if let Some(text) = raw_text.strip_prefix("r#") {
- Name::new_text(SmolStr::new(text))
- } else {
- Name::new_text(raw_text.into())
+ match raw_text.strip_prefix("r#") {
+ Some(text) => Name::new_text(SmolStr::new(text)),
+ None => Name::new_text(raw_text.into()),
  }
  }
 

crates/hir_ty/src/autoderef.rs

@@ -109,10 +109,9 @@ pub(crate) fn deref(
  ty: InEnvironment<&Canonical<Ty>>,
 ) -> Option<Canonical<Ty>> {
  let _p = profile::span("deref");
- if let Some(derefed) = builtin_deref(&ty.goal.value) {
- Some(Canonical { value: derefed, binders: ty.goal.binders.clone() })
- } else {
- deref_by_trait(db, krate, ty)
+ match builtin_deref(&ty.goal.value) {
+ Some(derefed) => Some(Canonical { value: derefed, binders: ty.goal.binders.clone() }),
+ None => deref_by_trait(db, krate, ty),
  }
 }
 

crates/hir_ty/src/chalk_ext.rs

@@ -104,10 +104,9 @@ impl TyExt for Ty {
  }
 
  fn as_fn_def(&self, db: &dyn HirDatabase) -> Option<FunctionId> {
- if let Some(CallableDefId::FunctionId(func)) = self.callable_def(db) {
- Some(func)
- } else {
- None
+ match self.callable_def(db) {
+ Some(CallableDefId::FunctionId(func)) => Some(func),
+ Some(CallableDefId::StructId(_) | CallableDefId::EnumVariantId(_)) | None => None,
  }
  }
  fn as_reference(&self) -> Option<(&Ty, Lifetime, Mutability)> {

crates/hir_ty/src/diagnostics/match_check/deconstruct_pat.rs

@@ -105,10 +105,9 @@ impl IntRange {
 
  #[inline]
  fn from_range(lo: u128, hi: u128, scalar_ty: Scalar) -> IntRange {
- if let Scalar::Bool = scalar_ty {
- IntRange { range: lo..=hi }
- } else {
- unimplemented!()
+ match scalar_ty {
+ Scalar::Bool => IntRange { range: lo..=hi },
+ _ => unimplemented!(),
  }
  }
 

crates/hir_ty/src/display.rs

@@ -167,10 +167,9 @@ impl<'a> HirFormatter<'a> {
  }
 
  pub fn should_truncate(&self) -> bool {
- if let Some(max_size) = self.max_size {
- self.curr_size >= max_size
- } else {
- false
+ match self.max_size {
+ Some(max_size) => self.curr_size >= max_size,
+ None => false,
  }
  }
 

crates/hir_ty/src/infer/expr.rs

@@ -264,10 +264,9 @@ impl<'a> InferenceContext<'a> {
 
  // collect explicitly written argument types
  for arg_type in arg_types.iter() {
- let arg_ty = if let Some(type_ref) = arg_type {
- self.make_ty(type_ref)
- } else {
- self.table.new_type_var()
+ let arg_ty = match arg_type {
+ Some(type_ref) => self.make_ty(type_ref),
+ None => self.table.new_type_var(),
  };
  sig_tys.push(arg_ty);
  }

crates/hir_ty/src/infer/pat.rs

@@ -204,10 +204,9 @@ impl<'a> InferenceContext<'a> {
  } else {
  BindingMode::convert(*mode)
  };
- let inner_ty = if let Some(subpat) = subpat {
- self.infer_pat(*subpat, &expected, default_bm)
- } else {
- expected
+ let inner_ty = match subpat {
+ Some(subpat) => self.infer_pat(*subpat, &expected, default_bm),
+ None => expected,
  };
  let inner_ty = self.insert_type_vars_shallow(inner_ty);
 

crates/hir_ty/src/infer/unify.rs

@@ -324,10 +324,9 @@ impl<'a> InferenceTable<'a> {
 
  /// Unify two types and register new trait goals that arise from that.
  pub(crate) fn unify(&mut self, ty1: &Ty, ty2: &Ty) -> bool {
- let result = if let Ok(r) = self.try_unify(ty1, ty2) {
- r
- } else {
- return false;
+ let result = match self.try_unify(ty1, ty2) {
+ Ok(r) => r,
+ Err(_) => return false,
  };
  self.register_infer_ok(result);
  true