From be5eb27f16e63ee6c5753e4fd669c14a5d3471f0 Mon Sep 17 00:00:00 2001
From: Stephen Jennings <stephen.g.jennings@gmail.com>
Date: Thu, 5 Sep 2024 16:26:28 -0700
Subject: [PATCH] fix: Add `enabled` config for fix tools

Adds an optional `fix.tools.TOOL.enabled` config that disables use of a fix
tool (if omitted, the tool is enabled). This is useful for defining tools in
the user's configuration without enabling them for all repositories:

```toml
# ~/.jjconfig.toml
[fix.tools.rustfmt]
enabled = false
command = ["rustfmt", "--emit", "stdout"]
patterns = ["glob:'**/*.rs'"]
```

Then to enable it in a repository:

```shell
$ jj config set --repo fix.tools.rustfmt.enabled true
```
---
 CHANGELOG.md                     |  4 ++
 cli/src/commands/fix.rs          | 22 ++++++++++-
 cli/src/config-schema.json       |  5 +++
 cli/tests/cli-reference@.md.snap |  3 ++
 cli/tests/test_fix_command.rs    | 67 ++++++++++++++++++++++++++++++++
 docs/config.md                   | 21 ++++++++++
 6 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 57747bebf..d8b193c27 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -77,6 +77,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * New `git.sign-on-push` config option to automatically sign commits which are being
   pushed to a Git remote.
 
+* New `fix.tools.TOOL.enabled` config option to enable/disable tools. This is
+  useful for defining disabled tools in user configuration that can be enabled
+  in individual repositories with one config setting.
+
 ### Fixed bugs
 
 * Fixed diff selection by external tools with `jj split`/`commit -i FILESETS`.
diff --git a/cli/src/commands/fix.rs b/cli/src/commands/fix.rs
index 7de74a5e7..a4ccb808c 100644
--- a/cli/src/commands/fix.rs
+++ b/cli/src/commands/fix.rs
@@ -85,6 +85,9 @@ use crate::ui::Ui;
 ///    empty, no files will be affected by the tool. If there are multiple
 ///    patterns, the tool is applied only once to each file in the union of the
 ///    patterns.
+///  - `enabled`: Enables or disables the tool. If omitted, the tool is enabled.
+///    This is useful for defining disabled tools in user configuration that can
+///    be enabled in individual repositories with one config setting.
 ///
 /// For example, the following configuration defines how two code formatters
 /// (`clang-format` and `black`) will apply to three different file extensions
@@ -407,6 +410,8 @@ struct ToolConfig {
     command: CommandNameAndArgs,
     /// The matcher that determines if this tool matches a file.
     matcher: Box<dyn Matcher>,
+    /// Whether the tool is enabled
+    enabled: bool,
     // TODO: Store the `name` field here and print it with the command's stderr, to clearly
     // associate any errors/warnings with the tool and its configuration entry.
 }
@@ -424,6 +429,12 @@ struct ToolsConfig {
 struct RawToolConfig {
     command: CommandNameAndArgs,
     patterns: Vec<String>,
+    #[serde(default = "default_tool_enabled")]
+    enabled: bool,
+}
+
+fn default_tool_enabled() -> bool {
+    true
 }
 
 /// Parses the `fix.tools` config table.
@@ -432,7 +443,7 @@ struct RawToolConfig {
 /// not check for issues that might still occur later like missing executables.
 /// This is a place where we could fail earlier in some cases, though.
 fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig, CommandError> {
-    let tools: Vec<ToolConfig> = settings
+    let mut tools: Vec<ToolConfig> = settings
         .table_keys("fix.tools")
         // Sort keys early so errors are deterministic.
         .sorted()
@@ -458,11 +469,18 @@ fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig,
             Ok(ToolConfig {
                 command: tool.command,
                 matcher: expression.to_matcher(),
+                enabled: tool.enabled,
             })
         })
         .try_collect()?;
     if tools.is_empty() {
-        Err(config_error("No `fix.tools` are configured"))
+        return Err(config_error("No `fix.tools` are configured"));
+    }
+    tools.retain(|t| t.enabled);
+    if tools.is_empty() {
+        Err(config_error(
+            "At least one entry of `fix.tools` must be enabled.".to_string(),
+        ))
     } else {
         Ok(ToolsConfig { tools })
     }
diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json
index 1ea267263..0ae17f803 100644
--- a/cli/src/config-schema.json
+++ b/cli/src/config-schema.json
@@ -617,6 +617,11 @@
                                     "type": "string"
                                 },
                                 "description": "Filesets that will be affected by this tool"
