Commit 59e984f
authored
Report an empty TOML command value as a handled error (#4041)
`install_command = []` in TOML raises `AttributeError: 'NoneType' object
has no attribute 'args'` with a traceback and exit code 2.
`list_dependencies_command = []` does the same. The ini spelling
`install_command=` reports `unable to determine pip install command:
attempting to parse '' into a command failed` and exits 1.
## The cause
`Convert.to_command` is declared `Command | None`, and the two loaders
disagree on what `None` means. `StrConvert.to_command("")` raises
`ValueError`; `TomlLoader.to_command([])` returns `None`, which is what
lets #3388 drop empty entries inside `commands`. For a scalar
`of_type=Command` that `None` is cast straight through to the caller,
which then dereferences it.
## The fix
`to_command` now returns a `Command` or raises, in every loader. The
TOML rule that an empty entry inside a list of commands is dropped moves
into `TomlLoader.to_list`, which is where `StrConvert.to_list` already
drops empty tokens. `convert.py` loses its `Command`-shaped branch in
the shared list path, so the format-specific rule no longer leaks into
the format-agnostic layer.
Making the abstract return type `Command` also surfaced that
`StrConvert.to_command` never returned `None` in the first place - `ty`
rejected the override until the annotation was corrected.
Both formats now stop before the dereference:
```
$ printf '[env.py]\ninstall_command = []\n' > tox.toml && tox r -e py
py: failed to load py.install_command: attempting to parse [] into a command failed
```
Drive-by: `TomlLoader.to_set` was a byte-for-byte copy of `to_list` and
now delegates to it.
## Verification
`test_toml_loader_command_empty_nok` covers the scalar rejection and
`test_toml_loader_command_list_drops_empty` pins the #3388 behaviour;
removing either half of the change fails the matching test.
`test_pip_toml_empty_command_error` is parametrised over
`install_command` and `list_dependencies_command`, the two scalar
`Command` settings.
Full suite passes, as do `tox -e type` (ty, mypy, pyrefly) and `tox -e
fix`.
- [x] ran the linter to address style issues (`tox -e fix`)
- [x] wrote descriptive pull request text
- [x] ensured there are test(s) validating the fix
- [x] added news fragment in `docs/changelog` folder
- [ ] updated/extended the documentation
Co-authored-by: Dylan Pulver <dylanpulver@users.noreply.github.com>1 parent fcb513e commit 59e984f
7 files changed
Lines changed: 47 additions & 13 deletions
File tree
- docs/changelog
- src/tox/config/loader
- toml
- tests
- config/loader
- tox_env/python/pip
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
63 | 63 | | |
64 | 64 | | |
65 | 65 | | |
66 | | - | |
67 | | - | |
68 | 66 | | |
69 | 67 | | |
70 | 68 | | |
| |||
180 | 178 | | |
181 | 179 | | |
182 | 180 | | |
183 | | - | |
| 181 | + | |
184 | 182 | | |
185 | 183 | | |
186 | 184 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
53 | 53 | | |
54 | 54 | | |
55 | 55 | | |
56 | | - | |
| 56 | + | |
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
74 | 74 | | |
75 | 75 | | |
76 | 76 | | |
77 | | - | |
| 77 | + | |
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
105 | 105 | | |
106 | 106 | | |
107 | 107 | | |
108 | | - | |
109 | | - | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
110 | 113 | | |
111 | 114 | | |
112 | 115 | | |
113 | | - | |
114 | | - | |
| 116 | + | |
115 | 117 | | |
116 | 118 | | |
117 | 119 | | |
| |||
123 | 125 | | |
124 | 126 | | |
125 | 127 | | |
126 | | - | |
127 | | - | |
128 | | - | |
129 | | - | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
130 | 133 | | |
131 | 134 | | |
132 | 135 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
122 | 122 | | |
123 | 123 | | |
124 | 124 | | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
125 | 135 | | |
126 | 136 | | |
127 | 137 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
| 15 | + | |
14 | 16 | | |
15 | 17 | | |
16 | 18 | | |
| |||
50 | 52 | | |
51 | 53 | | |
52 | 54 | | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
53 | 74 | | |
54 | 75 | | |
55 | 76 | | |
| |||
0 commit comments