Repository: dotenv-linter/dotenv-linter
Pull request #348 (2020-12-04 14:20:55)

fix(#345): Find all problems
<!--
Thank you for your pull request. Please provide a description above and review
the requirements below.

Bug fixes and new features should include tests.
-->

This fixes the problems described in #345.

I had to change some tests to make them pass again. Since this is my first commit, I am not quite sure if every change I made is acceptable so this will need some review.

Also I'm not sure if this fix requires any updates on the documentation repo.

<!-- _Please make sure to review and check all of these items:_ -->

#### ✔ 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);
- [ ] 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._ -->


vbrandl: I added a testcase for `export FOOBAR` and had to change the parameter used to generate the error message. Also `get_key` will now always return the stripped `str`.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f2428e6..2b021ac 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ ### 🚀 Added
 - Add export prefix support [#340](https://github.com/dotenv-linter/dotenv-linter/pull/340)([@skonik](https://github.com/skonik))
 
 ### 🔧 Changed
+- Find all problems on the first run for `KeyWithoutValue` [#348](https://github.com/dotenv-linter/dotenv-linter/pull/348) ([@vbrandl](https://github.com/vbrandl))
 - Add [commitlint](https://github.com/conventional-changelog/commitlint) action [#347](https://github.com/dotenv-linter/dotenv-linter/pull/347) ([@mgrachev](https://github.com/mgrachev))
 - Add [cargo-deny](https://github.com/EmbarkStudios/cargo-deny) action [#346](https://github.com/dotenv-linter/dotenv-linter/pull/346) ([@mgrachev](https://github.com/mgrachev))
 - Remade flags to subcommands [#342](https://github.com/dotenv-linter/dotenv-linter/pull/342) ([@mgrachev](https://github.com/mgrachev))
diff --git a/src/checks.rs b/src/checks.rs
index 9fb2f94..a77c398 100644
--- a/src/checks.rs
+++ b/src/checks.rs
@@ -170,7 +170,7 @@ fn skip_one_check() {
         );
         let lines: Vec<LineEntry> = vec![line1, line2, blank_line_entry(3, 3)];
         let expected: Vec<Warning> = vec![warning];
-        let skip_checks: Vec<&str> = vec!["KeyWithoutValue"];
+        let skip_checks: Vec<&str> = vec!["KeyWithoutValue", "UnorderedKey"];
 
         assert_eq!(expected, run(&lines, &skip_checks));
     }
@@ -197,7 +197,7 @@ fn skip_one_check_via_comment() {
         );
         let lines: Vec<LineEntry> = vec![line1, line2, line3, blank_line_entry(4, 4)];
         let expected: Vec<Warning> = vec![warning];
-        let skip_checks: Vec<&str> = Vec::new();
+        let skip_checks: Vec<&str> = vec!["UnorderedKey"];
 
         assert_eq!(expected, run(&lines, &skip_checks));
     }
@@ -214,7 +214,7 @@ fn skip_collision() {
         );
         let lines: Vec<LineEntry> = vec![line1, line2, line3, blank_line_entry(4, 4)];
         let expected: Vec<Warning> = vec![warning];
-        let skip_checks: Vec<&str> = vec!["KeyWithoutValue"];
+        let skip_checks: Vec<&str> = vec!["KeyWithoutValue", "UnorderedKey"];
 
         assert_eq!(expected, run(&lines, &skip_checks));
     }
diff --git a/src/checks/duplicated_key.rs b/src/checks/duplicated_key.rs
index e576377..f38a638 100644
--- a/src/checks/duplicated_key.rs
+++ b/src/checks/duplicated_key.rs
@@ -28,11 +28,11 @@ impl Check for DuplicatedKeyChecker<'_> {
     fn run(&mut self, line: &LineEntry) -> Option<Warning> {
         let key = line.get_key()?;
 
-        if self.keys.contains(&key) {
+        if self.keys.contains(key) {
             return Some(Warning::new(line.clone(), self.name(), self.message(&key)));
         }
 
-        self.keys.insert(key);
+        self.keys.insert(key.to_string());
         None
     }
 
diff --git a/src/checks/incorrect_delimiter.rs b/src/checks/incorrect_delimiter.rs
index 9a35a6e..6986f25 100644
--- a/src/checks/incorrect_delimiter.rs
+++ b/src/checks/incorrect_delimiter.rs
@@ -135,8 +135,12 @@ fn failing_with_whitespace_run() {
     fn unformatted_run() {
         let mut checker = IncorrectDelimiterChecker::default();
         let line = line_entry(1, 1, "FOO-BAR");
-        // there's no key, so KeyWithoutValueChecker should catch this error
-        assert_eq!(None, checker.run(&line));
+        let expected = Some(Warning::new(
+            line.clone(),
+            "IncorrectDelimiter",
+            String::from("The FOO-BAR key has incorrect delimiter"),
+        ));
+        assert_eq!(expected, checker.run(&line));
     }
 
     #[test]
diff --git a/src/checks/key_without_value.rs b/src/checks/key_without_value.rs
index 8a4a72c..14c4297 100644
--- a/src/checks/key_without_value.rs
+++ b/src/checks/key_without_value.rs
@@ -27,7 +27,7 @@ fn run(&mut self, line: &LineEntry) -> Option<Warning> {
             Some(Warning::new(
                 line.clone(),
                 self.name(),
-                self.message(&line.raw_string),
+                self.message(line.get_key().unwrap_or(&line.raw_string)),
             ))
         } else {
             None
diff --git a/src/checks/unordered_key.rs b/src/checks/unordered_key.rs
index af20a38..6c3ac32 100644
--- a/src/checks/unordered_key.rs
+++ b/src/checks/unordered_key.rs
@@ -34,7 +34,7 @@ fn run(&mut self, line: &LineEntry) -> Option<Warning> {
         }
 
         let key = line.get_key()?;
-        self.keys.push(key.clone());
+        self.keys.push(key.to_string());
 
         let mut sorted_keys = self.keys.clone();
         sorted_keys.sort();
@@ -43,7 +43,7 @@ fn run(&mut self, line: &LineEntry) -> Option<Warning> {
             return None;
         }
 
-        let another_key = sorted_keys.iter().skip_while(|&s| s != &key).nth(1)?;
+        let another_key = sorted_keys.iter().skip_while(|&s| s != key).nth(1)?;
 
         Some(Warning::new(
             line.clone(),
diff --git a/src/common/line_entry.rs b/src/common/line_entry.rs
index dd6de57..d814c99 100644
--- a/src/common/line_entry.rs
+++ b/src/common/line_entry.rs
@@ -24,15 +24,13 @@ pub fn is_comment(&self) -> bool {
         self.trimmed_string().starts_with('#')
     }
 
-    pub fn get_key(&self) -> Option<String> {
+    pub fn get_key(&self) -> Option<&str> {
         if self.is_empty_or_comment() {
             return None;
         }
 
-        match self.stripped_export_string().find('=') {
-            Some(index) => Some(self.stripped_export_string()[..index].to_owned()),
-            None => None,
-        }
+        let stripped = self.stripped_export_string();
+        Some(stripped.split('=').next().unwrap_or(stripped))
     }
 
     pub fn get_value(&self) -> Option<String> {
@@ -124,7 +122,7 @@ fn empty_line_test() {
         #[test]
         fn stripped_export_prefix_test() {
             let input = line_entry(1, 1, "export FOO=BAR");
-            let expected = Some(String::from("FOO"));
+            let expected = Some("FOO");
 
             assert_eq!(expected, input.get_key());
         }
@@ -132,7 +130,7 @@ fn stripped_export_prefix_test() {
         #[test]
         fn correct_line_test() {
             let input = line_entry(1, 1, "FOO=BAR");
-            let expected = Some(String::from("FOO"));
+            let expected = Some("FOO");
 
             assert_eq!(expected, input.get_key());
         }
@@ -140,7 +138,7 @@ fn correct_line_test() {
         #[test]
         fn line_without_value_test() {
             let input = line_entry(1, 1, "FOO=");
-            let expected = Some(String::from("FOO"));
+            let expected = Some("FOO");
 
             assert_eq!(expected, input.get_key());
         }
@@ -148,7 +146,7 @@ fn line_without_value_test() {
         #[test]
         fn missing_value_and_equal_sign_test() {
             let input = line_entry(1, 1, "FOOBAR");
-            let expected = None;
+            let expected = Some("FOOBAR");
 
             assert_eq!(expected, input.get_key());
         }
diff --git a/src/fixes/duplicated_key.rs b/src/fixes/duplicated_key.rs
index c6bdb7d..f8d74a9 100644
--- a/src/fixes/duplicated_key.rs
+++ b/src/fixes/duplicated_key.rs
@@ -37,7 +37,7 @@ fn fix_warnings(
                 continue;
             }
 
-            if let Some(key) = line.get_key() {
+            if let Some(key) = line.get_key().map(str::to_string) {
                 if keys.contains(&key) {
                     self.fix_line(line)?;
                 } else {


Test patch:
diff --git a/tests/checks/key_without_value.rs b/tests/checks/key_without_value.rs
new file mode 100644
index 0000000..577e894
--- /dev/null
+++ b/tests/checks/key_without_value.rs
@@ -0,0 +1,21 @@
+use crate::common::*;
+
+#[test]
+fn also_detect_lowercase_key() {
+    let contents = vec!["test\n", "export test\n"];
+
+    for contents in contents {
+        let testdir = TestDir::new();
+        let testfile = testdir.create_testfile(".env", contents);
+        let args = &[testfile.as_str()];
+        let expected_output = check_output(&[(
+            ".env",
+            &[
+                ".env:1 KeyWithoutValue: The test key should be with a value or have an equal sign",
+                ".env:1 LowercaseKey: The test key should be in uppercase",
+            ],
+        )]);
+
+        testdir.test_command_fail_with_args(args, expected_output);
+    }
+}
diff --git a/tests/checks/mod.rs b/tests/checks/mod.rs
index e01e14a..96017d3 100644
--- a/tests/checks/mod.rs
+++ b/tests/checks/mod.rs
@@ -1,3 +1,4 @@
 mod ending_blank_line;
 mod extra_blank_line;
+mod key_without_value;
 mod quote_character;