Skip to content

Commit c630de4

Browse files
Copilotnathanjmcdougall
Copilot
and
nathanjmcdougall
authored
Add managed per-file-ignore support; ignore RUF059 in tests by default (#1463)
* Initial plan * Add managed per-file-ignore support and ignore RUF059 in tests by default - Add `tests_ignored` and `nontests_ignored` fields to `RuleConfig` for managed per-file-ignores (can be removed when tool is removed) - Add `unignore_rules_in_glob` method to `RuffTool` for removing per-file-ignores - Update `apply_rule_config` to handle managed per-file-ignores - Update `remove_rule_config` to handle managed per-file-ignores - Add `RUF059` to `_get_basic_rule_config()` as a `tests_ignored` rule - Add tests for new behavior - Update existing test expectations Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> Agent-Logs-Url: https://github.com/usethis-python/usethis-python/sessions/1a70829b-ea2b-443e-9475-5cd4a579a3b9 * Apply formatter fixes from prek Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> Agent-Logs-Url: https://github.com/usethis-python/usethis-python/sessions/1a70829b-ea2b-443e-9475-5cd4a579a3b9 * Improve comment clarity in subset_related_to_tests Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> Agent-Logs-Url: https://github.com/usethis-python/usethis-python/sessions/1a70829b-ea2b-443e-9475-5cd4a579a3b9 * Fix hallucinated ticket ID * Add tests for full coverage of RuleConfig.__repr__ Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> Agent-Logs-Url: https://github.com/usethis-python/usethis-python/sessions/692d2eb5-b2c9-4970-8ec5-b1c5737da738 * Apply ruff formatter --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> Co-authored-by: Nathan McDougall <nathan.j.mcdougall@gmail.com>
1 parent b0505d5 commit c630de4

6 files changed

Lines changed: 181 additions & 2 deletions

File tree

‎src/usethis/_core/tool.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,9 @@ def _get_basic_rule_config() -> RuleConfig:
509509
"PLR2004", # https://github.com/usethis-python/usethis-python/issues/105
510510
"SIM108", # https://github.com/usethis-python/usethis-python/issues/118
511511
],
512+
tests_ignored=[
513+
"RUF059", # https://github.com/usethis-python/usethis-python/issues/907
514+
],
512515
)
513516

514517
return rule_config

‎src/usethis/_tool/impl/base/ruff.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,24 @@ def ignore_rules_in_glob(self, rules: Sequence[Rule], *, glob: str) -> None:
242242
keys = self._get_per_file_ignore_keys(file_manager, glob=glob)
243243
file_manager.extend_list(keys=keys, values=rules)
244244

