Fix precedence of ALL selector in TOML configurations #2952
Description
Summary
astral-sh/ruff#22832 added support for the ALL selector, but the ALL selector doesn't (always) work as intended when loading configurations from TOML.
TOML doesn't specify any order for key/value pairs in tables:
Under that, and until the next header or EOF, are the key/values of that table. Key/value pairs within tables are not guaranteed to be in any specific order.
https://toml.io/en/v1.1.0#table
Meaning, the following two rules tables are semantically equivalent:
[rules]
unresolved-reference = "warn"
all = "ignore"[rules]
all = "ignore"
unresolved-reference = "warn"ty's current configuration handling, by accident, resolves both configurations to the same settings, with unresolved-reference taking precedence over all (probably what the user intended). However, this only works because the toml crate sorts table keys lexicographically
By default it entries are stored in lexicographic order of the keys. Enable the preserve_order feature to store entries in the order they appear in the source file.
https://docs.rs/toml/latest/toml/type.Table.html
However, all doesn't work as intended for rules sorting before all, e.g. abstract-method-in-final-class, as demonstrated by the following test:
/// `"abstract-method-in-final-class"` sorts lexicographically *before* `"all"`, so the `toml`
/// crate feeds it to the `OrderMap` first. Because `to_rule_selection` processes entries
/// sequentially, `all = "warn"` is applied second and silently overrides the specific
/// `abstract-method-in-final-class = "error"` setting — the opposite of what the user intended.
#[test]
fn configuration_all_overrides_rule_that_sorts_before_all() -> anyhow::Result<()> {
let case = CliTest::with_files([
(
"pyproject.toml",
r#"
[tool.ty.rules]
all = "warn"
abstract-method-in-final-class = "error"
"#,
),
(
"test.py",
r#"
from typing import final
from abc import ABC, abstractmethod
class Base(ABC):
@abstractmethod
def foo(self) -> int:
raise NotImplementedError
@final
class Derived(Base):
pass
"#,
),
])?;
// The user wrote `abstract-method-in-final-class = "error"` after `all = "warn"`,
// intending it as an override. But the `toml` crate sorts keys lexicographically,
// placing `abstract-method-in-final-class` *before* `all`. The sequential processing
// then applies `all = "warn"` last, overriding the specific rule back to `warn`.
assert_cmd_snapshot!(case.command(), @"
success: true
exit_code: 0
----- stdout -----
warning[abstract-method-in-final-class]: Final class `Derived` has unimplemented abstract methods
--> test.py:11:7
|
10 | @final
11 | class Derived(Base):
| ^^^^^^^ `foo` is unimplemented
12 | pass
|
::: test.py:7:9
|
5 | class Base(ABC):
6 | @abstractmethod
7 | def foo(self) -> int:
| --- `foo` declared as abstract on superclass `Base`
8 | raise NotImplementedError
|
info: rule `abstract-method-in-final-class` was selected in the configuration file
Found 1 diagnostic
----- stderr -----
");
Ok(())
}There are multiple solutions to this problem:
- Only when loading the configuration from a file, sort the
allselector before all other selectors (but preserve ordering otherwise). - Use a
priorityfield similar to cargo that allows users to explicitly express the precedence of different selectors (mainly important if we think we'll have overlapping selectors). - ...?
Version
No response