Repository: dotenv-linter/dotenv-linter
Pull request #428 (2021-06-08 15:38:12)

feat: add substitution key fixer
#### ✔ Checklist:
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [X] Commit messages have been written in [Conventional Commits](https://www.conventionalcommits.org) format;
- [x] This PR has been added to [CHANGELOG.md](https://github.com/dotenv-linter/dotenv-linter/blob/master/CHANGELOG.md) (at the top of the list);
- [X] Tests for the changes have been added (for bug fixes / features);
- [x] Docs have been added / updated on the [dotenv-linter.github.io](https://github.com/dotenv-linter/dotenv-linter.github.io) (for bug fixes / features).

<!-- _NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open._ -->


github-actions[bot]: ## Benchmark for 9fb88e1
  <details>
    <summary>Click to view benchmark</summary>

| Test | PR Benchmark | Master Benchmark | % |
|------|--------------|------------------|---|
| dotenv_linter check | **54.9±2.25µs** | 60.4±5.19µs | **-9.11%** |
| dotenv_linter compare | 47.7±4.21µs | 49.4±3.84µs | -3.44% |
| dotenv_linter fix | 199.7±153.40µs | **182.1±13.17µs** | **+9.67%** |
| dotenv_linter fix with backup | 239.4±73.68µs | 235.8±51.11µs | +1.53% |


  </details>
  
github-actions[bot]: ## Benchmark for 3069e6b
  <details>
    <summary>Click to view benchmark</summary>

| Test | PR Benchmark | Master Benchmark | % |
|------|--------------|------------------|---|
| dotenv_linter check | **59.2±0.16µs** | 59.4±0.95µs | **-0.34%** |
| dotenv_linter compare | 47.8±0.24µs | 47.9±0.62µs | -0.21% |
| dotenv_linter fix | 129.7±1.93µs | 129.9±20.93µs | -0.15% |
| dotenv_linter fix with backup | **160.0±2.59µs** | 165.4±3.17µs | **-3.26%** |


  </details>
  

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8278676..3e3fa89 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@ # Changelog
 
 ## [Unreleased]
 ### 🚀 Added
+- Add fixer: Substitution Key [#428](https://github.com/dotenv-linter/dotenv-linter/pull/428) ([@DDtKey](https://github.com/DDtKey))
 - Refactor check unit tests [#421](https://github.com/dotenv-linter/dotenv-linter/pull/421) [@mc1098](https://github.com/mc1098)
 - Add missing integration tests [#420](https://github.com/dotenv-linter/dotenv-linter/pull/420) ([@gosolivs](https://github.com/gosolivs))
 - Add checker: Substitution Key [#414](https://github.com/dotenv-linter/dotenv-linter/pull/414) ([@de-sh](https://github.com/de-sh))
diff --git a/src/fixes.rs b/src/fixes.rs
index e80c608..4a114f9 100644
--- a/src/fixes.rs
+++ b/src/fixes.rs
@@ -9,6 +9,7 @@
 mod lowercase_key;
 mod quote_character;
 mod space_character;
+mod substitution_key;
 mod trailing_whitespace;
 mod unordered_key;
 
@@ -53,6 +54,7 @@ fn fixlist() -> Vec<Box<dyn Fix>> {
         Box::new(quote_character::QuoteCharacterFixer::default()),
         Box::new(incorrect_delimiter::IncorrectDelimiterFixer::default()),
         Box::new(extra_blank_line::ExtraBlankLineFixer::default()),
+        Box::new(substitution_key::SubstitutionKeyFixer::default()),
         // Then we should run the fixers that handle the line entry collection at whole
         Box::new(unordered_key::UnorderedKeyFixer::default()),
         Box::new(duplicated_key::DuplicatedKeyFixer::default()),
diff --git a/src/fixes/substitution_key.rs b/src/fixes/substitution_key.rs
new file mode 100644
index 0000000..c086c07
--- /dev/null
+++ b/src/fixes/substitution_key.rs
@@ -0,0 +1,125 @@
+use crate::common::LineEntry;
+use crate::fixes::Fix;
+
+pub(crate) struct SubstitutionKeyFixer<'a> {
+    name: &'a str,
+}
+
+impl Default for SubstitutionKeyFixer<'_> {
+    fn default() -> Self {
+        Self {
+            name: "SubstitutionKey",
+        }
+    }
+}
+
+impl Fix for SubstitutionKeyFixer<'_> {
+    fn name(&self) -> &str {
+        self.name
+    }
+
+    // TODO: refactor
+    fn fix_line(&mut self, line: &mut LineEntry) -> Option<()> {
+        let mut value = line
+            .get_value()
+            .map(str::trim)
+            .filter(|val| !val.starts_with('\''))?;
+
+        let is_escaped =
+            |prefix: &str| prefix.chars().rev().take_while(|ch| *ch == '\\').count() % 2 == 1;
+
+        let mut result = String::with_capacity(value.len());
+
+        // Checks if keys used in value have both '{' '}' or neither
+        while let Some((prefix, raw_key)) = value.split_once('$') {
+            result.push_str(prefix);
+            result.push('$');
+
+            // Separate initial key from the rest
+            let (initial_key, rest) = raw_key
+                .find('$')
+                .map(|i| raw_key.split_at(i))
+                .unwrap_or_else(|| (raw_key, ""));
+
+            value = &rest;
+
+            let stripped_key = initial_key.strip_prefix('{').unwrap_or(initial_key);
+
+            let correct_end_index = stripped_key
+                .find(|c: char| !c.is_ascii_alphanumeric() && c != '_')
+                .unwrap_or_else(|| stripped_key.len());
+
+            if is_escaped(prefix) || correct_end_index == 0 {
+                result.push_str(stripped_key);
+                continue;
+            }
+
+            result.push('{');
+            result.push_str(&stripped_key[..correct_end_index]);
+
+            let rest_part = &stripped_key[correct_end_index..];
+            if !rest_part.starts_with('}') {
+                result.push('}')
+            }
+
+            result.push_str(rest_part);
+        }
+
+        line.raw_string = format!("{}={}", line.get_key()?, result);
+        Some(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::common::tests::*;
+    use crate::common::Warning;
+
+    #[test]
+    fn fix_line_test() {
+        let mut fixer = SubstitutionKeyFixer::default();
+        let mut line = line_entry(1, 1, "FOO=${BAR");
+
+        assert_eq!(Some(()), fixer.fix_line(&mut line));
+        assert_eq!("FOO=${BAR}", line.raw_string);
+    }
+
+    #[test]
+    fn fix_warnings_test() {
+        let mut fixer = SubstitutionKeyFixer::default();
+        let mut lines = vec![
+            line_entry(1, 7, "FOO=${BAR-$ABC_ROOT}"),
+            line_entry(2, 7, "Z=$Y"),
+            line_entry(3, 7, "BAR=$Y}-$OPTS"),
+            line_entry(4, 7, "ABC=${BAR$XYZ}"),
+            line_entry(5, 7, "FOO=\"\\$BAR}"),
+            line_entry(6, 7, "GOD=${BAR!}"),
+            blank_line_entry(7, 7),
+        ];
+        let mut warnings = warnings(&lines, &[1, 3, 4, 6]);
+
+        assert_eq!(
+            Some(4),
+            fixer.fix_warnings(warnings.iter_mut().collect(), &mut lines)
+        );
+        assert_eq!("FOO=${BAR}-${ABC_ROOT}", lines[0].raw_string);
+        assert_eq!("BAR=${Y}-${OPTS}", lines[2].raw_string);
+        assert_eq!("ABC=${BAR}${XYZ}", lines[3].raw_string);
+        assert_eq!("GOD=${BAR}!}", lines[5].raw_string);
+    }
+
+    fn warnings(lines: &Vec<LineEntry>, warning_lines: &[usize]) -> Vec<Warning> {
+        lines
+            .iter()
+            .filter(|l| warning_lines.contains(&l.number))
+            .map(|l| {
+                Warning::new(
+                    l.clone(),
+                    "SubstitutionKey",
+                    String::from("The key is not assigned properly"),
+                )
+            })
+            .collect()
+    }
+}


Test patch:
diff --git a/tests/fixes/mod.rs b/tests/fixes/mod.rs
index d091c78..0cc133a 100644
--- a/tests/fixes/mod.rs
+++ b/tests/fixes/mod.rs
@@ -11,6 +11,7 @@
 mod lowercase_key;
 mod quote_character;
 mod space_character;
+mod substitution_key;
 mod trailing_whitespace;
 mod unordered_key;
 
diff --git a/tests/fixes/substitution_key.rs b/tests/fixes/substitution_key.rs
new file mode 100644
index 0000000..e8b18c2
--- /dev/null
+++ b/tests/fixes/substitution_key.rs
@@ -0,0 +1,27 @@
+use crate::common::*;
+
+#[test]
+fn substitution_key() {
+    let testdir = TestDir::new();
+    let testfile = testdir.create_testfile(
+        ".env",
+        "ABC=${BAR$XYZ}\nBYZ=${BAR!}\n\nFOO=${ABC-$BAR}\nGOD=${ENVS${BAR}\nXYZ=\\$BAR}\n",
+    );
+    let expected_output = fix_output(&[(
+        ".env",
+        &[
+            ".env:1 SubstitutionKey: The ABC key is not assigned properly",
+            ".env:2 SubstitutionKey: The BYZ key is not assigned properly",
+            ".env:4 SubstitutionKey: The FOO key is not assigned properly",
+            ".env:5 SubstitutionKey: The GOD key is not assigned properly",
+        ],
+    )]);
+    testdir.test_command_fix_success(expected_output);
+
+    assert_eq!(
+        testfile.contents().as_str(),
+        "ABC=${BAR}${XYZ}\nBYZ=${BAR}!}\n\nFOO=${ABC}-${BAR}\nGOD=${ENVS}${BAR}\nXYZ=\\$BAR}\n"
+    );
+
+    testdir.close();
+}