Skip to content

Commit

Permalink
fix(config): don't ignore include when ignore is set (biomejs#1548)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos authored and ematipico committed Jan 24, 2024
1 parent 34c919a commit de9b3e1
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 39 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ Read our [guidelines for writing a good changelog entry](https:/biom

- Fix [1440](https:/biomejs/biome/issues/1440), a case where `extends` and `overrides` weren't correctly emitting the final configuration. Contributed by @arendjr

- Correctly handle `include` when `ignore` is set (#1468). Contributed by @Conaclos

Previously, Biome ignored `include` if `ignore` was set.
Now, Biome check both `include` and `ignore`.
A file is processed if it is included and not ignored.
If `include` is not set all files are considered included.

### Editors

### Formatter
Expand Down
92 changes: 92 additions & 0 deletions crates/biome_cli/tests/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1340,6 +1340,98 @@ fn does_not_format_ignored_directories() {
));
}

#[test]
fn does_not_format_non_included_and_ignored_files() {
let config = r#"{
"files": {
"include": ["file1.js", "file2.js", "file3.js"],
"ignore": ["file2.js"]
},
"formatter": {
"include": ["file2.js"],
"ignore": ["file3.js"]
}
}"#;
let files = [("file1.js", true), ("file2.js", true), ("file3.js", false)];

let mut console = BufferConsole::default();
let mut fs = MemoryFileSystem::default();
let file_path = Path::new("biome.json");
fs.insert(file_path.into(), config);
for (file_path, _) in files {
let file_path = Path::new(file_path);
fs.insert(file_path.into(), UNFORMATTED.as_bytes());
}

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Args::from([("format"), ("."), ("--write")].as_slice()),
);
assert!(result.is_ok(), "run_cli returned {result:?}");

for (file_path, expect_formatted) in files {
let expected = if expect_formatted {
FORMATTED
} else {
UNFORMATTED
};
assert_file_contents(&fs, Path::new(file_path), expected);
}

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"does_not_format_non_included_and_ignored_files",
fs,
console,
result,
));
}

#[test]
fn does_not_format_ignored_file_in_included_directory() {
let config = r#"{
"formatter": {
"include": ["src"],
"ignore": ["src/file2.js"]
}
}"#;
let files = [("src/file1.js", true), ("src/file2.js", false)];

let mut console = BufferConsole::default();
let mut fs = MemoryFileSystem::default();
let file_path = Path::new("biome.json");
fs.insert(file_path.into(), config);
for (file_path, _) in files {
let file_path = Path::new(file_path);
fs.insert(file_path.into(), UNFORMATTED.as_bytes());
}

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Args::from([("format"), ("."), ("--write")].as_slice()),
);
assert!(result.is_ok(), "run_cli returned {result:?}");

for (file_path, expect_formatted) in files {
let expected = if expect_formatted {
FORMATTED
} else {
UNFORMATTED
};
assert_file_contents(&fs, Path::new(file_path), expected);
}

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"does_not_format_ignored_file_in_included_directory",
fs,
console,
result,
));
}

