I want to do this one but is my first time trying to contribute to a project and I'm a bit lost.
For now I forked and cloned the repository and run the tests as it is. 1 failed like this

failures:

---- fixes::fixtures stdout ----
thread 'fixes::fixtures' panicked at 'assertion failed: `(left == right)`
  left: `"\n# dotenv-linter:off UnorderedKey\n_ENV=000\nA=B\nA_ENV_11=B\nA_ENV_12=C\nA_ENV_13=D\nA_ENV_14=123\nA_ENV_15=345\nAB_ENV_16=777\nAB_ENV_17=alpha\nAB_ENV_18=beta\nAB_ENV_19=alpha12\nAB_ENV_20=beta13\nB=C\nC=D\nD=123\n# dotenv-linter:off SpaceCharacter\nE=345\nENV11=C\n# A_ENV_12=C\nSOME_ENV_WITHOUT_EQ=\nANOTHER_ENV=\nENV12_=D\n# dotenv-linter:on UnorderedKey\n\n# comment\n\n# Another\n# multiline\n# comment\n# ANOTHER_ENV=\n# A_ENV_12=C\nENV10=B\n# ENV11=C\n# ENV12_=D\n# single line comment\nENV13=123\nENV14=345\n# Multiline\n# comment\nSOME_ENV=\nVAR_1 = 1234\nVAR_2=1234\nVAR_3=1234\nVAR_4=1234\n# dotenv-linter:on SpaceCharacter\n# AB_ENV_19=alpha14\n# 
AB_ENV_19=alpha12\nENV15=777\nENV16=alpha\nENV18=alpha12\nENV19=beta13\nENV21=2\nENV22=3\nENV_17=beta\nF=777\nG=alpha\nH=beta\nI=alpha12\nJ=beta13\nKEY=VALUE\n# dotenv-linter:off QuoteCharacter\nENV24=\'key56\'\nVAR_1=1234\n# VAR_2=1234\n# VAR_3=1234\n# VAR_4=1234\nVAR_5=1234\nVAR_6=1234\nVAR_7=1234\n"`,
 right: `"\r\n# dotenv-linter:off UnorderedKey\r\n_ENV=000\r\nA=B\r\nA_ENV_11=B\r\nA_ENV_12=C\r\nA_ENV_13=D\r\nA_ENV_14=123\r\nA_ENV_15=345\r\nAB_ENV_16=777\r\nAB_ENV_17=alpha\r\nAB_ENV_18=beta\r\nAB_ENV_19=alpha12\r\nAB_ENV_20=beta13\r\nB=C\r\nC=D\r\nD=123\r\n# dotenv-linter:off SpaceCharacter\r\nE=345\r\nENV11=C\r\n# A_ENV_12=C\r\nSOME_ENV_WITHOUT_EQ=\r\nANOTHER_ENV=\r\nENV12_=D\r\n# dotenv-linter:on UnorderedKey\r\n\r\n# comment\r\n\r\n# Another\r\n# multiline\r\n# comment\r\n# ANOTHER_ENV=\r\n# A_ENV_12=C\r\nENV10=B\r\n# ENV11=C\r\n# ENV12_=D\r\n# single line comment\r\nENV13=123\r\nENV14=345\r\n# Multiline\r\n# comment\r\nSOME_ENV=\r\nVAR_1 = 1234\r\nVAR_2=1234\r\nVAR_3=1234\r\nVAR_4=1234\r\n# dotenv-linter:on SpaceCharacter\r\n# AB_ENV_19=alpha14\r\n# AB_ENV_19=alpha12\r\nENV15=777\r\nENV16=alpha\r\nENV18=alpha12\r\nENV19=beta13\r\nENV21=2\r\nENV22=3\r\nENV_17=beta\r\nF=777\r\nG=alpha\r\nH=beta\r\nI=alpha12\r\nJ=beta13\r\nKEY=VALUE\r\n# dotenv-linter:off QuoteCharacter\r\nENV24=\'key56\'\r\nVAR_1=1234\r\n# VAR_2=1234\r\n# VAR_3=1234\r\n# VAR_4=1234\r\nVAR_5=1234\r\nVAR_6=1234\r\nVAR_7=1234\r\n"`', tests\fixes\mod.rs:115:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    fixes::fixtures

test result: FAILED. 75 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

I want to do this one but is my first time trying to contribute to a project and I'm a bit lost.
For now I forked and cloned the repository and run the tests as it is. 1 failed like this

As I can see, the whole difference is in line breaks (\n vs \r\n).
Could you please provide your OS (is Windows?) and rustс version?

Could you please provide your OS (is Windows?) and rustс version?

Sure