245+
def unignore_rules_in_glob(self, rules: Sequence[Rule], *, glob: str) -> None:
246+
"""Stop ignoring Ruff rules in the project for a specific glob pattern."""
247+
rules = sorted(set(rules) & set(self.get_ignored_rules_in_glob(glob)))
248+
249+
if not rules:
250+
return
251+
252+
rules_str = ", ".join([f"'{rule}'" for rule in rules])
253+
s = "" if len(rules) == 1 else "s"
254+
255+
(file_manager,) = self.get_active_config_file_managers()
256+
ensure_managed_file_exists(file_manager)
257+
tick_print(
258+
f"No longer ignoring {self.name} rule{s} {rules_str} for '{glob}' in '{file_manager.name}'."
259+
)
260+
keys = self._get_per_file_ignore_keys(file_manager, glob=glob)
261+
file_manager.remove_from_list(keys=keys, values=rules)
262+
245263
def get_ignored_rules_in_glob(self, glob: str) -> list[Rule]:
246264
"""Get the Ruff rules ignored in the project for a specific glob pattern."""
247265
(file_manager,) = self.get_active_config_file_managers()
@@ -273,6 +291,10 @@ def apply_rule_config(self, rule_config: RuleConfig) -> None:
273291
):
274292
# Only add test-related directory ignore rules if the tests directory exists
275293
if (usethis_config.cpd() / "tests").exists():
294+
self.ignore_rules_in_glob(rule_config.tests_ignored, glob="tests/**")
295+
self.ignore_rules_in_glob(
296+
rule_config.nontests_ignored, glob="!tests/**/*.py"
297+
)
276298
self.ignore_rules_in_glob(
277299
rule_config.tests_unmanaged_ignored, glob="tests/**"
278300
)
@@ -287,6 +309,8 @@ def remove_rule_config(self, rule_config: RuleConfig) -> None:
287309
"""
288310
self.deselect_rules(rule_config.selected)
289311
self.unignore_rules(rule_config.ignored)
312+
self.unignore_rules_in_glob(rule_config.tests_ignored, glob="tests/**")
313+
self.unignore_rules_in_glob(rule_config.nontests_ignored, glob="!tests/**/*.py")
290314

291315
def set_docstyle(self, style: Literal["numpy", "google", "pep257"]) -> None:
292316
(file_manager,) = self.get_active_config_file_managers()

‎src/usethis/_tool/rule.py‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ class RuleConfig(BaseModel):
2929
ignored: Managed ignored rules.
3030
unmanaged_selected: Unmanaged selected rules.
3131
unmanaged_ignored: Unmanaged ignored rules.
32+
tests_ignored: Managed cases of rules ignored for specifically the tests
33+
directory.
34+
nontests_ignored: Managed cases of rules ignored for specifically non-test
35+
directories (using !tests/**/*.py glob).
3236
tests_unmanaged_ignored: Unmanaged cases of rules ignored for specifically the
3337
tests directory.
3438
nontests_unmanaged_ignored: Unmanaged cases of rules ignored for specifically
@@ -39,6 +43,8 @@ class RuleConfig(BaseModel):
3943
ignored: list[Rule] = Field(default_factory=list)
4044
unmanaged_selected: list[Rule] = Field(default_factory=list)
4145
unmanaged_ignored: list[Rule] = Field(default_factory=list)
46+
tests_ignored: list[Rule] = Field(default_factory=list)
47+
nontests_ignored: list[Rule] = Field(default_factory=list)
4248
tests_unmanaged_ignored: list[Rule] = Field(default_factory=list)
4349
nontests_unmanaged_ignored: list[Rule] = Field(default_factory=list)
4450

@@ -61,6 +67,10 @@ def subset_related_to_tests(self) -> Self:
6167
self.ignored = []
6268
self.unmanaged_selected = []
6369
self.unmanaged_ignored = []
70+
# tests_ignored, nontests_ignored, tests_unmanaged_ignored, and
71+
# nontests_unmanaged_ignored are preserved because they are per-file-ignores
72+
# specific to test directories and should not be cleared when removing global
73+
# rules.
6474
return self
6575

6676
@property
@@ -71,14 +81,21 @@ def empty(self) -> bool:
7181
and not self.ignored
7282
and not self.unmanaged_selected
7383
and not self.unmanaged_ignored
84+
and not self.tests_ignored
85+
and not self.nontests_ignored
7486
and not self.tests_unmanaged_ignored
7587
and not self.nontests_unmanaged_ignored
7688
)
7789

7890
@property
7991
def is_related_to_tests(self) -> bool:
8092
"""Check if the rule config has any tests-related rules."""
81-
return bool(self.tests_unmanaged_ignored or self.nontests_unmanaged_ignored)
93+
return bool(
94+
self.tests_ignored
95+
or self.nontests_ignored
96+
or self.tests_unmanaged_ignored
97+
or self.nontests_unmanaged_ignored
98+
)
8299

83100
@override
84101
def __repr__(self) -> str:
@@ -92,6 +109,10 @@ def __repr__(self) -> str:
92109
args.append(f"unmanaged_selected={self.unmanaged_selected}")
93110
if self.unmanaged_ignored:
94111
args.append(f"unmanaged_ignored={self.unmanaged_ignored}")
112+
if self.tests_ignored:
113+
args.append(f"tests_ignored={self.tests_ignored}")
114+
if self.nontests_ignored:
115+
args.append(f"nontests_ignored={self.nontests_ignored}")
95116
if self.tests_unmanaged_ignored:
96117
args.append(f"tests_unmanaged_ignored={self.tests_unmanaged_ignored}")
97118
if self.nontests_unmanaged_ignored:
@@ -120,6 +141,8 @@ def __or__(self, other: Self) -> Self:
120141
new.ignored = self.ignored + other.ignored
121142
new.unmanaged_selected = self.unmanaged_selected + other.unmanaged_selected
122143
new.unmanaged_ignored = self.unmanaged_ignored + other.unmanaged_ignored
144+
new.tests_ignored = self.tests_ignored + other.tests_ignored
145+
new.nontests_ignored = self.nontests_ignored + other.nontests_ignored
123146
new.tests_unmanaged_ignored = (
124147
self.tests_unmanaged_ignored + other.tests_unmanaged_ignored
125148
)

‎tests/usethis/_core/test_core_tool.py‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3007,7 +3007,7 @@ def test_import_linter_inp_rules(self, tmp_path: Path):
30073007
# Assert
30083008
with change_cwd(tmp_path), files_manager():
30093009
contents = (tmp_path / "ruff.toml").read_text()
3010-
assert """"tests/**" = ["INP"]""" in contents
3010+
assert """"tests/**" = ["RUF059", "INP"]""" in contents
30113011

30123012
class TestBitbucketIntegration:
30133013
def test_no_backend(
@@ -3679,6 +3679,30 @@ def test_doesnt_overwrite_existing_line_length(self, uv_init_dir: Path):
36793679
# Assert
36803680
assert RuffTOMLManager()[["line-length"]] == 100
36813681

3682+
@pytest.mark.usefixtures("_vary_network_conn")
3683+
def test_ruf059_ignored_in_tests(self, uv_init_dir: Path):
3684+
# https://github.com/usethis-python/usethis-python/issues/1186
3685+
# Arrange
3686+
(uv_init_dir / "tests").mkdir()
3687+
3688+
# Act
3689+
with change_cwd(uv_init_dir), files_manager():
3690+
use_ruff()
3691+
3692+
# Assert
3693+
assert "RUF059" in RuffTool().get_ignored_rules_in_glob("tests/**")
3694+
3695+
@pytest.mark.usefixtures("_vary_network_conn")
3696+
def test_ruf059_not_ignored_in_tests_without_tests_dir(self, uv_init_dir: Path):
3697+
# Arrange - no tests/ directory
3698+
3699+
# Act
3700+
with change_cwd(uv_init_dir), files_manager():
3701+
use_ruff()
3702+
3703+
# Assert
3704+
assert "RUF059" not in RuffTool().get_ignored_rules_in_glob("tests/**")
3705+
36823706
def test_only_add_linter(
36833707
self, uv_init_dir: Path, capfd: pytest.CaptureFixture[str]
36843708
):

‎tests/usethis/_tool/impl/base/test_ruff.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,40 @@ def test_dont_break_config(self, tmp_path: Path):
342342
"""
343343
)
344344

