RUF023: Don't sort __match_args__, only __slots__ (#9724)

Fixes #9723. I'm pretty embarrassed I forgot that order was important
here :(
This commit is contained in:
Alex Waygood 2024-01-30 22:44:49 +00:00 committed by GitHub
parent 541aef4e6c
commit 6bb126415d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 82 additions and 123 deletions

View file

@ -4,14 +4,14 @@
class Klass:
__slots__ = ["d", "c", "b", "a"] # a comment that is untouched
__match_args__ = ("d", "c", "b", "a")
__slots__ = ("d", "c", "b", "a")
# Quoting style is retained,
# but unnecessary parens are not
__slots__: set = {'b', "c", ((('a')))}
# Trailing commas are also not retained for single-line definitions
# (but they are in multiline definitions)
__match_args__: tuple = ("b", "c", "a",)
__slots__: tuple = ("b", "c", "a",)
class Klass2:
if bool():
@ -19,7 +19,7 @@ class Klass2:
else:
__slots__ = "foo3", "foo2", "foo1" # NB: an implicit tuple (without parens)
__match_args__: list[str] = ["the", "three", "little", "pigs"]
__slots__: list[str] = ["the", "three", "little", "pigs"]
__slots__ = ("parenthesized_item"), "in", ("an_unparenthesized_tuple")
# we use natural sort,
# not alphabetical sort or "isort-style" sort
@ -37,7 +37,7 @@ class Klass3:
# a comment regarding 'a0':
"a0"
)
__match_args__ = [
__slots__ = [
"d",
"c", # a comment regarding 'c'
"b",
@ -61,7 +61,7 @@ class Klass4:
) # comment6
# comment7
__match_args__ = [ # comment0
__slots__ = [ # comment0
# comment1
# comment2
"dx", "cx", "bx", "ax" # comment3
@ -139,7 +139,7 @@ class SlotUser:
'distance': 'measured in kilometers'}
class Klass5:
__match_args__ = (
__slots__ = (
"look",
(
"a_veeeeeeeeeeeeeeeeeeery_long_parenthesized_item"
@ -194,14 +194,14 @@ class BezierBuilder4:
class Klass6:
__slots__ = ()
__match_args__ = []
__slots__ = []
__slots__ = ("single_item",)
__match_args__ = (
__slots__ = (
"single_item_multiline",
)
__slots__ = {"single_item",}
__slots__ = {"single_item_no_trailing_comma": "docs for that"}
__match_args__ = [
__slots__ = [
"single_item_multiline_no_trailing_comma"
]
__slots__ = ("not_a_tuple_just_a_string")
@ -218,11 +218,11 @@ class Klass6:
__slots__ = ("b", "a", "e", "d")
__slots__ = ["b", "a", "e", "d"]
__match_args__ = ["foo", "bar", "antipasti"]
__slots__ = ["foo", "bar", "antipasti"]
class Klass6:
__slots__ = (9, 8, 7)
__match_args__ = ( # This is just an empty tuple,
__slots__ = ( # This is just an empty tuple,
# but,
# it's very well
) # documented
@ -245,10 +245,10 @@ class Klass6:
__slots__ = [
()
]
__match_args__ = (
__slots__ = (
()
)
__match_args__ = (
__slots__ = (
[]
)
__slots__ = (
@ -257,12 +257,9 @@ class Klass6:
__slots__ = (
[],
)
__match_args__ = (
__slots__ = (
"foo", [], "bar"
)
__match_args__ = [
__slots__ = [
"foo", (), "bar"
]
__match_args__ = {"a", "set", "for", "__match_args__", "is invalid"}
__match_args__ = {"this": "is", "also": "invalid"}

View file

@ -2,7 +2,7 @@
///
/// Examples where these are useful:
/// - Sorting `__all__` in the global scope,
/// - Sorting `__slots__` or `__match_args__` in a class scope
/// - Sorting `__slots__` in a class scope
use std::borrow::Cow;
use std::cmp::Ordering;

View file

@ -1,5 +1,4 @@
use std::borrow::Cow;
use std::fmt::Display;
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
@ -17,14 +16,12 @@ use crate::rules::ruff::rules::sequence_sorting::{
use itertools::izip;
/// ## What it does
/// Checks for `__slots__` and `__match_args__`
/// definitions that are not ordered according to a
/// Checks for `__slots__` definitions that are not ordered according to a
/// [natural sort](https://en.wikipedia.org/wiki/Natural_sort_order).
///
/// ## Why is this bad?
/// Consistency is good. Use a common convention for
/// these special variables to make your code more
/// readable and idiomatic.
/// Consistency is good. Use a common convention for this special variable
/// to make your code more readable and idiomatic.
///
/// ## Example
/// ```python
@ -40,7 +37,6 @@ use itertools::izip;
#[violation]
pub struct UnsortedDunderSlots {
class_name: String,
class_variable: SpecialClassDunder,
}
impl Violation for UnsortedDunderSlots {
@ -48,43 +44,18 @@ impl Violation for UnsortedDunderSlots {
#[derive_message_formats]
fn message(&self) -> String {
let UnsortedDunderSlots {
class_name,
class_variable,
} = self;
format!("`{class_name}.{class_variable}` is not sorted")
format!("`{}.__slots__` is not sorted", self.class_name)
}
fn fix_title(&self) -> Option<String> {
let UnsortedDunderSlots {
class_name,
class_variable,
} = self;
Some(format!(
"Apply a natural sort to `{class_name}.{class_variable}`"
"Apply a natural sort to `{}.__slots__`",
self.class_name
))
}
}
/// Enumeration of the two special class dunders
/// that we're interested in for this rule: `__match_args__` and `__slots__`
#[derive(Debug, Eq, PartialEq, Copy, Clone)]
enum SpecialClassDunder {
Slots,
MatchArgs,
}
impl Display for SpecialClassDunder {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let string = match self {
Self::MatchArgs => "__match_args__",
Self::Slots => "__slots__",
};
write!(f, "{string}")
}
}
/// Sort a `__slots__`/`__match_args__` definition
/// Sort a `__slots__` definition
/// represented by a `StmtAssign` AST node.
/// For example: `__slots__ = ["b", "c", "a"]`.
pub(crate) fn sort_dunder_slots_assign(
@ -96,7 +67,7 @@ pub(crate) fn sort_dunder_slots_assign(
}
}
/// Sort a `__slots__`/`__match_args__` definition
/// Sort a `__slots__` definition
/// represented by a `StmtAnnAssign` AST node.
/// For example: `__slots__: list[str] = ["b", "c", "a"]`.
pub(crate) fn sort_dunder_slots_ann_assign(checker: &mut Checker, node: &ast::StmtAnnAssign) {
@ -107,8 +78,7 @@ pub(crate) fn sort_dunder_slots_ann_assign(checker: &mut Checker, node: &ast::St
const SORTING_STYLE: SortingStyle = SortingStyle::Natural;
/// Sort a tuple, list, dict or set that defines `__slots__`
/// or `__match_args__` in a class scope.
/// Sort a tuple, list, dict or set that defines `__slots__` in a class scope.
///
/// This routine checks whether the display is sorted, and emits a
/// violation if it is not sorted. If the tuple/list/set was not sorted,
@ -118,13 +88,11 @@ fn sort_dunder_slots(checker: &mut Checker, target: &ast::Expr, node: &ast::Expr
return;
};
let dunder_kind = match id.as_str() {
"__slots__" => SpecialClassDunder::Slots,
"__match_args__" => SpecialClassDunder::MatchArgs,
_ => return,
};
if id != "__slots__" {
return;
}
// We're only interested in `__slots__`/`__match_args__` in the class scope
// We're only interested in `__slots__` in the class scope
let ScopeKind::Class(ast::StmtClassDef {
name: class_name, ..
}) = checker.semantic().current_scope().kind
@ -132,7 +100,7 @@ fn sort_dunder_slots(checker: &mut Checker, target: &ast::Expr, node: &ast::Expr
return;
};
let Some(display) = StringLiteralDisplay::new(node, dunder_kind) else {
let Some(display) = StringLiteralDisplay::new(node) else {
return;
};
@ -144,7 +112,6 @@ fn sort_dunder_slots(checker: &mut Checker, target: &ast::Expr, node: &ast::Expr
let mut diagnostic = Diagnostic::new(
UnsortedDunderSlots {
class_name: class_name.to_string(),
class_variable: dunder_kind,
},
display.range,
);
@ -179,9 +146,9 @@ impl Ranged for StringLiteralDisplay<'_> {
}
impl<'a> StringLiteralDisplay<'a> {
fn new(node: &'a ast::Expr, dunder_kind: SpecialClassDunder) -> Option<Self> {
let result = match (dunder_kind, node) {
(_, ast::Expr::List(ast::ExprList { elts, range, .. })) => {
fn new(node: &'a ast::Expr) -> Option<Self> {
let result = match node {
ast::Expr::List(ast::ExprList { elts, range, .. }) => {
let display_kind = DisplayKind::Sequence(SequenceKind::List);
Self {
elts: Cow::Borrowed(elts),
@ -189,7 +156,7 @@ impl<'a> StringLiteralDisplay<'a> {
display_kind,
}
}
(_, ast::Expr::Tuple(tuple_node @ ast::ExprTuple { elts, range, .. })) => {
ast::Expr::Tuple(tuple_node @ ast::ExprTuple { elts, range, .. }) => {
let display_kind = DisplayKind::Sequence(SequenceKind::Tuple(tuple_node));
Self {
elts: Cow::Borrowed(elts),
@ -197,7 +164,7 @@ impl<'a> StringLiteralDisplay<'a> {
display_kind,
}
}
(SpecialClassDunder::Slots, ast::Expr::Set(ast::ExprSet { elts, range })) => {
ast::Expr::Set(ast::ExprSet { elts, range }) => {
let display_kind = DisplayKind::Sequence(SequenceKind::Set);
Self {
elts: Cow::Borrowed(elts),
@ -205,21 +172,16 @@ impl<'a> StringLiteralDisplay<'a> {
display_kind,
}
}
(
SpecialClassDunder::Slots,
ast::Expr::Dict(ast::ExprDict {
keys,
values,
range,
}),
) => {
ast::Expr::Dict(ast::ExprDict {
keys,
values,
range,
}) => {
let mut narrowed_keys = Vec::with_capacity(values.len());
for key in keys {
if let Some(key) = key {
// This is somewhat unfortunate,
// *but* only `__slots__` can be a dict out of
// `__all__`, `__slots__` and `__match_args__`,
// and even for `__slots__`, using a dict is very rare
// *but* using a dict for __slots__ is very rare
narrowed_keys.push(key.to_owned());
} else {
return None;

View file

@ -6,7 +6,7 @@ RUF023.py:6:17: RUF023 [*] `Klass.__slots__` is not sorted
5 | class Klass:
6 | __slots__ = ["d", "c", "b", "a"] # a comment that is untouched
| ^^^^^^^^^^^^^^^^^^^^ RUF023
7 | __match_args__ = ("d", "c", "b", "a")
7 | __slots__ = ("d", "c", "b", "a")
|
= help: Apply a natural sort to `Klass.__slots__`
@ -16,27 +16,27 @@ RUF023.py:6:17: RUF023 [*] `Klass.__slots__` is not sorted
5 5 | class Klass:
6 |- __slots__ = ["d", "c", "b", "a"] # a comment that is untouched
6 |+ __slots__ = ["a", "b", "c", "d"] # a comment that is untouched
7 7 | __match_args__ = ("d", "c", "b", "a")
7 7 | __slots__ = ("d", "c", "b", "a")
8 8 |
9 9 | # Quoting style is retained,
RUF023.py:7:22: RUF023 [*] `Klass.__match_args__` is not sorted
RUF023.py:7:17: RUF023 [*] `Klass.__slots__` is not sorted
|
5 | class Klass:
6 | __slots__ = ["d", "c", "b", "a"] # a comment that is untouched
7 | __match_args__ = ("d", "c", "b", "a")
| ^^^^^^^^^^^^^^^^^^^^ RUF023
7 | __slots__ = ("d", "c", "b", "a")
| ^^^^^^^^^^^^^^^^^^^^ RUF023
8 |
9 | # Quoting style is retained,
|
= help: Apply a natural sort to `Klass.__match_args__`
= help: Apply a natural sort to `Klass.__slots__`
Safe fix
4 4 |
5 5 | class Klass:
6 6 | __slots__ = ["d", "c", "b", "a"] # a comment that is untouched
7 |- __match_args__ = ("d", "c", "b", "a")
7 |+ __match_args__ = ("a", "b", "c", "d")
7 |- __slots__ = ("d", "c", "b", "a")
7 |+ __slots__ = ("a", "b", "c", "d")
8 8 |
9 9 | # Quoting style is retained,
10 10 | # but unnecessary parens are not
@ -60,25 +60,25 @@ RUF023.py:11:22: RUF023 [*] `Klass.__slots__` is not sorted
11 |+ __slots__: set = {'a', 'b', "c"}
12 12 | # Trailing commas are also not retained for single-line definitions
13 13 | # (but they are in multiline definitions)
14 14 | __match_args__: tuple = ("b", "c", "a",)
14 14 | __slots__: tuple = ("b", "c", "a",)
RUF023.py:14:29: RUF023 [*] `Klass.__match_args__` is not sorted
RUF023.py:14:24: RUF023 [*] `Klass.__slots__` is not sorted
|
12 | # Trailing commas are also not retained for single-line definitions
13 | # (but they are in multiline definitions)
14 | __match_args__: tuple = ("b", "c", "a",)
| ^^^^^^^^^^^^^^^^ RUF023
14 | __slots__: tuple = ("b", "c", "a",)
| ^^^^^^^^^^^^^^^^ RUF023
15 |
16 | class Klass2:
|
= help: Apply a natural sort to `Klass.__match_args__`
= help: Apply a natural sort to `Klass.__slots__`
Safe fix
11 11 | __slots__: set = {'b', "c", ((('a')))}
12 12 | # Trailing commas are also not retained for single-line definitions
13 13 | # (but they are in multiline definitions)
14 |- __match_args__: tuple = ("b", "c", "a",)
14 |+ __match_args__: tuple = ("a", "b", "c")
14 |- __slots__: tuple = ("b", "c", "a",)
14 |+ __slots__: tuple = ("a", "b", "c")
15 15 |
16 16 | class Klass2:
17 17 | if bool():
@ -111,7 +111,7 @@ RUF023.py:20:21: RUF023 [*] `Klass2.__slots__` is not sorted
20 | __slots__ = "foo3", "foo2", "foo1" # NB: an implicit tuple (without parens)
| ^^^^^^^^^^^^^^^^^^^^^^ RUF023
21 |
22 | __match_args__: list[str] = ["the", "three", "little", "pigs"]
22 | __slots__: list[str] = ["the", "three", "little", "pigs"]
|
= help: Apply a natural sort to `Klass2.__slots__`
@ -122,33 +122,33 @@ RUF023.py:20:21: RUF023 [*] `Klass2.__slots__` is not sorted
20 |- __slots__ = "foo3", "foo2", "foo1" # NB: an implicit tuple (without parens)
20 |+ __slots__ = "foo1", "foo2", "foo3" # NB: an implicit tuple (without parens)
21 21 |
22 22 | __match_args__: list[str] = ["the", "three", "little", "pigs"]
22 22 | __slots__: list[str] = ["the", "three", "little", "pigs"]
23 23 | __slots__ = ("parenthesized_item"), "in", ("an_unparenthesized_tuple")
RUF023.py:22:33: RUF023 [*] `Klass2.__match_args__` is not sorted
RUF023.py:22:28: RUF023 [*] `Klass2.__slots__` is not sorted
|
20 | __slots__ = "foo3", "foo2", "foo1" # NB: an implicit tuple (without parens)
21 |
22 | __match_args__: list[str] = ["the", "three", "little", "pigs"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF023
22 | __slots__: list[str] = ["the", "three", "little", "pigs"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF023
23 | __slots__ = ("parenthesized_item"), "in", ("an_unparenthesized_tuple")
24 | # we use natural sort,
|
= help: Apply a natural sort to `Klass2.__match_args__`
= help: Apply a natural sort to `Klass2.__slots__`
Safe fix
19 19 | else:
20 20 | __slots__ = "foo3", "foo2", "foo1" # NB: an implicit tuple (without parens)
21 21 |
22 |- __match_args__: list[str] = ["the", "three", "little", "pigs"]
22 |+ __match_args__: list[str] = ["little", "pigs", "the", "three"]
22 |- __slots__: list[str] = ["the", "three", "little", "pigs"]
22 |+ __slots__: list[str] = ["little", "pigs", "the", "three"]
23 23 | __slots__ = ("parenthesized_item"), "in", ("an_unparenthesized_tuple")
24 24 | # we use natural sort,
25 25 | # not alphabetical sort or "isort-style" sort
RUF023.py:23:17: RUF023 [*] `Klass2.__slots__` is not sorted
|
22 | __match_args__: list[str] = ["the", "three", "little", "pigs"]
22 | __slots__: list[str] = ["the", "three", "little", "pigs"]
23 | __slots__ = ("parenthesized_item"), "in", ("an_unparenthesized_tuple")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF023
24 | # we use natural sort,
@ -159,7 +159,7 @@ RUF023.py:23:17: RUF023 [*] `Klass2.__slots__` is not sorted
Safe fix
20 20 | __slots__ = "foo3", "foo2", "foo1" # NB: an implicit tuple (without parens)
21 21 |
22 22 | __match_args__: list[str] = ["the", "three", "little", "pigs"]
22 22 | __slots__: list[str] = ["the", "three", "little", "pigs"]
23 |- __slots__ = ("parenthesized_item"), "in", ("an_unparenthesized_tuple")
23 |+ __slots__ = "an_unparenthesized_tuple", "in", "parenthesized_item"
24 24 | # we use natural sort,
@ -199,7 +199,7 @@ RUF023.py:33:17: RUF023 [*] `Klass3.__slots__` is not sorted
38 | | "a0"
39 | | )
| |_____^ RUF023
40 | __match_args__ = [
40 | __slots__ = [
41 | "d",
|
= help: Apply a natural sort to `Klass3.__slots__`
@ -218,15 +218,15 @@ RUF023.py:33:17: RUF023 [*] `Klass3.__slots__` is not sorted
38 |- "a0"
38 |+ "d0"
39 39 | )
40 40 | __match_args__ = [
40 40 | __slots__ = [
41 41 | "d",
RUF023.py:40:22: RUF023 [*] `Klass3.__match_args__` is not sorted
RUF023.py:40:17: RUF023 [*] `Klass3.__slots__` is not sorted
|
38 | "a0"
39 | )
40 | __match_args__ = [
| ______________________^
40 | __slots__ = [
| _________________^
41 | | "d",
42 | | "c", # a comment regarding 'c'
43 | | "b",
@ -237,12 +237,12 @@ RUF023.py:40:22: RUF023 [*] `Klass3.__match_args__` is not sorted
47 |
48 | ##################################
|
= help: Apply a natural sort to `Klass3.__match_args__`
= help: Apply a natural sort to `Klass3.__slots__`
Safe fix
38 38 | "a0"
39 39 | )
40 40 | __match_args__ = [
40 40 | __slots__ = [
41 |- "d",
41 |+ # a comment regarding 'a':
42 |+ "a",
@ -296,12 +296,12 @@ RUF023.py:54:17: RUF023 [*] `Klass4.__slots__` is not sorted
61 64 | ) # comment6
62 65 | # comment7
RUF023.py:64:22: RUF023 [*] `Klass4.__match_args__` is not sorted
RUF023.py:64:17: RUF023 [*] `Klass4.__slots__` is not sorted
|
62 | # comment7
63 |
64 | __match_args__ = [ # comment0
| ______________________^
64 | __slots__ = [ # comment0
| _________________^
65 | | # comment1
66 | | # comment2
67 | | "dx", "cx", "bx", "ax" # comment3
@ -313,12 +313,12 @@ RUF023.py:64:22: RUF023 [*] `Klass4.__match_args__` is not sorted
72 |
73 | # from cpython/Lib/pathlib/__init__.py
|
= help: Apply a natural sort to `Klass4.__match_args__`
= help: Apply a natural sort to `Klass4.__slots__`
Safe fix
62 62 | # comment7
63 63 |
64 64 | __match_args__ = [ # comment0
64 64 | __slots__ = [ # comment0
65 |+ "ax",
66 |+ "bx",
67 |+ "cx",
@ -499,11 +499,11 @@ RUF023.py:138:17: RUF023 `SlotUser.__slots__` is not sorted
|
= help: Apply a natural sort to `SlotUser.__slots__`
RUF023.py:142:22: RUF023 `Klass5.__match_args__` is not sorted
RUF023.py:142:17: RUF023 `Klass5.__slots__` is not sorted
|
141 | class Klass5:
142 | __match_args__ = (
| ______________________^
142 | __slots__ = (
| _________________^
143 | | "look",
144 | | (
145 | | "a_veeeeeeeeeeeeeeeeeeery_long_parenthesized_item"
@ -513,7 +513,7 @@ RUF023.py:142:22: RUF023 `Klass5.__match_args__` is not sorted
148 | __slots__ = (
149 | "b",
|
= help: Apply a natural sort to `Klass5.__match_args__`
= help: Apply a natural sort to `Klass5.__slots__`
RUF023.py:148:17: RUF023 `Klass5.__slots__` is not sorted
|