📎 Implement more targeted file scanner

[arendjr]

Description

With Biome 2, we have a file scanner that is responsible for indexing a repository for the purpose of multi-file analysis. But our initial implementation is rather naive and has some unintended implications.

The important thing to understand is that the file scanner needs to index node_modules, because it needs to be able to extract type information from dependencies. But it’s not just type information: We also use the file scanner for building our module graph, which we use for module resolution, cycle detection, and other features. For these features to work properly we need to index files that may be ignored in user configurations, such as generated files, package.json files, and indeed any dependencies of your code in general.

But there are also files that we know we never need to index: directories such as .git, .nx, .cache and more.

Our current strategy is to index everything except the files/directories on a hardcoded exclusion list. It works for most projects, but obviously it can miss things and end up indexing things it shouldn’t. Most of the time, scanning too much isn’t a big deal, but of course it impacts performance. Sometimes unacceptably so. And even for projects where it works acceptably, it can hardly be called optimal.

So here is a list of ideas for improvement:

• Currently dist and build are not on the exclude list. By convention we can be pretty sure they’re unneeded, the only reason I didn’t was caution: Maybe they are used for legitimate sources in some projects? We can easily reconsider this.
• I’ve considered swapping the logic around: Instead of using an exclude list, we could start with files.includes and just add node_modules so we grab the dependencies too. It looks appealing at the surface, but quickly becomes even more complex to navigate: What if someone ignores package.json files in their files.includes? What about generated files they’ve excluded, but which are still relevant for type extraction? How does .gitignore factor in in this scenario? And for all that complexity we still don’t solve another major issue: Indexing of irrelevant dependencies in node_modules.
• We can give users control of the exclude list in their config files. We’re not a fan of this approach as it would not be very user-friendly. It’s confusing and hard to explain why in addition to files.includes (which already supports exclusions) we’d have another mechanism for excluding files with different behaviour that’s harder to observe.
• We can speed up the processing of node_modules by skipping all .js files that have a corresponding .d.ts file. This is probably something we want to do regardless, so it would make a good subtask of this one.
• We can first scan the files we want to process, extract which dependencies they reference, and scan those dependencies, check which transitive dependencies those have, then scan those and so on. This is possible, but comes with a lot of complexity:
  • We still need to scan all package.json files (including inside node_modules) for the module resolution to work.
  • We need separate strategies for the LSP (which wants to index the entire project) and the CLI (which wants to look only at the files for a given invocation).
  • It is harder to parallelise the work, as you get “choke points” during dependency resolution.
  • File watching becomes harder, because changing a file means its dependencies may change, meaning the scope of what needs to be scanned may change at any time.
• We could take a shortcut on all of this, and just distinguish between dependencies and devDependencies when it comes to scanning the node_modules. Many projects have a lot more dev dependencies than they have real dependencies, so we could restrict ourselves to scanning dependencies only. There would still be some complexity in figuring out transitive dependencies, but it would be a lot easier than handling everything per file. The downside is that scripts that use dev dependencies may have reduced functionality with some lint rules, but that may be an acceptable compromise.
• Other ideas/a combination of the above.

Feedback and ideas are welcome on this thread, but please check which suggestions have been made before. Giving thumbs up/down to suggestions gives us a better overview what we ideas are best to pursue.

[fregante]

I hope that this scanning is only enabled when actually used. I literally just switched to Biome last month because it's so fast at formatting and running a handful of rules on pre-commit. Those rules did not need type information and likely still don't. I hope that the speed isn't affected when not needed.

In #5633 (reply in thread) I posted my config, with 3/4 non-type-aware rules.

[ematipico]

Here are a few ideas I had to make the scanner more of an opt-in feature, rather than an opt-out feature. They might be random ideas, hopefully there's something good among them. They might conflict with each other:

• We can have a "light" scanner to discover known files, e.g., biome.json, package.json, tsconfig.json, ignore files, etc. I don't know what the impact of multiple scanning phases is performance-wise, but I doubt that the majority of users will see any differences—probably only the enterprise ones.
• Currently, we rely on the scanner - among other things - to read nested ignore files. Maybe we can make support of nested ignore files opt-in via configuration. If users opt-in, then we scan the project, if not, we do what we used to do before
• biome format should only use a light scan, because we only need to know which files are ignored. This connects to the previous bullet point.
• Any command that passes positional arguments shouldn't use the scanner. E.g. biome check ./src/components, biome check ./scripts/build.js, or use only the light scanner for known files. In this case, users want to check only a small part of the project, or we could assume that's the intention.
• Rules that require services that are project-wide (module graph, types, etc.) should have specific metadata.
  • We can panic if rule tries to query project-wide services, but the metadata isn't set
  • We can have a visitor that tracks this metadata in the configured rules. If no rule requires project-aware services, we don't need a full scanner
• Commands without positional arguments will use the full-fledged scanner

[fregante]

If it can be opt-in, I think at this point it should definitely be opt-in, at least until performance is restored and the kinks are straightened out.

[Conaclos]

Commands without positional arguments will use the full-fledged scanner

I find this really confusing because before we didn't allow commands without positional args. In my view biome check . should behave like biome check.

[ematipico]

Yes, I suppose you're right, so you understand what I meant :)

[maou-shonen]

Not having an exclude list has caused me some trouble and confusion.
My unit test files are located next to the source files (e.g., src/foo.test.ts).
I'm using Biome as the main formatter, but I don't need to lint the unit test files.
As a result, I have to include the test files in files.include and then re-exclude them in the format options.
This becomes quite troublesome in certain directory structures, such as with nuxt(The source code is not located under the src directory).

Example:

{
  "$schema": "https://biomejs.dev/schemas/2.0.0-beta.1/schema.json",
  "vcs": {
    "enabled": false,
    "clientKind": "git",
    "useIgnoreFile": false
  },
  "files": {
    "ignoreUnknown": true,
    "includes": [
      "**",
      "!dist",
      "!node_modules",
      "!coverage",
      "!.cache",
      "!.git",
      "!.turbo", // turbo repo
      "!patches", // pnpm patches
      "!generated" // some generated types
    ]
  },
  "formatter": {
    "enabled": true,
    "indentStyle": "space"
  },
  "linter": {
    "enabled": true,
    "includes": [
      "!**/*.test.ts", // <-- exclude test files
      // ↓ same as files.include
      "**",
      "!dist",
      "!node_modules",
      "!coverage",
      "!.cache",
      "!.git",
      "!.turbo",
      "!patches",
      "!generated"
    ],
    "rules": {
      "recommended": true
    }
  },
  "json": {},
  "assist": {
    "enabled": true,
    "actions": { "source": { "organizeImports": "on" } }
  }
}

I believe having exclude list option would provide more flexibility and align better with user intuition.

[Conaclos]

Not having an exclude list has caused me some trouble and confusion.

How is this related to the current issue?

Note that "!**/*.test.ts" should certainly be placed after "**". Otherwise, you just overrides it.

Also, linter.includes is a refinement of files.includes.
Thus the following config should be enough:

{
  "$schema": "https://biomejs.dev/schemas/2.0.0-beta.1/schema.json",
  "files": {
    "ignoreUnknown": true,
    "includes": [
      "**",
      "!dist",
      "!node_modules",
      "!coverage",
      "!.cache",
      "!.git",
      "!.turbo", // turbo repo
      "!patches", // pnpm patches
      "!generated" // some generated types
    ]
  },
  "formatter": {
    "indentStyle": "space"
  },
  "linter": {
    "includes": [
      "**",
      "!**/*.test.ts", // <-- exclude test files
    ]
  }
}

Thus, includes can act as an excludes.