Feedback Wanted: Project-aware ESLint

[nzakas]

As we are continuing to gather requirements for the next version of ESLint, one common question that keeps coming up is whether or not ESLint should be "project-aware." This is a bit of a difficult question to answer because there is no consistent definition of "project." TypeScript does have tsconfig.json that loosely defines a project, but what about regular JavaScript?

Feedback Wanted

In this discussion, I'd like to get feedback on when it would be helpful for ESLint to know about more than one file at a time:

• Does "project-aware" simply mean that ESLint will follow import statements to know whether or not imports exist? Is there something else?
• If you're a plugin author, what would this allow you to do?
• How would the ESLint CLI know that you are working on a project instead of a single file?
• What would it mean for ESLint to be project-aware for your JavaScript project?
• What would it mean for ESLint to be project-aware for your TypeScript project?

We aren't talking deep implementation details or whether or not something is possible right now. What I'm hoping to get out of this is the wish list that everyone has in the back of their head. We are doing some serious rethinking of our APIs right now, so every seemingly outlandish wish is a possibility!

We want to make your life easier. Help us do it!

cc @bradzacher @ljharb @JoshuaKGoldberg @jfmengels

[ljharb]

I'd define a package root as a directory that contains a package.json, that has more fields than just "type" or "main" defined. I'd prefer eslint to automatically set root: true at package boundaries, so that config had to explicitly opt in to looking upwards for further config.

That would work automatically for single-project repos, and for monorepos, would mean that subprojects would use config to define "yes, look upwards", and then the root would automatically apply config to all the subprojects.

Additionally, project awareness would mean that rules could be told, by eslint, what the root(s) are, so that it's clear when imports are reaching outside of the package or not.

[nzakas]

I'd define a package root as a directory that contains a package.json, that has more fields than just "type" or "main" defined. I'd prefer eslint to automatically set root: true at package boundaries, so that config had to explicitly opt in to looking upwards for further config.

With flat config, I think we would need users to define that behavior for themselves, although flat config is root: true automatically so maybe it's not all that important anymore?

Additionally, project awareness would mean that rules could be told, by eslint, what the root(s) are, so that it's clear when imports are reaching outside of the package or not.

Ah interesting. So that would be maybe context.projectRoot in addition to context.cwd?

[ljharb]

Yep, that sounds great

[bradzacher]

Being told the project root would be super valuable for @typescript-eslint as well.

Currently we have parserOptions.tsconfigRootDir which we ask users to set to __dirname so that we know where the root of their project is.

Without this we have no reference frame from which to resolve relative paths passed to parserOptions.project - so we just use process.cwd() which obviously will break in a lot of cases.

[bradzacher]

I'll split my respond up over a few comments to aid with threading and responses.

Does "project-aware" simply mean that ESLint will follow import statements to know whether or not imports exist? Is there something else?

For me a "project" is simply a distinct set of files in a given folder. It doesn't matter if those files are part of a shared dependency graph or anything like that - just that given a folder you can idempotently define 1..n groupings of those files.

Given that definition it's clear that a JS project is going to be a little different to a TS project, which is also going to be different to a project.

TBH even frameworks can have different concepts of a project - Vue vs Angular vs React are all structured differently and might want to be grouped differently. Heck - individual repos might have a different concept of a project.

[nzakas]

TBH even frameworks can have different concepts of a project - Vue vs Angular vs React are all structured differently and might want to be grouped differently. Heck - individual repos might have a different concept of a project.

Yup, this is the challenge. I'm not sure ESLint should attempt to be project-aware for this reason, but it's worth exploring to see if there's something obvious that we are missing.

[bradzacher]

What would it mean for ESLint to be project-aware for your TypeScript project?

Given all that I would like it if ESLint would allow "plugins" to define the shape of a project.

For typescript-eslint eslint (and likely most if not all TS repos - regardless of framework) we'd probably define one project per tsconfig plus one additional project for files not covered by a tsconfig.

So I'd probably expect that eslint has no real idea of what a TS project is or looks like - just that each project is identified by a tsconfig path and each identifier resolves to a set of files.

[nzakas]

This is an interesting idea, although it would be a dramatic departure from how ESLint is configured right now.

In my mind, what this would look like is something like this in an eslint.config.js file:

import ts from "typescript-eslint";
export default [
    {
        projectType: ts.projectTypes.base,
        rules: { /* ... */ }
    }
];

And then maybe the project type is passed the cwd or some other information? Does that seem along the lines of what you are thinking?

[jfmengels]

elm-review is a linter with looking at multiple files (and more) close to its core. Doing so has made the tool very precise and report few false positives. From my experience, the more data you provide the more accurate a rule can be, and new opportunities arise.

