Add rule to remove if expressions

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+* add `remove_if_expression` rule ([#221](https://github.com/seaofvoices/darklua/pull/221))
+
 ## 0.14.0
 
 * 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))

site/content/rules/remove_if_expression.md

@@ -0,0 +1,12 @@
+---
+description: Remove if expressions
+added_in: "unreleased"
+parameters: []
+examples:
+  - content: |
+      local variable = if condition() then { option = true } else { option = false }
+---
+
+This rule removes all `if` expressions (not if statements!) and replaces them with an equivalent expression.
+
+**Note:** this rule is useful if you are converting Luau code into regular Lua code.

site/package.json

@@ -8,44 +8,44 @@
     "url": "https://github.com/seaofvoices/darklua/issues"
   },
   "dependencies": {
-    "@babel/eslint-parser": "^7.23.10",
-    "@emotion/react": "^11.11.4",
-    "@emotion/styled": "^11.11.0",
-    "@fontsource-variable/comfortaa": "^5.0.18",
+    "@babel/eslint-parser": "^7.25.8",
+    "@emotion/react": "^11.13.3",
+    "@emotion/styled": "^11.13.0",
+    "@fontsource-variable/comfortaa": "^5.1.0",
     "@mui/icons-material": "^5.15.13",
     "@mui/material": "^5.15.13",
     "acorn": "^8.11.3",
     "acorn-import-assertions": "^1.9.0",
     "babel-plugin-prismjs": "^2.1.0",
     "darklua": "./darklua-wasm/pkg",
-    "gatsby": "^5.13.3",
+    "gatsby": "^5.13.7",
     "gatsby-plugin-catch-links": "^5.13.1",
     "gatsby-plugin-image": "^3.13.1",
     "gatsby-plugin-manifest": "^5.13.1",
     "gatsby-plugin-react-helmet": "^6.13.1",
     "gatsby-plugin-sharp": "^5.13.1",
     "gatsby-remark-copy-linked-files": "^6.13.1",
-    "gatsby-remark-images": "^7.13.1",
-    "gatsby-remark-prismjs": "^7.13.1",
-    "gatsby-remark-responsive-iframe": "^6.13.1",
+    "gatsby-remark-images": "^7.13.2",
+    "gatsby-remark-prismjs": "^7.13.2",
+    "gatsby-remark-responsive-iframe": "^6.13.2",
     "gatsby-remark-smartypants": "^6.13.1",
     "gatsby-source-filesystem": "^5.13.1",
     "gatsby-transformer-remark": "^6.13.1",
     "gatsby-transformer-sharp": "^5.13.1",
-    "joi": "^17.12.2",
+    "joi": "^17.13.3",
     "json5": "^2.2.3",
-    "mdast-util-from-markdown": "^2.0.0",
-    "mdast-util-to-hast": "^13.1.0",
-    "monaco-editor": "^0.47.0",
+    "mdast-util-from-markdown": "^2.0.1",
+    "mdast-util-to-hast": "^13.2.0",
+    "monaco-editor": "^0.52.0",
     "monaco-editor-webpack-plugin": "^7.1.0",
     "prismjs": "^1.29.0",
-    "react": "^18.2.0",
-    "react-dom": "^18.2.0",
+    "react": "^18.3.1",
+    "react-dom": "^18.3.1",
     "react-helmet": "^6.1.0",
-    "react-use-ref-effect": "^1.2.0",
+    "react-use-ref-effect": "^1.3.0",
     "rehype-react": "^8.0.0",
-    "typescript": "^5.4.2",
-    "unified": "^11.0.4"
+    "typescript": "^5.6.3",
+    "unified": "^11.0.5"
   },
   "devDependencies": {
     "babel-preset-gatsby": "^3.13.1",
@@ -56,7 +56,7 @@
   },
   "overrides": {
     "react-server-dom-webpack@0.0.0-experimental-c8b778b7f-20220825": {
-      "react": "^18.2.0"
+      "react": "^18.3.1"
     }
   },
   "homepage": "https://github.com/seaofvoices/darklua",

src/process/evaluator/mod.rs