OS: Windows 10 Home Single Language Version 2004
rustup -V
rustup 1.23.1 (3df2264a9 2020-11-30)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.49.0 (e1884a8e3 2020-12-29)`
cargo --version
cargo 1.49.0 (d00d64df9 2020-12-05)

@PatatasDelPapa I can't reproduce this behavior (and for Windows - CI passed successfully ) 🙁
Just to make sure: You haven't changed any files and the git diff output is empty?
Did you just clone the repo and run cargo test?

The right part of the assertion is just the content of the file from the repo (without any changes), but with additional carriage return (\r) for you 🤔

Just to make sure: You haven't changed any files and the git diff output is empty?
Did you just clone the repo and run cargo test?

Yes and just to be sure I cloned the repo again then run cargo test

Exactly what I done was

1. Open GitHub Desktop
2. Cloned the repo (from https://github.com/dotenv-linter/dotenv-linter.git) to desktop/rust
3. Open visual studio code through Github Desktop
4. Open a terminal in vscode then run cargo test

Got pretty much the same as last time

failures:

---- fixes::fixtures stdout ----
thread 'fixes::fixtures' panicked at 'assertion failed: `(left == right)`
  left: `"\n# dotenv-linter:off UnorderedKey\n_ENV=000\nA=B\nA_ENV_11=B\nA_ENV_12=C\nA_ENV_13=D\nA_ENV_14=123\nA_ENV_15=345\nAB_ENV_16=777\nAB_ENV_17=alpha\nAB_ENV_18=beta\nAB_ENV_19=alpha12\nAB_ENV_20=beta13\nB=C\nC=D\nD=123\n# dotenv-linter:off SpaceCharacter\nE=345\nENV11=C\n# A_ENV_12=C\nSOME_ENV_WITHOUT_EQ=\nANOTHER_ENV=\nENV12_=D\n# dotenv-linter:on UnorderedKey\n\n# comment\n\n# Another\n# multiline\n# comment\n# ANOTHER_ENV=\n# A_ENV_12=C\nENV10=B\n# ENV11=C\n# ENV12_=D\n# single line comment\nENV13=123\nENV14=345\n# Multiline\n# comment\nSOME_ENV=\nVAR_1 = 1234\nVAR_2=1234\nVAR_3=1234\nVAR_4=1234\n# dotenv-linter:on SpaceCharacter\n# AB_ENV_19=alpha14\n# 
AB_ENV_19=alpha12\nENV15=777\nENV16=alpha\nENV18=alpha12\nENV19=beta13\nENV21=2\nENV22=3\nENV_17=beta\nF=777\nG=alpha\nH=beta\nI=alpha12\nJ=beta13\nKEY=VALUE\n# dotenv-linter:off QuoteCharacter\nENV24=\'key56\'\nVAR_1=1234\n# VAR_2=1234\n# VAR_3=1234\n# VAR_4=1234\nVAR_5=1234\nVAR_6=1234\nVAR_7=1234\n"`,
 right: `"\r\n# dotenv-linter:off UnorderedKey\r\n_ENV=000\r\nA=B\r\nA_ENV_11=B\r\nA_ENV_12=C\r\nA_ENV_13=D\r\nA_ENV_14=123\r\nA_ENV_15=345\r\nAB_ENV_16=777\r\nAB_ENV_17=alpha\r\nAB_ENV_18=beta\r\nAB_ENV_19=alpha12\r\nAB_ENV_20=beta13\r\nB=C\r\nC=D\r\nD=123\r\n# dotenv-linter:off SpaceCharacter\r\nE=345\r\nENV11=C\r\n# A_ENV_12=C\r\nSOME_ENV_WITHOUT_EQ=\r\nANOTHER_ENV=\r\nENV12_=D\r\n# dotenv-linter:on UnorderedKey\r\n\r\n# comment\r\n\r\n# Another\r\n# multiline\r\n# comment\r\n# ANOTHER_ENV=\r\n# A_ENV_12=C\r\nENV10=B\r\n# ENV11=C\r\n# ENV12_=D\r\n# single line comment\r\nENV13=123\r\nENV14=345\r\n# Multiline\r\n# comment\r\nSOME_ENV=\r\nVAR_1 = 1234\r\nVAR_2=1234\r\nVAR_3=1234\r\nVAR_4=1234\r\n# dotenv-linter:on SpaceCharacter\r\n# AB_ENV_19=alpha14\r\n# AB_ENV_19=alpha12\r\nENV15=777\r\nENV16=alpha\r\nENV18=alpha12\r\nENV19=beta13\r\nENV21=2\r\nENV22=3\r\nENV_17=beta\r\nF=777\r\nG=alpha\r\nH=beta\r\nI=alpha12\r\nJ=beta13\r\nKEY=VALUE\r\n# dotenv-linter:off QuoteCharacter\r\nENV24=\'key56\'\r\nVAR_1=1234\r\n# VAR_2=1234\r\n# VAR_3=1234\r\n# VAR_4=1234\r\nVAR_5=1234\r\nVAR_6=1234\r\nVAR_7=1234\r\n"`', tests\fixes\mod.rs:115:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    fixes::fixtures

test result: FAILED. 75 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '--test cli'

