diff --git a/cli/render.mbt b/cli/render.mbt index 3e88a3a..6034e83 100644 --- a/cli/render.mbt +++ b/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() } diff --git a/cli/render_wbtest.mbt b/cli/render_wbtest.mbt index 45e4f41..dab408a 100644 --- a/cli/render_wbtest.mbt +++ b/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") } } diff --git a/cli/scan_wbtest.mbt b/cli/scan_wbtest.mbt index 0b1fc15..6c8520a 100644 --- a/cli/scan_wbtest.mbt +++ b/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}") diff --git a/e2etests/BUILTIN_RULES.md b/e2etests/BUILTIN_RULES.md index e4e89bc..6a8f34c 100644 --- a/e2etests/BUILTIN_RULES.md +++ b/e2etests/BUILTIN_RULES.md @@ -73,6 +73,21 @@ source: 3 > inspect(flag, content="true") 4 | } +testdata/builtin-rules-all/simplifiable_assignment.mbt:4:3-4:19 +rule: moonbitlang/simplifiable_assignment +description: + Assignment repeats its target as the operand of a binary expression. + Prefer the corresponding augmented assignment, such as foo += 1, foo.bar += 1, foo[i] +=1. + Note: Apply this rewrite only when the target expression and any index expression + are free of side effects. Augmented assignment may evaluate them a different + number of times. +source: +2 | fn simplifiable_assignment(step : Int) -> Int { +3 | let mut foo = 0 +4 > foo = foo + step +5 | foo +6 | } + testdata/builtin-rules-all/cstyle_forward_simple_forloop.mbt:3:3-5:4 rule: moonbitlang/cstyle_forward_simple_forloop description: @@ -135,6 +150,7 @@ $ cd "$TESTDIR"/.. && moonrun "$TESTDIR"/moongrep.wasm -- scan --output-json --e {"file":"testdata/builtin-rules-all/inspect_number.mbt","rule_id":"moonbitlang/inspect_number","description":"Found inspect() snapshots whose expected value is a plain number.\nPrefer numeric assertions for numeric checks.","range":{"start":{"line":3,"column":3},"end":{"line":3,"column":26}},"matched_source":"inspect(1, content=\"1\")","source_context":[{"line":1,"text":"///|","is_match":false},{"line":2,"text":"fn number_snapshot() -> Unit {","is_match":false},{"line":3,"text":" inspect(1, content=\"1\")","is_match":true},{"line":4,"text":"}","is_match":false}]} {"file":"testdata/builtin-rules-all/unnessary_else.mbt","rule_id":"moonbitlang/unnessary_else","description":"Found an if expression whose else branch is empty or only returns ().\nPrefer omitting the unnecessary else branch.","range":{"start":{"line":3,"column":3},"end":{"line":6,"column":12}},"matched_source":"if flag {\n prepare()\n finish()\n } else {}","source_context":[{"line":1,"text":"///|","is_match":false},{"line":2,"text":"fn unnecessary_empty_else(flag : Bool) -> Unit {","is_match":false},{"line":3,"text":" if flag {","is_match":true},{"line":4,"text":" prepare()","is_match":true},{"line":5,"text":" finish()","is_match":true},{"line":6,"text":" } else {}","is_match":true},{"line":7,"text":"}","is_match":false},{"line":8,"text":"///|","is_match":false}]} {"file":"testdata/builtin-rules-all/inspect_boolean.mbt","rule_id":"moonbitlang/inspect_boolean","description":"Found inspect(), debug_inspect(), or json_inspect() snapshots whose expected value is true or false.\nPrefer assert_true(...) or assert_false(...) for boolean checks.","range":{"start":{"line":3,"column":3},"end":{"line":3,"column":32}},"matched_source":"inspect(flag, content=\"true\")","source_context":[{"line":1,"text":"///|","is_match":false},{"line":2,"text":"fn boolean_snapshot(flag : Bool) -> Unit {","is_match":false},{"line":3,"text":" inspect(flag, content=\"true\")","is_match":true},{"line":4,"text":"}","is_match":false}]} +{"file":"testdata/builtin-rules-all/simplifiable_assignment.mbt","rule_id":"moonbitlang/simplifiable_assignment","description":"Assignment repeats its target as the operand of a binary expression.\nPrefer the corresponding augmented assignment, such as foo += 1, foo.bar += 1, foo[i] +=1.\nNote: Apply this rewrite only when the target expression and any index expression\nare free of side effects. Augmented assignment may evaluate them a different\nnumber of times.","range":{"start":{"line":4,"column":3},"end":{"line":4,"column":19}},"matched_source":"foo = foo + step","source_context":[{"line":2,"text":"fn simplifiable_assignment(step : Int) -> Int {","is_match":false},{"line":3,"text":" let mut foo = 0","is_match":false},{"line":4,"text":" foo = foo + step","is_match":true},{"line":5,"text":" foo","is_match":false},{"line":6,"text":"}","is_match":false}]} {"file":"testdata/builtin-rules-all/cstyle_forward_simple_forloop.mbt","rule_id":"moonbitlang/cstyle_forward_simple_forloop","description":"C-style forward for loops that can be rewritten as simple for-in loops.","range":{"start":{"line":3,"column":3},"end":{"line":5,"column":4}},"matched_source":"for i = 0; i < limit; i = i + 1 {\n tick()\n }","source_context":[{"line":1,"text":"///|","is_match":false},{"line":2,"text":"fn forward_simple_loop(limit : Int) -> Unit {","is_match":false},{"line":3,"text":" for i = 0; i < limit; i = i + 1 {","is_match":true},{"line":4,"text":" tick()","is_match":true},{"line":5,"text":" }","is_match":true},{"line":6,"text":"}","is_match":false}]} {"file":"testdata/builtin-rules-all/cstyle_backward_simple_forloop.mbt","rule_id":"moonbitlang/cstyle_backward_simple_forloop","description":"C-style backward for loops that can be rewritten as simple for-in loops.","range":{"start":{"line":3,"column":3},"end":{"line":5,"column":4}},"matched_source":"for i = limit; i > 0; i = i - 1 {\n tick_back()\n }","source_context":[{"line":1,"text":"///|","is_match":false},{"line":2,"text":"fn backward_simple_loop(limit : Int) -> Unit {","is_match":false},{"line":3,"text":" for i = limit; i > 0; i = i - 1 {","is_match":true},{"line":4,"text":" tick_back()","is_match":true},{"line":5,"text":" }","is_match":true},{"line":6,"text":"}","is_match":false}]} {"file":"testdata/builtin-rules-all/cstyle_forward_array_iteration.mbt","rule_id":"moonbitlang/cstyle_forward_array_iteration","description":"C-style forward array iteration that can be rewritten as simple for-in loops.","range":{"start":{"line":3,"column":3},"end":{"line":5,"column":4}},"matched_source":"for i = 0; i < items.length(); i = i + 1 {\n consume(items[i])\n }","source_context":[{"line":1,"text":"///|","is_match":false},{"line":2,"text":"fn forward_array_loop(items : Array[Int]) -> Unit {","is_match":false},{"line":3,"text":" for i = 0; i < items.length(); i = i + 1 {","is_match":true},{"line":4,"text":" consume(items[i])","is_match":true},{"line":5,"text":" }","is_match":true},{"line":6,"text":"}","is_match":false}]} @@ -153,3 +169,41 @@ moonbitlang/catch_all 7:3 moonbitlang/catch_all 11:3 moonbitlang/catch_all 17:3 ``` + +## Simplifiable assignment variants + +All eighteen rule shapes are reported: local variables, fields, and indexed +values using `+`, `-`, `*`, or `/`, including both operand orders supported for +commutative operators and compound expressions in the other operand. + +```mooncram +$ cd "$TESTDIR"/.. && moonrun "$TESTDIR"/moongrep.wasm -- scan --output-json --enable-builtin-rules testdata/builtin-simplifiable-assignment-variants/positive.mbt | sed -n 's#.*"rule_id":"moonbitlang/simplifiable_assignment".*"range":{"start":{"line":\([0-9][0-9]*\),"column":\([0-9][0-9]*\)}.*"matched_source":"\([^"]*\)".*#\1:\2 \3#p' +14:3 add_left = add_left + step * 2 +16:3 add_right = step * 2 + add_right +18:3 subtract = subtract - step +20:3 multiply_left = multiply_left * (step + 1) +22:3 multiply_right = (step + 1) * multiply_right +24:3 divide = divide / step +32:3 target.add_left = target.add_left + step * 2 +33:3 target.add_right = step * 2 + target.add_right +34:3 target.subtract = target.subtract - step +35:3 target.multiply_left = target.multiply_left * (step + 1) +36:3 target.multiply_right = (step + 1) * target.multiply_right +37:3 target.divide = target.divide / step +42:3 target[0] = target[0] + step * 2 +43:3 target[1] = step * 2 + target[1] +44:3 target[2] = target[2] - step +45:3 target[3] = target[3] * (step + 1) +46:3 target[4] = (step + 1) * target[4] +47:3 target[5] = target[5] / step +``` + +The negative fixture checks unsupported operators, different local, field, or +indexed targets, different field receivers or indexed collections, reversed +`-` and `/`, and existing augmented assignments. It produces no findings for +this rule. + +```mooncram +$ cd "$TESTDIR"/.. && moonrun "$TESTDIR"/moongrep.wasm -- scan --output-json --enable-builtin-rules testdata/builtin-simplifiable-assignment-variants/negative.mbt | sed -n 's#"rule_id":"moonbitlang/simplifiable_assignment"#&#p' | wc -l +0 +``` diff --git a/main.mbt b/main.mbt index f2d0eaf..90579c2 100644 --- a/main.mbt +++ b/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") } } } diff --git a/matching/ellipsis.mbt b/matching/ellipsis.mbt index 79744ec..6f4bca2 100644 --- a/matching/ellipsis.mbt +++ b/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) diff --git a/matching/matching.mbt b/matching/matching.mbt index d14ea41..36d2725 100644 --- a/matching/matching.mbt +++ b/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 } } diff --git a/matching/matching_test.mbt b/matching/matching_test.mbt index 52207ca..258dd4d 100644 --- a/matching/matching_test.mbt +++ b/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(_)) diff --git a/matching/untyped_matching_test.mbt b/matching/untyped_matching_test.mbt index 87efe25..04efe49 100644 --- a/matching/untyped_matching_test.mbt +++ b/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}") } } } diff --git a/query/query_test.mbt b/query/query_test.mbt index 72497d5..1bcabb7 100644 --- a/query/query_test.mbt +++ b/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") } ///| diff --git a/rule/apply/apply.mbt b/rule/apply/apply.mbt index f863b02..8b584d7 100644 --- a/rule/apply/apply.mbt +++ b/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 } } diff --git a/rule/apply/ast_bucket.mbt b/rule/apply/ast_bucket.mbt index d91c417..ebfbd86 100644 --- a/rule/apply/ast_bucket.mbt +++ b/rule/apply/ast_bucket.mbt @@ -122,9 +122,10 @@ fn ExprPatternBuckets::from_patterns( for index in 0.. wildcard.push(indexed) - Some(key) => push_keyed_pattern(keyed, key, indexed) + if key is Some(key) { + push_keyed_pattern(keyed, key, indexed) + } else { + wildcard.push(indexed) } } { wildcard, keyed } diff --git a/rule/apply/ast_bucket_wbtest.mbt b/rule/apply/ast_bucket_wbtest.mbt index fda72af..8514361 100644 --- a/rule/apply/ast_bucket_wbtest.mbt +++ b/rule/apply/ast_bucket_wbtest.mbt @@ -75,9 +75,10 @@ fn manual_source_pattern(shape : String) -> CompiledRulePattern raise { ///| fn root_key_signature(key : @untyped_ast.NodeKind?) -> String { - match key { - None => "*" - Some(kind) => kind.to_string() + if key is Some(kind) { + kind.to_string() + } else { + "*" } } diff --git a/rule/builtin/builtin.mbt b/rule/builtin/builtin.mbt index e8ce110..84ee886 100644 --- a/rule/builtin/builtin.mbt +++ b/rule/builtin/builtin.mbt @@ -45,6 +45,10 @@ fn builtin_rule_sources() -> Array[BuiltinRuleSource] { path: "builtin/moonbitlang/unnessary_else.yaml", yaml: @moonbitlang_rules.unnessary_else_yaml, }, + { + path: "builtin/moonbitlang/simplifiable_assignment.yaml", + yaml: @moonbitlang_rules.simplifiable_assignment_yaml, + }, { path: "builtin/moonbitlang/cstyle_forward_simple_forloop.yaml", yaml: @moonbitlang_rules.cstyle_forward_simple_forloop_yaml, diff --git a/rule/builtin/builtin_test.mbt b/rule/builtin/builtin_test.mbt index 7be2c2f..5a13fea 100644 --- a/rule/builtin/builtin_test.mbt +++ b/rule/builtin/builtin_test.mbt @@ -14,6 +14,7 @@ test "builtin rules load moonbitlang rules" { #| "moonbitlang/inspect_number", #| "moonbitlang/match_option", #| "moonbitlang/unnessary_else", + #| "moonbitlang/simplifiable_assignment", #| "moonbitlang/cstyle_forward_simple_forloop", #| "moonbitlang/cstyle_backward_simple_forloop", #| "moonbitlang/cstyle_forward_array_iteration", @@ -103,3 +104,46 @@ test "builtin catch_all matches async function signature variants" { assert_eq(finding.rule_id, "moonbitlang/catch_all") } } + +///| +test "builtin simplifiable_assignment matches supported augmented assignment operators" { + let source = + #|fn sample(step : Int) -> Unit { + #| let mut add = 0 + #| add = add + step + #| let mut subtract = 0 + #| subtract = subtract - step + #| let mut multiply = 1 + #| multiply = multiply * step + #| let mut divide = 1 + #| divide = divide / step + #| let mut remainder = 1 + #| remainder = remainder % step + #| let mut different = 0 + #| different = add + step + #| let mut reversed = 0 + #| reversed = step - reversed + #| add += step + #|} + #| + let (impls, reports) = @parser.parse_string( + source, + name="simplifiable_assignment test", + ) + guard reports is [] else { fail("expected source to parse") } + let plan = @apply.ScanPlan::from_rules( + @compile.compile_rules(@builtin.load_rules()), + ) + let findings = @apply.apply_structural_scan_plan_to_node( + "sample.mbt", + @untyped_ast.from_impls(impls), + plan, + ) + let matches : Array[String] = [] + for finding in findings { + if finding.rule_id == "moonbitlang/simplifiable_assignment" { + matches.push("\{finding.pattern_index}@\{finding.loc.start.lnum}") + } + } + inspect(matches.join(","), content="0@3,6@5,9@7,15@9") +} diff --git a/rule/compile/compile.mbt b/rule/compile/compile.mbt index ff97744..4337e6e 100644 --- a/rule/compile/compile.mbt +++ b/rule/compile/compile.mbt @@ -350,9 +350,10 @@ fn ast_child_is_empty_list( name : String, kind : @untyped_ast.NodeKind, ) -> Bool { - match ast_child(node, name) { - Some(child) => child.kind == kind && child.children.is_empty() - None => false + if ast_child(node, name) is Some(child) { + child.kind == kind && child.children.is_empty() + } else { + false } } @@ -362,9 +363,10 @@ fn ast_child_has_kind( name : String, kind : @untyped_ast.NodeKind, ) -> Bool { - match ast_child(node, name) { - Some(child) => child.kind == kind - None => false + if ast_child(node, name) is Some(child) { + child.kind == kind + } else { + false } } @@ -1131,16 +1133,16 @@ fn whole_source_target(ast : @untyped_ast.Node) -> TaintTarget? { if node_child(ast, "self") is Some(self) && is_source_placeholder_node(self) { Some(Receiver) + } else if node_child(ast, "args") is Some(args) { + source_target_from_args(args) } else { - match node_child(ast, "args") { - Some(args) => source_target_from_args(args) - None => None - } + None } Expr_Apply => - match node_child(ast, "args") { - Some(args) => source_target_from_args(args) - None => None + if node_child(ast, "args") is Some(args) { + source_target_from_args(args) + } else { + None } _ => None } diff --git a/rule/compile/ellipsis_metavars.mbt b/rule/compile/ellipsis_metavars.mbt index 9f8863b..b155903 100644 --- a/rule/compile/ellipsis_metavars.mbt +++ b/rule/compile/ellipsis_metavars.mbt @@ -305,9 +305,10 @@ fn ellipsis_direct_raw(node : @untyped_ast.Node) -> String? { let raw = match node.kind { Expr_Ident => node.normalized_expr_identifier_name() Pattern_Var => - match ellipsis_child(node, "value") { - Some(binder) => ellipsis_leaf_string_child(binder, "name") - None => None + if ellipsis_child(node, "value") is Some(binder) { + ellipsis_leaf_string_child(binder, "name") + } else { + None } Binder => ellipsis_leaf_string_child(node, "name") Type_Name => ellipsis_type_placeholder_name(node) diff --git a/rule/compile/metavar_expr.mbt b/rule/compile/metavar_expr.mbt index 60fa1b9..6ae7e2a 100644 --- a/rule/compile/metavar_expr.mbt +++ b/rule/compile/metavar_expr.mbt @@ -309,38 +309,38 @@ fn MetavarRewriteContext::rewrite_expr_var( ) -> @syntax.Var raise { match var_.name { Ident(name~) => - match parse_metavar(name, ctx.path, ctx.context) { - None => var_ - Some(syntax) => - match ctx.collect_or_resolve_metavar(syntax, BareExpressionPosition) { - None => var_ - Some((kind, meta_name)) => { - if kind == "arg" || kind == "type" { - ctx.reject_metavar_position(kind, meta_name) - } - ctx.register_metavar(kind, meta_name) - { name: Ident(name=meta_name), loc: var_.loc } - } + if parse_metavar(name, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BareExpressionPosition) + is Some((kind, meta_name)) { + if kind == "arg" || kind == "type" { + ctx.reject_metavar_position(kind, meta_name) } + ctx.register_metavar(kind, meta_name) + { name: Ident(name=meta_name), loc: var_.loc } + } else { + var_ + } + } else { + var_ } Dot(pkg~, id~) => - match parse_metavar(id, ctx.path, ctx.context) { - None => var_ - Some(syntax) => - match ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) { - None => var_ - Some((kind, meta_name)) => { - if kind == "arg" || kind == "type" { - ctx.reject_metavar_position(kind, meta_name) - } else if kind == "exp" { - ctx.reject_metavar_position(kind, meta_name) - } else if kind == "const" { - ctx.reject_qualified_metavar(kind, meta_name) - } - ctx.register_metavar(kind, meta_name) - { name: Dot(pkg~, id=meta_name), loc: var_.loc } - } + if parse_metavar(id, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) + is Some((kind, meta_name)) { + if kind == "arg" || kind == "type" { + ctx.reject_metavar_position(kind, meta_name) + } else if kind == "exp" { + ctx.reject_metavar_position(kind, meta_name) + } else if kind == "const" { + ctx.reject_qualified_metavar(kind, meta_name) } + ctx.register_metavar(kind, meta_name) + { name: Dot(pkg~, id=meta_name), loc: var_.loc } + } else { + var_ + } + } else { + var_ } } } @@ -352,40 +352,34 @@ fn MetavarRewriteContext::rewrite_var( ) -> @syntax.Var raise { match var_.name { Ident(name~) => - match parse_metavar(name, ctx.path, ctx.context) { - None => var_ - Some(syntax) => - match ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) { - None => var_ - Some((kind, meta_name)) => { - if kind == "exp" || - kind == "const" || - kind == "arg" || - kind == "type" { - ctx.reject_metavar_position(kind, meta_name) - } - ctx.register_metavar(kind, meta_name) - { name: Ident(name=meta_name), loc: var_.loc } - } + if parse_metavar(name, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) + is Some((kind, meta_name)) { + if kind == "exp" || kind == "const" || kind == "arg" || kind == "type" { + ctx.reject_metavar_position(kind, meta_name) } + ctx.register_metavar(kind, meta_name) + { name: Ident(name=meta_name), loc: var_.loc } + } else { + var_ + } + } else { + var_ } Dot(pkg~, id~) => - match parse_metavar(id, ctx.path, ctx.context) { - None => var_ - Some(syntax) => - match ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) { - None => var_ - Some((kind, meta_name)) => { - if kind == "exp" || - kind == "const" || - kind == "arg" || - kind == "type" { - ctx.reject_metavar_position(kind, meta_name) - } - ctx.register_metavar(kind, meta_name) - { name: Dot(pkg~, id=meta_name), loc: var_.loc } - } + if parse_metavar(id, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) + is Some((kind, meta_name)) { + if kind == "exp" || kind == "const" || kind == "arg" || kind == "type" { + ctx.reject_metavar_position(kind, meta_name) } + ctx.register_metavar(kind, meta_name) + { name: Dot(pkg~, id=meta_name), loc: var_.loc } + } else { + var_ + } + } else { + var_ } } } @@ -395,19 +389,19 @@ fn MetavarRewriteContext::rewrite_binder( ctx : MetavarRewriteContext, binder : @syntax.Binder, ) -> @syntax.Binder raise { - match parse_metavar(binder.name, ctx.path, ctx.context) { - None => binder - Some(syntax) => - match ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) { - None => binder - Some((kind, meta_name)) => { - if kind == "exp" || kind == "const" || kind == "arg" || kind == "type" { - ctx.reject_metavar_position(kind, meta_name) - } - ctx.register_metavar(kind, meta_name) - { name: meta_name, loc: binder.loc } - } + if parse_metavar(binder.name, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) + is Some((kind, meta_name)) { + if kind == "exp" || kind == "const" || kind == "arg" || kind == "type" { + ctx.reject_metavar_position(kind, meta_name) } + ctx.register_metavar(kind, meta_name) + { name: meta_name, loc: binder.loc } + } else { + binder + } + } else { + binder } } @@ -416,19 +410,19 @@ fn MetavarRewriteContext::rewrite_label( ctx : MetavarRewriteContext, label : @syntax.Label, ) -> @syntax.Label raise { - match parse_metavar(label.name, ctx.path, ctx.context) { - None => label - Some(syntax) => - match ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) { - None => label - Some((kind, meta_name)) => { - if kind == "exp" || kind == "const" || kind == "arg" || kind == "type" { - ctx.reject_metavar_position(kind, meta_name) - } - ctx.register_metavar(kind, meta_name) - { name: meta_name, loc: label.loc } - } + if parse_metavar(label.name, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) + is Some((kind, meta_name)) { + if kind == "exp" || kind == "const" || kind == "arg" || kind == "type" { + ctx.reject_metavar_position(kind, meta_name) } + ctx.register_metavar(kind, meta_name) + { name: meta_name, loc: label.loc } + } else { + label + } + } else { + label } } @@ -447,19 +441,19 @@ fn MetavarRewriteContext::rewrite_constr_name( ctx : MetavarRewriteContext, name : @syntax.ConstrName, ) -> @syntax.ConstrName raise { - match parse_metavar(name.name, ctx.path, ctx.context) { - None => name - Some(syntax) => - match ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) { - None => name - Some((kind, meta_name)) => { - if kind == "exp" || kind == "const" || kind == "arg" || kind == "type" { - ctx.reject_metavar_position(kind, meta_name) - } - ctx.register_metavar(kind, meta_name) - { name: meta_name, loc: name.loc } - } + if parse_metavar(name.name, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) + is Some((kind, meta_name)) { + if kind == "exp" || kind == "const" || kind == "arg" || kind == "type" { + ctx.reject_metavar_position(kind, meta_name) } + ctx.register_metavar(kind, meta_name) + { name: meta_name, loc: name.loc } + } else { + name + } + } else { + name } } @@ -498,40 +492,34 @@ fn MetavarRewriteContext::rewrite_type_long_ident( ) -> @syntax.LongIdent raise { match long_ident { Ident(name~) => - match parse_metavar(name, ctx.path, ctx.context) { - None => long_ident - Some(syntax) => - match ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) { - None => long_ident - Some((kind, meta_name)) => { - if kind == "exp" || - kind == "const" || - kind == "arg" || - kind == "type" { - ctx.reject_metavar_position(kind, meta_name) - } - ctx.register_metavar(kind, meta_name) - Ident(name=meta_name) - } + if parse_metavar(name, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) + is Some((kind, meta_name)) { + if kind == "exp" || kind == "const" || kind == "arg" || kind == "type" { + ctx.reject_metavar_position(kind, meta_name) } + ctx.register_metavar(kind, meta_name) + Ident(name=meta_name) + } else { + long_ident + } + } else { + long_ident } Dot(pkg~, id~) => - match parse_metavar(id, ctx.path, ctx.context) { - None => long_ident - Some(syntax) => - match ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) { - None => long_ident - Some((kind, meta_name)) => { - if kind == "exp" || - kind == "const" || - kind == "arg" || - kind == "type" { - ctx.reject_metavar_position(kind, meta_name) - } - ctx.register_metavar(kind, meta_name) - Dot(pkg~, id=meta_name) - } + if parse_metavar(id, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BareIdentifierPosition) + is Some((kind, meta_name)) { + if kind == "exp" || kind == "const" || kind == "arg" || kind == "type" { + ctx.reject_metavar_position(kind, meta_name) } + ctx.register_metavar(kind, meta_name) + Dot(pkg~, id=meta_name) + } else { + long_ident + } + } else { + long_ident } } } diff --git a/rule/compile/metavar_pattern.mbt b/rule/compile/metavar_pattern.mbt index 3d60db2..3837601 100644 --- a/rule/compile/metavar_pattern.mbt +++ b/rule/compile/metavar_pattern.mbt @@ -50,42 +50,37 @@ fn MetavarRewriteContext::rewrite_pattern_var_binder( ctx : MetavarRewriteContext, binder : @syntax.Binder, ) -> @syntax.Binder raise { - match parse_metavar(binder.name, ctx.path, ctx.context) { - None => binder - Some(syntax) => - match - ctx.collect_or_resolve_metavar(syntax, BarePatternVariablePosition) { - None => binder - Some((kind, meta_name)) => { - if kind == "exp" || kind == "arg" || kind == "type" { - ctx.reject_metavar_position(kind, meta_name) - } else if kind == "pat" { - validate_declared_metavar(ctx.path, ctx.context, meta_name, true) - let marker = temporary_pattern_metavar_marker(meta_name) - if !array_contains(ctx.identifier_metavars, marker) { - ctx.ensure_no_other_metavar_kind( - meta_name, - ctx.expr_metavars, - ctx.identifier_metavars, - ) - ctx.ensure_no_other_metavar_kind( - meta_name, - ctx.constant_metavars, - [], - ) - ctx.ensure_no_other_metavar_kind(meta_name, ctx.arg_metavars, []) - ctx.ensure_no_other_metavar_kind(meta_name, ctx.type_metavars, []) - ctx.expr_metavars.push(meta_name) - ctx.identifier_metavars.push(marker) - ctx.constant_metavars.push(meta_name) - ctx.pattern_metavars.push(meta_name) - } - return { name: metavar_text("pat", meta_name), loc: binder.loc } - } - ctx.register_metavar(kind, meta_name) - { name: meta_name, loc: binder.loc } + if parse_metavar(binder.name, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BarePatternVariablePosition) + is Some((kind, meta_name)) { + if kind == "exp" || kind == "arg" || kind == "type" { + ctx.reject_metavar_position(kind, meta_name) + } else if kind == "pat" { + validate_declared_metavar(ctx.path, ctx.context, meta_name, true) + let marker = temporary_pattern_metavar_marker(meta_name) + if !array_contains(ctx.identifier_metavars, marker) { + ctx.ensure_no_other_metavar_kind( + meta_name, + ctx.expr_metavars, + ctx.identifier_metavars, + ) + ctx.ensure_no_other_metavar_kind(meta_name, ctx.constant_metavars, []) + ctx.ensure_no_other_metavar_kind(meta_name, ctx.arg_metavars, []) + ctx.ensure_no_other_metavar_kind(meta_name, ctx.type_metavars, []) + ctx.expr_metavars.push(meta_name) + ctx.identifier_metavars.push(marker) + ctx.constant_metavars.push(meta_name) + ctx.pattern_metavars.push(meta_name) } + return { name: metavar_text("pat", meta_name), loc: binder.loc } } + ctx.register_metavar(kind, meta_name) + { name: meta_name, loc: binder.loc } + } else { + binder + } + } else { + binder } } diff --git a/rule/compile/metavar_toplevel.mbt b/rule/compile/metavar_toplevel.mbt index e1c4f6c..77362a3 100644 --- a/rule/compile/metavar_toplevel.mbt +++ b/rule/compile/metavar_toplevel.mbt @@ -274,9 +274,10 @@ fn MetavarRewriteContext::rewrite_optional_constr_params( ctx : MetavarRewriteContext, params : @list.List[@syntax.ConstrParam]?, ) -> @list.List[@syntax.ConstrParam]? raise { - match params { - None => None - Some(params) => Some(ctx.rewrite_constr_params(params)) + if params is Some(params) { + Some(ctx.rewrite_constr_params(params)) + } else { + None } } @@ -352,8 +353,9 @@ fn MetavarRewriteContext::rewrite_optional_parameters( ctx : MetavarRewriteContext, parameters : @list.List[@syntax.Parameter]?, ) -> @list.List[@syntax.Parameter]? raise { - match parameters { - None => None - Some(parameters) => Some(ctx.rewrite_parameters(parameters)) + if parameters is Some(parameters) { + Some(ctx.rewrite_parameters(parameters)) + } else { + None } } diff --git a/rule/compile/metavar_type.mbt b/rule/compile/metavar_type.mbt index 757147e..8bbeae9 100644 --- a/rule/compile/metavar_type.mbt +++ b/rule/compile/metavar_type.mbt @@ -35,19 +35,19 @@ fn MetavarRewriteContext::rewrite_type_metavar( return None } guard constr_id.id is Ident(name~) else { return None } - match parse_metavar(name, ctx.path, ctx.context) { - None => None - Some(syntax) => - match ctx.collect_or_resolve_metavar(syntax, BareTypePosition) { - None => None - Some((kind, meta_name)) => { - if kind != "type" { - ctx.reject_metavar_position(kind, meta_name) - } - ctx.register_metavar(kind, meta_name) - Some(type_metavar_marker(meta_name, loc)) - } + if parse_metavar(name, ctx.path, ctx.context) is Some(syntax) { + if ctx.collect_or_resolve_metavar(syntax, BareTypePosition) + is Some((kind, meta_name)) { + if kind != "type" { + ctx.reject_metavar_position(kind, meta_name) } + ctx.register_metavar(kind, meta_name) + Some(type_metavar_marker(meta_name, loc)) + } else { + None + } + } else { + None } } @@ -61,9 +61,10 @@ fn MetavarRewriteContext::rewrite_optional_type( ctx : MetavarRewriteContext, ty : @syntax.Type?, ) -> @syntax.Type? raise { - match ty { - None => None - Some(ty) => Some(ctx.rewrite_type(ty)) + if ty is Some(ty) { + Some(ctx.rewrite_type(ty)) + } else { + None } } diff --git a/rule/compile/parse_shape.mbt b/rule/compile/parse_shape.mbt index 8b6f54d..c51d125 100644 --- a/rule/compile/parse_shape.mbt +++ b/rule/compile/parse_shape.mbt @@ -102,9 +102,10 @@ fn attach_toplevel_shape_docstring( selected = Some(comments) } } - let doc = match selected { - None => @syntax.DocString::empty() - Some(comments) => shape_docstring_from_comments(comments) + let doc = if selected is Some(comments) { + shape_docstring_from_comments(comments) + } else { + @syntax.DocString::empty() } match item { TopFuncDef(fun_decl~, ..) => fun_decl.doc = doc diff --git a/rule/internal/rules/moonbitlang/moon.pkg b/rule/internal/rules/moonbitlang/moon.pkg index 136121d..63b24b2 100644 --- a/rule/internal/rules/moonbitlang/moon.pkg +++ b/rule/internal/rules/moonbitlang/moon.pkg @@ -25,6 +25,12 @@ dev_build( output: "unnessary_else.mbt", ) +dev_build( + rule: "embed", + input: "simplifiable_assignment.yaml", + output: "simplifiable_assignment.mbt", +) + dev_build( rule: "embed", input: "cstyle_forward_simple_forloop.yaml", diff --git a/rule/internal/rules/moonbitlang/pkg.generated.mbti b/rule/internal/rules/moonbitlang/pkg.generated.mbti index 6404d8f..c54aa64 100644 --- a/rule/internal/rules/moonbitlang/pkg.generated.mbti +++ b/rule/internal/rules/moonbitlang/pkg.generated.mbti @@ -18,6 +18,8 @@ pub let inspect_number_yaml : String pub let match_option_yaml : String +pub let simplifiable_assignment_yaml : String + pub let unnessary_else_yaml : String // Errors diff --git a/rule/internal/rules/moonbitlang/simplifiable_assignment.mbt b/rule/internal/rules/moonbitlang/simplifiable_assignment.mbt new file mode 100644 index 0000000..48051d0 --- /dev/null +++ b/rule/internal/rules/moonbitlang/simplifiable_assignment.mbt @@ -0,0 +1,53 @@ +// Generated by moonbit-community/embed from ./rule/internal/rules/moonbitlang/simplifiable_assignment.yaml. + +///| +let _embed_simplifiable_assignment_yaml : String = + #|id: simplifiable_assignment + #|description: | + #| Assignment repeats its target as the operand of a binary expression. + #| Prefer the corresponding augmented assignment, such as foo += 1, foo.bar += 1, foo[i] +=1. + #| Note: Apply this rewrite only when the target expression and any index expression + #| are free of side effects. Augmented assignment may evaluate them a different + #| number of times. + #| + #|patterns: + #| - shape: | + #| $(target:id) = $(target:id) + $_ + #| - shape: | + #| $target.$(field:id) = $target.$(field:id) + $_ + #| - shape: | + #| $target[$index] = $target[$index] + $_ + #| - shape: | + #| $(target:id) = $_ + $(target:id) + #| - shape: | + #| $target.$(field:id) = $_ + $target.$(field:id) + #| - shape: | + #| $target[$index] = $_ + $target[$index] + #| - shape: | + #| $(target:id) = $(target:id) - $_ + #| - shape: | + #| $target.$(field:id) = $target.$(field:id) - $_ + #| - shape: | + #| $target[$index] = $target[$index] - $_ + #| - shape: | + #| $(target:id) = $(target:id) * $_ + #| - shape: | + #| $target.$(field:id) = $target.$(field:id) * $_ + #| - shape: | + #| $target[$index] = $target[$index] * $_ + #| - shape: | + #| $(target:id) = $_ * $(target:id) + #| - shape: | + #| $target.$(field:id) = $_ * $target.$(field:id) + #| - shape: | + #| $target[$index] = $_ * $target[$index] + #| - shape: | + #| $(target:id) = $(target:id) / $_ + #| - shape: | + #| $target.$(field:id) = $target.$(field:id) / $_ + #| - shape: | + #| $target[$index] = $target[$index] / $_ + #| + +///| +pub let simplifiable_assignment_yaml : String = _embed_simplifiable_assignment_yaml diff --git a/rule/internal/rules/moonbitlang/simplifiable_assignment.yaml b/rule/internal/rules/moonbitlang/simplifiable_assignment.yaml new file mode 100644 index 0000000..71ea74d --- /dev/null +++ b/rule/internal/rules/moonbitlang/simplifiable_assignment.yaml @@ -0,0 +1,45 @@ +id: simplifiable_assignment +description: | + Assignment repeats its target as the operand of a binary expression. + Prefer the corresponding augmented assignment, such as foo += 1, foo.bar += 1, foo[i] +=1. + Note: Apply this rewrite only when the target expression and any index expression + are free of side effects. Augmented assignment may evaluate them a different + number of times. + +patterns: + - shape: | + $(target:id) = $(target:id) + $_ + - shape: | + $target.$(field:id) = $target.$(field:id) + $_ + - shape: | + $target[$index] = $target[$index] + $_ + - shape: | + $(target:id) = $_ + $(target:id) + - shape: | + $target.$(field:id) = $_ + $target.$(field:id) + - shape: | + $target[$index] = $_ + $target[$index] + - shape: | + $(target:id) = $(target:id) - $_ + - shape: | + $target.$(field:id) = $target.$(field:id) - $_ + - shape: | + $target[$index] = $target[$index] - $_ + - shape: | + $(target:id) = $(target:id) * $_ + - shape: | + $target.$(field:id) = $target.$(field:id) * $_ + - shape: | + $target[$index] = $target[$index] * $_ + - shape: | + $(target:id) = $_ * $(target:id) + - shape: | + $target.$(field:id) = $_ * $target.$(field:id) + - shape: | + $target[$index] = $_ * $target[$index] + - shape: | + $(target:id) = $(target:id) / $_ + - shape: | + $target.$(field:id) = $target.$(field:id) / $_ + - shape: | + $target[$index] = $target[$index] / $_ diff --git a/rule/model/rulespec_parse.mbt b/rule/model/rulespec_parse.mbt index caaef66..ee2ea97 100644 --- a/rule/model/rulespec_parse.mbt +++ b/rule/model/rulespec_parse.mbt @@ -179,21 +179,20 @@ fn expect_inside_expr( doc : Map[String, @yaml.Yaml], path : String, ) -> Array[RulePatternSpec] raise { - match doc.get("inside-expr") { - None => [] - Some(value) => { - let patterns = parse_rule_clause_array( - expect_yaml_array_value(value, path, "inside-expr"), - path, - "inside-expr", - allow_guard=true, - allow_match_mode=false, - ) - if patterns.is_empty() { - raise invalid_rule(path~, info="inside-expr must not be empty") - } - patterns + if doc.get("inside-expr") is Some(value) { + let patterns = parse_rule_clause_array( + expect_yaml_array_value(value, path, "inside-expr"), + path, + "inside-expr", + allow_guard=true, + allow_match_mode=false, + ) + if patterns.is_empty() { + raise invalid_rule(path~, info="inside-expr must not be empty") } + patterns + } else { + [] } } @@ -202,21 +201,20 @@ fn expect_inside_toplevel( doc : Map[String, @yaml.Yaml], path : String, ) -> Array[RulePatternSpec] raise { - match doc.get("inside-toplevel") { - None => [] - Some(value) => { - let patterns = parse_rule_clause_array( - expect_yaml_array_value(value, path, "inside-toplevel"), - path, - "inside-toplevel", - allow_guard=true, - allow_match_mode=true, - ) - if patterns.is_empty() { - raise invalid_rule(path~, info="inside-toplevel must not be empty") - } - patterns + if doc.get("inside-toplevel") is Some(value) { + let patterns = parse_rule_clause_array( + expect_yaml_array_value(value, path, "inside-toplevel"), + path, + "inside-toplevel", + allow_guard=true, + allow_match_mode=true, + ) + if patterns.is_empty() { + raise invalid_rule(path~, info="inside-toplevel must not be empty") } + patterns + } else { + [] } } diff --git a/rule/prefilter/prefilter.mbt b/rule/prefilter/prefilter.mbt index 517c96f..7cb0666 100644 --- a/rule/prefilter/prefilter.mbt +++ b/rule/prefilter/prefilter.mbt @@ -244,9 +244,10 @@ fn collect_node_children_literals( ) -> Unit { for entry in node.children { let (child_name, child) = entry - let ignored = match child_name { - Some(name) => prefilter_ignores_field(compiled, node.kind, name) - None => false + let ignored = if child_name is Some(name) { + prefilter_ignores_field(compiled, node.kind, name) + } else { + false } if !ignored { collect_node_literals(child, compiled, literals) diff --git a/rule/taint_lowering/taint_lowering.mbt b/rule/taint_lowering/taint_lowering.mbt index c0d9437..ed10ad2 100644 --- a/rule/taint_lowering/taint_lowering.mbt +++ b/rule/taint_lowering/taint_lowering.mbt @@ -117,9 +117,10 @@ fn target_value_for_match( _ => match pattern_match.bindings.get("__SOURCE__") { Some(Single(source)) => - match target_value_at_expr(call, source) { - Some(value) => Some(value) - None => target_value(call, target) + if target_value_at_expr(call, source) is Some(value) { + Some(value) + } else { + target_value(call, target) } _ => None } diff --git a/taint/ast_path.mbt b/taint/ast_path.mbt index cff684c..d90573d 100644 --- a/taint/ast_path.mbt +++ b/taint/ast_path.mbt @@ -14,13 +14,13 @@ fn function_like_from_impl(node : @untyped_ast.Node) -> FunctionLike raise { let decl_body = node_child(node, "decl_body") match decl_body { Some(body) if body.kind == DeclBody_DeclBody => { - let body_expr = match node_child(body, "expr") { - Some(expr) => expr - None => - raise TaintAnalysisError::UnsupportedFunctionLike( - kind="TopFuncDef.DeclBody", - loc=node.loc, - ) + let body_expr = if node_child(body, "expr") is Some(expr) { + expr + } else { + raise TaintAnalysisError::UnsupportedFunctionLike( + kind="TopFuncDef.DeclBody", + loc=node.loc, + ) } { name: function_decl_name(fun_decl), @@ -50,13 +50,13 @@ fn function_like_from_impl(node : @untyped_ast.Node) -> FunctionLike raise { let decl_body = node_child(node, "body") match decl_body { Some(body) if body.kind == DeclBody_DeclBody => { - let body_expr = match node_child(body, "expr") { - Some(expr) => expr - None => - raise TaintAnalysisError::UnsupportedFunctionLike( - kind="TopImpl.DeclBody", - loc=node.loc, - ) + let body_expr = if node_child(body, "expr") is Some(expr) { + expr + } else { + raise TaintAnalysisError::UnsupportedFunctionLike( + kind="TopImpl.DeclBody", + loc=node.loc, + ) } { name: impl_method_name(node), @@ -92,29 +92,22 @@ fn function_like_from_impl(node : @untyped_ast.Node) -> FunctionLike raise { ///| fn function_decl_name(fun_decl : @untyped_ast.Node?) -> String { - match fun_decl { - Some(decl) => - match node_child(decl, "name") { - Some(name) => - match node_binder_name(name) { - Some(value) => value - None => "" - } - None => "" - } - None => "" + if fun_decl is Some(decl) && + node_child(decl, "name") is Some(name) && + node_binder_name(name) is Some(value) { + value + } else { + "" } } ///| fn impl_method_name(node : @untyped_ast.Node) -> String { - match node_child(node, "method_name") { - Some(name) => - match node_binder_name(name) { - Some(value) => value - None => "" - } - None => "" + if node_child(node, "method_name") is Some(name) && + node_binder_name(name) is Some(value) { + value + } else { + "" } } @@ -140,9 +133,10 @@ fn paths_from_optional_parameters( parent : @untyped_ast.Node?, name : String, ) -> Array[StoragePath] { - match parent { - Some(node) => paths_from_parameter_list(node_optional_child(node, name)) - None => [] + if parent is Some(node) { + paths_from_parameter_list(node_optional_child(node, name)) + } else { + [] } } @@ -185,9 +179,10 @@ fn storage_path_from_var(var_ : @untyped_ast.Node) -> StoragePath? { fn storage_path_from_expr(expr : @untyped_ast.Node) -> StoragePath? { match expr.kind { Expr_Ident => - match node_child(expr, "id") { - Some(id) => storage_path_from_var(id) - None => None + if node_child(expr, "id") is Some(id) { + storage_path_from_var(id) + } else { + None } Expr_Field => match (node_child(expr, "record"), node_child(expr, "accessor")) { @@ -210,9 +205,10 @@ fn storage_path_from_expr(expr : @untyped_ast.Node) -> StoragePath? { _ => None } Expr_Group | Expr_Constraint => - match node_child(expr, "expr") { - Some(inner) => storage_path_from_expr(inner) - None => None + if node_child(expr, "expr") is Some(inner) { + storage_path_from_expr(inner) + } else { + None } _ => None } @@ -263,9 +259,10 @@ fn append_path_segment( ///| fn segment_from_index_expr(index : @untyped_ast.Node) -> PathSegment { - match constant_integer_text(index) { - Some(value) => ConstIndex(value) - None => AnyIndex + if constant_integer_text(index) is Some(value) { + ConstIndex(value) + } else { + AnyIndex } } @@ -275,9 +272,10 @@ fn constant_integer_text(expr : @untyped_ast.Node) -> String? { guard node_child(expr, "constant") is Some(constant) else { return None } match constant.kind { Constant_Int | Constant_UInt => - match node_first_child(constant) { - Some(value) => node_leaf_string(value) - None => None + if node_first_child(constant) is Some(value) { + node_leaf_string(value) + } else { + None } _ => None } @@ -290,18 +288,17 @@ fn segment_from_argument_kind( ) -> PathSegment { match kind.kind { ArgumentKind_Labelled | ArgumentKind_LabelledPun => - match node_first_label_name(kind) { - Some(label) => Field(label) - None => TupleIndex(positional_index) + if node_first_label_name(kind) is Some(label) { + Field(label) + } else { + TupleIndex(positional_index) } ArgumentKind_LabelledOption | ArgumentKind_LabelledOptionPun => - match node_child(kind, "label") { - Some(label) => - match node_label_name(label) { - Some(value) => Field(value) - None => TupleIndex(positional_index) - } - None => TupleIndex(positional_index) + if node_child(kind, "label") is Some(label) && + node_label_name(label) is Some(value) { + Field(value) + } else { + TupleIndex(positional_index) } ArgumentKind_Positional => TupleIndex(positional_index) _ => TupleIndex(positional_index) @@ -314,9 +311,10 @@ fn argument_label(kind : @untyped_ast.Node) -> String? { ArgumentKind_Labelled | ArgumentKind_LabelledPun => node_first_label_name(kind) ArgumentKind_LabelledOption | ArgumentKind_LabelledOptionPun => - match node_child(kind, "label") { - Some(label) => node_label_name(label) - None => None + if node_child(kind, "label") is Some(label) { + node_label_name(label) + } else { + None } ArgumentKind_Positional => None _ => None diff --git a/taint/engine.mbt b/taint/engine.mbt index 41a5309..73fcb38 100644 --- a/taint/engine.mbt +++ b/taint/engine.mbt @@ -120,21 +120,22 @@ fn eval_expr( Expr_Record => eval_record_expr(node_child_items(expr, "fields"), state, spec, findings) Expr_Group | Expr_Constraint => - match node_child(expr, "expr") { - Some(inner) => eval_expr(inner, state, spec, findings) - None => normal_result(state, []) + if node_child(expr, "expr") is Some(inner) { + eval_expr(inner, state, spec, findings) + } else { + normal_result(state, []) } Expr_Sequence => - match node_child(expr, "last_expr") { - Some(last_expr) => - eval_sequence( - node_child_items(expr, "exprs"), - last_expr, - state, - spec, - findings, - ) - None => normal_result(state, []) + if node_child(expr, "last_expr") is Some(last_expr) { + eval_sequence( + node_child_items(expr, "exprs"), + last_expr, + state, + spec, + findings, + ) + } else { + normal_result(state, []) } Expr_Let => match @@ -226,53 +227,53 @@ fn eval_expr( _ => normal_result(state, []) } Expr_Match => - match node_child(expr, "expr") { - Some(scrutinee) => - eval_match( - scrutinee, - node_child_items(expr, "cases"), - state, - spec, - findings, - ) - None => normal_result(state, []) + if node_child(expr, "expr") is Some(scrutinee) { + eval_match( + scrutinee, + node_child_items(expr, "cases"), + state, + spec, + findings, + ) + } else { + normal_result(state, []) } Expr_Lexmatch => - match node_child(expr, "expr") { - Some(scrutinee) => - eval_lex_match( - scrutinee, - node_child_items(expr, "cases"), - state, - spec, - findings, - ) - None => normal_result(state, []) + if node_child(expr, "expr") is Some(scrutinee) { + eval_lex_match( + scrutinee, + node_child_items(expr, "cases"), + state, + spec, + findings, + ) + } else { + normal_result(state, []) } Expr_Lexscan => - match node_child(expr, "expr") { - Some(scrutinee) => - eval_lex_scan( - scrutinee, - node_child_items(expr, "cases"), - state, - spec, - findings, - ) - None => normal_result(state, []) + if node_child(expr, "expr") is Some(scrutinee) { + eval_lex_scan( + scrutinee, + node_child_items(expr, "cases"), + state, + spec, + findings, + ) + } else { + normal_result(state, []) } Expr_Try => - match node_child(expr, "body") { - Some(body) => - eval_try( - body, - node_child_items(expr, "catch"), - node_optional_child(expr, "try_else"), - state, - spec, - findings, - ) - None => normal_result(state, []) + if node_child(expr, "body") is Some(body) { + eval_try( + body, + node_child_items(expr, "catch"), + node_optional_child(expr, "try_else"), + state, + spec, + findings, + ) + } else { + normal_result(state, []) } Expr_While => match (node_child(expr, "loop_cond"), node_child(expr, "loop_body")) { @@ -288,19 +289,19 @@ fn eval_expr( _ => normal_result(state, []) } Expr_For => - match node_child(expr, "body") { - Some(body) => - eval_for( - node_child_items(expr, "binders"), - node_optional_child(expr, "condition"), - node_child_items(expr, "continue_block"), - body, - node_optional_child(expr, "for_else"), - state, - spec, - findings, - ) - None => normal_result(state, []) + if node_child(expr, "body") is Some(body) { + eval_for( + node_child_items(expr, "binders"), + node_optional_child(expr, "condition"), + node_child_items(expr, "continue_block"), + body, + node_optional_child(expr, "for_else"), + state, + spec, + findings, + ) + } else { + normal_result(state, []) } Expr_ForEach => match (node_child(expr, "expr"), node_child(expr, "body")) { @@ -328,41 +329,38 @@ fn eval_expr( _ => normal_result(state, []) } Expr_LetAnd => - match node_child(expr, "body") { - Some(body) => - eval_clean_root_scope( - body, - let_and_bound_roots(node_child_items(expr, "bindings")), - state, - spec, - findings, - ) - None => normal_result(state, []) + if node_child(expr, "body") is Some(body) { + eval_clean_root_scope( + body, + let_and_bound_roots(node_child_items(expr, "bindings")), + state, + spec, + findings, + ) + } else { + normal_result(state, []) } Expr_Function => normal_result(state, []) Expr_Return => - match node_optional_child(expr, "return_value") { - Some(value) => { - let result = eval_expr(value, state, spec, findings) - flow_result(FlowReturn, result.state, result.value) - } - None => flow_result(FlowReturn, state, []) + if node_optional_child(expr, "return_value") is Some(value) { + let result = eval_expr(value, state, spec, findings) + flow_result(FlowReturn, result.state, result.value) + } else { + flow_result(FlowReturn, state, []) } Expr_Break => - match node_optional_child(expr, "arg") { - Some(value) => { - let result = eval_expr(value, state, spec, findings) - flow_result(FlowBreak, result.state, result.value) - } - None => flow_result(FlowBreak, state, []) + if node_optional_child(expr, "arg") is Some(value) { + let result = eval_expr(value, state, spec, findings) + flow_result(FlowBreak, result.state, result.value) + } else { + flow_result(FlowBreak, state, []) } Expr_Raise => - match node_child(expr, "err_value") { - Some(value) => { - let result = eval_expr(value, state, spec, findings) - flow_result(FlowRaise, result.state, result.value) - } - None => flow_result(FlowRaise, state, []) + if node_child(expr, "err_value") is Some(value) { + let result = eval_expr(value, state, spec, findings) + flow_result(FlowRaise, result.state, result.value) + } else { + flow_result(FlowRaise, state, []) } Expr_Continue => { let result = eval_children( @@ -1174,28 +1172,31 @@ fn site_for_expr(expr : @untyped_ast.Node, path : StoragePath?) -> ValueSite { fn callee_name_from_expr(expr : @untyped_ast.Node) -> String? { match expr.kind { Expr_Ident => - match node_child(expr, "id") { - Some(id) => node_var_name(id) - None => None + if node_child(expr, "id") is Some(id) { + node_var_name(id) + } else { + None } Expr_Field => - match node_child(expr, "accessor") { - Some(accessor) => - match accessor.kind { - Accessor_Label => node_first_label_name(accessor) - _ => None - } - None => None + if node_child(expr, "accessor") is Some(accessor) { + match accessor.kind { + Accessor_Label => node_first_label_name(accessor) + _ => None + } + } else { + None } Expr_Method => - match node_child(expr, "method_name") { - Some(method_name) => node_label_name(method_name) - None => None + if node_child(expr, "method_name") is Some(method_name) { + node_label_name(method_name) + } else { + None } Expr_Group | Expr_Constraint => - match node_child(expr, "expr") { - Some(inner) => callee_name_from_expr(inner) - None => None + if node_child(expr, "expr") is Some(inner) { + callee_name_from_expr(inner) + } else { + None } _ => None } @@ -1493,9 +1494,10 @@ fn bind_pattern( ) -> TaintState { match pattern.kind { Pattern_Var => - match node_first_child(pattern) { - Some(binder) => bind_binder(binder, value, state) - None => state + if node_first_child(pattern) is Some(binder) { + bind_binder(binder, value, state) + } else { + state } Pattern_Alias => { let next = if node_child(pattern, "pat") is Some(pat) { @@ -1510,9 +1512,10 @@ fn bind_pattern( } } Pattern_Constraint => - match node_child(pattern, "pat") { - Some(pat) => bind_pattern(pat, value, state) - None => state + if node_child(pattern, "pat") is Some(pat) { + bind_pattern(pat, value, state) + } else { + state } Pattern_Tuple => { let mut next = state @@ -1551,9 +1554,10 @@ fn bind_pattern( next } Pattern_Array => - match node_child(pattern, "pats") { - Some(pats) => bind_array_pattern(pats, value, state) - None => state + if node_child(pattern, "pats") is Some(pats) { + bind_array_pattern(pats, value, state) + } else { + state } Pattern_Constr => { let mut next = state @@ -1723,9 +1727,10 @@ fn eval_condition_scope( ) -> ConditionEval { match cond.kind { Expr_Group | Expr_Constraint => - match node_child(cond, "expr") { - Some(expr) => eval_condition_scope(expr, state, spec, findings) - None => condition_from_eval_result(normal_result(state, [])) + if node_child(cond, "expr") is Some(expr) { + eval_condition_scope(expr, state, spec, findings) + } else { + condition_from_eval_result(normal_result(state, [])) } Expr_Is => match (node_child(cond, "expr"), node_child(cond, "pat")) { @@ -1746,36 +1751,35 @@ fn eval_condition_scope( _ => condition_from_eval_result(normal_result(state, [])) } Expr_IsLexmatch => - match node_child(cond, "expr") { - Some(expr) => { - let matched = eval_expr(expr, state, spec, findings) - if matched.flow != FlowNormal { - return condition_from_eval_result(matched) - } - let patterns = node_child_items(cond, "pat") - let pattern_result = eval_lex_top_pattern_exprs( + if node_child(cond, "expr") is Some(expr) { + let matched = eval_expr(expr, state, spec, findings) + if matched.flow != FlowNormal { + return condition_from_eval_result(matched) + } + let patterns = node_child_items(cond, "pat") + let pattern_result = eval_lex_top_pattern_exprs( + patterns, + matched.state, + spec, + findings, + ) + if pattern_result.flow != FlowNormal { + return condition_from_eval_result(pattern_result) + } + let roots = lex_top_patterns_bound_roots(patterns) + { + base_state: pattern_result.state, + true_state: bind_lex_top_patterns( patterns, - matched.state, - spec, - findings, - ) - if pattern_result.flow != FlowNormal { - return condition_from_eval_result(pattern_result) - } - let roots = lex_top_patterns_bound_roots(patterns) - { - base_state: pattern_result.state, - true_state: bind_lex_top_patterns( - patterns, - matched.value, - pattern_result.state, - ), - bound_roots: roots, - flow: FlowNormal, - value: matched.value, - } + matched.value, + pattern_result.state, + ), + bound_roots: roots, + flow: FlowNormal, + value: matched.value, } - None => condition_from_eval_result(normal_result(state, [])) + } else { + condition_from_eval_result(normal_result(state, [])) } Expr_RegexMatch => match (node_child(cond, "expr"), node_child(cond, "pat")) { @@ -1849,9 +1853,10 @@ fn eval_and_condition_scope( ///| fn is_logical_and_expr(expr : @untyped_ast.Node) -> Bool { - match node_child(expr, "op") { - Some(op) => node_var_name(op) == Some("&&") - None => false + if node_child(expr, "op") is Some(op) { + node_var_name(op) == Some("&&") + } else { + false } } @@ -1867,9 +1872,10 @@ fn eval_lex_top_pattern_exprs( for pattern in patterns { let result = match pattern.kind { LexTopPattern_Pattern => - match node_first_child(pattern) { - Some(pat) => eval_lex_pattern_exprs(pat, current, spec, findings) - None => normal_result(current, []) + if node_first_child(pattern) is Some(pat) { + eval_lex_pattern_exprs(pat, current, spec, findings) + } else { + normal_result(current, []) } LexTopPattern_Binder | LexTopPattern_Wildcard => normal_result(current, []) @@ -1893,9 +1899,10 @@ fn eval_lex_pattern_exprs( ) -> EvalResult { match pattern.kind { LexPattern_Alias => - match node_child(pattern, "pat") { - Some(pat) => eval_lex_pattern_exprs(pat, state, spec, findings) - None => normal_result(state, []) + if node_child(pattern, "pat") is Some(pat) { + eval_lex_pattern_exprs(pat, state, spec, findings) + } else { + normal_result(state, []) } LexPattern_RegexInterp => eval_interp_elems( @@ -2349,13 +2356,15 @@ fn eval_pattern_case( spec : TaintSpec, findings : Array[SinkFinding], ) -> EvalResult { - let pattern = match node_child(case, "pattern") { - Some(pattern) => pattern - None => return normal_result(base_state, []) + let pattern = if node_child(case, "pattern") is Some(pattern) { + pattern + } else { + return normal_result(base_state, []) } - let body = match node_child(case, "body") { - Some(body) => body - None => return normal_result(base_state, []) + let body = if node_child(case, "body") is Some(body) { + body + } else { + return normal_result(base_state, []) } let roots = pattern_bound_roots(pattern) let bound = bind_pattern(pattern, value, base_state) @@ -2440,9 +2449,10 @@ fn eval_lex_case( if pattern_result.flow != FlowNormal { return pattern_result } - let body = match node_child(case, "body") { - Some(body) => body - None => return normal_result(pattern_result.state, []) + let body = if node_child(case, "body") is Some(body) { + body + } else { + return normal_result(pattern_result.state, []) } let roots = lex_top_patterns_bound_roots(patterns) let bound = bind_lex_top_patterns(patterns, value, pattern_result.state) @@ -2520,13 +2530,15 @@ fn eval_lex_scan_case( spec : TaintSpec, findings : Array[SinkFinding], ) -> EvalResult { - let pattern = match node_child(case, "pat") { - Some(pattern) => pattern - None => return normal_result(base_state, []) + let pattern = if node_child(case, "pat") is Some(pattern) { + pattern + } else { + return normal_result(base_state, []) } - let body = match node_child(case, "body") { - Some(body) => body - None => return normal_result(base_state, []) + let body = if node_child(case, "body") is Some(body) { + body + } else { + return normal_result(base_state, []) } let roots = lex_scan_pattern_bound_roots(pattern) let bound = bind_lex_scan_pattern(pattern, value, base_state) @@ -2676,7 +2688,7 @@ fn eval_while( break } current = joined - iteration = iteration + 1 + iteration += 1 } if while_else is Some(expr) { let else_result = eval_expr(expr, current, spec, findings) @@ -2801,7 +2813,7 @@ fn eval_for( break } current = joined - iteration = iteration + 1 + iteration += 1 } let result = if for_else is Some(expr) { eval_expr(expr, current, spec, findings) @@ -2895,7 +2907,7 @@ fn eval_for_each( break } current = joined - iteration = iteration + 1 + iteration += 1 } let result = if else_block is Some(expr) { eval_expr(expr, current, spec, findings) @@ -2928,18 +2940,18 @@ fn eval_unknown_expr( findings, ) Expr_Unary => - match node_child(expr, "expr") { - Some(inner) => eval_expr(inner, state, spec, findings) - None => normal_result(state, []) + if node_child(expr, "expr") is Some(inner) { + eval_expr(inner, state, spec, findings) + } else { + normal_result(state, []) } Expr_As | Expr_TryOperator | Expr_ProofAssert | Expr_TemplateWriting => - match node_child(expr, "expr") { - Some(inner) => eval_expr(inner, state, spec, findings) - None => - match node_child(expr, "body") { - Some(body) => eval_expr(body, state, spec, findings) - None => normal_result(state, []) - } + if node_child(expr, "expr") is Some(inner) { + eval_expr(inner, state, spec, findings) + } else if node_child(expr, "body") is Some(body) { + eval_expr(body, state, spec, findings) + } else { + normal_result(state, []) } Expr_ArraySet | Expr_ArrayAugmentedSet => eval_children( @@ -2978,29 +2990,29 @@ fn eval_unknown_expr( _ => normal_result(state, []) } Expr_Is | Expr_RegexMatch => - match node_child(expr, "expr") { - Some(inner) => eval_expr(inner, state, spec, findings) - None => normal_result(state, []) + if node_child(expr, "expr") is Some(inner) { + eval_expr(inner, state, spec, findings) + } else { + normal_result(state, []) } Expr_IsLexmatch => - match node_child(expr, "expr") { - Some(inner) => { - let matched = eval_expr(inner, state, spec, findings) - if matched.flow != FlowNormal { - return matched - } - let pattern_result = eval_lex_top_pattern_exprs( - node_child_items(expr, "pat"), - matched.state, - spec, - findings, - ) - if pattern_result.flow != FlowNormal { - return pattern_result - } - normal_result(pattern_result.state, matched.value) + if node_child(expr, "expr") is Some(inner) { + let matched = eval_expr(inner, state, spec, findings) + if matched.flow != FlowNormal { + return matched + } + let pattern_result = eval_lex_top_pattern_exprs( + node_child_items(expr, "pat"), + matched.state, + spec, + findings, + ) + if pattern_result.flow != FlowNormal { + return pattern_result } - None => normal_result(state, []) + normal_result(pattern_result.state, matched.value) + } else { + normal_result(state, []) } Expr_Interp | Expr_BytesInterp => eval_interp_elems(node_child_items(expr, "elems"), state, spec, findings) @@ -3020,20 +3032,19 @@ fn eval_unknown_expr( normal_result(current, value) } Expr_RecordUpdate => - match node_child(expr, "record") { - Some(record) => { - let base = eval_expr(record, state, spec, findings) - if base.flow != FlowNormal { - return base - } - eval_record_expr( - node_child_items(expr, "fields"), - base.state, - spec, - findings, - ) + if node_child(expr, "record") is Some(record) { + let base = eval_expr(record, state, spec, findings) + if base.flow != FlowNormal { + return base } - None => normal_result(state, []) + eval_record_expr( + node_child_items(expr, "fields"), + base.state, + spec, + findings, + ) + } else { + normal_result(state, []) } _ => normal_result(state, []) } diff --git a/taint/node_helpers.mbt b/taint/node_helpers.mbt index bffda9f..1ec48a3 100644 --- a/taint/node_helpers.mbt +++ b/taint/node_helpers.mbt @@ -96,9 +96,10 @@ fn node_var_name(node : @untyped_ast.Node) -> String? { ///| fn node_first_label_name(node : @untyped_ast.Node) -> String? { - match node_first_child(node) { - Some(child) => node_label_name(child) - None => None + if node_first_child(node) is Some(child) { + node_label_name(child) + } else { + None } } diff --git a/testdata/builtin-rules-all/simplifiable_assignment.mbt b/testdata/builtin-rules-all/simplifiable_assignment.mbt new file mode 100644 index 0000000..ba0f54f --- /dev/null +++ b/testdata/builtin-rules-all/simplifiable_assignment.mbt @@ -0,0 +1,13 @@ +///| +fn simplifiable_assignment(step : Int) -> Int { + let mut foo = 0 + foo = foo + step + foo +} + +///| +fn already_augmented(step : Int) -> Int { + let mut foo = 0 + foo += step + foo +} diff --git a/testdata/builtin-simplifiable-assignment-variants/negative.mbt b/testdata/builtin-simplifiable-assignment-variants/negative.mbt new file mode 100644 index 0000000..ce586da --- /dev/null +++ b/testdata/builtin-simplifiable-assignment-variants/negative.mbt @@ -0,0 +1,75 @@ +///| +struct AssignmentPair { + mut left : Int + mut right : Int +} + +///| +fn unsupported_assignments( + first : AssignmentPair, + second : AssignmentPair, + values : Array[Int], + other_values : Array[Int], + step : Int, +) -> Unit { + let mut remainder = 1 + remainder = remainder % step + + let mut target = 0 + let other = 1 + target = other + step + target = step + other + target = other - step + target = step - target + target = other * step + target = step * other + target = other / step + target = step / target + + first.left = first.right + step + first.left = step + first.right + first.left = first.right - step + first.left = step - first.left + first.left = first.right * step + first.left = step * first.right + first.left = first.right / step + first.left = step / first.left + + first.left = second.left + step + first.left = step + second.left + first.left = second.left - step + first.left = second.left * step + first.left = step * second.left + first.left = second.left / step + + values[0] = values[0] % step + values[0] = values[1] + step + values[0] = step + values[1] + values[0] = values[1] - step + values[0] = step - values[0] + values[0] = values[1] * step + values[0] = step * values[1] + values[0] = values[1] / step + values[0] = step / values[0] + + values[0] = other_values[0] + step + values[0] = step + other_values[0] + values[0] = other_values[0] - step + values[0] = other_values[0] * step + values[0] = step * other_values[0] + values[0] = other_values[0] / step + + let mut add = 0 + add += step + let mut subtract = 0 + subtract -= step + let mut multiply = 1 + multiply *= step + let mut divide = 1 + divide /= step + + values[0] += step + values[0] -= step + values[0] *= step + values[0] /= step +} diff --git a/testdata/builtin-simplifiable-assignment-variants/positive.mbt b/testdata/builtin-simplifiable-assignment-variants/positive.mbt new file mode 100644 index 0000000..cac1860 --- /dev/null +++ b/testdata/builtin-simplifiable-assignment-variants/positive.mbt @@ -0,0 +1,48 @@ +///| +struct AssignmentTarget { + mut add_left : Int + mut add_right : Int + mut subtract : Int + mut multiply_left : Int + mut multiply_right : Int + mut divide : Int +} + +///| +fn supported_local_assignments(step : Int) -> Unit { + let mut add_left = 0 + add_left = add_left + step * 2 + let mut add_right = 0 + add_right = step * 2 + add_right + let mut subtract = 0 + subtract = subtract - step + let mut multiply_left = 1 + multiply_left = multiply_left * (step + 1) + let mut multiply_right = 1 + multiply_right = (step + 1) * multiply_right + let mut divide = 1 + divide = divide / step +} + +///| +fn supported_field_assignments( + target : AssignmentTarget, + step : Int, +) -> Unit { + target.add_left = target.add_left + step * 2 + target.add_right = step * 2 + target.add_right + target.subtract = target.subtract - step + target.multiply_left = target.multiply_left * (step + 1) + target.multiply_right = (step + 1) * target.multiply_right + target.divide = target.divide / step +} + +///| +fn supported_index_assignments(target : Array[Int], step : Int) -> Unit { + target[0] = target[0] + step * 2 + target[1] = step * 2 + target[1] + target[2] = target[2] - step + target[3] = target[3] * (step + 1) + target[4] = (step + 1) * target[4] + target[5] = target[5] / step +} diff --git a/untyped_ast/scoped_visit.mbt b/untyped_ast/scoped_visit.mbt index 05a7312..4468952 100644 --- a/untyped_ast/scoped_visit.mbt +++ b/untyped_ast/scoped_visit.mbt @@ -97,10 +97,10 @@ fn visit_node_root_named_expr( shadowed : Array[String], visitor : (ScopedExprNode) -> NodeVisitAction, ) -> NodeVisitAction { - match node_child(node, name) { - Some(child) => - visit_node_scoped_expr_roots_with_shadow(child, shadowed, visitor) - None => NodeVisitContinue + if node_child(node, name) is Some(child) { + visit_node_scoped_expr_roots_with_shadow(child, shadowed, visitor) + } else { + NodeVisitContinue } } @@ -508,9 +508,10 @@ fn visit_node_named_expr( shadowed : Array[String], visitor : (ScopedExprNode) -> NodeVisitAction, ) -> NodeVisitAction { - match node_child(node, name) { - Some(child) => visit_node_descendant_exprs(child, shadowed, visitor) - None => NodeVisitContinue + if node_child(node, name) is Some(child) { + visit_node_descendant_exprs(child, shadowed, visitor) + } else { + NodeVisitContinue } } @@ -521,9 +522,10 @@ fn visit_node_named_container_exprs( shadowed : Array[String], visitor : (ScopedExprNode) -> NodeVisitAction, ) -> NodeVisitAction { - match node_child(node, name) { - Some(child) => visit_node_all_child_exprs(child, shadowed, visitor) - None => NodeVisitContinue + if node_child(node, name) is Some(child) { + visit_node_all_child_exprs(child, shadowed, visitor) + } else { + NodeVisitContinue } } @@ -582,9 +584,10 @@ fn visit_node_case_list_exprs( shadowed : Array[String], visitor : (ScopedExprNode) -> NodeVisitAction, ) -> NodeVisitAction { - match node_child(node, name) { - Some(list) => visit_node_case_container_exprs(list, shadowed, visitor) - None => NodeVisitContinue + if node_child(node, name) is Some(list) { + visit_node_case_container_exprs(list, shadowed, visitor) + } else { + NodeVisitContinue } } @@ -1054,9 +1057,10 @@ fn node_binder_name(node : Node) -> String? { ///| fn node_infix_op(node : Node, op : String) -> Bool { - match node_child(node, "op") { - Some(op_node) => node_var_name(op_node) == Some(op) - None => false + if node_child(node, "op") is Some(op_node) { + node_var_name(op_node) == Some(op) + } else { + false } } diff --git a/untyped_ast/untyped_ast_test.mbt b/untyped_ast/untyped_ast_test.mbt index d8fa29e..f2c68e1 100644 --- a/untyped_ast/untyped_ast_test.mbt +++ b/untyped_ast/untyped_ast_test.mbt @@ -235,9 +235,10 @@ fn test_pattern_var(name : String) -> Node { ///| fn require_test_child(node : Node, name : String) -> Node { - match node_test_child(node, name) { - Some(child) => child - None => abort("expected child \{name}") + if node_test_child(node, name) is Some(child) { + child + } else { + abort("expected child \{name}") } } @@ -546,9 +547,8 @@ test "node scoped visitor prune skips children" { visit_node_scoped_exprs(from_expr(parse_test_expr("target + source")), fn( scoped, ) { - match node_test_expr_ident_name(scoped.expr) { - Some(name) => visited.push(name) - None => () + if node_test_expr_ident_name(scoped.expr) is Some(name) { + visited.push(name) } NodeVisitPrune }) @@ -561,12 +561,11 @@ test "node scoped visitor return stops sibling traversal" { visit_node_scoped_exprs(from_expr(parse_test_expr("target + source")), fn( scoped, ) { - match node_test_expr_ident_name(scoped.expr) { - Some(name) => { - visited.push(name) - NodeVisitReturn - } - None => NodeVisitContinue + if node_test_expr_ident_name(scoped.expr) is Some(name) { + visited.push(name) + NodeVisitReturn + } else { + NodeVisitContinue } }) assert_eq(visited.join("|"), "target") @@ -579,12 +578,11 @@ test "node scoped root visitor return stops later roots" { let root = from_impls(List([first, second])) let visited : Array[String] = [] visit_node_scoped_expr_roots(root, fn(scoped) { - match node_test_expr_ident_name(scoped.expr) { - Some(name) => { - visited.push(name) - NodeVisitReturn - } - None => NodeVisitContinue + if node_test_expr_ident_name(scoped.expr) is Some(name) { + visited.push(name) + NodeVisitReturn + } else { + NodeVisitContinue } }) assert_eq(visited.join("|"), "target")