Skip to content

Commit 1e920ff

Browse files
committed
add unnessary_else
1 parent 270c3fa commit 1e920ff

8 files changed

Lines changed: 80 additions & 0 deletions

File tree

e2etests/BUILTIN_RULES.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,21 @@ source:
4747
3 > inspect(1, content="1")
4848
4 | }
4949
50+
testdata/builtin-rules-all/unnessary_else.mbt:3:3-6:12
51+
rule: moonbitlang/unnessary_else
52+
description:
53+
Found an if expression whose else branch is empty or only returns ().
54+
Prefer omitting the unnecessary else branch.
55+
source:
56+
1 | ///|
57+
2 | fn unnecessary_empty_else(flag : Bool) -> Unit {
58+
3 > if flag {
59+
4 > prepare()
60+
5 > finish()
61+
6 > } else {}
62+
7 | }
63+
8 | ///|
64+
5065
testdata/builtin-rules-all/inspect_boolean.mbt:3:3-3:32
5166
rule: moonbitlang/inspect_boolean
5267
description:
@@ -118,6 +133,7 @@ $ cd "$TESTDIR"/.. && moonrun "$TESTDIR"/moongrep.wasm -- scan --output-json --e
118133
{"file":"testdata/builtin-rules-all/catch_all.mbt","rule_id":"moonbitlang/catch_all","description":"Single catch arm handles every error, which can hide unexpected failures.\nPrefer matching only the specific error cases that can be recovered from.","range":{"start":{"line":3,"column":3},"end":{"line":5,"column":4}},"matched_source":"try risky() catch {\n _ => recover()\n }","source_context":[{"line":1,"text":"///|","is_match":false},{"line":2,"text":"async fn catches_everything(_) -> Unit {","is_match":false},{"line":3,"text":" try risky() catch {","is_match":true},{"line":4,"text":" _ => recover()","is_match":true},{"line":5,"text":" }","is_match":true},{"line":6,"text":"}","is_match":false}]}
119134
{"file":"testdata/builtin-rules-all/match_option.mbt","rule_id":"moonbitlang/match_option","description":"Found an Option value handled with match over Some and None.\nPrefer if + is for simple Option checks.","range":{"start":{"line":3,"column":3},"end":{"line":11,"column":4}},"matched_source":"match value {\n Some(inner) => {\n let prepared = prepare(inner)\n let validated = validate(prepared)\n let normalized = normalize(validated)\n finish(normalized)\n }\n None => false\n }","source_context":[{"line":1,"text":"///|","is_match":false},{"line":2,"text":"fn option_match(value : Int?) -> Bool {","is_match":false},{"line":3,"text":" match value {","is_match":true},{"line":4,"text":" Some(inner) => {","is_match":true},{"line":5,"text":" let prepared = prepare(inner)","is_match":true},{"line":6,"text":" let validated = validate(prepared)","is_match":true},{"line":7,"text":" let normalized = normalize(validated)","is_match":true},{"line":8,"text":" finish(normalized)","is_match":true},{"line":9,"text":" }","is_match":true},{"line":10,"text":" None => false","is_match":true},{"line":11,"text":" }","is_match":true},{"line":12,"text":"}","is_match":false}]}
120135
{"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}]}
136+
{"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}]}
121137
{"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}]}
122138
{"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}]}
123139
{"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}]}

rule/builtin/builtin.mbt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ fn builtin_rule_sources() -> Array[BuiltinRuleSource] {
4141
path: "builtin/moonbitlang/match_option.yaml",
4242
yaml: @moonbitlang_rules.match_option_yaml,
4343
},
44+
{
45+
path: "builtin/moonbitlang/unnessary_else.yaml",
46+
yaml: @moonbitlang_rules.unnessary_else_yaml,
47+
},
4448
{
4549
path: "builtin/moonbitlang/cstyle_forward_simple_forloop.yaml",
4650
yaml: @moonbitlang_rules.cstyle_forward_simple_forloop_yaml,

rule/builtin/builtin_test.mbt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ test "builtin rules load moonbitlang rules" {
1313
#| "moonbitlang/inspect_boolean",
1414
#| "moonbitlang/inspect_number",
1515
#| "moonbitlang/match_option",
16+
#| "moonbitlang/unnessary_else",
1617
#| "moonbitlang/cstyle_forward_simple_forloop",
1718
#| "moonbitlang/cstyle_backward_simple_forloop",
1819
#| "moonbitlang/cstyle_forward_array_iteration",

rule/internal/rules/moonbitlang/moon.pkg

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ dev_build(
1919

2020
dev_build(rule: "embed", input: "match_option.yaml", output: "match_option.mbt")
2121

22+
dev_build(
23+
rule: "embed",
24+
input: "unnessary_else.yaml",
25+
output: "unnessary_else.mbt",
26+
)
27+
2228
dev_build(
2329
rule: "embed",
2430
input: "cstyle_forward_simple_forloop.yaml",

rule/internal/rules/moonbitlang/pkg.generated.mbti

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ pub let inspect_number_yaml : String
1818

1919
pub let match_option_yaml : String
2020

21+
pub let unnessary_else_yaml : String
22+
2123
// Errors
2224

2325
// Types and methods
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Generated by moonbit-community/embed from ./rule/internal/rules/moonbitlang/unnessary_else.yaml.
2+
3+
///|
4+
let _embed_unnessary_else_yaml : String =
5+
#|id: unnessary_else
6+
#|description: |
7+
#| Found an if expression whose else branch is empty or only returns ().
8+
#| Prefer omitting the unnecessary else branch.
9+
#|
10+
#|patterns:
11+
#| - shape: |
12+
#| if $_ {
13+
#| $_
14+
#| } else {
15+
#| ()
16+
#| }
17+
#| - shape: |
18+
#| if $_ {
19+
#| $_
20+
#| } else {}
21+
#|
22+
23+
///|
24+
pub let unnessary_else_yaml : String = _embed_unnessary_else_yaml
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
id: unnessary_else
2+
description: |
3+
Found an if expression whose else branch is empty or only returns ().
4+
Prefer omitting the unnecessary else branch.
5+
6+
patterns:
7+
- shape: |
8+
if $_ {
9+
$_
10+
} else {
11+
()
12+
}
13+
- shape: |
14+
if $_ {
15+
$_
16+
} else {}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
///|
2+
fn unnecessary_empty_else(flag : Bool) -> Unit {
3+
if flag {
4+
prepare()
5+
finish()
6+
} else {}
7+
}
8+
///|
9+
fn non_unit_else(flag : Bool) -> Unit {
10+
ignore(if flag { first() } else { second() })
11+
}

0 commit comments

Comments
 (0)