Code structure of "utilities" is inconsistent

[platinumazure]

The modules that CLIEngine or Linter depend on are sometimes stored in inconsistent locations (at least, it seems that way to me).

At this point, it can be hard to see a consistent difference between files stored in lib/, vs files stored in lib/util/. That is, in both directories, we can see both "core" and "utility" modules. I would like to see if we could store only "core" modules in lib/, and only "utility" modules in lib/util/. (The rule of thumb I would use for what constitutes a "core" module is as follows: Anything important enough to document in our Node.js API documentation page, or anything directly called by CLIEngine or Linter that performs a key piece of linting, should probably be "core" and should probably be in lib/.)

I think it could help new contributors find their way around the codebase better if we try to be more consistent about placing core modules in lib/ and utility modules in lib/util/.

Some examples of modules that I think should be moved from lib/ to lib/util/:

• lib/ast-utils.js: This is used by many rules, but it is not a strategic part of our API. I think this should be in lib/util.
• lib/file-finder.js: This is used in the config file operations for helping find files on the filesystem. Config APIs themselves (e.g., in lib/config.js) should be "core" and be in the lib directory, but I think this file should be in lib/util.
• lib/logging.js: This is clearly not a core module, but rather something used for internal logging only. Should be in lib/util.
• lib/timing.js: This is clearly not a core module, but rather something to help with performance testing only.

(I do think there could be other modules on this list, especially around rule management, but this is a good start.)

Some examples of modules that I think should be moved from lib/util/ to lib/:

• lib/util/rule-fixer.js: This API is used in all core and custom rules that support autofixing, which is a key component of the linter.
• lib/util/source-code-fixer.js: This code is a key part of tracking and applying autofixes, which is a key component of the linter.

---

Would love to get feedback from the rest of the team on this.

[platinumazure]

I've created #10579 for what I think are the least controversial of the moves I am recommending (lib/logging.js and lib/timing.js into the lib/util directory). I'll wait on making any further PRs until some discussion occurs or a few days/weeks pass without objections.

[aladdin-add]

what about lib/ast-utils.js => lib/util/ast-utils.js?
if so, all utils will be in lib/util/ dir.

and personally I'd like to rename ast-utils.js => ast-util.js, so it could be consistent to other util, like npm-util.js, path-util.js.

[not-an-aardvark]

Given how much code uses ast-utils and variables like astUtils, it would probably be simpler to rename npm-util to npm-utils and path-util to path-utils.

[aladdin-add]

we might also need to move corresponding test files. 😄

[platinumazure]

@aladdin-add Can you name examples where we didn't do that correctly? (In my commits, lib/util/logging and lib/util/timing didn't have test files, and I did move tests/lib/file-finder correctly.)

[aladdin-add]

I moved lib/ast-utils to lib/util/ast-utils, and renamed *-util to *-utils in #10680. but I'm sorry to forgot changing the tests. 😂

[platinumazure]

Okay, I've taken another look after the PRs we've sent so far. Here is what I think we could still move (including some new ones):

Moving to utils:

• lib/ignored-paths.js --> lib/util/ignored-paths.js
• lib/load-rules.js --> lib/util/load-rules.js
• lib/report-translator.js --> lib/util/report-translator.js

Moving from utils:

• lib/util/rule-fixer.js --> lib/rule-fixer.js
• lib/util/source-code-fixer.js --> lib/source-code-fixer.js
• lib/util/source-code.js --> lib/source-code.js

Also wondering if it might be worth creating a lib/autofix folder for everything related to autofix.

[nzakas]

@platinumazure are you still working on this?

[platinumazure]

@nzakas Thank you for the reminder! I'll look at this again this week and determine if there are other changes to pursue. (I believe there will be other changes, but this hasn't been a priority.)

[eslint-deprecated]

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that accepted issues failing to be implemented after 90 days tend to
never be implemented, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.