With that in mind, we are very likely to allow having rules ask for additional files. One example we have is a documentation rule getting access to the changelog file to check that the current version (found in yet another file, elm.json) has a changelog entry. This is not possible currently.

So I definitely think having multi-file-aware rules is a move in the right direction, albeit a probably very complicated one.

---

For Elm/elm-review, the list of Elm files is in elm.json (an equivalent of package.json) 's source-directories (example for an application, for packages it's always src/). From that information, elm-review just finds all the Elm files, the same way that the Elm compiler does. That is IMO simpler than following import statements (just glob once, no need to parse files), also allows finding/linting files that are not being imported at all.

This unfortunately doesn't work for ESLint out of the box because the information is not anywhere. I would argue that there should be a field similar to Elm's source-directories in ESLint's configuration. That would have the additional benefit of not having to run eslint with a list of paths.

The downside is that this only works for a single Elm project, as defined by the combination of files and dependencies listed in elm.json. When there are multiple projects (sharing some code), elm-review doesn't have a great solution yet. There are workable hacks/solutions to this, like creating a root elm.json encompassing all sub-projects, but other than that, I haven't found a great solution yet.

Similar to ESLint, the elm-review CLI accepts a list of path arguments (elm-review src/ tests) which override which paths to look at. In practice, I found that to be a problem, and I'm planning of removing that (rather, make it more cumbersome) in the next major version.

Currently, the rules that look at multiple files assume that they look at the entire project, which I believe is a reasonable premise for the tool. Say we're writing a rule that reports unused exports. If an exported function fn is only used somewhere in lib-user/ and we run elm-review lib-admin/ (instead of just elm-review which would include both folders). Now the imports/usages of fn are not found anymore, resulting in an effective false positive.

A side-effect of this is that a configuration can somewhat get tied to a project, but in practice it's not been much of a problem for elm-review. It might be a bit more for ESLint and its philosophy though (like sharing/extending configurations).

---

The way that elm-review collects and combines data from multiple modules is done in a specific way that is likable to a map-reduce algorithm. By virtue of the language the rules are written in, rules can't write to global variables or perform side-effects, so there is a structure to the collection, that I have designed in a way that allows re-visiting only some of the files (like in watch mode or in fix mode when some files get changed), with caching in consistent results in mind.

I would definitely encourage you to go through some of elm-review's docs, as I think it might give a bit of inspiration and prior art. The "main" Rule module shows how you can create rules, starting from "module rules", which only look at a single modules in isolation, and then goes on to show how to create "project rules" which look at multiple files. There are some small examples in there, so those might be helpful too. Hit me up if you want help or a walkthrough.

---

Here are off the top of my head a few things that we can do with the knowledge of multiple files:

Be a lot more precise in targeting functions. If you have two imports import * as Module from './a.js'; import * as Module from './b.js' and then a reference like Module.someFunction, then being able to look at the contents of the imported files can help determine from which module someFunction comes from. This is a probably lot more useful in Elm due to how modules are made, and this information is used in nearly all rules (and made easy through the API). For ESLint, that will be able to help determine which of a.js or b.js might be considered unnecessarily imported. In practice this will be less useful for JS/TS because references are so complex to resolve (method calling rather than function calling, shadowed/overriden references) that I think rule authors will usually not bother doing this. At least not without type information (which typescript-eslint provides).

Similarly, being able to determine whether a module has side-effects in its code can determine whether it is safe to remove an import or not.

Being able to determine which functions/values/types are exported but unused

For TS, being able to determine which enum values) are unused can help remove the unnecessary ones.

elm-review has documentation rules, which access the README. We can detect incorrect links to modules or their functions/types from there, or from one module to another. That helps fix typos, or show where to update docs when something has been removed or renamed.

Reporting usages of functions from others modules that are marked as deprecated. More generally, being able to figure out any information about something that would be written where the thing is defined. typescript-eslint provides the type, but we might want to know the JSDoc contents or some other details (in Elm,we often want to know the possible variants of types for instance).

---

I'm sorry if you were not asking about all these details. It was a bit unclear to me what you meant by "project" so I preferred adding more information that too little.

[nzakas]

No, this is exactly what I was looking for and why I CCed you on this thread. This is very helpful, thank you.

So it sounds like elm-review is already project-aware from the start. The idea of adding an equivalent of source-files is interesting, although I do wonder if we could leverage package.json in some way for that.

Am I correct in understanding that elm-review always lints the entire project rather than a single file?

[jfmengels]

Am I correct in understanding that elm-review always lints the entire project rather than a single file?

Yes. Unless you override that behavior through CLI arguments. Because elm-review has multi-file rules, sometimes skipping (not including) some files results in false positives (ex: an export now "looks" unused), which is why I intend to make "harder" to override that. The tool still tries to skip running rules for performance reasons when we know the results will be ignored (for single-file rules for instance).

