@evgeniy-r Thank you for your contribution! 🚀

This PR has a lot of changes. I guess I need more time to review this feature.

I added the field with a checker name to Warning (it is possible to determine the checker by the text of the message, but I don't like this approach).

I'd like to minimize the changes in this PR, so I suppose to transfer this item to another PR.

I'd like to minimize the changes in this PR, so I suppose to transfer this item to another PR.

I'm agree, I will do it in a couple of days.

@DDtKey, thank you for feedback!

Will this API accurately make it easy to scale the application (add new fixes for checks)?

Important question. Yes, I experimented a bit and I'm sure that it will be possible to implement all existing checks based on this approach.

Is there enough information in the warnings to correct various types of warnings?

Sometimes (in several checks) we will work with the entire line entry collection and will use warnings only for indication (Fixed).
These checks (preliminarily): DuplicatedKey, ExtraBlankLine, UnorderedKey.

Perhaps the fixers should still be able to work without a preliminary check, what do you think?

I’ll think it over, but it seems to me that in terms of performance, checking first and then fixing is not a problem. On the other hand, we clearly link checks and fixes now.

The fact is that we are unlikely to be able to use the information, let's say about the sorting order, we’ll just have to fix the whole file.

For some checks, this is true, for others - not.

I apologize, I deleted the question before I received the answer, because I answered myself to some extent 😞

I will return it to leave a story:

Thanks for the contribution, autofix are a really important part of development 🚀

I would like to consider this PR a little more when I will have a bit more free time.

It seems important to me to discuss the moment, with a fix using the Warnings.
The ability to categorize as “fixed” and “not fixed” really looks great! 🔥

But some questions worry me:

1. Will this API accurately make it easy to scale the application (add new fixes for checks)?
2. Is there enough information in the warnings to correct various types of warnings?

Perhaps the fixers should still be able to work without a preliminary check, what do you think?

The fact is that we are unlikely to be able to use the information, let's say about the sorting order, we’ll just have to fix the whole file. And roughly speaking the Warning information gives not so much in such cases. It also possibly affects performance, although not critically.

At the same time, it is absolutely correct to display warnings that we cannot correct or are ambiguous (for example: IncorrectDelimiterChecker).

Maybe I'm completely wrong, but I want to raise this issue to look into the distance 🤔

@evgeniy-r Thanks for the detailed answer, I agree that the approach justifies itself.

I have splitted this PR into two (#234).
I can't push the branch to this repo and rebase this PR to another branch, so there are 2 commits for now (one for each PR).

@evgeniy-r The PR #234 has been merged. Please, update this PR.

I have updated PR.

@mgrachev, @DDtKey thank you very much for your feedback.
I will make changes.

I changed the output format and there is no need in Option<bool> anymore (since the output format of each message no longer depends on the fixing option). I will simplify the code.

Also there is some problem with tests on Windows platform - I will look in more detail.

Also there is some problem with tests on Windows platform - I will look in more detail.

I have fixed tests, everything is OK.

Codecov Report

Merging #228 into master will decrease coverage by 0.20%.
The diff coverage is 95.76%.

@@            Coverage Diff             @@
##           master     #228      +/-   ##
==========================================
- Coverage   97.35%   97.14%   -0.21%     
==========================================
  Files          16       18       +2     
  Lines        1510     1679     +169     
==========================================
+ Hits         1470     1631     +161     
- Misses         40       48       +8     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e37ef8f...93e05f9. Read the comment docs.

I seem to have answered all the comments.
I can rebase on the master branch, if it is needed.

I think, we should use something like that:

        let file_name = String::from(".env");
        let tempdir = tempdir().expect("create tempdir");
        let path = tempdir.path().join(file_name);

This solves the problem of unique names and concurrency.
How this works in integration tests.

I was able to reproduce this.
If the tests (with temp_dir + same file name) are in the same test module, we easily come across this on Windows,
and I have it reproduced on Linux including
Moreover, this does not happen with the cargo test - --test-threads 1 command, which confirms the above.

👍 It is an excellent finding!
I agree that we should use randomized file names.

I added tempdir usage to tests. Also rebased to the fresh master.

In that case we should print an error instead of panic. Maybe we should use the Result enum instead of Option

OK, I will take a look.

@evgeniy-r Thanks a lot! 🙏 You've done a great job! 🔥

You are welcome!