From bc3bd4e18cd08c19855999247bb711644f273e14 Mon Sep 17 00:00:00 2001
From: Victorien Elvinger <victorien@elvinger.fr>
Date: Sat, 13 Jan 2024 12:23:55 +0100
Subject: [PATCH] fix(config): don't ignore `include` when `ignore` is set
 (#1548)

---
 CHANGELOG.md                                  |  7 ++
 crates/biome_cli/tests/commands/format.rs     | 92 +++++++++++++++++++
 ...at_ignored_file_in_included_directory.snap | 35 +++++++
 ...format_non_included_and_ignored_files.snap | 46 ++++++++++
 crates/biome_service/src/settings.rs          | 10 +-
 crates/biome_service/src/workspace/server.rs  | 40 +++-----
 .../content/docs/guides/how-biome-works.mdx   | 19 ++--
 .../src/content/docs/internals/changelog.mdx  |  7 ++
 8 files changed, 217 insertions(+), 39 deletions(-)
 create mode 100644 crates/biome_cli/tests/snapshots/main_commands_format/does_not_format_ignored_file_in_included_directory.snap
 create mode 100644 crates/biome_cli/tests/snapshots/main_commands_format/does_not_format_non_included_and_ignored_files.snap

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 060b9d77b307..502b3f8d2c23 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,13 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
 
 - Fix [1440](https://github.com/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
diff --git a/crates/biome_cli/tests/commands/format.rs b/crates/biome_cli/tests/commands/format.rs
index a7b6d8b77c5c..064b1db208ed 100644
--- a/crates/biome_cli/tests/commands/format.rs
+++ b/crates/biome_cli/tests/commands/format.rs
@@ -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();
diff --git a/crates/biome_cli/tests/snapshots/main_commands_format/does_not_format_ignored_file_in_included_directory.snap b/crates/biome_cli/tests/snapshots/main_commands_format/does_not_format_ignored_file_in_included_directory.snap
new file mode 100644
index 000000000000..224b027e8bec
--- /dev/null
+++ b/crates/biome_cli/tests/snapshots/main_commands_format/does_not_format_ignored_file_in_included_directory.snap
@@ -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>
+```
+
+
diff --git a/crates/biome_cli/tests/snapshots/main_commands_format/does_not_format_non_included_and_ignored_files.snap b/crates/biome_cli/tests/snapshots/main_commands_format/does_not_format_non_included_and_ignored_files.snap
new file mode 100644
index 000000000000..b0e58459f434
--- /dev/null
+++ b/crates/biome_cli/tests/snapshots/main_commands_format/does_not_format_non_included_and_ignored_files.snap
@@ -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>
+```
+
+
diff --git a/crates/biome_service/src/settings.rs b/crates/biome_service/src/settings.rs
index 2415df5913d0..e8aff3061057 100644
--- a/crates/biome_service/src/settings.rs
+++ b/crates/biome_service/src/settings.rs
@@ -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 {
diff --git a/crates/biome_service/src/workspace/server.rs b/crates/biome_service/src/workspace/server.rs
index 17dc9d012f3c..8c4dff110cd0 100644
--- a/crates/biome_service/src/workspace/server.rs
+++ b/crates/biome_service/src/workspace/server.rs
@@ -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)
     }
 }
 
@@ -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 => {
@@ -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);
         }
 
diff --git a/website/src/content/docs/guides/how-biome-works.mdx b/website/src/content/docs/guides/how-biome-works.mdx
index ade54b080e7b..bf38ec07375c 100644
--- a/website/src/content/docs/guides/how-biome-works.mdx
+++ b/website/src/content/docs/guides/how-biome-works.mdx
@@ -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
@@ -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
diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx
index 0ceda15560e9..8897521b3370 100644
--- a/website/src/content/docs/internals/changelog.mdx
+++ b/website/src/content/docs/internals/changelog.mdx
@@ -31,6 +31,13 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
 
 - Fix [1440](https://github.com/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