881 usethis tool import linter doesnt give message for ignoring inp (#885)

nathanjmcdougall · web-flow · commit 4927e0aa9262 · 2025-07-22T17:59:02.000+12:00
* Clarify behaviour for duplicates in `KeyValueFileManager.extend_list` docstring

* Give a message regarding the addition of INP per-file ignores when adding Import Linter.

Also change the rule management methods on `Tool` to return `bool` for whether rules were added.

* Reword comment for clarity and conciseness
diff --git a/src/usethis/_io.py b/src/usethis/_io.py
@@ -203,7 +203,10 @@ def set_value(
 
     @abstractmethod
     def extend_list(self, *, keys: Sequence[Key], values: list[Any]) -> None:
-        """Extend a list in the configuration file."""
+        """Extend a list in the configuration file.
+
+        This method will always extend the list, even if it results in duplicates.
+        """
         raise NotImplementedError
 
     @abstractmethod
diff --git a/src/usethis/_tool/base.py b/src/usethis/_tool/base.py
@@ -567,18 +567,26 @@ def is_managed_rule(self, rule: Rule) -> bool:
         """Determine if a rule is managed by this tool."""
         return False
 
-    def select_rules(self, rules: list[Rule]) -> None:
+    def select_rules(self, rules: list[Rule]) -> bool:
         """Select the rules managed by the tool.
 
         These rules are not validated; it is assumed they are valid rules for the tool,
         and that the tool will be able to manage them.
+
+        Args:
+            rules: The rules to select. If any of these rules are already selected, they
+                   will be skipped.
+
+        Returns:
+            True if any rules were selected, False if no rules were selected.
         """
+        return False
 
     def get_selected_rules(self) -> list[Rule]:
         """Get the rules managed by the tool that are currently selected."""
         return []
 
-    def ignore_rules(self, rules: list[Rule]) -> None:
+    def ignore_rules(self, rules: list[Rule]) -> bool:
         """Ignore rules managed by the tool.
 
         Ignoring a rule is different from deselecting it - it means that even if it
@@ -587,21 +595,45 @@ def ignore_rules(self, rules: list[Rule]) -> None:
 
         These rules are not validated; it is assumed they are valid rules for the tool,
         and that the tool will be able to manage them.
+
+        Args:
+            rules: The rules to ignore. If any of these rules are already ignored, they
+                   will be skipped.
+
+        Returns:
+            True if any rules were ignored, False if no rules were ignored.
         """
+        return False
 
-    def unignore_rules(self, rules: list[str]) -> None:
+    def unignore_rules(self, rules: list[str]) -> bool:
         """Stop ignoring rules managed by the tool.
 
         These rules are not validated; it is assumed they are valid rules for the tool,
         and that the tool will be able to manage them.
+
+        Args:
+            rules: The rules to unignore. If any of these rules are not ignored, they
+                   will be skipped.
+
+        Returns:
+            True if any rules were unignored, False if no rules were unignored.
         """
+        return False
 
     def get_ignored_rules(self) -> list[Rule]:
         """Get the ignored rules managed by the tool."""
         return []
 
-    def deselect_rules(self, rules: list[Rule]) -> None:
+    def deselect_rules(self, rules: list[Rule]) -> bool:
         """Deselect the rules managed by the tool.
 
         Any rules that aren't already selected are ignored.
+
+        Args:
+            rules: The rules to deselect. If any of these rules are not selected, they
+                   will be skipped.
+
+        Returns:
+            True if any rules were deselected, False if no rules were deselected.
         """
+        return False
diff --git a/src/usethis/_tool/impl/deptry.py b/src/usethis/_tool/impl/deptry.py
@@ -110,10 +110,11 @@ def get_bitbucket_steps(self) -> list[BitbucketStep]:
     def is_managed_rule(self, rule: Rule) -> bool:
         return rule.startswith("DEP") and rule[3:].isdigit()
 
-    def select_rules(self, rules: list[Rule]) -> None:
+    def select_rules(self, rules: list[Rule]) -> bool:
         """Does nothing for deptry - all rules are automatically enabled by default."""
         if rules:
             info_print(f"All {self.name} rules are always implicitly selected.")
+        return False
 
     def get_selected_rules(self) -> list[Rule]:
         """No notion of selection for deptry.
@@ -123,14 +124,15 @@ def get_selected_rules(self) -> list[Rule]:
         """
         return []
 
-    def deselect_rules(self, rules: list[Rule]) -> None:
+    def deselect_rules(self, rules: list[Rule]) -> bool:
         """Does nothing for deptry - all rules are automatically enabled by default."""
+        return False
 
-    def ignore_rules(self, rules: list[Rule]) -> None:
+    def ignore_rules(self, rules: list[Rule]) -> bool:
         rules = sorted(set(rules) - set(self.get_ignored_rules()))
 
         if not rules:
-            return
+            return False
 
         rules_str = ", ".join([f"'{rule}'" for rule in rules])
         s = "" if len(rules) == 1 else "s"
@@ -143,11 +145,13 @@ def ignore_rules(self, rules: list[Rule]) -> None:
         keys = self._get_ignore_keys(file_manager)
         file_manager.extend_list(keys=keys, values=rules)
 
-    def unignore_rules(self, rules: list[str]) -> None:
+        return True
+
+    def unignore_rules(self, rules: list[str]) -> bool:
         rules = sorted(set(rules) & set(self.get_ignored_rules()))
 
         if not rules:
-            return
+            return False
 
         rules_str = ", ".join([f"'{rule}'" for rule in rules])
         s = "" if len(rules) == 1 else "s"
@@ -160,6 +164,8 @@ def unignore_rules(self, rules: list[str]) -> None:
         keys = self._get_ignore_keys(file_manager)
         file_manager.remove_from_list(keys=keys, values=rules)
 
+        return True
+
     def get_ignored_rules(self) -> list[Rule]:
         (file_manager,) = self.get_active_config_file_managers()
         keys = self._get_ignore_keys(file_manager)
diff --git a/src/usethis/_tool/impl/ruff.py b/src/usethis/_tool/impl/ruff.py
@@ -5,6 +5,7 @@
 
 from typing_extensions import assert_never
 
+from usethis._config import usethis_config
 from usethis._config_file import DotRuffTOMLManager, RuffTOMLManager
 from usethis._console import box_print, tick_print
 from usethis._integrations.ci.bitbucket.anchor import (
@@ -297,12 +298,12 @@ def get_bitbucket_steps(self) -> list[BitbucketStep]:
 
         return steps
 
-    def select_rules(self, rules: list[Rule]) -> None:
+    def select_rules(self, rules: list[Rule]) -> bool:
         """Add Ruff rules to the project."""
         rules = sorted(set(rules) - set(self.get_selected_rules()))
 
         if not rules:
-            return
+            return False
 
         rules_str = ", ".join([f"'{rule}'" for rule in rules])
         s = "" if len(rules) == 1 else "s"
@@ -315,12 +316,14 @@ def select_rules(self, rules: list[Rule]) -> None:
         keys = self._get_select_keys(file_manager)
         file_manager.extend_list(keys=keys, values=rules)
 
-    def ignore_rules(self, rules: list[Rule]) -> None:
+        return True
+
+    def ignore_rules(self, rules: list[Rule]) -> bool:
         """Ignore Ruff rules in the project."""
         rules = sorted(set(rules) - set(self.get_ignored_rules()))
 
         if not rules:
-            return
+            return False
 
         rules_str = ", ".join([f"'{rule}'" for rule in rules])
         s = "" if len(rules) == 1 else "s"
@@ -333,12 +336,14 @@ def ignore_rules(self, rules: list[Rule]) -> None:
         keys = self._get_ignore_keys(file_manager)
         file_manager.extend_list(keys=keys, values=rules)
 
-    def unignore_rules(self, rules: list[str]) -> None:
+        return True
+
+    def unignore_rules(self, rules: list[str]) -> bool:
         """Unignore Ruff rules in the project."""
         rules = list(set(rules) & set(self.get_ignored_rules()))
 
         if not rules:
-            return
+            return False
 
         rules_str = ", ".join([f"'{rule}'" for rule in rules])
         s = "" if len(rules) == 1 else "s"
@@ -350,13 +355,14 @@ def unignore_rules(self, rules: list[str]) -> None:
         )
         keys = self._get_ignore_keys(file_manager)
         file_manager.remove_from_list(keys=keys, values=rules)
+        return True
 
-    def deselect_rules(self, rules: list[Rule]) -> None:
+    def deselect_rules(self, rules: list[Rule]) -> bool:
         """Ensure Ruff rules are not selected in the project."""
         rules = list(set(rules) & set(self.get_selected_rules()))
 
         if not rules:
-            return
+            return False
 
         rules_str = ", ".join([f"'{rule}'" for rule in rules])
         s = "" if len(rules) == 1 else "s"
@@ -368,6 +374,7 @@ def deselect_rules(self, rules: list[Rule]) -> None:
         )
         keys = self._get_select_keys(file_manager)
         file_manager.remove_from_list(keys=keys, values=rules)
+        return True
 
     def get_selected_rules(self) -> list[Rule]:
         """Get the Ruff rules selected in the project."""
@@ -397,8 +404,14 @@ def ignore_rules_in_glob(self, rules: list[Rule], *, glob: str) -> None:
         if not rules:
             return
 
+        rules_str = ", ".join([f"'{rule}'" for rule in rules])
+        s = "" if len(rules) == 1 else "s"
+
         (file_manager,) = self.get_active_config_file_managers()
         ensure_managed_file_exists(file_manager)
+        tick_print(
+            f"Ignoring {self.name} rule{s} {rules_str} for '{glob}' in '{file_manager.name}'."
+        )
         keys = self._get_per_file_ignore_keys(file_manager, glob=glob)
         file_manager.extend_list(keys=keys, values=rules)
 
@@ -407,9 +420,19 @@ def apply_rule_config(self, rule_config: RuleConfig) -> None:
 
         Note, this will add both managed and unmanaged config.
         """
-        self.select_rules(rule_config.get_all_selected())
-        self.ignore_rules(rule_config.get_all_ignored())
-        self.ignore_rules_in_glob(rule_config.tests_unmanaged_ignored, glob="tests/**")
+        is_selected = self.select_rules(rule_config.get_all_selected())
+        is_ignored = self.ignore_rules(rule_config.get_all_ignored())
+
+        # We don't want to spam the user with verbose messages about per-file ignores.
+        # On the other hand, if we haven't displayed any messages at all, we need to
+        # avoid a misleading silence, which would imply we haven't modified a file.
+        # This is probably a workaround until there is more sophisticated support for
+        # verbosity control.
+        # https://github.com/usethis-python/usethis-python/issues/884
+        with usethis_config.set(alert_only=is_selected or is_ignored):
+            self.ignore_rules_in_glob(
+                rule_config.tests_unmanaged_ignored, glob="tests/**"
+            )
 
     def remove_rule_config(self, rule_config: RuleConfig) -> None:
         """Remove the Ruff rules associated with a rule config from the project.
diff --git a/tests/usethis/_core/test_core_tool.py b/tests/usethis/_core/test_core_tool.py
@@ -1352,16 +1352,54 @@ def test_inp_rules_selected(self, tmp_path: Path):
                 assert "INP" in RuffTool().get_selected_rules()
 
         @pytest.mark.usefixtures("_vary_network_conn")
-        def test_inp_rules_not_selected_for_tests_dir(self, tmp_path: Path):
+        def test_inp_rules_not_selected_for_tests_dir(
+            self, uv_init_dir: Path, capfd: pytest.CaptureFixture[str]
+        ):
             # Arrange
-            (tmp_path / "ruff.toml").touch()
+            (uv_init_dir / "ruff.toml").touch()
 
-            with change_cwd(tmp_path), files_manager():
+            with change_cwd(uv_init_dir), files_manager():
                 # Act
                 use_import_linter()
 
             # Assert
-            contents = (tmp_path / "ruff.toml").read_text()
+            contents = (uv_init_dir / "ruff.toml").read_text()
+            assert (
+                contents
+                == """\
+[lint]
+select = ["INP"]
+
+[lint.per-file-ignores]
+"tests/**" = ["INP"]
+"""
+            )
+            out, err = capfd.readouterr()
+            assert not err
+            assert out == (
+                "✔ Adding dependency 'import-linter' to the 'dev' group in 'pyproject.toml'.\n"
+                "☐ Install the dependency 'import-linter'.\n"
+                "✔ Adding Import Linter config to 'pyproject.toml'.\n"
+                "✔ Selecting Ruff rule 'INP' in 'ruff.toml'.\n"
+                "☐ Run 'uv run lint-imports' to run Import Linter.\n"
+            )
+
+        @pytest.mark.usefixtures("_vary_network_conn")
+        def test_message_for_already_selected_but_needs_ignoring(
+            self, uv_init_dir: Path, capfd: pytest.CaptureFixture[str]
+        ):
+            # Arrange
+            (uv_init_dir / "ruff.toml").write_text("""\
+[lint]
+select = ["INP"]
+""")
+
+            with change_cwd(uv_init_dir), files_manager():
+                # Act
+                use_import_linter()
+
+            # Assert
+            contents = (uv_init_dir / "ruff.toml").read_text()
             assert (
                 contents
                 == """\
@@ -1372,6 +1410,15 @@ def test_inp_rules_not_selected_for_tests_dir(self, tmp_path: Path):
 "tests/**" = ["INP"]
 """
             )
+            out, err = capfd.readouterr()
+            assert not err
+            assert out == (
+                "✔ Adding dependency 'import-linter' to the 'dev' group in 'pyproject.toml'.\n"
+                "☐ Install the dependency 'import-linter'.\n"
+                "✔ Adding Import Linter config to 'pyproject.toml'.\n"
+                "✔ Ignoring Ruff rule 'INP' for 'tests/**' in 'ruff.toml'.\n"
+                "☐ Run 'uv run lint-imports' to run Import Linter.\n"
+            )
 
         @pytest.mark.usefixtures("_vary_network_conn")
         def test_inp_rules_dont_break_config(self, uv_init_dir: Path):