345+
class TestUnignoreRulesInGlob:
346+
def test_removes_rule(self, tmp_path: Path):
347+
# Arrange
348+
(tmp_path / "pyproject.toml").write_text("""\
349+
[tool.ruff]
350+
lint.select = [ "RUF" ]
351+
lint.per-file-ignores."tests/**" = ["RUF059"]
352+
""")
353+
354+
with change_cwd(tmp_path), files_manager():
355+
# Act
356+
RuffTool().unignore_rules_in_glob(["RUF059"], glob="tests/**")
357+
358+
# Assert
359+
contents = (tmp_path / "pyproject.toml").read_text()
360+
assert "RUF059" not in contents
361+
362+
def test_no_op_when_not_ignored(
363+
self, tmp_path: Path, capfd: pytest.CaptureFixture[str]
364+
):
365+
# Arrange
366+
(tmp_path / "pyproject.toml").write_text("""\
367+
[tool.ruff]
368+
lint.select = [ "RUF" ]
369+
""")
370+
371+
with change_cwd(tmp_path), files_manager():
372+
# Act
373+
RuffTool().unignore_rules_in_glob(["RUF059"], glob="tests/**")
374+
375+
# Assert - no changes, no output
376+
out, _err = capfd.readouterr()
377+
assert "RUF059" not in out
378+
345379
class TestAddConfig:
346380
def test_empty_dir(self, tmp_path: Path):
347381
# Expect ruff.toml to be preferred

‎tests/usethis/_tool/test_rule.py‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
from usethis._tool.rule import RuleConfig
2+
3+
4+
class TestRepr:
5+
def test_empty(self):
6+
assert repr(RuleConfig()) == "RuleConfig()"
7+
8+
def test_selected(self):
9+
assert repr(RuleConfig(selected=["A"])) == "RuleConfig(selected=['A'])"
10+
11+
def test_ignored(self):
12+
assert repr(RuleConfig(ignored=["A"])) == "RuleConfig(ignored=['A'])"
13+
14+
def test_unmanaged_selected(self):
15+
assert (
16+
repr(RuleConfig(unmanaged_selected=["A"]))
17+
== "RuleConfig(unmanaged_selected=['A'])"
18+
)
19+
20+
def test_unmanaged_ignored(self):
21+
assert (
22+
repr(RuleConfig(unmanaged_ignored=["A"]))
23+
== "RuleConfig(unmanaged_ignored=['A'])"
24+
)
25+
26+
def test_tests_ignored(self):
27+
assert (
28+
repr(RuleConfig(tests_ignored=["A"])) == "RuleConfig(tests_ignored=['A'])"
29+
)
30+
31+
def test_nontests_ignored(self):
32+
assert (
33+
repr(RuleConfig(nontests_ignored=["A"]))
34+
== "RuleConfig(nontests_ignored=['A'])"
35+
)
36+
37+
def test_tests_unmanaged_ignored(self):
38+
assert (
39+
repr(RuleConfig(tests_unmanaged_ignored=["A"]))
40+
== "RuleConfig(tests_unmanaged_ignored=['A'])"
41+
)
42+
43+
def test_nontests_unmanaged_ignored(self):
44+
assert (
45+
repr(RuleConfig(nontests_unmanaged_ignored=["A"]))
46+
== "RuleConfig(nontests_unmanaged_ignored=['A'])"
47+
)
48+
49+
def test_all_fields(self):
50+
rc = RuleConfig(
51+
selected=["A"],
52+
ignored=["B"],
53+
unmanaged_selected=["C"],
54+
unmanaged_ignored=["D"],
55+
tests_ignored=["E"],
56+
nontests_ignored=["F"],
57+
tests_unmanaged_ignored=["G"],
58+
nontests_unmanaged_ignored=["H"],
59+
)
60+
assert repr(rc) == (
61+
"RuleConfig("
62+
"selected=['A'], "
63+
"ignored=['B'], "
64+
"unmanaged_selected=['C'], "
65+
"unmanaged_ignored=['D'], "
66+
"tests_ignored=['E'], "
67+
"nontests_ignored=['F'], "
68+
"tests_unmanaged_ignored=['G'], "
69+
"nontests_unmanaged_ignored=['H']"
70+
")"
71+
)

0 commit comments

Comments
 (0)