@@ -80,12 +80,14 @@ impl Evaluator {
     #[allow(clippy::only_used_in_recursion)]
     pub fn can_return_multiple_values(&self, expression: &Expression) -> bool {
         match expression {
-            Expression::Binary(_)
-            | Expression::Call(_)
+            Expression::Call(_)
             | Expression::Field(_)
             | Expression::Index(_)
             | Expression::Unary(_)
             | Expression::VariableArguments(_) => true,
+            Expression::Binary(binary) => {
+                !matches!(binary.operator(), BinaryOperator::And | BinaryOperator::Or)
+            }
             Expression::False(_)
             | Expression::Function(_)
             | Expression::Identifier(_)

src/rules/mod.rs

@@ -18,6 +18,7 @@ mod remove_call_match;
 mod remove_comments;
 mod remove_compound_assign;
 mod remove_debug_profiling;
+mod remove_if_expression;
 mod remove_interpolated_string;
 mod remove_nil_declarations;
 mod remove_spaces;
@@ -47,6 +48,7 @@ pub use remove_assertions::*;
 pub use remove_comments::*;
 pub use remove_compound_assign::*;
 pub use remove_debug_profiling::*;
+pub use remove_if_expression::*;
 pub use remove_interpolated_string::*;
 pub use remove_nil_declarations::*;
 pub use remove_spaces::*;
@@ -242,6 +244,7 @@ pub fn get_all_rule_names() -> Vec<&'static str> {
         REMOVE_UNUSED_VARIABLE_RULE_NAME,
         REMOVE_UNUSED_WHILE_RULE_NAME,
         RENAME_VARIABLES_RULE_NAME,
+        REMOVE_IF_EXPRESSION_RULE_NAME,
     ]
 }
 
@@ -275,6 +278,7 @@ impl FromStr for Box<dyn Rule> {
             REMOVE_UNUSED_VARIABLE_RULE_NAME => Box::<RemoveUnusedVariable>::default(),
             REMOVE_UNUSED_WHILE_RULE_NAME => Box::<RemoveUnusedWhile>::default(),
             RENAME_VARIABLES_RULE_NAME => Box::<RenameVariables>::default(),
+            REMOVE_IF_EXPRESSION_RULE_NAME => Box::<RemoveIfExpression>::default(),
             _ => return Err(format!("invalid rule name: {}", string)),
         };
 

src/rules/remove_if_expression.rs

@@ -0,0 +1,145 @@
+use crate::nodes::{
+    BinaryExpression, BinaryOperator, Block, Expression, IndexExpression, TableEntry,
+    TableExpression,
+};
+use crate::process::{DefaultVisitor, Evaluator, NodeProcessor, NodeVisitor};
+use crate::rules::{
+    Context, FlawlessRule, RuleConfiguration, RuleConfigurationError, RuleProperties,
+};
+
+use super::verify_no_rule_properties;
+
+#[derive(Default)]
+struct Processor {
+    evaluator: Evaluator,
+}
+
+impl Processor {
+    fn wrap_in_table(&self, expression: Expression) -> Expression {
+        TableExpression::new(vec![TableEntry::Value({
+            if self.evaluator.can_return_multiple_values(&expression) {
+                expression.in_parentheses()
+            } else {
+                expression
+            }
+        })])
+        .into()
+    }
+
+    fn convert_if_branch(
+        &self,
+        condition: Expression,
+        result: Expression,
+        else_result: Expression,
+    ) -> Expression {
+        if self
+            .evaluator
+            .evaluate(&result)
+            .is_truthy()
+            .unwrap_or_default()
+        {
+            BinaryExpression::new(
+                BinaryOperator::Or,
+                BinaryExpression::new(BinaryOperator::And, condition, result),
+                else_result,
+            )
+            .into()
+        } else {
+            IndexExpression::new(
+                Expression::from(BinaryExpression::new(
+                    BinaryOperator::Or,
+                    BinaryExpression::new(
+                        BinaryOperator::And,
+                        condition,
+                        self.wrap_in_table(result),
+                    ),
+                    self.wrap_in_table(else_result),
+                )),
+                Expression::from(1),
+            )
+            .into()
+        }
+    }
+}
+
+impl NodeProcessor for Processor {
+    fn process_expression(&mut self, expression: &mut Expression) {
+        if let Expression::If(if_expression) = expression {
+            let else_result = if_expression.iter_branches().fold(
+                if_expression.get_else_result().clone(),
+                |else_result, branch| {
+                    self.convert_if_branch(
+                        branch.get_condition().clone(),
+                        branch.get_result().clone(),
+                        else_result,
+                    )
+                },
+            );
+
+            *expression = self.convert_if_branch(
+                if_expression.get_condition().clone(),
+                if_expression.get_result().clone(),
+                else_result,
+            );
+        }
+    }
+}
+
+pub const REMOVE_IF_EXPRESSION_RULE_NAME: &str = "remove_if_expression";
+
+/// A rule that removes trailing `nil` in local assignments.
+#[derive(Debug, Default, PartialEq, Eq)]
+pub struct RemoveIfExpression {}
+
+impl FlawlessRule for RemoveIfExpression {
+    fn flawless_process(&self, block: &mut Block, _: &Context) {
+        let mut processor = Processor::default();
+        DefaultVisitor::visit_block(block, &mut processor);
+    }
+}
+
+impl RuleConfiguration for RemoveIfExpression {
+    fn configure(&mut self, properties: RuleProperties) -> Result<(), RuleConfigurationError> {
+        verify_no_rule_properties(&properties)?;
+
+        Ok(())
+    }
+
+    fn get_name(&self) -> &'static str {
+        REMOVE_IF_EXPRESSION_RULE_NAME
+    }
+
+    fn serialize_to_properties(&self) -> RuleProperties {
+        RuleProperties::new()
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::rules::Rule;
+
+    use insta::assert_json_snapshot;
+
+    fn new_rule() -> RemoveIfExpression {
+        RemoveIfExpression::default()
+    }
+
+    #[test]
+    fn serialize_default_rule() {
+        let rule: Box<dyn Rule> = Box::new(new_rule());
+
+        assert_json_snapshot!("default_remove_if_expression", rule);
+    }
+
+    #[test]
+    fn configure_with_extra_field_error() {
+        let result = json5::from_str::<Box<dyn Rule>>(
+            r#"{
+            rule: 'remove_if_expression',
+            prop: "something",
+        }"#,
+        );
+        pretty_assertions::assert_eq!(result.unwrap_err().to_string(), "unexpected field 'prop'");
+    }
+}

