Repository: seaofvoices/darklua (rust)

Issue:
Remove interpolated strings
Add rule to remove interpolated strings. Simple strings can be converted to string expressions:

**Examples:** 

```lua
return `...`
```

Converts into:

```lua
return "..."
```

More complex interpolated strings can be a chain of concatenation

```lua
return `...{true}`
```

Converts into:

```lua
return "..." .. tostring(true)
```


Pull request:
Add rule to remove interpolated strings
Closes #92 

This new rule replaces interpolated strings with `string.format` calls. The rule has a `strategy` property that defines how it converts the interpolated strings. 

The default strategy `"string"` uses the `%s` string specifier in `string.format` calls and wrap each interpolated value in a `tostring` call. 

The other available strategy is `tostring`, which uses the `%*` string specifier in `string.format`. This strategy does not need to wrap interpolated values.

Example:

```lua
local _ = `{object}`
-- generates:
local _ = tostring(object)
```

```lua
local _ = `Value = {object}`
-- generates:
local _ = string.format("Value = %s", tostring(object))
-- or generates with the `tostring` strategy:
local _ = string.format("Value = %*", object)
```

Todo:
- [x] add entry to the changelog
- [x] add rule docs


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3140266..bdac5ed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* add rule to remove interpolated strings (`remove_interpolated_string`) ([#156](https://github.com/seaofvoices/darklua/pull/156))
 * add support for floor division (`//`) operator in binary expressions ([#155](https://github.com/seaofvoices/darklua/pull/155))
 * add support for Luau interpolated strings ([#94](https://github.com/seaofvoices/darklua/pull/94))
 * add rule to append text comments ([#141](https://github.com/seaofvoices/darklua/pull/141))
diff --git a/site/content/rules/remove_interpolated_string.md b/site/content/rules/remove_interpolated_string.md
new file mode 100644
index 0000000..467a49a
--- /dev/null
+++ b/site/content/rules/remove_interpolated_string.md
@@ -0,0 +1,16 @@
+---
+description: Removes interpolated strings (backtick strings)
+added_in: "unreleased"
+parameters:
+  - name: strategy
+    type: '"string" or "tostring"'
+    description: Defines how darklua converts the interpolated strings into `string.format` calls. The "string" strategy will make the rule use the `%s` specifier and the "tostring" strategy will use the `%*` specifier.
+    default: string
+examples:
+  - content: "return `abc`"
+  - content: "return ``"
+  - content: "return `+{value} (in seconds)`"
+  - content: "return `Total = {#elements}`"
+---
+
+This rule removes all interpolated strings and replaces them with `string.format` calls.
diff --git a/src/nodes/arguments.rs b/src/nodes/arguments.rs
index 8b43bf2..3e15ee9 100644
--- a/src/nodes/arguments.rs
+++ b/src/nodes/arguments.rs
@@ -1,3 +1,5 @@
+use std::iter;
+
 use crate::nodes::{Expression, StringExpression, TableExpression, Token};
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -131,6 +133,15 @@ impl From<Arguments> for TupleArguments {
     }
 }
 
+impl iter::FromIterator<Expression> for TupleArguments {
+    fn from_iter<T: IntoIterator<Item = Expression>>(iter: T) -> Self {
+        Self {
+            values: iter.into_iter().collect(),
+            tokens: None,
+        }
+    }
+}
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum Arguments {
     Tuple(TupleArguments),
diff --git a/src/nodes/expressions/interpolated_string.rs b/src/nodes/expressions/interpolated_string.rs
index 04de8b6..5f124a9 100644
--- a/src/nodes/expressions/interpolated_string.rs
+++ b/src/nodes/expressions/interpolated_string.rs
@@ -44,6 +44,16 @@ impl StringSegment {
         self.token = None;
     }
 
+    #[inline]
+    pub fn len(&self) -> usize {
+        self.value.len()
+    }
+
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        self.value.is_empty()
+    }
+
     pub fn clear_comments(&mut self) {
         if let Some(token) = &mut self.token {
             token.clear_comments();
@@ -309,6 +319,19 @@ impl InterpolatedStringExpression {
         self.segments.iter_mut()
     }
 
+    #[inline]
+    pub fn len(&self) -> usize {
+        self.segments.len()
+    }
+
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        self.segments.iter().all(|segment| match segment {
+            InterpolationSegment::String(string_segment) => string_segment.is_empty(),
+            InterpolationSegment::Value(_) => false,
+        })
+    }
+
     pub fn push_segment(&mut self, segment: impl Into<InterpolationSegment>) {
         let new_segment = segment.into();
         match new_segment {
diff --git a/src/parser.rs b/src/parser.rs
index 2de39f0..9b629a6 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -678,6 +678,21 @@ mod test {
                 r#return: spaced_token(0, 6),
                 commas: Vec::new(),
             }),
+            return_backtick_string_with_escaped_backtick("return `\\``") => ReturnStatement::one(
+                InterpolatedStringExpression::empty()
+                .with_segment(
+                    StringSegment::from_value('`').with_token(token_at_first_line(8, 10))
+                )
+                .with_tokens(
+                    InterpolatedStringTokens {
+                        opening_tick: token_at_first_line(7, 8),
+                        closing_tick: token_at_first_line(10, 11),
+                    }
+                )
+            ).with_tokens(ReturnTokens {
+                r#return: spaced_token(0, 6),
+                commas: Vec::new(),
+            }),
             return_backtick_string_hello("return `hello`") => ReturnStatement::one(
                 InterpolatedStringExpression::new(vec![
                     StringSegment::from_value("hello")
diff --git a/src/process/scope_visitor.rs b/src/process/scope_visitor.rs
index e258f59..5419d1e 100644
--- a/src/process/scope_visitor.rs
+++ b/src/process/scope_visitor.rs
@@ -151,7 +151,7 @@ impl<T: NodeProcessor + Scope> NodeVisitor<T> for ScopeVisitor {
     }
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Default)]
 pub(crate) struct IdentifierTracker {
     identifiers: Vec<HashSet<String>>,
 }
diff --git a/src/rules/mod.rs b/src/rules/mod.rs
index 452110a..b630676 100644
--- a/src/rules/mod.rs
+++ b/src/rules/mod.rs
@@ -15,6 +15,7 @@ mod method_def;
 mod no_local_function;
 mod remove_comments;
 mod remove_compound_assign;
+mod remove_interpolated_string;
 mod remove_nil_declarations;
 mod remove_spaces;
 mod remove_types;
@@ -40,6 +41,7 @@ pub use method_def::*;
 pub use no_local_function::*;
 pub use remove_comments::*;
 pub use remove_compound_assign::*;
+pub use remove_interpolated_string::*;
 pub use remove_nil_declarations::*;
 pub use remove_spaces::*;
 pub use remove_types::*;
@@ -221,6 +223,7 @@ pub fn get_all_rule_names() -> Vec<&'static str> {
         REMOVE_COMPOUND_ASSIGNMENT_RULE_NAME,
         REMOVE_EMPTY_DO_RULE_NAME,
         REMOVE_FUNCTION_CALL_PARENS_RULE_NAME,
+        REMOVE_INTERPOLATED_STRING_RULE_NAME,
         REMOVE_METHOD_DEFINITION_RULE_NAME,
         REMOVE_NIL_DECLARATION_RULE_NAME,
         REMOVE_SPACES_RULE_NAME,
@@ -250,6 +253,7 @@ impl FromStr for Box<dyn Rule> {
             REMOVE_COMPOUND_ASSIGNMENT_RULE_NAME => Box::<RemoveCompoundAssignment>::default(),
             REMOVE_EMPTY_DO_RULE_NAME => Box::<RemoveEmptyDo>::default(),
             REMOVE_FUNCTION_CALL_PARENS_RULE_NAME => Box::<RemoveFunctionCallParens>::default(),
+            REMOVE_INTERPOLATED_STRING_RULE_NAME => Box::<RemoveInterpolatedString>::default(),
             REMOVE_METHOD_DEFINITION_RULE_NAME => Box::<RemoveMethodDefinition>::default(),
             REMOVE_NIL_DECLARATION_RULE_NAME => Box::<RemoveNilDeclaration>::default(),
             REMOVE_SPACES_RULE_NAME => Box::<RemoveSpaces>::default(),
diff --git a/src/rules/remove_interpolated_string.rs b/src/rules/remove_interpolated_string.rs
new file mode 100644
index 0000000..000fe3c
--- /dev/null
+++ b/src/rules/remove_interpolated_string.rs
@@ -0,0 +1,266 @@
+use std::{iter, ops};
+
+use crate::nodes::{
+    Block, Expression, FieldExpression, FunctionCall, Identifier, InterpolatedStringExpression,
+    InterpolationSegment, LocalAssignStatement, Prefix, StringExpression, TupleArguments,
+    TypedIdentifier,
+};
+use crate::process::{IdentifierTracker, NodeProcessor, NodeVisitor, ScopeVisitor};
+use crate::rules::{
+    Context, FlawlessRule, RuleConfiguration, RuleConfigurationError, RuleProperties,
+};
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum ReplacementStrategy {
+    StringSpecifier,
+    ToStringSpecifier,
+}
+
+impl Default for ReplacementStrategy {
+    fn default() -> Self {
+        Self::StringSpecifier
+    }
+}
+
+struct RemoveInterpolatedStringProcessor {
+    string_format_identifier: String,
+    tostring_identifier: String,
+    define_string_format: bool,
+    define_tostring: bool,
+    identifier_tracker: IdentifierTracker,
+    strategy: ReplacementStrategy,
+}
+
+impl ops::Deref for RemoveInterpolatedStringProcessor {
+    type Target = IdentifierTracker;
+
+    fn deref(&self) -> &Self::Target {
+        &self.identifier_tracker
+    }
+}
+
+impl ops::DerefMut for RemoveInterpolatedStringProcessor {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.identifier_tracker
+    }
+}
+
+const DEFAULT_TOSTRING_IDENTIFIER: &str = "tostring";
+const DEFAULT_STRING_LIBRARY: &str = "string";
+const DEFAULT_STRING_FORMAT_NAME: &str = "format";
+
+impl RemoveInterpolatedStringProcessor {
+    fn new(
+        strategy: ReplacementStrategy,
+        string_format_identifier: impl Into<String>,
+        tostring_identifier: impl Into<String>,
+    ) -> Self {
+        Self {
+            string_format_identifier: string_format_identifier.into(),
+            tostring_identifier: tostring_identifier.into(),
+            define_string_format: false,
+            define_tostring: false,
+            identifier_tracker: Default::default(),
+            strategy,
+        }
+    }
+
+    fn replace_with(&mut self, string: &InterpolatedStringExpression) -> Expression {
+        if string.is_empty() {
+            StringExpression::from_value("").into()
+        } else if string.len() == 1 {
+            match string.iter_segments().next().unwrap() {
+                InterpolationSegment::String(string_segment) => {
+                    StringExpression::from_value(string_segment.get_value()).into()
+                }
+                InterpolationSegment::Value(value_segment) => FunctionCall::from_name(
+                    if self.is_identifier_used(DEFAULT_TOSTRING_IDENTIFIER) {
+                        self.define_tostring = true;
+                        &self.tostring_identifier
+                    } else {
+                        DEFAULT_TOSTRING_IDENTIFIER
+                    },
+                )
+                .with_argument(value_segment.get_expression().clone())
+                .into(),
+            }
+        } else {
+            let arguments = iter::once(
+                StringExpression::from_value(string.iter_segments().fold(
+                    String::new(),
+                    |mut format_string, segment| {
+                        match segment {
+                            InterpolationSegment::String(string_segment) => {
+                                format_string
+                                    .push_str(&string_segment.get_value().replace('%', "%%"));
+                            }
+                            InterpolationSegment::Value(_) => {
+                                format_string.push_str(match self.strategy {
+                                    ReplacementStrategy::StringSpecifier => "%s",
+                                    ReplacementStrategy::ToStringSpecifier => "%*",
+                                });
+                            }
+                        }
+                        format_string
+                    },
+                ))
+                .into(),
+            )
+            .chain(
+                string
+                    .iter_segments()
+                    .filter_map(|segment| match segment {
+                        InterpolationSegment::Value(segment) => {
+                            Some(segment.get_expression().clone())
+                        }
+                        InterpolationSegment::String(_) => None,
+                    })
+                    .map(|value| match self.strategy {
+                        ReplacementStrategy::ToStringSpecifier => value,
+                        ReplacementStrategy::StringSpecifier => FunctionCall::from_name(
+                            if self.is_identifier_used(DEFAULT_TOSTRING_IDENTIFIER) {
+                                self.define_tostring = true;
+                                &self.tostring_identifier
+                            } else {
+                                DEFAULT_TOSTRING_IDENTIFIER
+                            },
+                        )
+                        .with_argument(value)
+                        .into(),
+                    }),
+            )
+            .collect::<TupleArguments>();
+
+            FunctionCall::from_prefix(if self.is_identifier_used(DEFAULT_STRING_LIBRARY) {
+                self.define_string_format = true;
+                Prefix::from_name(&self.string_format_identifier)
+            } else {
+                FieldExpression::new(
+                    Prefix::from_name(DEFAULT_STRING_LIBRARY),
+                    DEFAULT_STRING_FORMAT_NAME,
+                )
+                .into()
+            })
+            .with_arguments(arguments)
+            .into()
+        }
+    }
+}
+
+impl NodeProcessor for RemoveInterpolatedStringProcessor {
+    fn process_expression(&mut self, expression: &mut Expression) {
+        if let Expression::InterpolatedString(string) = expression {
+            *expression = self.replace_with(string);
+        }
+    }
+}
+
+pub const REMOVE_INTERPOLATED_STRING_RULE_NAME: &str = "remove_interpolated_string";
+
+/// A rule that removes interpolated strings.
+#[derive(Debug, Default, PartialEq, Eq)]
+pub struct RemoveInterpolatedString {
+    strategy: ReplacementStrategy,
+}
+
+impl FlawlessRule for RemoveInterpolatedString {
+    fn flawless_process(&self, block: &mut Block, _: &Context) {
+        const STRING_FORMAT_IDENTIFIER: &str = "__DARKLUA_STR_FMT";
+        const TOSTRING_IDENTIFIER: &str = "__DARKLUA_TO_STR";
+
+        let mut processor = RemoveInterpolatedStringProcessor::new(
+            self.strategy,
+            STRING_FORMAT_IDENTIFIER,
+            TOSTRING_IDENTIFIER,
+        );
+        ScopeVisitor::visit_block(block, &mut processor);
+
+        if processor.define_string_format || processor.define_tostring {
+            let mut variables = Vec::new();
+            let mut values = Vec::new();
+
+            if processor.define_string_format {
+                variables.push(TypedIdentifier::new(STRING_FORMAT_IDENTIFIER));
+                values.push(
+                    FieldExpression::new(
+                        Prefix::from_name(DEFAULT_STRING_LIBRARY),
+                        DEFAULT_STRING_FORMAT_NAME,
+                    )
+                    .into(),
+                );
+            }
+
+            if processor.define_tostring {
+                variables.push(TypedIdentifier::new(TOSTRING_IDENTIFIER));
+                values.push(Identifier::new(DEFAULT_TOSTRING_IDENTIFIER).into());
+            }
+
+            block.insert_statement(0, LocalAssignStatement::new(variables, values));
+        }
+    }
+}
+
+impl RuleConfiguration for RemoveInterpolatedString {
+    fn configure(&mut self, properties: RuleProperties) -> Result<(), RuleConfigurationError> {
+        for (key, value) in properties {
+            match key.as_str() {
+                "strategy" => {
+                    self.strategy = match value.expect_string(&key)?.as_str() {
+                        "string" => ReplacementStrategy::StringSpecifier,
+                        "tostring" => ReplacementStrategy::ToStringSpecifier,
+                        unexpected => {
+                            return Err(RuleConfigurationError::UnexpectedValue {
+                                property: "strategy".to_owned(),
+                                message: format!(
+                                    "invalid value `{}` (must be `string` or `tostring`)",
+                                    unexpected
+                                ),
+                            })
+                        }
+                    };
+                }
+                _ => return Err(RuleConfigurationError::UnexpectedProperty(key)),
+            }
+        }
+
+        Ok(())
+    }
+
+    fn get_name(&self) -> &'static str {
+        REMOVE_INTERPOLATED_STRING_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() -> RemoveInterpolatedString {
+        RemoveInterpolatedString::default()
+    }
+
+    #[test]
+    fn serialize_default_rule() {
+        let rule: Box<dyn Rule> = Box::new(new_rule());
+
+        assert_json_snapshot!("default_remove_interpolated_string", rule);
+    }
+
+    #[test]
+    fn configure_with_extra_field_error() {
+        let result = json5::from_str::<Box<dyn Rule>>(
+            r#"{
+            rule: 'remove_interpolated_string',
+            prop: "something",
+        }"#,
+        );
+        pretty_assertions::assert_eq!(result.unwrap_err().to_string(), "unexpected field 'prop'");
+    }
+}


Test patch:
diff --git a/src/rules/snapshots/darklua_core__rules__remove_interpolated_string__test__default_remove_interpolated_string.snap b/src/rules/snapshots/darklua_core__rules__remove_interpolated_string__test__default_remove_interpolated_string.snap
new file mode 100644
index 0000000..36bb2d7
--- /dev/null
+++ b/src/rules/snapshots/darklua_core__rules__remove_interpolated_string__test__default_remove_interpolated_string.snap
@@ -0,0 +1,5 @@
+---
+source: src/rules/remove_interpolated_string.rs
+expression: rule
+---
+"remove_interpolated_string"
diff --git a/src/rules/snapshots/darklua_core__rules__test__all_rule_names.snap b/src/rules/snapshots/darklua_core__rules__test__all_rule_names.snap
index c8cfc30..341ad99 100644
--- a/src/rules/snapshots/darklua_core__rules__test__all_rule_names.snap
+++ b/src/rules/snapshots/darklua_core__rules__test__all_rule_names.snap
@@ -15,6 +15,7 @@ expression: rule_names
   "remove_compound_assignment",
   "remove_empty_do",
   "remove_function_call_parens",
+  "remove_interpolated_string",
   "remove_method_definition",
   "remove_nil_declaration",
   "remove_spaces",
diff --git a/tests/rule_tests/mod.rs b/tests/rule_tests/mod.rs
index 9113ccc..18ddc67 100644
--- a/tests/rule_tests/mod.rs
+++ b/tests/rule_tests/mod.rs
@@ -295,6 +295,7 @@ mod remove_call_parens;
 mod remove_comments;
 mod remove_compound_assignment;
 mod remove_empty_do;
+mod remove_interpolated_string;
 mod remove_method_definition;
 mod remove_nil_declaration;
 mod remove_types;
diff --git a/tests/rule_tests/remove_interpolated_string.rs b/tests/rule_tests/remove_interpolated_string.rs
new file mode 100644
index 0000000..417e167
--- /dev/null
+++ b/tests/rule_tests/remove_interpolated_string.rs
@@ -0,0 +1,70 @@
+use darklua_core::rules::{RemoveInterpolatedString, Rule};
+
+test_rule!(
+    remove_interpolated_string,
+    RemoveInterpolatedString::default(),
+    empty_string("return ``") => "return ''",
+    regular_string("return `abc`") => "return 'abc'",
+    string_with_single_quote("return `'`") => "return \"'\"",
+    string_with_double_quote("return `\"`") => "return '\"'",
+    string_with_variable("return `{object}`") => "return tostring(object)",
+    nested_interpolated_string("return `{'+' .. `{object}`}`") => "return tostring('+' .. tostring(object))",
+    string_prefix_with_variable("return `-{object}`") => "return string.format('-%s', tostring(object))",
+    string_prefix_need_escaping_with_variable("return `%{object}`") => "return string.format('%%%s', tostring(object))",
+    string_suffix_with_variable("return `{object}-`") => "return string.format('%s-', tostring(object))",
+    string_with_variable_shadowing_tostring("local tostring return `{object}`")
+        => "local __DARKLUA_TO_STR = tostring local tostring return __DARKLUA_TO_STR(object)",
+    string_prefix_need_escaping_with_variable_shadowing_tostring("local tostring return `%{object}`")
+        => "local __DARKLUA_TO_STR = tostring local tostring return string.format('%%%s', __DARKLUA_TO_STR(object))",
+    string_prefix_need_escaping_with_variable_shadowing_string("local string return `%{object}`")
+        => "local __DARKLUA_STR_FMT = string.format local string return __DARKLUA_STR_FMT('%%%s', tostring(object))",
+    string_prefix_need_escaping_with_variable_shadowing_string_and_tostring("local string, tostring return `%{object}`")
+        => "local __DARKLUA_STR_FMT, __DARKLUA_TO_STR = string.format, tostring local string, tostring return __DARKLUA_STR_FMT('%%%s', __DARKLUA_TO_STR(object))",
+    two_strings_with_variable_shadowing_tostring("local tostring local a, b = `{object}`, `{var}`")
+    => "local __DARKLUA_TO_STR = tostring local tostring local a, b = __DARKLUA_TO_STR(object), __DARKLUA_TO_STR(var)",
+);
+
+test_rule!(
+    remove_interpolated_string_using_tostring_specifier,
+    json5::from_str::<Box<dyn Rule>>(
+        r#"{
+        rule: 'remove_interpolated_string',
+        strategy: 'tostring',
+    }"#,
+    )
+    .unwrap(),
+    empty_string("return ``") => "return ''",
+    regular_string("return `abc`") => "return 'abc'",
+    string_with_single_quote("return `'`") => "return \"'\"",
+    string_with_double_quote("return `\"`") => "return '\"'",
+    string_with_variable("return `{object}`") => "return tostring(object)",
+    nested_interpolated_string("return `{'+' .. `{object}`}`") => "return tostring('+' .. tostring(object))",
+    string_prefix_with_variable("return `-{object}`") => "return string.format('-%*', object)",
+    string_prefix_need_escaping_with_variable("return `%{object}`") => "return string.format('%%%*', object)",
+    string_suffix_with_variable("return `{object}-`") => "return string.format('%*-', object)",
+    string_with_variable_shadowing_tostring("local tostring return `{object}`")
+        => "local __DARKLUA_TO_STR = tostring local tostring return __DARKLUA_TO_STR(object)",
+    string_prefix_need_escaping_with_variable_shadowing_tostring("local tostring return `%{object}`")
+        => "local tostring return string.format('%%%*', object)",
+    string_prefix_need_escaping_with_variable_shadowing_string("local string return `%{object}`")
+        => "local __DARKLUA_STR_FMT = string.format local string return __DARKLUA_STR_FMT('%%%*', object)",
+    string_prefix_need_escaping_with_variable_shadowing_string_and_tostring("local string, tostring return `%{object}`")
+        => "local __DARKLUA_STR_FMT = string.format local string, tostring return __DARKLUA_STR_FMT('%%%*', object)",
+    two_strings_with_variable_shadowing_tostring("local tostring local a, b = `{object}`, `{var}`")
+        => "local __DARKLUA_TO_STR = tostring local tostring local a, b = __DARKLUA_TO_STR(object), __DARKLUA_TO_STR(var)",
+);
+
+#[test]
+fn deserialize_from_object_notation() {
+    json5::from_str::<Box<dyn Rule>>(
+        r#"{
+        rule: 'remove_interpolated_string',
+    }"#,
+    )
+    .unwrap();
+}
+
+#[test]
+fn deserialize_from_string() {
+    json5::from_str::<Box<dyn Rule>>("'remove_interpolated_string'").unwrap();
+}