[jfmengels]

To be clear, it is possible to only run it on a single file, but then to avoid false positives, you'll likely only run a subset of rules that are the ones you care about and that you know are single-file rules elm-review src/SomeFile.elm --rules SomeRuleName. I don't know if people ever run it like that in practice, that may be arcane knowledge that I only know about.

[bradzacher]

If you're a plugin author, what would this allow you to do?

TBH by itself - nothing in particular.
The import thing we'd need is for ESLint to use project information to provide a deterministic file order for linting.

Right now eslint's file ordering is, for all intents and purposes, random. ESLint could lint a file from project A then a file from project B, them ten from project A, etc. As a plugin author - it's not an implementation detail we can rely on atm.

If ESLint used the project grouping to order files - I.e. All files in a project are linted before the next project is linted (or in a multi-threaded world - one project per thread), then we can make decisions about how we manage memory.

Right now we accumulate ts.Program (the data structure that represents a tsconfig) instances over the course of a lint run.
We cannot dispose of these objects ever because (a) we don't know if we're in a single-run or persistent-run scenario (b) we don't know the list of files being linted.
We have multiple in-flight because we don't control the ordering of files as well (as before - ESLint might lint a file from project A then project B, etc).

So if we (a) can tell ESLint the projects, (b) rely on the ordering of files, (c) know the set of files being linted and (d) know if we're in a single- or persistent-run then that enables a huge performance improvement for us - we can guarantee that there is exactly one ts.Program in memory at any given time. No more OOMs on large repos and no more wasted time checking which program to use from our list.

Really, really big wins for us for the single-run scenario.

Also worth noting (I touched on this earlier) - we define projects and these projects can be used to guide parallel linting. Without projects ESLint cannot do parallel type-aware linting efficiently because it will cause us to waste time duicating ts.Programs across threads. But with projects we can guarantee no overlap and exactly one ts.Program per thread.

[nzakas]

So if we (a) can tell ESLint the projects, (b) rely on the ordering of files, (c) know the set of files being linted and (d) know if we're in a single- or persistent-run then that enables a huge performance improvement for us - we can guarantee that there is exactly one ts.Program in memory at any given time. No more OOMs on large repos and no more wasted time checking which program to use from our list.

Noted. 👍

[bradzacher]

How would the ESLint CLI know that you are working on a project instead of a single file?

I breifly touched on this in my other comment.

IMO ESLint needs a new model for how it lints. Right now you pass ESLint a set of globs to resolve, a set of files to enumerate, and/or a set of specific files.

From my experience for the most part repos will do a cli lint run in one of two ways:

1. an entire repo lint that looks like this: eslint . (or sometimes they specify specific subfolders known to contain lintable code like eslint src test).
2. a "staged file" lint run which passes just the changed files git diff --name-only | xargs eslint

I think the best future for ESLint is to simplify its CLI and just allow these two cases.

Instead of relying on CLI args to drive the set of files - IMO the ESLint config should cover that. For example I'd expect that I could just run eslint and ESLint would load the config, gather the set of lintable files based on the config, gather the set of projects from the config, use the plugin(s) to resolve those projects to a set of files, then lint the files.

Given we're talking about the "project" concept here I'd also expect that eslint would accept a project ID which is resolved by a plugin to a set of files.
For example eslint -p package/foo/tsconfig.json would cause eslint to ask the plugin for the files for the project package/foo/tsconfig.json, which typescript-eslint would resolve to the files included in that tsconfig.
For a JS project perhaps that instead would be eslint -p package/foo/package.json or something?

For the "set of known paths" case I'd expect that ESLint would ask the plugin to group them into projects, then lint them like that. Eg eslint package/foo/src/index.ts would cause eslint to ask the plugin to group package/foo/src/index.ts, and the plugin would lookup the configured projects to find the matching one.

[ljharb]

I definitely run eslint on specific file paths with custom config - pretty-printing generated output, for example - but that is very much an edge case, compared to these two primary cases.

[nzakas]

Instead of relying on CLI args to drive the set of files - IMO the ESLint config should cover that. For example I'd expect that I could just run eslint and ESLint would load the config, gather the set of lintable files based on the config,

This is how ESLint will work once we remove eslintrc. 👍

In general, it sounds like you are on-board with the idea that ESLint should work on projects by default and files only as an override?

[aladdin-add]

I like it. just a question: should it also add first-class monorepo support? nowadays I'm seeing mainly 2 ways to use eslint in a monorepo:

• one config for all the workspaces.
• every workspace has its own config.

[nzakas]

I'm not sure what first-class monorepo support means. :) If you want to discuss monorepos, let's start a separate discussion so we can keep this one focused on what a "project" means.