Repository: dotenv-linter/dotenv-linter
Pull request #324 (2020-10-17 18:04:54)

Add default exclude list
First stab at #323
This implementation hard-excludes files and there is no way to run checks on them.
Even running the linter with these files listed explicitly will still skip them.

<!--
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:_ -->

#### ✔ 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);
- [x] 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._ -->


codecov-io: # [Codecov](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/324?src=pr&el=h1) Report
> Merging [#324](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/324?src=pr&el=desc) into [master](https://codecov.io/gh/dotenv-linter/dotenv-linter/commit/f6adeec541e774b81ed4af21c8e2fcbdd96db13e?el=desc) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/324/graphs/tree.svg?width=650&height=150&src=pr&token=wQybePfHI1)](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/324?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #324   +/-   ##
=======================================
  Coverage   96.98%   96.99%           
=======================================
  Files          32       32           
  Lines        2292     2294    +2     
=======================================
+ Hits         2223     2225    +2     
  Misses         69       69           
```


| [Impacted Files](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/324?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/common/file\_entry.rs](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/324/diff?src=pr&el=tree#diff-c3JjL2NvbW1vbi9maWxlX2VudHJ5LnJz) | `98.64% <100.00%> (+0.03%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/324?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/324?src=pr&el=footer). Last update [f6adeec...7f6ac4d](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/324?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ametalon: Sure, just wanted to make sure that you ok with the approach I took.
Readme doesn't mention exclusion option at all and redirects to the docs for more info.
Should I copy paragraph from the docs or leave readme as is?
I'm failing to see how I can add stuff to the docs, could you give me directions?
mstruebing: > Readme doesn't mention exclusion option at all and redirects to the docs for more info.

Sorry, my bad, just leave it as it is :)
mgrachev: > Sure, just wanted to make sure that you ok with the approach I took.
> Readme doesn't mention exclusion option at all and redirects to the docs for more info.
> Should I copy paragraph from the docs or leave readme as is?
> I'm failing to see how I can add stuff to the docs, could you give me directions?

All documentation lives here: https://github.com/dotenv-linter/dotenv-linter.github.io

Please add information about these changes to this page https://dotenv-linter.github.io/#/usage (at the bottom of the page)

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 82aa608..2244aab 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,8 +7,10 @@ # Changelog
 ## [Unreleased]
 ### 🚀 Added
 - Add action-yamllint [#317](https://github.com/dotenv-linter/dotenv-linter/pull/317) ([@vk26](https://github.com/vk26))
+- Add default exclude list [#324](https://github.com/dotenv-linter/dotenv-linter/pull/324) ([@ametalon](https://github.com/ametalon))
 
 ### 🔧 Changed
+- Fix linter rechecking files if they were listed more than once [#327](https://github.com/dotenv-linter/dotenv-linter/pull/327) ([@Aashu23](https://github.com/Aashu23))
 - Added docker build step to the CI pipeline [#322](https://github.com/dotenv-linter/dotenv-linter/pull/322) ([@JoeAmedeo](https://github.com/JoeAmedeo))
 - Change soon deprecated `set-env` action [#320](https://github.com/dotenv-linter/dotenv-linter/pull/320) ([@marcodenisi](https://github.com/marcodenisi))
 - Fix docker release [#319](https://github.com/dotenv-linter/dotenv-linter/pull/319) ([@mgrachev](https://github.com/mgrachev))
diff --git a/src/common/file_entry.rs b/src/common/file_entry.rs
index 3fd619a..af2645c 100644
--- a/src/common/file_entry.rs
+++ b/src/common/file_entry.rs
@@ -4,6 +4,8 @@
 
 use crate::common::*;
 
+const EXCLUDED_FILES: &[&str] = &[".envrc"];
+
 #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
 pub struct FileEntry {
     pub path: PathBuf,
@@ -51,6 +53,7 @@ pub fn from(path: PathBuf) -> Option<(Self, Vec<String>)> {
     pub fn is_env_file(path: &PathBuf) -> bool {
         let pattern = ".env";
         Self::get_file_name(path)
+            .filter(|file_name| !EXCLUDED_FILES.contains(&file_name.as_str()))
             .filter(|file_name| file_name.starts_with(pattern) || file_name.ends_with(pattern))
             .is_some()
     }
@@ -101,7 +104,7 @@ fn path_with_file_test() {
 
     #[test]
     fn is_env_file_test() {
-        let assertions = vec![
+        let mut assertions = vec![
             (".env", true),
             ("foo.env", true),
             (".env.foo", true),
@@ -114,6 +117,8 @@ fn is_env_file_test() {
             ("dev.env.js", false),
         ];
 
+        assertions.extend(EXCLUDED_FILES.iter().map(|file| (*file, false)));
+
         for (file_name, expected) in assertions {
             assert_eq!(
                 expected,
diff --git a/src/lib.rs b/src/lib.rs
index 1323c2d..f60779f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -98,8 +98,8 @@ fn get_file_paths(
         .filter(|entry| !excludes.contains(entry))
         .collect();
 
-    file_paths.sort();
     file_paths.extend(nested_paths);
+    file_paths.sort();
     file_paths.dedup();
     file_paths
 }


Test patch:
diff --git a/tests/args/specific_path.rs b/tests/args/specific_path.rs
index 595208f..fa61539 100644
--- a/tests/args/specific_path.rs
+++ b/tests/args/specific_path.rs
@@ -82,6 +82,50 @@ fn checks_two_specific_files() {
     testdir.test_command_fail_with_args(args, expected_output);
 }
 
+#[test]
+fn checks_each_file_only_once_when_listing_same_path_twice() {
+    let testdir = TestDir::new();
+
+    let subdir = testdir.subdir();
+    let testfile_1 = subdir.create_testfile(".env.a", " FOO=\n");
+    let testfile_2 = subdir.create_testfile(".env.b", "FOO=BAR\nBAR=foo\n");
+
+    let args = &[subdir.as_str(), subdir.as_str()];
+    let expected_output = format!(
+        "{}:1 LeadingCharacter: Invalid leading character detected\n{}:2 UnorderedKey: The BAR key should go before the FOO key\n\nFound 2 problems\n",
+        Path::new(&testdir.relative_path(&subdir))
+            .join(testfile_1.shortname_as_str())
+            .to_str().expect("multi-platform path to test .env file"),
+        Path::new(&testdir.relative_path(&subdir))
+            .join(testfile_2.shortname_as_str())
+            .to_str().expect("multi-platform path to test .env file")
+    );
+
+    testdir.test_command_fail_with_args(args, expected_output);
+}
+
+#[test]
+fn checks_each_file_only_once_when_listing_one_path_and_one_file() {
+    let testdir = TestDir::new();
+
+    let subdir = testdir.subdir();
+    let testfile_1 = subdir.create_testfile(".env.a", " FOO=\n");
+    let testfile_2 = subdir.create_testfile(".env.b", "FOO=val\nBAR=foo\n");
+
+    let args = &[subdir.as_str(), testfile_2.as_str()];
+    let expected_output = format!(
+        "{}:1 LeadingCharacter: Invalid leading character detected\n{}:2 UnorderedKey: The BAR key should go before the FOO key\n\nFound 2 problems\n",
+        Path::new(&testdir.relative_path(&subdir))
+            .join(testfile_1.shortname_as_str())
+            .to_str().expect("multi-platform path to test .env file"),
+        Path::new(&testdir.relative_path(&subdir))
+            .join(testfile_2.shortname_as_str())
+            .to_str().expect("multi-platform path to test .env file")
+    );
+
+    testdir.test_command_fail_with_args(args, expected_output);
+}
+
 #[test]
 fn checks_one_specific_file_and_one_path() {
     let testdir = TestDir::new();
@@ -93,11 +137,11 @@ fn checks_one_specific_file_and_one_path() {
 
     let args = &[testfile_2.as_str(), subdir.as_str()];
     let expected_output = format!(
-        "{}:2 UnorderedKey: The BAR key should go before the FOO key\n{}:2 DuplicatedKey: The FOO key is duplicated\n\nFound 2 problems\n",
-        testfile_2.shortname_as_str(),
+        "{}:2 DuplicatedKey: The FOO key is duplicated\n{}:2 UnorderedKey: The BAR key should go before the FOO key\n\nFound 2 problems\n",
         Path::new(&testdir.relative_path(&subdir))
             .join(testfile_3.shortname_as_str())
             .to_str().expect("multi-platform path to test .env file"),
+        testfile_2.shortname_as_str(),
     );
 
     testdir.test_command_fail_with_args(args, expected_output);
diff --git a/tests/flags/recursive.rs b/tests/flags/recursive.rs
index 7357536..f30ea5a 100644
--- a/tests/flags/recursive.rs
+++ b/tests/flags/recursive.rs
@@ -33,14 +33,14 @@ fn checks_files_in_deep_subdirs() {
 
     let args = &["--recursive"];
     let expected_output = format!(
-        "{}:2 UnorderedKey: The BAR key should go before the FOO key\n{}:1 EndingBlankLine: No blank line at the end of the file\n\nFound 2 problems\n",
-        Path::new(&test_dir.relative_path(&test_subdir_2))
-            .join(testfile_2.shortname_as_str())
-            .to_str()
-            .expect("multi-platform path to test .env file"),
+        "{}:1 EndingBlankLine: No blank line at the end of the file\n{}:2 UnorderedKey: The BAR key should go before the FOO key\n\nFound 2 problems\n",
         Path::new(&test_dir.relative_path(&test_subdir_3))
             .join(testfile_3.shortname_as_str())
             .to_str()
+            .expect("multi-platform path to test .env file"),
+        Path::new(&test_dir.relative_path(&test_subdir_2))
+            .join(testfile_2.shortname_as_str())
+            .to_str()
             .expect("multi-platform path to test .env file")
     );