Skip to content

Commit f49501b

Browse files
jeparlefrancaisjiwonz
authored andcommitted
Add rule to remove if expressions (seaofvoices#221)
This PR add a new rule that remove `if` expressions and convert them into an equivalent expression. --------- Co-authored-by: jiwonz <jiwonz0113@gmail.com>
1 parent 0309be3 commit f49501b

12 files changed

+7722
-4147
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## Unreleased
44

5+
* add `remove_if_expression` rule ([#221](https://github.com/seaofvoices/darklua/pull/221))
6+
57
## 0.14.0
68

79
* migrate parser to the latest version. Reduce stack overflow issues, add support for compound assignments using floor division and leading symbols in union and intersection types ([#219](https://github.com/seaofvoices/darklua/pull/219))
+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
description: Remove if expressions
3+
added_in: "unreleased"
4+
parameters: []
5+
examples:
6+
- content: |
7+
local variable = if condition() then { option = true } else { option = false }
8+
---
9+
10+
This rule removes all `if` expressions (not if statements!) and replaces them with an equivalent expression.
11+
12+
**Note:** this rule is useful if you are converting Luau code into regular Lua code.

site/package-lock.json

+7,485-4,126
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

site/package.json

+18-18
Original file line numberDiff line numberDiff line change
@@ -8,44 +8,44 @@
88
"url": "https://github.com/seaofvoices/darklua/issues"
99
},
1010
"dependencies": {
11-
"@babel/eslint-parser": "^7.23.10",
12-
"@emotion/react": "^11.11.4",
13-
"@emotion/styled": "^11.11.0",
14-
"@fontsource-variable/comfortaa": "^5.0.18",
11+
"@babel/eslint-parser": "^7.25.8",
12+
"@emotion/react": "^11.13.3",
13+
"@emotion/styled": "^11.13.0",
14+
"@fontsource-variable/comfortaa": "^5.1.0",
1515
"@mui/icons-material": "^5.15.13",
1616
"@mui/material": "^5.15.13",
1717
"acorn": "^8.11.3",
1818
"acorn-import-assertions": "^1.9.0",
1919
"babel-plugin-prismjs": "^2.1.0",
2020
"darklua": "./darklua-wasm/pkg",
21-
"gatsby": "^5.13.3",
21+
"gatsby": "^5.13.7",
2222
"gatsby-plugin-catch-links": "^5.13.1",
2323
"gatsby-plugin-image": "^3.13.1",
2424
"gatsby-plugin-manifest": "^5.13.1",
2525
"gatsby-plugin-react-helmet": "^6.13.1",
2626
"gatsby-plugin-sharp": "^5.13.1",
2727
"gatsby-remark-copy-linked-files": "^6.13.1",
28-
"gatsby-remark-images": "^7.13.1",
29-
"gatsby-remark-prismjs": "^7.13.1",
30-
"gatsby-remark-responsive-iframe": "^6.13.1",
28+
"gatsby-remark-images": "^7.13.2",
29+
"gatsby-remark-prismjs": "^7.13.2",
30+
"gatsby-remark-responsive-iframe": "^6.13.2",
3131
"gatsby-remark-smartypants": "^6.13.1",
3232
"gatsby-source-filesystem": "^5.13.1",
3333
"gatsby-transformer-remark": "^6.13.1",
3434
"gatsby-transformer-sharp": "^5.13.1",
35-
"joi": "^17.12.2",
35+
"joi": "^17.13.3",
3636
"json5": "^2.2.3",
37-
"mdast-util-from-markdown": "^2.0.0",
38-
"mdast-util-to-hast": "^13.1.0",
39-
"monaco-editor": "^0.47.0",
37+
"mdast-util-from-markdown": "^2.0.1",
38+
"mdast-util-to-hast": "^13.2.0",
39+
"monaco-editor": "^0.52.0",
4040
"monaco-editor-webpack-plugin": "^7.1.0",
4141
"prismjs": "^1.29.0",
42-
"react": "^18.2.0",
43-
"react-dom": "^18.2.0",
42+
"react": "^18.3.1",
43+
"react-dom": "^18.3.1",
4444
"react-helmet": "^6.1.0",
45-
"react-use-ref-effect": "^1.2.0",
45+
"react-use-ref-effect": "^1.3.0",
4646
"rehype-react": "^8.0.0",
47-
"typescript": "^5.4.2",
48-
"unified": "^11.0.4"
47+
"typescript": "^5.6.3",
48+
"unified": "^11.0.5"
4949
},
5050
"devDependencies": {
5151
"babel-preset-gatsby": "^3.13.1",
@@ -56,7 +56,7 @@
5656
},
5757
"overrides": {
5858
"react-server-dom-webpack@0.0.0-experimental-c8b778b7f-20220825": {
59-
"react": "^18.2.0"
59+
"react": "^18.3.1"
6060
}
6161
},
6262
"homepage": "https://github.com/seaofvoices/darklua",

src/process/evaluator/mod.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,14 @@ impl Evaluator {
8080
#[allow(clippy::only_used_in_recursion)]
8181
pub fn can_return_multiple_values(&self, expression: &Expression) -> bool {
8282
match expression {
83-
Expression::Binary(_)
84-
| Expression::Call(_)
83+
Expression::Call(_)
8584
| Expression::Field(_)
8685
| Expression::Index(_)
8786
| Expression::Unary(_)
8887
| Expression::VariableArguments(_) => true,
88+
Expression::Binary(binary) => {
89+
!matches!(binary.operator(), BinaryOperator::And | BinaryOperator::Or)
90+
}
8991
Expression::False(_)
9092
| Expression::Function(_)
9193
| Expression::Identifier(_)

src/rules/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ mod remove_call_match;
1818
mod remove_comments;
1919
mod remove_compound_assign;
2020
mod remove_debug_profiling;
21+
mod remove_if_expression;
2122
mod remove_interpolated_string;
2223
mod remove_nil_declarations;
2324
mod remove_spaces;
@@ -47,6 +48,7 @@ pub use remove_assertions::*;
4748
pub use remove_comments::*;
4849
pub use remove_compound_assign::*;
4950
pub use remove_debug_profiling::*;
51+
pub use remove_if_expression::*;
5052
pub use remove_interpolated_string::*;
5153
pub use remove_nil_declarations::*;
5254
pub use remove_spaces::*;
@@ -242,6 +244,7 @@ pub fn get_all_rule_names() -> Vec<&'static str> {
242244
REMOVE_UNUSED_VARIABLE_RULE_NAME,
243245
REMOVE_UNUSED_WHILE_RULE_NAME,
244246
RENAME_VARIABLES_RULE_NAME,
247+
REMOVE_IF_EXPRESSION_RULE_NAME,
245248
]
246249
}
247250

@@ -275,6 +278,7 @@ impl FromStr for Box<dyn Rule> {
275278
REMOVE_UNUSED_VARIABLE_RULE_NAME => Box::<RemoveUnusedVariable>::default(),
276279
REMOVE_UNUSED_WHILE_RULE_NAME => Box::<RemoveUnusedWhile>::default(),
277280
RENAME_VARIABLES_RULE_NAME => Box::<RenameVariables>::default(),
281+
REMOVE_IF_EXPRESSION_RULE_NAME => Box::<RemoveIfExpression>::default(),
278282
_ => return Err(format!("invalid rule name: {}", string)),
279283
};
280284

src/rules/remove_if_expression.rs

+145
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
use crate::nodes::{
2+
BinaryExpression, BinaryOperator, Block, Expression, IndexExpression, TableEntry,
3+
TableExpression,
4+
};
5+
use crate::process::{DefaultVisitor, Evaluator, NodeProcessor, NodeVisitor};
6+
use crate::rules::{
7+
Context, FlawlessRule, RuleConfiguration, RuleConfigurationError, RuleProperties,
8+
};
9+
10+
use super::verify_no_rule_properties;
11+
12+
#[derive(Default)]
13+
struct Processor {
14+
evaluator: Evaluator,
15+
}
16+
17+
impl Processor {
18+
fn wrap_in_table(&self, expression: Expression) -> Expression {
19+
TableExpression::new(vec![TableEntry::Value({
20+
if self.evaluator.can_return_multiple_values(&expression) {
21+
expression.in_parentheses()
22+
} else {
23+
expression
24+
}
25+
})])
26+
.into()
27+
}
28+
29+
fn convert_if_branch(
30+
&self,
31+
condition: Expression,
32+
result: Expression,
33+
else_result: Expression,
34+
) -> Expression {
35+
if self
36+
.evaluator
37+
.evaluate(&result)
38+
.is_truthy()
39+
.unwrap_or_default()
40+
{
41+
BinaryExpression::new(
42+
BinaryOperator::Or,
43+
BinaryExpression::new(BinaryOperator::And, condition, result),
44+
else_result,
45+
)
46+
.into()
47+
} else {
48+
IndexExpression::new(
49+
Expression::from(BinaryExpression::new(
50+
BinaryOperator::Or,
51+
BinaryExpression::new(
52+
BinaryOperator::And,
53+
condition,
54+
self.wrap_in_table(result),
55+
),
56+
self.wrap_in_table(else_result),
57+
)),
58+
Expression::from(1),
59+
)
60+
.into()
61+
}
62+
}
63+
}
64+
65+
impl NodeProcessor for Processor {
66+
fn process_expression(&mut self, expression: &mut Expression) {
67+
if let Expression::If(if_expression) = expression {
68+
let else_result = if_expression.iter_branches().fold(
69+
if_expression.get_else_result().clone(),
70+
|else_result, branch| {
71+
self.convert_if_branch(
72+
branch.get_condition().clone(),
73+
branch.get_result().clone(),
74+
else_result,
75+
)
76+
},
77+
);
78+
79+
*expression = self.convert_if_branch(
80+
if_expression.get_condition().clone(),
81+
if_expression.get_result().clone(),
82+
else_result,
83+
);
84+
}
85+
}
86+
}
87+
88+
pub const REMOVE_IF_EXPRESSION_RULE_NAME: &str = "remove_if_expression";
89+
90+
/// A rule that removes trailing `nil` in local assignments.
91+
#[derive(Debug, Default, PartialEq, Eq)]
92+
pub struct RemoveIfExpression {}
93+
94+
impl FlawlessRule for RemoveIfExpression {
95+
fn flawless_process(&self, block: &mut Block, _: &Context) {
96+
let mut processor = Processor::default();
97+
DefaultVisitor::visit_block(block, &mut processor);
98+
}
99+
}
100+
101+
impl RuleConfiguration for RemoveIfExpression {
102+
fn configure(&mut self, properties: RuleProperties) -> Result<(), RuleConfigurationError> {
103+
verify_no_rule_properties(&properties)?;
104+
105+
Ok(())
106+
}
107+
108+
fn get_name(&self) -> &'static str {
109+
REMOVE_IF_EXPRESSION_RULE_NAME
110+
}
111+
112+
fn serialize_to_properties(&self) -> RuleProperties {
113+
RuleProperties::new()
114+
}
115+
}
116+
117+
#[cfg(test)]
118+
mod test {
119+
use super::*;
120+
use crate::rules::Rule;
121+
122+
use insta::assert_json_snapshot;
123+
124+
fn new_rule() -> RemoveIfExpression {
125+
RemoveIfExpression::default()
126+
}
127+
128+
#[test]
129+
fn serialize_default_rule() {
130+
let rule: Box<dyn Rule> = Box::new(new_rule());
131+
132+
assert_json_snapshot!("default_remove_if_expression", rule);
133+
}
134+
135+
#[test]
136+
fn configure_with_extra_field_error() {
137+
let result = json5::from_str::<Box<dyn Rule>>(
138+
r#"{
139+
rule: 'remove_if_expression',
140+
prop: "something",
141+
}"#,
142+
);
143+
pretty_assertions::assert_eq!(result.unwrap_err().to_string(), "unexpected field 'prop'");
144+
}
145+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
source: src/rules/remove_continue.rs
3+
expression: rule
4+
---
5+
"remove_continue"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
source: src/rules/remove_if_expression.rs
3+
expression: rule
4+
---
5+
"remove_if_expression"

src/rules/snapshots/darklua_core__rules__test__all_rule_names.snap

+2-1
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,6 @@ expression: rule_names
2525
"remove_unused_if_branch",
2626
"remove_unused_variable",
2727
"remove_unused_while",
28-
"rename_variables"
28+
"rename_variables",
29+
"remove_if_expression"
2930
]

tests/rule_tests/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ mod remove_comments;
304304
mod remove_compound_assignment;
305305
mod remove_debug_profiling;
306306
mod remove_empty_do;
307+
mod remove_if_expression;
307308
mod remove_interpolated_string;
308309
mod remove_method_definition;
309310
mod remove_nil_declaration;
+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
use darklua_core::rules::{RemoveIfExpression, Rule};
2+
3+
test_rule!(
4+
remove_if_expression,
5+
RemoveIfExpression::default(),
6+
if_with_truthy_result("local a = if condition() then 1 else 2")
7+
=> "local a = condition() and 1 or 2",
8+
if_with_truthy_result_else_nil("local a = if condition() then '' else nil")
9+
=> "local a = condition() and '' or nil",
10+
if_with_truthy_result_else_false("local a = if condition() then {} else false")
11+
=> "local a = condition() and {} or false",
12+
if_with_nil_result_else_false("local a = if condition() then nil else false")
13+
=> "local a = (condition() and { nil } or { false })[1]",
14+
if_with_false_result_else_truthy("local a = if condition() then false else true")
15+
=> "local a = (condition() and { false } or { true })[1]",
16+
if_with_unknown_result_else_unknown("local a = if condition() then update() else default()")
17+
=> "local a = (condition() and { (update()) } or { (default()) })[1]",
18+
assign_if_expression_with_elseif("local a = if true then 1 elseif false then 2 else 3")
19+
=> "local a = true and 1 or (false and 2 or 3)",
20+
if_expression_with_varargs("local function f(...: string) return if condition(...) then ... else transform(...) end")
21+
=> "local function f(...: string) return (condition(...) and {(...)} or {(transform(...))})[1] end",
22+
if_expression_with_varargs_elseif("local function f(...: string) return if condition(...) then ... elseif condition2(...) then ... else transform(...) end")
23+
=> "local function f(...: string) return (condition(...) and {(...)} or { ((condition2(...) and {(...)} or { (transform(...)) })[1]) }) [1] end"
24+
);
25+
26+
#[test]
27+
fn deserialize_from_object_notation() {
28+
json5::from_str::<Box<dyn Rule>>(
29+
r#"{
30+
rule: 'remove_if_expression',
31+
}"#,
32+
)
33+
.unwrap();
34+
}
35+
36+
#[test]
37+
fn deserialize_from_string() {
38+
json5::from_str::<Box<dyn Rule>>("'remove_if_expression'").unwrap();
39+
}

0 commit comments

Comments
 (0)