#[test]
fn fs_error_read_only() {
let mut fs = MemoryFileSystem::new_read_only();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `biome.json`

```json
{
"formatter": {
"include": ["src"],
"ignore": ["src/file2.js"]
}
}
```

## `src/file1.js`

```js
statement();

```

## `src/file2.js`

```js
statement( )
```

# Emitted Messages

```block
Formatted 2 file(s) in <TIME>
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `biome.json`

```json
{
"files": {
"include": ["file1.js", "file2.js", "file3.js"],
"ignore": ["file2.js"]
},
"formatter": {
"include": ["file2.js"],
"ignore": ["file3.js"]
}
}
```

## `file1.js`

```js
statement();

```

## `file2.js`

```js
statement();

```

## `file3.js`

```js
statement( )
```

# Emitted Messages

```block
Formatted 3 file(s) in <TIME>
```


10 changes: 4 additions & 6 deletions crates/biome_service/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,12 +484,10 @@ fn to_file_settings(
vcs_config_path.clone(),
gitignore_matches,
)?,
included_files: to_matcher(
working_directory,
config.include.as_ref(),
vcs_config_path,
gitignore_matches,
)?,
// Don't set `gitignore` for the include matcher.
// TODO: it could better to create dedicated matcher types:
// one for ignore and the other for include.
included_files: to_matcher(working_directory, config.include.as_ref(), None, &[])?,
ignore_unknown: config.ignore_unknown.unwrap_or_default(),
})
} else {
Expand Down
40 changes: 14 additions & 26 deletions crates/biome_service/src/workspace/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,9 @@ impl WorkspaceServer {
/// Check whether a file is ignored in the top-level config `files.ignore`/`files.include`
fn is_ignored_by_top_level_config(&self, path: &Path) -> bool {
let settings = self.settings();

if !settings.as_ref().files.ignored_files.is_empty() {
return settings.as_ref().files.ignored_files.matches_path(path);
} else if !settings.as_ref().files.included_files.is_empty() {
return !settings.as_ref().files.included_files.matches_path(path);
}

false
let is_included = settings.as_ref().files.included_files.is_empty()
|| settings.as_ref().files.included_files.matches_path(path);
!is_included || settings.as_ref().files.ignored_files.matches_path(path)
}
}

Expand Down Expand Up @@ -278,26 +273,19 @@ impl Workspace for WorkspaceServer {
return Ok(false);
}

let excluded_by_override = settings.as_ref().override_settings.is_path_excluded(path);
let included_by_override = settings.as_ref().override_settings.is_path_included(path);

// Overrides have top priority
if let Some(excluded_by_override) = excluded_by_override {
if excluded_by_override {
return Ok(true);
}
let excluded_by_override = settings.as_ref().override_settings.is_path_excluded(path);
if excluded_by_override.unwrap_or_default() {
return Ok(true);
}

if let Some(included_by_override) = included_by_override {
if included_by_override {
return Ok(!included_by_override);
}
let included_by_override = settings.as_ref().override_settings.is_path_included(path);
if included_by_override.unwrap_or_default() {
return Ok(false);
}

let (ignored_files, included_files) = match params.feature {
FeatureName::Format => {
let formatter = &settings.as_ref().formatter;

(&formatter.ignored_files, &formatter.included_files)
}
FeatureName::Lint => {
Expand All @@ -313,11 +301,11 @@ impl Workspace for WorkspaceServer {
}
};

if !ignored_files.is_empty() {
if ignored_files.matches_path(path) {
return Ok(true);
}
} else if !included_files.is_empty() && included_files.matches_path(path) {
// Tool include/ignore have priority over (global) files include/ignore
if ignored_files.matches_path(path) {
return Ok(true);
}
if !included_files.is_empty() && included_files.matches_path(path) {
return Ok(false);
}

Expand Down
19 changes: 12 additions & 7 deletions website/src/content/docs/guides/how-biome-works.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,17 @@ The following files are parsed as **`JSON` files** with the options `json.parse
- `.hintrc`
- `.babelrc`

## `ignore` and `include` explained
## `include` and `ignore` explained

Biome will resolve the globs specified in `ignore` and `include` relatively from the working directory.
Biome will resolve the globs specified in `include` and `ignore` relatively from the working directory.

The working directory is the directory where you usually run a CLI command. This means that you have to place **particular attention** when the configuration file is placed in a
different directory that is different from where you execute your command.
The working directory is the directory where you usually run a CLI command.
This means that you have to place **particular attention** when the configuration file is placed in
a different directory from where you execute your command.

For example, you have a project that contains two directories called `backend/` and `frontend/`, and decide to place your `biome.json` at the root folder of the project. Inside the `frontend/` project, you have your `package.json` with some scripts that runs Biome:
For example, you have a project that contains two directories called `backend/` and `frontend/`,
and decide to place your `biome.json` at the root folder of the project.
Inside the `frontend/` project, you have your `package.json` with some scripts that run Biome:

```
├── backend
Expand Down Expand Up @@ -130,10 +133,12 @@ For example, you have a project that contains two directories called `backend/`
}
```

When you run the script `format` inside `frontend/package.json`, the working directory resolved by that script will be `frontend/`, the globs `src/**/*.js` and `src/**/*.ts` will have as "base" directory `frontend/`.
When you run the script `format` inside `frontend/package.json`,
the working directory resolved by that script will be `frontend/`,
the globs `src/**/*.js` and `src/**/*.ts` will have as "base" directory `frontend/`.

:::note
When both `ignore` and `include` are specified, Biome gives **precedence** to `ignore`.
When both `include` and `ignore` are specified, Biome gives **precedence** to `ignore`.
:::

:::caution
Expand Down
7 changes: 7 additions & 0 deletions website/src/content/docs/internals/changelog.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ Read our [guidelines for writing a good changelog entry](https:/biom

- Fix [1440](https:/biomejs/biome/issues/1440), a case where `extends` and `overrides` weren't correctly emitting the final configuration. Contributed by @arendjr

- Correctly handle `include` when `ignore` is set (#1468). Contributed by @Conaclos

Previously, Biome ignored `include` if `ignore` was set.
Now, Biome check both `include` and `ignore`.
A file is processed if it is included and not ignored.
If `include` is not set all files are considered included.

### Editors

### Formatter
Expand Down

0 comments on commit de9b3e1

Please sign in to comment.