src/rules/snapshots/darklua_core__rules__remove_continue__test__default_remove_continue.snap

@@ -0,0 +1,5 @@
+---
+source: src/rules/remove_continue.rs
+expression: rule
+---
+"remove_continue"

src/rules/snapshots/darklua_core__rules__remove_if_expression__test__default_remove_if_expression.snap

@@ -0,0 +1,5 @@
+---
+source: src/rules/remove_if_expression.rs
+expression: rule
+---
+"remove_if_expression"

src/rules/snapshots/darklua_core__rules__test__all_rule_names.snap

@@ -25,5 +25,6 @@ expression: rule_names
   "remove_unused_if_branch",
   "remove_unused_variable",
   "remove_unused_while",
-  "rename_variables"
+  "rename_variables",
+  "remove_if_expression"
 ]

tests/rule_tests/mod.rs

@@ -304,6 +304,7 @@ mod remove_comments;
 mod remove_compound_assignment;
 mod remove_debug_profiling;
 mod remove_empty_do;
+mod remove_if_expression;
 mod remove_interpolated_string;
 mod remove_method_definition;
 mod remove_nil_declaration;

tests/rule_tests/remove_if_expression.rs

@@ -0,0 +1,39 @@
+use darklua_core::rules::{RemoveIfExpression, Rule};
+
+test_rule!(
+    remove_if_expression,
+    RemoveIfExpression::default(),
+    if_with_truthy_result("local a = if condition() then 1 else 2")
+        => "local a = condition() and 1 or 2",
+    if_with_truthy_result_else_nil("local a = if condition() then '' else nil")
+        => "local a = condition() and '' or nil",
+    if_with_truthy_result_else_false("local a = if condition() then {} else false")
+        => "local a = condition() and {} or false",
+    if_with_nil_result_else_false("local a = if condition() then nil else false")
+        => "local a = (condition() and { nil } or { false })[1]",
+    if_with_false_result_else_truthy("local a = if condition() then false else true")
+        => "local a = (condition() and { false } or { true })[1]",
+    if_with_unknown_result_else_unknown("local a = if condition() then update() else default()")
+        => "local a = (condition() and { (update()) } or { (default()) })[1]",
+    assign_if_expression_with_elseif("local a = if true then 1 elseif false then 2 else 3")
+        => "local a = true and 1 or (false and 2 or 3)",
+    if_expression_with_varargs("local function f(...: string) return if condition(...) then ... else transform(...) end")
+        => "local function f(...: string) return (condition(...) and {(...)} or {(transform(...))})[1] end",
+    if_expression_with_varargs_elseif("local function f(...: string) return if condition(...) then ... elseif condition2(...) then ... else transform(...) end")
+        => "local function f(...: string) return (condition(...) and {(...)} or { ((condition2(...) and {(...)} or { (transform(...)) })[1]) }) [1] end"
+);
+
+#[test]
+fn deserialize_from_object_notation() {
+    json5::from_str::<Box<dyn Rule>>(
+        r#"{
+        rule: 'remove_if_expression',
+    }"#,
+    )
+    .unwrap();
+}
+
+#[test]
+fn deserialize_from_string() {
+    json5::from_str::<Box<dyn Rule>>("'remove_if_expression'").unwrap();
+}