🐛 Ignore pattern in linter feature wrongly applies to formatter

[Sec-ant]

Environment information

CLI:
  Version:                      1.6.1
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           linux

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v21.7.1"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "npm/10.5.0"

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 true

Workspace:
  Open Documents:               0

What happened?

1. Clone the repro repo:

   git clone https://github.com/Sec-ant/biome-pattern-matching

2. cd into the repo folder

   File structure:

   ├── biome.json
   ├── build
   │   └── a.js
   ├── package-lock.json
   └── package.json
   

   Biome configuration biome.json:

   {
    "$schema": "https://biomejs.dev/schemas/1.6.1/schema.json",
    "linter": {
        "ignore": ["**/build"]
    }
   }

   a.js in the build folder:

   const  a   =      "123";

3. Run the following command

   npm i
   npx biome check .

4. The output doesn't report any formatting suggestions. But we didn't ignore any files in the formatter, so it should report a formatting suggestion for build/a.js.

5. If we change the ignore pattern from ["**/build"] to ["**/build/*"] or ["**/build/**"], the formatting suggestion will be printed as expected.

6. Also, Biome doesn't seem to support trailing slashes in the pattern. **/build/ will not match the build dir. But I think this may be considered as another issue.

Expected result

Ignore patterns in linter feature shouldn't apply to formatter or other features.

Code of Conduct

• I agree to follow Biome's Code of Conduct

[Sec-ant]

And I suspect this issue and #2080 have the same root cause, which may be related to how we are matching paths for including and ignoring files.

[Sec-ant]

Some debugging:

The TraversalOptions::can_handle method will treat directories and files differently.

biome/crates/biome_cli/src/execute/traverse.rs

Lines 694 to 712 in 8dc5011

	if !self.fs.path_is_file(biome_path.as_path()) {
	// handle:
	// - directories
	// - symlinks
	// - unresolved symlinks
	// e.g `symlink/subdir` where symlink points to a directory that includes `subdir`.
	// Note that `symlink/subdir` is not an existing file.
	let can_handle = !self
	.workspace
	.is_path_ignored(IsPathIgnoredParams {
	biome_path: biome_path.clone(),
	feature: self.execution.as_feature_name(),
	})
	.unwrap_or_else(|err| {
	self.push_diagnostic(err.into());
	false
	});
	return can_handle;
	}

As we can see it will first check if the directory is ignored by the feature (lint or format) with this method:

biome/crates/biome_cli/src/execute/traverse.rs

Lines 703 to 706 in 8dc5011

	.is_path_ignored(IsPathIgnoredParams {
	biome_path: biome_path.clone(),
	feature: self.execution.as_feature_name(),
	})

In which self.execution.as_feature_name() will cast any traversal mode to Lint feature except for the Format mode, which can be seen in:

biome/crates/biome_cli/src/execute/mod.rs

Lines 31 to 36 in 8dc5011

	pub(crate) fn as_feature_name(&self) -> FeatureName {
	match self.traversal_mode {
	TraversalMode::Format { .. } => FeatureName::Format,
	_ => FeatureName::Lint,
	}
	}

So biome check and biome ci will use the ignore patterns in the linter config section to check if a directory is ignored or not. And once the directory is ignored, it will prevent biome from further formatting the files in that directory.

The reason why ignore patterns like **/build/* or **/build/** will work is because they are checked against files instead of directories.

Unfortunately, I don't have a good idea to fix this issue. This might be due to the limitation of the matching function.

[Sec-ant]

It looks like this behavior (ignoring directories) is introduced in rome/tools#4276. @ematipico Do you have any suggestions?

[ematipico]

@Sec-ant do you experience the same issue with the LSP?

[Sec-ant]

do you experience the same issue with the LSP?

Sorry but I'm not very certain how to test LSP? I use Biome via the extension in VSCode, but that will only execute format instead of check or ci when I format a file, right?

[Conaclos]

Unfortunately, I don't have a good idea to fix this issue. This might be due to the limitation of the matching function.

If I am not wrong, can_handle is a preliminary check to completely ignore a directory or a file.
A file can still be ignored in a lower layer.

I could try to change the code like following:

impl Execution {
    pub(crate) fn as_feature_name(&self) -> Option<FeatureName> {
        match self.traversal_mode {
            TraversalMode::Format { .. } => Some(FeatureName::Format),
            TraversalMode::Lint { .. } => Some(FeatureName::Lint),
            _ => None,
        }
    }
}

This will require some change to allow querying the server without a feature.

[Sec-ant]

Unfortunately, I don't have a good idea to fix this issue. This might be due to the limitation of the matching function.

If I am not wrong, can_handle is a preliminary check to completely ignore a directory or a file. A file can still be ignored in a lower layer.

I could try to change the code like following:

impl Execution {
    pub(crate) fn as_feature_name(&self) -> Option<FeatureName> {
        match self.traversal_mode {
            TraversalMode::Format { .. } => Some(FeatureName::Format),
            TraversalMode::Lint { .. } => Some(FeatureName::Lint),
            _ => None,
        }
    }
}

This will require some change to allow querying the server without a feature.

Yes, I did exactly this, but build/a.js cannot be matched against **/build. The file will not be ignored by the linter.

[ematipico]

Thank you, @Sec-ant, for your findings! Those are very useful and will save a ton of time

[ematipico]

I am looking at the issue, I should be able to fix it

[ematipico]

Looking better at the use case, I believe it's expected, although I also think the users' expectations are misinterpreted.

The glob **/build is very specific: a folder foo/build matches the blog, but foo/build/file.js DOESN'T match the glob. And this is not an issue with our glob matching, it's how the glob works.

Now, there's definitely an issue on how we mark folders as ignored, but the outcome of changing the globs is actually expected.

Maybe we should write a better guide on how ignore and include comes into play with multiple tools, although a user should understand how to control the globs and how they work.

EDIT:

The issue becomes more subtle because **/build is in the linter section, and the user is using the check command, which runs all the tools at once. When we traverse the file system, we check if a folder is ignored and it is not, because **/build is still eligible for formatting and import sorting, so we traverse it. Once we start traversing the files, the paths we traverse will be like build/a.js, which doesn't match the initial glob, and hence the linting is applied.

The correct fix is to use build/** or **/build/** globs.

[ematipico]

We could add a lint rule that could suggest users avoid globs that would look like **/<something> inside formatter.ignore/linter.ignore.

[Sec-ant]

Maybe we should write a better guide on how ignore and include comes into play with multiple tools, although a user should understand how to control the globs and how they work.

I agree. Many .gitignore files out there simply use folder names to ignore both the folders themselves and everything within them, for example, node_modules, dist. Consequently, many users, myself included, might develop a path dependence regarding the setting of ignore patterns. When executing multi-tool commands, this could easily become a confusing point if not explained with clarity.

We could add a lint rule that could suggest users avoid globs that would look like **/<something> inside formatter.ignore/linter.ignore.

This may be difficult to achieve because we don't know if the user wants to ignore a file or a folder.