Repository: seaofvoices/darklua (rust)

Issue:
`remove_unused_variable` incorrect behavior
`remove_unused_variable` incorrectly removes unassigned local variables in grouped local assignments after a function call and assigned variable.

# Reproduction
`a.luau`:
```lua
local c = 0;
(function()
    local a, b, c = (function()end)(), 0;
    c = 1;
end)();
assert(c == 0);
```

`.darklua.json`:
```json
{
    "rules": [
        "remove_unused_variable"
    ]
}
```

```
darklua process a.luau out.luau --verbose
 INFO > using configuration file `.darklua.json`
 INFO > successfully processed `a.luau`
 INFO > executed work in 1.2508ms
successfully processed 1 file (in 2.049ms)
```
```
darklua --version
darklua 0.13.1
```

`out.luau`:
```lua
local c = 0;
(function()
local a=(function()end)();
    c = 1;
end)();
assert(c == 0);
```
This output is incorrect. When combined with `group_local_assignment` and `rename_variables`, this bug is more likely to break scripts.

Pull request description:
Fix `remove_unused_variable` rule bug with used unassigned variables
Closes #209 

Fixed the `remove_unused_variable` rule to correctly handle trailing unassigned but used variables.

- [x] add entry to the changelog


Patch:
diff --git a/src/rules/remove_unused_variable.rs b/src/rules/remove_unused_variable.rs
index be4b942..5c39e42 100644
--- a/src/rules/remove_unused_variable.rs
+++ b/src/rules/remove_unused_variable.rs
@@ -129,22 +129,33 @@ impl NodeProcessor for RemoveUnusedVariableProcessor {
                                 .collect();
 
                             let length = assignments.len();
+                            let mut remaining_unassigned_variables = Vec::new();
+
                             if let Some((last, value)) = assignments.last_mut() {
+                                let remaining =
+                                    assign.iter_variables().zip(usages.iter()).skip(length);
                                 if self.evaluator.can_return_multiple_values(value) {
-                                    last.extend(
-                                        assign.iter_variables().zip(usages.iter()).skip(length),
+                                    last.extend(remaining);
+                                } else {
+                                    remaining_unassigned_variables.extend(
+                                        remaining
+                                            .filter(|(_, used)| **used)
+                                            .map(|(identifier, _)| identifier.clone()),
                                     );
                                 }
                             }
 
-                            let mut variables = Vec::new();
-                            let mut values = Vec::new();
+                            let mut values: Vec<_> = remaining_unassigned_variables
+                                .iter()
+                                .map(|_| Expression::nil())
+                                .collect();
+                            let mut variables = remaining_unassigned_variables;
 
                             for (mut identifiers, value) in assignments {
-                                if !self.evaluator.has_side_effects(value) {
-                                    while identifiers.last().filter(|(_, used)| !*used).is_some() {
-                                        identifiers.pop();
-                                    }
+                                let mut last_popped = None;
+
+                                while identifiers.last().filter(|(_, used)| !*used).is_some() {
+                                    last_popped = identifiers.pop();
                                 }
 
                                 if !identifiers.is_empty() {
@@ -154,6 +165,11 @@ impl NodeProcessor for RemoveUnusedVariableProcessor {
                                             .map(|(identifier, _)| identifier.clone()),
                                     );
                                     values.push(value.clone());
+                                } else if self.evaluator.has_side_effects(value) {
+                                    if let Some((last_identifier, _)) = last_popped {
+                                        variables.push(last_identifier.clone());
+                                        values.push(value.clone());
+                                    }
                                 }
                             }
 


Test patch:
diff --git a/tests/rule_tests/remove_unused_variable.rs b/tests/rule_tests/remove_unused_variable.rs
index 3fb344f..b8f2c08 100644
--- a/tests/rule_tests/remove_unused_variable.rs
+++ b/tests/rule_tests/remove_unused_variable.rs
@@ -56,6 +56,9 @@ test_rule!(
     remove_unused_variable_but_keep_require_side_effect("local _requireZero = require('./requireZero.roblox.lua')") => "require('./requireZero.roblox.lua')",
     remove_unused_variable_but_keep_require_side_effect_in_parens("local _requireZero = (require('./requireZero.roblox.lua'))") => "require('./requireZero.roblox.lua')",
     remove_unused_variable_but_keep_require_side_effect_in_parens_with_type_cast("local _requireZero = (require('./requireZero.roblox.lua') :: any)") => "require('./requireZero.roblox.lua')",
+    remove_unused_variable_but_not_initially_assigned_variable("local a, b = true b = false") => "local b = nil b = false",
+    remove_unused_variable_but_not_initially_assigned_variable_with_unused_trailing_variable("local a, b, c = true b = false") => "local b = nil b = false",
+    keep_used_variable_bounded_by_unused_variable_with_side_effect("local a, b, c = call() b = false") => "local a, b = call() b = false",
     // remove variables that are used more than once, but never read
     // remove_if_only_assigned("local a = true a = false") => "",
     // remove_if_only_field_assigned("local a = {} a.foo = false") => "",