fix(eslint-plugin): [sort-ngmodule-metadata-arrays]: properly handle multiple fixes at once #695
Conversation
Nx Cloud ReportCI ran the following commands for commit 6ddb806. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch Sent with 💌 from NxCloud. |
| }, | ||
| fixable: 'code', | ||
| schema: [], | ||
| messages: { | ||
| sortNgmoduleMetadataArrays: | ||
| '`NgModule` metadata arrays should be sorted in ASC alphabetical order', | ||
| suggestFix: 'Sort members (WARNING! comments will probably be messed up)', |
There was a problem hiding this comment.
Please, suggest a better messageId/description if that's the case.
| ...(hasComments | ||
| ? { suggest: [{ messageId: 'suggestFix', fix }] } | ||
| : { fix }), |
There was a problem hiding this comment.
I even tried to use fix, even if there were comments, but besides being very costly to maintain such an implementation, it's also very prone to failure. 😞
Example of hard cases can be found here, for example.
There was a problem hiding this comment.
One thing that came to mind now is, since this is something we have control over and wouldn't cause any compilation errors if applied, we could add an option called something like alwaysFixEvenWithComments/forceFix (I'm really bad at naming) to the rule to ignore comments and always fix the reports. It would be false by default, of course.
Let me know what you think, so I can send another PR for this if that's the case.
| AppModule1, | ||
| AppModule2, |
There was a problem hiding this comment.
This change is due to the fact that the fixer now, fixes everything at once, not one-by-one as previously.
| bProvider, | ||
| cProvider, | ||
| DProvider, | ||
| ~~~~~~~~~ |
There was a problem hiding this comment.
This change is due to the fact that the fixer now, puts everything that isn't a Identifier at the end, to avoid conflicts and simplify the implementation.
Codecov Report
@@ Coverage Diff @@
## master #695 +/- ##
==========================================
+ Coverage 85.37% 85.47% +0.09%
==========================================
Files 83 83
Lines 1922 1935 +13
Branches 339 341 +2
==========================================
+ Hits 1641 1654 +13
Misses 170 170
Partials 111 111
|
…multiple fixes at once
115d84a to
6ddb806
Compare
|
I emailed Rafael about 2 months ago asking if wished to continue working on this but sadly have not received a response in that time. I will therefore close this so that it is clear that it is not being worked on and that somebody else can pick this up if it is important to them. Many thanks to Rafael for his fantastic contributions to angular-eslint to date 🙏 |
Fixes #675 (there's more context there on this change).