Skip to content

Commit 685d685

Browse files
nathanjmcdougallnathanjmcdougall
authored
Don't break Ruff TOML config when adding Import Linter (#865)
* Add failing tests * Use `_set_value_in_existing` in `TOMLFileManager.extend_list` to avoid deep nesting bug. * Rework `RuleConfig` docstring for clarity * Tweak test output in `test_inp_rules_dont_break_config` * Fix incorrect issue number in comments * Weaken test to drop irrelevant output * Generalize and strengthen `test_dependencies_added` * Set CWD explicitly in `test_target_python_version` tests * use `uv_init_dir` in `test_inp_rules_dont_break_config` test This might focus Import Linter to build a graph of an actual package rather than looking inan unrelated lib dir
1 parent 257e49f commit 685d685

8 files changed

Lines changed: 128 additions & 25 deletions

File tree

‎src/usethis/_integrations/file/toml/io_.py‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ def set_value(
167167
except KeyError:
168168
_set_value_in_existing(
169169
toml_document=toml_document,
170-
current_container=d,
170+
shared_container=d,
171+
shared_keys=shared_keys,
171172
keys=keys,
172-
current_keys=shared_keys,
173173
value=value,
174174
)
175175
except ValidationError:
@@ -179,9 +179,9 @@ def set_value(
179179
else:
180180
_set_value_in_existing(
181181
toml_document=toml_document,
182-
current_container=d,
182+
shared_keys=shared_keys,
183+
shared_container=d,
183184
keys=keys,
184-
current_keys=shared_keys,
185185
value=value,
186186
)
187187
else:
@@ -269,12 +269,14 @@ def extend_list(self, *, keys: Sequence[Key], values: list[Any]) -> None:
269269

270270
toml_document = copy.copy(self.get())
271271

272+
shared_keys: list[str] = []
273+
d = toml_document
272274
try:
273-
d = toml_document
274275
for key in keys[:-1]:
275276
TypeAdapter(dict).validate_python(d)
276277
assert isinstance(d, dict)
277278
d = d[key]
279+
shared_keys.append(key)
278280
p_parent = d
279281
TypeAdapter(dict).validate_python(p_parent)
280282
assert isinstance(p_parent, dict)
@@ -284,7 +286,13 @@ def extend_list(self, *, keys: Sequence[Key], values: list[Any]) -> None:
284286
for key in reversed(keys):
285287
contents = {key: contents}
286288
assert isinstance(contents, dict)
287-
toml_document = mergedeep.merge(toml_document, contents)
289+
_set_value_in_existing(
290+
toml_document=toml_document,
291+
shared_keys=shared_keys,
292+
shared_container=d,
293+
keys=keys,
294+
value=values,
295+
)
288296
assert isinstance(toml_document, TOMLDocument)
289297
except ValidationError:
290298
msg = (
@@ -349,11 +357,20 @@ def remove_from_list(self, *, keys: Sequence[Key], values: Collection[Any]) -> N
349357
def _set_value_in_existing(
350358
*,
351359
toml_document: TOMLDocument,
352-
current_container: TOMLDocument | Item | Container,
360+
shared_keys: Sequence[Key],
361+
shared_container: TOMLDocument | Item | Container,
353362
keys: Sequence[Key],
354-
current_keys: Sequence[Key],
355363
value: Any,
356364
) -> None:
365+
"""Set a new value in an existing container.
366+
367+
Args:
368+
toml_document: The overall document.
369+
shared_keys: Keys to the deepest container that actually exists.
370+
shared_container: The shared container itself that needs new contents.
371+
keys: Keys to the new value from the root of the document.
372+
value: The value at the keys.
373+
"""
357374
# The old configuration should be kept for all ID keys except the
358375
# final/deepest one which shouldn't exist anyway since we checked as much,
359376
# above. For example, if there is [tool.ruff] then we shouldn't overwrite it
@@ -369,12 +386,12 @@ def _set_value_in_existing(
369386
else:
370387
# Note that this alternative logic is just to avoid a bug:
371388
# https://github.com/usethis-python/usethis-python/issues/507
372-
TypeAdapter(dict).validate_python(current_container)
373-
assert isinstance(current_container, dict)
389+
TypeAdapter(dict).validate_python(shared_container)
390+
assert isinstance(shared_container, dict)
374391

375-
unshared_keys = keys[len(current_keys) :]
392+
unshared_keys = keys[len(shared_keys) :]
376393

377-
if len(current_keys) == 1:
394+
if len(shared_keys) == 1:
378395
# In this case, we need to "seed" the section to avoid another bug:
379396
# https://github.com/usethis-python/usethis-python/issues/558
380397

@@ -385,9 +402,9 @@ def _set_value_in_existing(
385402
for key in reversed(unshared_keys[1:]):
386403
contents = {key: contents}
387404

388-
current_container[keys[1]] = contents # type: ignore[reportAssignmentType]
405+
shared_container[keys[1]] = contents # type: ignore[reportAssignmentType]
389406
else:
390-
current_container[_get_unified_key(unshared_keys)] = value
407+
shared_container[_get_unified_key(unshared_keys)] = value
391408

392409

393410
def _validate_keys(keys: Sequence[Key]) -> list[str]:

‎src/usethis/_tool/rule.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@ class RuleConfig(BaseModel):
1414
"""Configuration for linter rules associated with a tool.
1515
1616
There is a distinction between selected and ignored rules. Selected rules are those
17-
which are enabled and will be run by the tool unless ignored. Ignored rules are
18-
those which are not run by the tool, even if they are selected. This follows the
19-
Ruff paradigm.
17+
which are enabled for the tool and will be used unless ignored. Ignored rules are
18+
those which are not run, even if they are selected. This follows the Ruff paradigm.
2019
2120
There is also a distinction between managed and unmanaged rule config. Managed
2221
selections (and ignores) are those which are managed exclusively by the one tool,

‎tests/usethis/_core/test_core_tool.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,6 +1373,40 @@ def test_inp_rules_not_selected_for_tests_dir(self, tmp_path: Path):
13731373
"""
13741374
)
13751375

1376+
@pytest.mark.usefixtures("_vary_network_conn")
1377+
def test_inp_rules_dont_break_config(self, uv_init_dir: Path):
1378+
# Arrange
1379+
(uv_init_dir / "pyproject.toml").write_text("""\
1380+
[project]
1381+
name = "test"
1382+
version = "0.1.0"
1383+
1384+
[tool.ruff]
1385+
line-length = 88
1386+
format.docstring-code-format = true
1387+
lint.select = [ "A", "C4", "E4", "E7", "E9", "F", "FLY", "FURB", "I", "INP", "PLE", "PLR", "PT", "RUF", "SIM", "UP" ]
1388+
lint.ignore = [ "PLR2004", "SIM108" ]
1389+
""")
1390+
1391+
with change_cwd(uv_init_dir), files_manager():
1392+
# Act
1393+
use_import_linter()
1394+
1395+
# Assert
1396+
contents = (uv_init_dir / "pyproject.toml").read_text()
1397+
assert contents.startswith("""\
1398+
[project]
1399+
name = "test"
1400+
version = "0.1.0"
1401+
1402+
[tool.ruff]
1403+
line-length = 88
1404+
format.docstring-code-format = true
1405+
lint.select = [ "A", "C4", "E4", "E7", "E9", "F", "FLY", "FURB", "I", "INP", "PLE", "PLR", "PT", "RUF", "SIM", "UP" ]
1406+
lint.per-file-ignores."tests/**" = ["INP"]
1407+
lint.ignore = [ "PLR2004", "SIM108" ]
1408+
""")
1409+
13761410
@pytest.mark.usefixtures("_vary_network_conn")
13771411
def test_ruff_passes(self, tmp_path: Path):
13781412
with change_cwd(tmp_path), files_manager():

‎tests/usethis/_integrations/ci/bitbucket/test_bitbucket_schema.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from usethis._integrations.file.pyproject_toml.requires_python import (
1010
get_requires_python,
1111
)
12-
from usethis._test import is_offline
12+
from usethis._test import change_cwd, is_offline
1313

1414

1515
class TestStep2:
@@ -42,7 +42,7 @@ def test_matches_schema_store(self):
4242
# TIP: go into debug mode to copy-and-paste into updated schema.json
4343
assert local_schema_json == online_schema_json
4444

45-
def test_target_python_version(self):
45+
def test_target_python_version(self, usethis_dev_dir: Path):
4646
# If this test fails, we should bump the version in the command in schema.py
47-
with PyprojectTOMLManager():
47+
with change_cwd(usethis_dev_dir), PyprojectTOMLManager():
4848
assert get_requires_python() == ">=3.10"

‎tests/usethis/_integrations/file/pyproject_toml/test_pyproject_toml_io_.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,5 +392,32 @@ def test_add_one(self, tmp_path: Path):
392392
== """\
393393
[tool.usethis]
394394
key = ["value1", "value2"]
395+
"""
396+
)
397+
398+
def test_deep_nesting_doesnt_break_config(self, tmp_path: Path):
399+
# https://github.com/usethis-python/usethis-python/issues/862
400+
# The issue is basically the same as this one though:
401+
# https://github.com/usethis-python/usethis-python/issues/507
402+
(tmp_path / "pyproject.toml").write_text("""\
403+
[tool.ruff]
404+
lint.select = [ "INP" ]
405+
""")
406+
407+
with change_cwd(tmp_path), PyprojectTOMLManager() as mgr:
408+
# Act
409+
mgr.extend_list(
410+
keys=["tool", "ruff", "lint", "per-file-ignores", "tests/**"],
411+
values=["INP"],
412+
)
413+
414+
# Assert
415+
contents = (tmp_path / "pyproject.toml").read_text()
416+
assert (
417+
contents
418+
== """\
419+
[tool.ruff]
420+
lint.select = [ "INP" ]
421+
lint.per-file-ignores."tests/**" = ["INP"]
395422
"""
396423
)

‎tests/usethis/_integrations/pre_commit/test_pre_commit_schema.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
get_requires_python,
1010
)
1111
from usethis._integrations.pre_commit.schema import HookDefinition, LocalRepo
12-
from usethis._test import is_offline
12+
from usethis._test import change_cwd, is_offline
1313

1414

1515
def test_multiple_per_repo():
@@ -44,7 +44,7 @@ def test_matches_schema_store(self):
4444
# TIP: go into debug mode to copy-and-paste into updated schema.json
4545
assert local_schema_json == online_schema_json.replace("\r\n", "\n\n")
4646

47-
def test_target_python_version(self):
47+
def test_target_python_version(self, usethis_dev_dir: Path):
4848
# If this test fails, we should bump the version in the command in schema.py
49-
with PyprojectTOMLManager():
49+
with change_cwd(usethis_dev_dir), PyprojectTOMLManager():
5050
assert get_requires_python() == ">=3.10"

‎tests/usethis/_interface/test_lint.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,5 @@ def test_dependencies_added(self, tmp_path: Path):
2323

2424
# Check Ruff formatter is not added
2525
txt = (tmp_path / "pyproject.toml").read_text()
26-
assert "ruff.format" not in txt
27-
assert "ruff.lint" in txt
26+
assert "format" not in txt
27+
assert "lint" in txt

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,3 +312,29 @@ def test_ruff_toml(self, tmp_path: Path):
312312
# Act
313313
with change_cwd(tmp_path), DotRuffTOMLManager():
314314
assert RuffTool().is_formatter_used()
315+
316+
class TestIgnoreRulesInGlob:
317+
def test_dont_break_config(self, tmp_path: Path):
318+
# https://github.com/usethis-python/usethis-python/issues/862
319+
# The issue is basically the same as this one though:
320+
# https://github.com/usethis-python/usethis-python/issues/507
321+
# Arrange
322+
(tmp_path / "pyproject.toml").write_text("""\
323+
[tool.ruff]
324+
lint.select = [ "INP" ]
325+
""")
326+
327+
with change_cwd(tmp_path), files_manager():
328+
# Act
329+
RuffTool().ignore_rules_in_glob(["INP"], glob="tests/**")
330+
331+
# Assert
332+
contents = (tmp_path / "pyproject.toml").read_text()
333+
assert (
334+
contents
335+
== """\
336+
[tool.ruff]
337+
lint.select = [ "INP" ]
338+
lint.per-file-ignores."tests/**" = ["INP"]
339+
"""
340+
)

0 commit comments

Comments
 (0)