I was able to reproduce this error on Windows and I found one way that the problem could be resolved.
The problem happens because Git can automatically convert LF endings into CRLF when you check out code (this includes cloning a repo).
The automatic conversion happens if you have the Git core.autocrlf configuration variable set to true, which is the recommended setting on Windows.
You can see what the variable is currently set to by running git config core.autocrlf from a terminal. In my case it was set to true as part of the global config. See the Formatting and Whitespace section of this page for an explanation of this behavior: https://git-scm.com/book/en/v2/Customizing-Git-Git-Configuration

When I cloned the repo and ran cargo test with core.autocrlf to true I got the same error.

I then set core.autocrlf to false by running git config --global core.autocrlf false and then cloned the repo to a new directory.
When I ran cargo test in the new repo directory I did not get the error.

It appears that you can also configure the end-of-line normalization for a specific path using the .gitattributes file. See the Checking-out and checking-in section here: http://git-scm.com/docs/gitattributes.
This might be the best way to ensure this issue doesn't happen in the future.
I think adding *.env.golden text eol=lf to the .gitattributes file might be the way to do that, but I wasn't able to test this functionality out.

I created a PR that adds a .gitattributes file to the /tests/fixes/fixtures/ directory so this problem shouldn't happen for future contributors.

@stevetaggart good catch! Thank you 🙂

Hey, does this also involve changing the function in the Check trait to accept Rc<LineEntry> instead of &LineEntry?

@PatatasDelPapa my PR was merged, so you should be able to have tests pass on Windows now with a fresh clone of the repo. 😃

@stevetaggart Yes! all test pass with a fresh clone sadly I'm now occupied but will try to help again on another issue when I have time

Hi, isn't it better to store borrowed reference to LineEntry inside Warning, instead of an Rc? Based on a quick glance, it looks like LineEntrys live as long as Warnings do, so it shouldn't be a problem. It'll keep LineEntrys nicely stored in a vector as in current design, instead of a vector of Rcs, which is much better. Or am I missing something? I'm gonna give this a shot, let me know what you think 😄

Or am I missing something? I'm gonna give this a shot, let me know what you think 😄

Feel free to do a PR 🙂

Just note that LineEntry must be mutable in Fixers which introduces restrictions 🤔

Getting back a bit late, but I was trying out few ideas. @DDtKey you're right that Fixer requiring mutable lines vector adds restrictions. But there's no need to preserve Warnings (holding borrowed reference to LineEntrys) across the fixture runs. Instead of passing a slice of Warning to fixes::run, I changed the code to pass a map of warning name to line numbers. I got the non-test modules building fine, but for the test modules, looks like I'll have to touch almost every single test for the new signature of Warning::new.

I think resolving #419 might be a better idea before I open a PR for this issue, to avoid unnecessarily repeating test changes. Should we reopen #419? If I get some time this week, I'll try to resolve that first.

Also, I noticed that warning and check/fixer are compared using their check_name and name strings. It could be avoided by using Enum to represent a lint's kind. If you think it's a good enhancement, I can open a new issue for it.

Also, I noticed that warning and check/fixer are compared using their check_name and name strings. It could be avoided by using Enum to represent a lint's kind. If you think it's a good enhancement, I can open a new issue for it.

@atsmtat Could you give an example of how would you implement this feature? 🤔

Also, I noticed that warning and check/fixer are compared using their check_name and name strings. It could be avoided by using Enum to represent a lint's kind. If you think it's a good enhancement, I can open a new issue for it.

@atsmtat Could you give an example of how would you implement this feature? 🤔

It could be done by defining an enum -- something like:

enum Lint {
   DuplicatedKey,
   EndingBlankLine,
   ExtraBlankLine,
   // ...
}

And using it in place of &str for Warning::check_name, Check::name, and Fix::name. We can implement Display for this enum for printing purposes. This way, we avoid string comparison (faster code), and get safety against typos in using strings (compile time checks).

It could be done by defining an enum -- something like:

enum Lint {
   DuplicatedKey,
   EndingBlankLine,
   ExtraBlankLine,
   // ...
}

And using it in place of &str for Warning::check_name, Check::name, and Fix::name. We can implement Display for this enum for printing purposes. This way, we avoid string comparison (faster code), and get safety against typos in using strings (compile time checks).

I like this idea. Could you implement it in another PR?

It could be done by defining an enum -- something like:

enum Lint {
   DuplicatedKey,
   EndingBlankLine,
   ExtraBlankLine,
   // ...
}

And using it in place of &str for Warning::check_name, Check::name, and Fix::name. We can implement Display for this enum for printing purposes. This way, we avoid string comparison (faster code), and get safety against typos in using strings (compile time checks).

I like this idea. Could you implement it in another PR?

I opened a new issue #426 to track it. I'm currently busy to implement it, but it'll be good first issue for someone new I think. I'm still waiting for #422 to merge, which will create conflict with the enum conversion.