+                            },
+                            "enabled": {
+                                "type": "boolean",
+                                "description": "Disables this tool if set to false",
+                                "default": true
                             }
                         }
                     },
diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap
index bfdfa3704..e90c4be4f 100644
--- a/cli/tests/cli-reference@.md.snap
+++ b/cli/tests/cli-reference@.md.snap
@@ -986,6 +986,9 @@ the values have the following properties:
    empty, no files will be affected by the tool. If there are multiple
    patterns, the tool is applied only once to each file in the union of the
    patterns.
+ - `enabled`: Enables or disables the tool. If omitted, the tool is enabled.
+   This is useful for defining disabled tools in user configuration that can
+   be enabled in individual repositories with one config setting.
 
 For example, the following configuration defines how two code formatters
 (`clang-format` and `black`) will apply to three different file extensions
diff --git a/cli/tests/test_fix_command.rs b/cli/tests/test_fix_command.rs
index 931101152..dc170ca10 100644
--- a/cli/tests/test_fix_command.rs
+++ b/cli/tests/test_fix_command.rs
@@ -144,6 +144,73 @@ fn test_config_multiple_tools_with_same_name() {
     insta::assert_snapshot!(content, @"Bar\n");
 }
 
+#[test]
+fn test_config_disabled_tools() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+    let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter");
+    assert!(formatter_path.is_file());
+    let formatter = to_toml_value(formatter_path.to_str().unwrap());
+    test_env.add_config(format!(
+        r###"
+        [fix.tools.tool-1]
+        # default is enabled
+        command = [{formatter}, "--uppercase"]
+        patterns = ["foo"]
+
+        [fix.tools.tool-2]
+        enabled = true
+        command = [{formatter}, "--lowercase"]
+        patterns = ["bar"]
+
+        [fix.tools.tool-3]
+        enabled = false
+        command = [{formatter}, "--lowercase"]
+        patterns = ["baz"]
+        "###
+    ));
+
+    std::fs::write(repo_path.join("foo"), "Foo\n").unwrap();
+    std::fs::write(repo_path.join("bar"), "Bar\n").unwrap();
+    std::fs::write(repo_path.join("baz"), "Baz\n").unwrap();
+
+    let (_stdout, _stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]);
+
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo", "-r", "@"]);
+    insta::assert_snapshot!(content, @"FOO\n");
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "bar", "-r", "@"]);
+    insta::assert_snapshot!(content, @"bar\n");
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "baz", "-r", "@"]);
+    insta::assert_snapshot!(content, @"Baz\n");
+}
+
+#[test]
+fn test_config_disabled_tools_warning_when_all_tools_are_disabled() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+    let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter");
+    assert!(formatter_path.is_file());
+    let formatter = to_toml_value(formatter_path.to_str().unwrap());
+    test_env.add_config(format!(
+        r###"
+        [fix.tools.tool-2]
+        enabled = false
+        command = [{formatter}, "--lowercase"]
+        patterns = ["bar"]
+        "###
+    ));
+
+    std::fs::write(repo_path.join("bar"), "Bar\n").unwrap();
+
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["fix"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Config error: At least one entry of `fix.tools` must be enabled.
+    For help, see https://jj-vcs.github.io/jj/latest/config/.
+    "###);
+}
+
 #[test]
 fn test_config_tables_overlapping_patterns() {
     let test_env = TestEnvironment::default();
diff --git a/docs/config.md b/docs/config.md
index 301a574e1..7af9a3e46 100644
--- a/docs/config.md
+++ b/docs/config.md
@@ -960,6 +960,27 @@ command = ["head", "-n", "10"]
 patterns = ["numbers.txt"]
 ```
 
+### Disabling and enabling tools
+
+Tools can be disabled and enabled with the optional `enabled` config. This
+allows you to define tools globally but enable them only for specific
+repositories.
+
+In the user configuration, define a disabled tool for running rustfmt:
+
+```toml
+[fix.tools.rustfmt]
+enabled = false
+command = ["rustfmt", "--emit", "stdout"]
+patterns = ["glob:'**/*.rs'"]
+```
+
+Then to use the tool in a specific repository, set the `enabled` config:
+
+```shell
+$ jj config set --repo fix.tools.rustfmt.enabled true
+```
+
 ## Commit Signing
 
 `jj` can be configured to sign and verify the commits it creates using either