Skip to content

Commit a6a5e8d

Browse files
[ty] Fix precedence of all selector in TOML configurations (#23723)
## Summary Closes astral-sh/ty#2952. TOML tables are unordered — the spec says key/value pairs within tables are not guaranteed to be in any specific order. However, the `toml` crate happens to sort keys lexicographically when deserializing. This means that rules like `abstract-method-in-final-class` sort *before* `all`, so the `all` selector silently overrides the more-specific rule when processed sequentially. This PR fixes the issue by sorting selectors from the same configuration file so that `all` is always applied first, then specific per-rule selectors are applied afterwards. This ensures that specific rules always take precedence over `all` within a given config file, regardless of lexicographic ordering. The fix is scoped to file-based configuration only — CLI argument order is still preserved as-is, since users have explicit control over ordering there. ## Test plan Added a test (`configuration_all_rules_with_rule_sorting_before_all`) that uses `abstract-method-in-final-class` (which sorts before `all`) to verify the specific rule takes precedence.
1 parent 2a5384b commit a6a5e8d

File tree

3 files changed

+170
-2
lines changed

3 files changed

+170
-2
lines changed

crates/ty/tests/cli/rule_selection.rs

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,3 +1092,129 @@ fn configuration_all_rules() -> anyhow::Result<()> {
10921092

10931093
Ok(())
10941094
}
1095+
1096+
/// In TOML, key order in a table is not semantically meaningful, so specific rules should
1097+
/// still override `all` even if they sort lexicographically before `all`.
1098+
#[test]
1099+
fn configuration_all_rules_with_rule_sorting_before_all() -> anyhow::Result<()> {
1100+
let case = CliTest::with_files([
1101+
(
1102+
"pyproject.toml",
1103+
r#"
1104+
[tool.ty.rules]
1105+
all = "warn"
1106+
abstract-method-in-final-class = "error"
1107+
"#,
1108+
),
1109+
(
1110+
"test.py",
1111+
r#"
1112+
from typing import final
1113+
from abc import ABC, abstractmethod
1114+
1115+
class Base(ABC):
1116+
@abstractmethod
1117+
def foo(self) -> int:
1118+
raise NotImplementedError
1119+
1120+
@final
1121+
class Derived(Base):
1122+
pass
1123+
"#,
1124+
),
1125+
])?;
1126+
1127+
assert_cmd_snapshot!(case.command(), @"
1128+
success: false
1129+
exit_code: 1
1130+
----- stdout -----
1131+
error[abstract-method-in-final-class]: Final class `Derived` has unimplemented abstract methods
1132+
--> test.py:11:7
1133+
|
1134+
10 | @final
1135+
11 | class Derived(Base):
1136+
| ^^^^^^^ `foo` is unimplemented
1137+
12 | pass
1138+
|
1139+
::: test.py:7:9
1140+
|
1141+
5 | class Base(ABC):
1142+
6 | @abstractmethod
1143+
7 | def foo(self) -> int:
1144+
| --- `foo` declared as abstract on superclass `Base`
1145+
8 | raise NotImplementedError
1146+
|
1147+
info: rule `abstract-method-in-final-class` was selected in the configuration file
1148+
1149+
Found 1 diagnostic
1150+
1151+
----- stderr -----
1152+
");
1153+
1154+
Ok(())
1155+
}
1156+
1157+
/// Same TOML key ordering issue, but within an override's `[rules]` table.
1158+
/// `abstract-method-in-final-class` sorts before `all` lexicographically, but
1159+
/// the specific rule should still take precedence over `all`.
1160+
#[test]
1161+
fn overrides_all_rules_with_rule_sorting_before_all() -> anyhow::Result<()> {
1162+
let case = CliTest::with_files([
1163+
(
1164+
"pyproject.toml",
1165+
r#"
1166+
[[tool.ty.overrides]]
1167+
include = ["src/**"]
1168+
1169+
[tool.ty.overrides.rules]
1170+
all = "warn"
1171+
abstract-method-in-final-class = "error"
1172+
"#,
1173+
),
1174+
(
1175+
"src/test.py",
1176+
r#"
1177+
from typing import final
1178+
from abc import ABC, abstractmethod
1179+
1180+
class Base(ABC):
1181+
@abstractmethod
1182+
def foo(self) -> int:
1183+
raise NotImplementedError
1184+
1185+
@final
1186+
class Derived(Base):
1187+
pass
1188+
"#,
1189+
),
1190+
])?;
1191+
1192+
assert_cmd_snapshot!(case.command(), @"
1193+
success: false
1194+
exit_code: 1
1195+
----- stdout -----
1196+
error[abstract-method-in-final-class]: Final class `Derived` has unimplemented abstract methods
1197+
--> src/test.py:11:7
1198+
|
1199+
10 | @final
1200+
11 | class Derived(Base):
1201+
| ^^^^^^^ `foo` is unimplemented
1202+
12 | pass
1203+
|
1204+
::: src/test.py:7:9
1205+
|
1206+
5 | class Base(ABC):
1207+
6 | @abstractmethod
1208+
7 | def foo(self) -> int:
1209+
| --- `foo` declared as abstract on superclass `Base`
1210+
8 | raise NotImplementedError
1211+
|
1212+
info: rule `abstract-method-in-final-class` was selected in the configuration file
1213+
1214+
Found 1 diagnostic
1215+
1216+
----- stderr -----
1217+
");
1218+
1219+
Ok(())
1220+
}

crates/ty_project/src/metadata/options.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use ruff_python_ast::PythonVersion;
2222
use rustc_hash::FxHasher;
2323
use serde::{Deserialize, Serialize};
2424
use std::borrow::Cow;
25+
use std::cmp::Ordering;
2526
use std::fmt::{self, Debug, Display};
2627
use std::hash::BuildHasherDefault;
2728
use std::ops::Deref;
@@ -107,10 +108,42 @@ pub struct Options {
107108
impl Options {
108109
pub fn from_toml_str(content: &str, source: ValueSource) -> Result<Self, TyTomlError> {
109110
let _guard = ValueSourceGuard::new(source, true);
110-
let options = toml::from_str(content)?;
111+
let mut options: Self = toml::from_str(content)?;
112+
options.prioritize_all_selectors();
111113
Ok(options)
112114
}
113115

116+
/// Ensures that the `all` selector is applied before per-rule selectors
117+
/// in all rule tables (top-level and overrides).
118+
///
119+
/// This must be called after deserializing from TOML and before any
120+
/// [`Combine::combine`] calls, because TOML tables are unordered and the
121+
/// `toml` crate sorts keys lexicographically.
122+
pub(crate) fn prioritize_all_selectors(&mut self) {
123+
// Stable sort that moves all `all` selectors before non-`all` selectors
124+
// while preserving relative order among non-`all` entries.
125+
let sort = |rules: &mut Rules| {
126+
rules.inner.sort_by(
127+
|key_a, _, key_b, _| match (**key_a == "all", **key_b == "all") {
128+
(true, false) => Ordering::Less,
129+
(false, true) => Ordering::Greater,
130+
_ => Ordering::Equal,
131+
},
132+
);
133+
};
134+
135+
if let Some(rules) = &mut self.rules {
136+
sort(rules);
137+
}
138+
if let Some(overrides) = &mut self.overrides {
139+
for override_option in &mut overrides.0 {
140+
if let Some(rules) = &mut override_option.rules {
141+
sort(rules);
142+
}
143+
}
144+
}
145+
}
146+
114147
pub fn deserialize_with<'de, D>(source: ValueSource, deserializer: D) -> Result<Self, D::Error>
115148
where
116149
D: serde::Deserializer<'de>,

crates/ty_project/src/metadata/pyproject.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,16 @@ impl PyProject {
3535
source: ValueSource,
3636
) -> Result<Self, PyProjectError> {
3737
let _guard = ValueSourceGuard::new(source, true);
38-
toml::from_str(content).map_err(PyProjectError::TomlSyntax)
38+
let mut pyproject: Self = toml::from_str(content).map_err(PyProjectError::TomlSyntax)?;
39+
// TOML tables are unordered and the `toml` crate sorts keys
40+
// lexicographically. Normalize rule order so that the `all` selector
41+
// is applied before per-rule selectors.
42+
if let Some(tool) = &mut pyproject.tool {
43+
if let Some(ty) = &mut tool.ty {
44+
ty.prioritize_all_selectors();
45+
}
46+
}
47+
Ok(pyproject)
3948
}
4049
}
4150

0 commit comments

Comments
 (0)