fix antipatterns

Author: myfreess

cli/render.mbt

@@ -36,10 +36,10 @@ fn render_skipped_file_warnings(skipped_files : Array[SkippedFile]) -> String {
 
 ///|
 fn render_skipped_file_warning(skipped : SkippedFile) -> String {
-  match skipped.block_start_line {
-    Some(line) =>
-      "warning: skipping \{skipped.file} block starting at line \{line}: \{skipped.reason}"
-    None => "warning: skipping \{skipped.file}: \{skipped.reason}"
+  if skipped.block_start_line is Some(line) {
+    "warning: skipping \{skipped.file} block starting at line \{line}: \{skipped.reason}"
+  } else {
+    "warning: skipping \{skipped.file}: \{skipped.reason}"
   }
 }
 
@@ -226,7 +226,7 @@ fn format_location(file : String, loc : @basic.Location) -> String {
 fn trim_trailing_newlines(input : String) -> String {
   let mut end = input.length()
   while end > 0 && input[end - 1:end].to_owned() == "\n" {
-    end = end - 1
+    end -= 1
   }
   input[:end].to_owned()
 }

cli/render_wbtest.mbt

@@ -52,8 +52,8 @@ async test "matched source line prefix is not dimmed" {
     content="12 | let value = \u{1b}[93mtarget()\u{1b}[39m",
   )
   let arr = [5, 6, 7]
-  for i = 0; i < arr.length(); i = i + 1 {
-    if arr[i] == 0 {
+  for i, value in arr {
+    if value == 0 {
       @stdio.stdout.write("\{i}\n")
     }
   }

cli/scan_wbtest.mbt

@@ -333,7 +333,7 @@ async test "scan excludes configured rule ids before planning" {
 async test "scan rejects unknown exclude rule ids" {
   try collect_directory_hits(unknown_exclude_rule_scan_options()) catch {
     CliError::Usage(message~, exit_code=found) => {
-      inspect(found, content="2")
+      assert_eq(found, 2)
       inspect(message, content="unknown rule id in --exclude-rule: missing")
     }
     err => fail("unexpected error \{err}")

main.mbt

@@ -9,9 +9,10 @@ extern "c" fn runtime_exit(code : Int) -> Unit = "exit"
 ///|
 async fn write_cli_output(output : String) -> Unit {
   if output != "" {
-    match output.strip_suffix("\n") {
-      Some(_) => @stdio.stdout.write(output)
-      None => @stdio.stdout.write(output + "\n")
+    if output.strip_suffix("\n") is Some(_) {
+      @stdio.stdout.write(output)
+    } else {
+      @stdio.stdout.write(output + "\n")
     }
   }
 }

matching/ellipsis.mbt

@@ -57,9 +57,10 @@ fn ellipsis_item_raw(item : @untyped_ast.Node) -> String? {
       matching_ellipsis_direct_raw(pattern)
     }
     ArrayPattern_Pattern | SpreadableElem_Regular =>
-      match child(item, "value") {
-        Some(value) => matching_ellipsis_direct_raw(value)
-        None => None
+      if child(item, "value") is Some(value) {
+        matching_ellipsis_direct_raw(value)
+      } else {
+        None
       }
     Parameter_Positional => {
       guard child(item, "binder") is Some(binder) else { return None }
@@ -77,9 +78,10 @@ fn matching_ellipsis_direct_raw(node : @untyped_ast.Node) -> String? {
   let raw = match node.kind {
     Expr_Ident => node.normalized_expr_identifier_name()
     Pattern_Var =>
-      match child(node, "value") {
-        Some(binder) => leaf_string_child(binder, "name")
-        None => None
+      if child(node, "value") is Some(binder) {
+        leaf_string_child(binder, "name")
+      } else {
+        None
       }
     Binder => leaf_string_child(node, "name")
     Type_Name => type_placeholder_name(node)

matching/matching.mbt

@@ -181,17 +181,18 @@ fn match_expr_placeholder(
     contains_name(compiled.expr_metavars, name) {
     Some(bind_value(bindings, name, candidate))
   } else if contains_name(compiled.identifier_metavars, name) {
-    match candidate.normalized_expr_identifier_name() {
-      Some(candidate_name) =>
-        Some(
-          bind_value(bindings, name, string_binding(candidate_name, candidate)),
-        )
-      None => Some(false)
+    if candidate.normalized_expr_identifier_name() is Some(candidate_name) {
+      Some(
+        bind_value(bindings, name, string_binding(candidate_name, candidate)),
+      )
+    } else {
+      Some(false)
     }
   } else if contains_name(compiled.constant_metavars, name) {
-    match expr_constant(candidate) {
-      Some(constant) => Some(bind_value(bindings, name, constant))
-      None => Some(false)
+    if expr_constant(candidate) is Some(constant) {
+      Some(bind_value(bindings, name, constant))
+    } else {
+      Some(false)
     }
   } else {
     None
@@ -216,17 +217,18 @@ fn match_pattern_placeholder(
   } else if is_ignore_placeholder(name) {
     Some(true)
   } else if contains_name(compiled.identifier_metavars, name) {
-    match candidate.normalized_pattern_identifier_name() {
-      Some(candidate_name) =>
-        Some(
-          bind_value(bindings, name, string_binding(candidate_name, candidate)),
-        )
-      None => Some(false)
+    if candidate.normalized_pattern_identifier_name() is Some(candidate_name) {
+      Some(
+        bind_value(bindings, name, string_binding(candidate_name, candidate)),
+      )
+    } else {
+      Some(false)
     }
   } else if contains_name(compiled.constant_metavars, name) {
-    match pattern_constant(candidate) {
-      Some(constant) => Some(bind_value(bindings, name, constant))
-      None => Some(false)
+    if pattern_constant(candidate) is Some(constant) {
+      Some(bind_value(bindings, name, constant))
+    } else {
+      Some(false)
     }
   } else {
     None
@@ -783,35 +785,35 @@ fn constructor_name(node : @untyped_ast.Node) -> String? {
 ///|
 fn normalized_constructor(constr : @untyped_ast.Node) -> String {
   let name = constructor_name(constr).unwrap_or("")
-  match child(constr, "extra_info") {
-    Some(extra) =>
-      match extra.kind {
-        ConstructorExtraInfo_NoExtraInfo => name
-        ConstructorExtraInfo_Package =>
-          if leaf_string_child(extra, "value") is Some(pkg) {
-            "@\{pkg}.\{name}"
+  if child(constr, "extra_info") is Some(extra) {
+    match extra.kind {
+      ConstructorExtraInfo_NoExtraInfo => name
+      ConstructorExtraInfo_Package =>
+        if leaf_string_child(extra, "value") is Some(pkg) {
+          "@\{pkg}.\{name}"
+        } else {
+          name
+        }
+      ConstructorExtraInfo_TypeName =>
+        if child(extra, "value") is Some(type_name) {
+          "\{normalized_type_name(type_name)}::\{name}"
+        } else {
+          name
+        }
+      ConstructorExtraInfo_TypeNameWithConstrPackage =>
+        if child(extra, "type_name") is Some(type_name) {
+          if leaf_string_child(extra, "pkg") is Some(pkg) {
+            "\{normalized_type_name(type_name)}::@\{pkg}.\{name}"
           } else {
-            name
-          }
-        ConstructorExtraInfo_TypeName =>
-          if child(extra, "value") is Some(type_name) {
             "\{normalized_type_name(type_name)}::\{name}"
-          } else {
-            name
           }
-        ConstructorExtraInfo_TypeNameWithConstrPackage =>
-          if child(extra, "type_name") is Some(type_name) {
-            if leaf_string_child(extra, "pkg") is Some(pkg) {
-              "\{normalized_type_name(type_name)}::@\{pkg}.\{name}"
-            } else {
-              "\{normalized_type_name(type_name)}::\{name}"
-            }
-          } else {
-            name
-          }
-        _ => name
-      }
-    None => name
+        } else {
+          name
+        }
+      _ => name
+    }
+  } else {
+    name
   }
 }
 

matching/matching_test.mbt

@@ -371,13 +371,13 @@ test "argument metavar captures complete argument nodes" {
     source in [
       "sink(value)", "sink(label=value)", "sink(label~)", "sink(label?=value)", "sink(label?)",
     ] {
-    match match_expr_pattern(compiled, parse_expr(source)) {
-      Some(result) =>
-        match result.bindings.get("arg") {
-          Some(Single(node)) => assert_true(node.kind == Argument)
-          _ => fail("expected argument binding")
-        }
-      None => fail("expected argument metavar to match \{source}")
+    if match_expr_pattern(compiled, parse_expr(source)) is Some(result) {
+      match result.bindings.get("arg") {
+        Some(Single(node)) => assert_true(node.kind == Argument)
+        _ => fail("expected argument binding")
+      }
+    } else {
+      fail("expected argument metavar to match \{source}")
     }
   }
   assert_false(match_expr_pattern(compiled, parse_expr("sink()")) is Some(_))

matching/untyped_matching_test.mbt

@@ -193,9 +193,10 @@ test "default matcher binds repeated expression metavars" {
 ///|
 test "default matcher treats only $_ as ignore placeholder" {
   let compiled = untyped_compiled(parse_untyped_test_metavar_node("foo($_)"))
-  match untyped_match(compiled, "foo(make())") {
-    Some(result) => inspect(result.bindings.length(), content="0")
-    None => fail("expected $_ ignore placeholder to match")
+  if untyped_match(compiled, "foo(make())") is Some(result) {
+    assert_eq(result.bindings.length(), 0)
+  } else {
+    fail("expected $_ ignore placeholder to match")
   }
   assert_true(untyped_match(compiled, "foo(other())") is Some(_))
   assert_true(
@@ -306,14 +307,14 @@ test "default matcher captures complex type nodes" {
       "let value : Array[Int] = input", "let value : Int? = input", "let value : (Int, String) = input",
       "let value : (Int) -> String = input",
     ] {
-    match untyped_match(compiled, source) {
-      Some(result) =>
-        match result.bindings.get("T") {
-          Some(Single(node)) =>
-            assert_true(node.kind.to_string().has_prefix("Type::"))
-          _ => fail("expected type binding for \{source}")
-        }
-      None => fail("expected type metavar to match \{source}")
+    if untyped_match(compiled, source) is Some(result) {
+      match result.bindings.get("T") {
+        Some(Single(node)) =>
+          assert_true(node.kind.to_string().has_prefix("Type::"))
+        _ => fail("expected type binding for \{source}")
+      }
+    } else {
+      fail("expected type metavar to match \{source}")
     }
   }
 }

query/query_test.mbt

@@ -148,7 +148,7 @@ test "query captures identifiers and constants by metavar name" {
     fail("expected second literal value")
   }
   inspect(first_value, content="danger")
-  inspect(second_value, content="42")
+  assert_eq(second_value, "42")
 }
 
 ///|
@@ -225,7 +225,7 @@ test "query raises parse error when relevant source is invalid" {
     #|}
     #|
   try query.captures(source_name="bad.mbt", source) catch {
-    err => inspect("\{Repr(err)}".contains("parse"), content="true")
+    err => assert_true("\{Repr(err)}".contains("parse"))
   } noraise {
     _ => fail("expected parse error")
   }
@@ -234,7 +234,7 @@ test "query raises parse error when relevant source is invalid" {
 ///|
 test "query raises when pattern is invalid" {
   try ExprQuery::ExprQuery("target(") catch {
-    err => inspect("\{Repr(err)}".contains("InvalidRule"), content="true")
+    err => assert_true("\{Repr(err)}".contains("InvalidRule"))
   } noraise {
     _ => fail("expected invalid pattern")
   }
@@ -267,7 +267,7 @@ test "query captures_from_ast matches a direct expression root" {
   guard query_constant_value(literal) is Some(value) else {
     fail("expected literal value")
   }
-  inspect(value, content="42")
+  assert_eq(value, "42")
 }
 
 ///|

rule/apply/apply.mbt

@@ -110,13 +110,12 @@ fn scan_plan_literal_matches(
   literal_cache : Map[String, Bool],
   literal : String,
 ) -> Bool {
-  match literal_cache.get(literal) {
-    Some(matched) => matched
-    None => {
-      let matched = source.contains(literal)
-      literal_cache[literal] = matched
-      matched
-    }
+  if literal_cache.get(literal) is Some(matched) {
+    matched
+  } else {
+    let matched = source.contains(literal)
+    literal_cache[literal] = matched
+    matched
   }
 }