Repository: dotenv-linter/dotenv-linter
Pull request #270 (2020-08-20 20:43:01)

solve #243: add DuplicatedKeyFixer
<!--
Thank you for your pull request. Please provide a description above and review
the requirements below.

Bug fixes and new features should include tests.
-->

<!-- _Please make sure to review and check all of these items:_ -->

Solves #243

#### ✔ Checklist:
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [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);
- [ ] Docs have been added / updated (for bug fixes / features).

<!-- _NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open._ -->


mgrachev: > > I guess we should run not only fixer, but also checker.
> 
> There are two options:
> 
> 1. We can implement the `fix_warnings` function, not the `fix_line`, and comment out all duplicated keys, not just the ones found during the initial check. Yes, there will be a slight duplication of code with the checker, but it seems to me that such an implementation is simpler and more reliable.
> 2. We can run the checker again somehow to find new warnings.
> 
> I prefer the first option but would like to discuss different approaches.

I'd think about it after we do all the fixer tasks 🤔

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4199fe0..1cd7e92 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@ # Changelog
 
 ## [Unreleased]
 ### 🚀 Added
+- Add fixer: DuplicatedKeyFixer [#270](https://github.com/dotenv-linter/dotenv-linter/pull/270) ([@utter-step](https://github.com/utter-step))
 - Add fixer: LeadingCharacter [#259](https://github.com/dotenv-linter/dotenv-linter/pull/259) ([@baile320](https://github.com/baile320))
 - Add fixer: IncorrectDelimiter [#258]((https://github.com/dotenv-linter/dotenv-linter/pull/258) ([@gillespiecd](https://github.com/gillespiecd))
 - Add support of comments to disable checks [#239](https://github.com/dotenv-linter/dotenv-linter/pull/239) ([@mgrachev](https://github.com/mgrachev))
diff --git a/src/fixes.rs b/src/fixes.rs
index 30501bb..a34cbba 100644
--- a/src/fixes.rs
+++ b/src/fixes.rs
@@ -1,5 +1,6 @@
 use crate::common::*;
 
+mod duplicated_key;
 mod ending_blank_line;
 mod incorrect_delimiter;
 mod key_without_value;
@@ -49,6 +50,7 @@ fn fixlist() -> Vec<Box<dyn Fix>> {
         // Then we should run the fixers that handle the line entry collection at whole.
         // And at the end we should run the fixer for ExtraBlankLine check (because the previous
         // fixers can create additional extra blank lines).
+        Box::new(duplicated_key::DuplicatedKeyFixer::default()),
         Box::new(ending_blank_line::EndingBlankLineFixer::default()),
     ]
 }
diff --git a/src/fixes/duplicated_key.rs b/src/fixes/duplicated_key.rs
new file mode 100644
index 0000000..a412514
--- /dev/null
+++ b/src/fixes/duplicated_key.rs
@@ -0,0 +1,142 @@
+use super::Fix;
+use crate::common::*;
+
+pub(crate) struct DuplicatedKeyFixer<'a> {
+    name: &'a str,
+}
+
+impl Default for DuplicatedKeyFixer<'_> {
+    fn default() -> Self {
+        Self {
+            name: "DuplicatedKey",
+        }
+    }
+}
+
+impl Fix for DuplicatedKeyFixer<'_> {
+    fn name(&self) -> &str {
+        self.name
+    }
+
+    fn fix_line(&self, line: &mut LineEntry) -> Option<()> {
+        line.raw_string = format!("# {}", line.raw_string);
+
+        Some(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn fix_warnings() {
+        let fixer = DuplicatedKeyFixer::default();
+        let mut lines = vec![
+            LineEntry {
+                number: 1,
+                file: FileEntry {
+                    path: ".env".into(),
+                    file_name: ".env".to_string(),
+                    total_lines: 4,
+                },
+                raw_string: "FOO=BAR".to_string(),
+            },
+            LineEntry {
+                number: 2,
+                file: FileEntry {
+                    path: ".env".into(),
+                    file_name: ".env".to_string(),
+                    total_lines: 4,
+                },
+                raw_string: "Z=Y".to_string(),
+            },
+            LineEntry {
+                number: 3,
+                file: FileEntry {
+                    path: ".env".into(),
+                    file_name: ".env".to_string(),
+                    total_lines: 4,
+                },
+                raw_string: "FOO=BAZ".to_string(),
+            },
+            LineEntry {
+                number: 4,
+                file: FileEntry {
+                    path: ".env".into(),
+                    file_name: ".env".to_string(),
+                    total_lines: 4,
+                },
+                raw_string: "Z=X".to_string(),
+            },
+        ];
+        let mut warnings = vec![
+            Warning::new(
+                lines[2].clone(),
+                "DuplicatedKey",
+                "The FOO key is duplicated".to_owned(),
+            ),
+            Warning::new(
+                lines[3].clone(),
+                "DuplicatedKey",
+                "The Z key is duplicated".to_owned(),
+            ),
+        ];
+
+        assert_eq!(
+            Some(2),
+            fixer.fix_warnings(warnings.iter_mut().collect(), &mut lines)
+        );
+        // what needed to be changed is changed
+        assert_eq!(
+            lines[2],
+            LineEntry {
+                number: 3,
+                file: FileEntry {
+                    path: ".env".into(),
+                    file_name: ".env".to_string(),
+                    total_lines: 4,
+                },
+                raw_string: "# FOO=BAZ".to_string(),
+            }
+        );
+        assert_eq!(
+            lines[3],
+            LineEntry {
+                number: 4,
+                file: FileEntry {
+                    path: ".env".into(),
+                    file_name: ".env".to_string(),
+                    total_lines: 4,
+                },
+                raw_string: "# Z=X".to_string(),
+            }
+        );
+        // anything else left untouched
+        assert_eq!(
+            &lines[..2],
+            &[
+                LineEntry {
+                    number: 1,
+                    file: FileEntry {
+                        path: ".env".into(),
+                        file_name: ".env".to_string(),
+                        total_lines: 4,
+                    },
+                    raw_string: "FOO=BAR".to_string(),
+                },
+                LineEntry {
+                    number: 2,
+                    file: FileEntry {
+                        path: ".env".into(),
+                        file_name: ".env".to_string(),
+                        total_lines: 4,
+                    },
+                    raw_string: "Z=Y".to_string(),
+                }
+            ]
+        );
+
+        assert!(warnings.iter().all(|w| w.is_fixed));
+    }
+}


Test patch:
diff --git a/tests/fixes/duplicated_key.rs b/tests/fixes/duplicated_key.rs
new file mode 100644
index 0000000..91de861
--- /dev/null
+++ b/tests/fixes/duplicated_key.rs
@@ -0,0 +1,20 @@
+use crate::common::TestDir;
+
+#[test]
+fn duplicated_key() {
+    let testdir = TestDir::new();
+    let testfile = testdir.create_testfile(".env", "ABC=DEF\nABC=XYZ\nFOO=BAR\nFOO=BAZ\n");
+    let expected_output = String::from(
+        "Fixed warnings:\n\
+        .env:2 DuplicatedKey: The ABC key is duplicated\n\
+        .env:4 DuplicatedKey: The FOO key is duplicated\n",
+    );
+    testdir.test_command_fix_success(expected_output);
+
+    assert_eq!(
+        testfile.contents().as_str(),
+        "ABC=DEF\n# ABC=XYZ\nFOO=BAR\n# FOO=BAZ\n"
+    );
+
+    testdir.close();
+}
diff --git a/tests/fixes/mod.rs b/tests/fixes/mod.rs
index 0fc40e8..b415f32 100644
--- a/tests/fixes/mod.rs
+++ b/tests/fixes/mod.rs
@@ -2,6 +2,7 @@
 
 use crate::common::TestDir;
 
+mod duplicated_key;
 mod ending_blank_line;
 mod incorrect_delimiter;
 mod leading_character;