feat: add the fixer function to eslint/rules/vars-order

[headlessNode]

Resolves #3222.

Description

What is the purpose of this pull request?

This pull request:

• Add a fixer function to eslint/rules/vars-order to allow auto-formatting during code review.

Related Issues

Does this pull request have any related issues?

This pull request:

• resolves [RFC]: Add fixer function to the ESLint rule vars-order #3222

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

• Read, understood, and followed the contributing guidelines.

---

@stdlib-js/reviewers

[headlessNode]

@Planeshifter @kgryte I would like your reviews on this. I got the fixer function working independently .i.e. outside stdlib. But in this branch it doesn't work. I guess there is a step to rebuild that I'd need run?

The rule falls in the --fix-type layout category so that is not the issue I think.

[Planeshifter]

@headlessNode Yes, you are correct: one currently needs to run make init to generate n custom ESLint bundle with our custom rules in order for them to be applied.

Please update the test fixtures of this package though to verify that the new lint rule works. See e.g. https://github.com/stdlib-js/stdlib/blob/develop/lib/node_modules/%40stdlib/_tools/eslint/rules/require-spaces/test/fixtures/invalid.js for how to specify expected output.

[headlessNode]

@Planeshifter I've updated the test fixtures. The last case fails and that is because of the fixer function's logic.

The fixer function currently swaps the two variables where the error arises. For the case like so:

  var x;
  var y;
  var xy;

The function first swaps y and xy, linter checks again, errors is thrown, then the fixer swaps xy and x. All variables are ordered.

The reason behind the swapping logic is that this solution seem more "elegant" to me and it is also more efficient than replacing the whole block.

What are your thoughts on this?

[kgryte]

While recursive linting may work, I am curious what is the relative performance cost? Linting can already take a long time for a single file, especially for devs running VMs. If recursive linting is significantly more costly, I'd be a bit leery about favoring that approach over single-pass ordering of all variables.

[headlessNode]

@kgryte Worst case scenario with recursive is definitely more resource intensive. For using the single-pass do we have a sorting function in stdlib?

[kgryte]

@headlessNode If you are working with normal arrays, just use the built-in x.sort() method.

[kgryte]

You will need a custom callback, however, in order to order by length, rather than lexicographically.

[headlessNode]

@kgryte @Planeshifter The PR now looks promising. I had introduced a few bugs due to experimenting but now they are addressed. I also add test cases for testing the order of variables in nested functions and they are passing 🥳.

But the nested case fails when the outer functions' variables are also not ordered correctly. The error statement is something like expected One error but got two.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no suggestions.

[Planeshifter]

PR looks good, thanks for your work @headlessNode.

With respect to the test failure when both inner and outer function's variable declarations were in the wrong order, we just had to list two elements in the errors array because now there are two lint failure instances in the code; see the commit I pushed to the branch.

I will go ahead and merge your PR shortly.

[stdlib-bot]

PR Commit Message

feat: add the `fixer` function to `eslint/rules/vars-order`

PR-URL: https://github.com/stdlib-js/stdlib/pull/3224
Closes: https://github.com/stdlib-js/stdlib/issues/3222

Co-authored-by: Philipp Burckhardt <[email protected]>
Reviewed-by: Philipp Burckhardt <[email protected]>
Signed-off-by: Muhammad Haris <[email protected]>

Please review the above commit message and make any necessary adjustments.

[headlessNode]

@Planeshifter Yep, makes sense. Thanks!