Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a rule/autofix to sort __slots__ and __match_args__ #9564

Merged
merged 23 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8a15678
[Proof of concept] Add a rule/autofix to sort `__slots__` and `__matc…
AlexWaygood Jan 17, 2024
785a002
Spruce up a little bit
AlexWaygood Jan 17, 2024
dfeb341
regenerate schema
AlexWaygood Jan 17, 2024
39a059b
Respond to some Micha comments from Discord
AlexWaygood Jan 17, 2024
37ee879
Avoid allocations in the common case
AlexWaygood Jan 18, 2024
883b561
Also implement the fix for multiline `__slots__` and `__match_args__`
AlexWaygood Jan 18, 2024
270432f
remove some unnecessary uses of `pub`
AlexWaygood Jan 18, 2024
6c94446
use static lifetimes
AlexWaygood Jan 19, 2024
90c413b
Address a bunch of Micha comments
AlexWaygood Jan 19, 2024
2b41a65
Introduce a `SortingStyle` enum
AlexWaygood Jan 19, 2024
902871a
explicitly test an edge case
AlexWaygood Jan 19, 2024
b667d2f
split the assertion in two
AlexWaygood Jan 19, 2024
cf6ca75
Rename `NodeAnalysis`
AlexWaygood Jan 19, 2024
b04bdb0
fix a few nits
AlexWaygood Jan 21, 2024
3dfdf8b
Address some Charlie comments
AlexWaygood Jan 21, 2024
606d85a
Fix edge-case bug involving `__slots__` or `__all__` that have non-st…
AlexWaygood Jan 21, 2024
1f59a16
More Charlie comments
AlexWaygood Jan 21, 2024
e64879a
Make `create_fix` a method in `sort_dunder_slots.rs`
AlexWaygood Jan 21, 2024
6645934
Put all the validation logic for the elements into a newtype construc…
AlexWaygood Jan 22, 2024
58bed1d
Merge branch 'main' into sort-dunder-slots
AlexWaygood Jan 22, 2024
7395585
Merge branch 'main' into sort-dunder-slots
AlexWaygood Jan 22, 2024
0ab0682
Implement `Clone` and `Copy` for `SortingStyle`
AlexWaygood Jan 22, 2024
333fe62
Small cleanups
AlexWaygood Jan 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
232 changes: 232 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF023.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
#########################
# Single-line definitions
#########################

class Klass:
__slots__ = ["d", "c", "b", "a"] # a comment that is untouched
__match_args__ = ("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",)

class Klass2:
if bool():
__slots__ = {"x": "docs for x", "m": "docs for m", "a": "docs for a"}
else:
__slots__ = "foo3", "foo2", "foo1" # NB: an implicit tuple (without parens)

__match_args__: list[str] = ["the", "three", "little", "pigs"]
__slots__ = ("parenthesized_item"), "in", ("an_unparenthesized_tuple")
# we use natural sort,
# not alphabetical sort or "isort-style" sort
__slots__ = {"aadvark237", "aadvark10092", "aadvark174", "aadvark532"}

############################
# Neat multiline definitions
############################

class Klass3:
__slots__ = (
"d0",
"c0", # a comment regarding 'c0'
"b0",
# a comment regarding 'a0':
"a0"
)
__match_args__ = [
"d",
"c", # a comment regarding 'c'
"b",
# a comment regarding 'a':
"a"
]

##########################################
# Messier multiline __all__ definitions...
##########################################

class Klass4:
# comment0
__slots__ = ("d", "a", # comment1
# comment2
"f", "b",
"strangely", # comment3
# comment4
"formatted",
# comment5
) # comment6
# comment7

__match_args__ = [ # comment0
# comment1
# comment2
"dx", "cx", "bx", "ax" # comment3
# comment4
# comment5
# comment6
] # comment7

# from cpython/Lib/pathlib/__init__.py
class PurePath:
__slots__ = (
# The `_raw_paths` slot stores unnormalized string paths. This is set
# in the `__init__()` method.
'_raw_paths',

# The `_drv`, `_root` and `_tail_cached` slots store parsed and
# normalized parts of the path. They are set when any of the `drive`,
# `root` or `_tail` properties are accessed for the first time. The
# three-part division corresponds to the result of
# `os.path.splitroot()`, except that the tail is further split on path
# separators (i.e. it is a list of strings), and that the root and
# tail are normalized.
'_drv', '_root', '_tail_cached',

# The `_str` slot stores the string representation of the path,
# computed from the drive, root and tail when `__str__()` is called
# for the first time. It's used to implement `_str_normcase`
'_str',

# The `_str_normcase_cached` slot stores the string path with
# normalized case. It is set when the `_str_normcase` property is
# accessed for the first time. It's used to implement `__eq__()`
# `__hash__()`, and `_parts_normcase`
'_str_normcase_cached',

# The `_parts_normcase_cached` slot stores the case-normalized
# string path after splitting on path separators. It's set when the
# `_parts_normcase` property is accessed for the first time. It's used
# to implement comparison methods like `__lt__()`.
'_parts_normcase_cached',

# The `_hash` slot stores the hash of the case-normalized string
# path. It's set when `__hash__()` is called for the first time.
'_hash',
)

# From cpython/Lib/pickletools.py
class ArgumentDescriptor(object):
__slots__ = (
# name of descriptor record, also a module global name; a string
'name',

# length of argument, in bytes; an int; UP_TO_NEWLINE and
# TAKEN_FROM_ARGUMENT{1,4,8} are negative values for variable-length
# cases
'n',

# a function taking a file-like object, reading this kind of argument
# from the object at the current position, advancing the current
# position by n bytes, and returning the value of the argument
'reader',

# human-readable docs for this arg descriptor; a string
'doc',
)

####################################
# Should be flagged, but not fixed
####################################

# from cpython/Lib/test/test_inspect.py.
# Multiline dicts are out of scope.
class SlotUser:
__slots__ = {'power': 'measured in kilowatts',
'distance': 'measured in kilometers'}

class Klass5:
__match_args__ = (
"look",
(
"a_veeeeeeeeeeeeeeeeeeery_long_parenthesized_item"
),
)
__slots__ = (
"b",
((
"c"
)),
"a"
)
__slots__ = ("don't" "care" "about", "__slots__" "with", "concatenated" "strings")

###################################
# These should all not get flagged:
###################################

class Klass6:
__slots__ = ()
__match_args__ = []
__slots__ = ("single_item",)
__match_args__ = (
"single_item_multiline",
)
__slots__ = {"single_item",}
__slots__ = {"single_item_no_trailing_comma": "docs for that"}
__match_args__ = [
"single_item_multiline_no_trailing_comma"
]
__slots__ = ("not_a_tuple_just_a_string")
__slots__ = ["a", "b", "c", "d"]
__slots__ += ["e", "f", "g"]
__slots__ = ("a", "b", "c", "d")

if bool():
__slots__ += ("e", "f", "g")
else:
__slots__ += ["alpha", "omega"]

__slots__ = {"not": "sorted", "but": "includes", **a_kwarg_splat}

__slots__ = ("b", "a", "e", "d")
__slots__ = ["b", "a", "e", "d"]
__match_args__ = ["foo", "bar", "antipasti"]

class Klass6:
__slots__ = (9, 8, 7)
__match_args__ = ( # This is just an empty tuple,
# but,
# it's very well
) # documented

# We don't deduplicate elements;
# this just ensures that duplicate elements aren't unnecessarily
# reordered by an autofix:
__slots__ = (
"duplicate_element", # comment1
"duplicate_element", # comment3
"duplicate_element", # comment2
"duplicate_element", # comment0
)

__slots__ =[
[]
]
__slots__ = [
()
]
__match_args__ = (
()
)
__match_args__ = (
[]
)
__slots__ = (
(),
)
__slots__ = (
[],
)
__match_args__ = (
"foo", [], "bar"
)
__match_args__ = [
"foo", (), "bar"
]

__match_args__ = {"a", "set", "for", "__match_args__", "is invalid"}
__match_args__ = {"this": "is", "also": "invalid"}
6 changes: 6 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.settings.rules.enabled(Rule::UnsortedDunderAll) {
ruff::rules::sort_dunder_all_assign(checker, assign);
}
if checker.enabled(Rule::UnsortedDunderSlots) {
ruff::rules::sort_dunder_slots_assign(checker, assign);
}
if checker.source_type.is_stub() {
if checker.any_enabled(&[
Rule::UnprefixedTypeParam,
Expand Down Expand Up @@ -1531,6 +1534,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.settings.rules.enabled(Rule::UnsortedDunderAll) {
ruff::rules::sort_dunder_all_ann_assign(checker, assign_stmt);
}
if checker.enabled(Rule::UnsortedDunderSlots) {
ruff::rules::sort_dunder_slots_ann_assign(checker, assign_stmt);
}
if checker.source_type.is_stub() {
if let Some(value) = value {
if checker.enabled(Rule::AssignmentDefaultInStub) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "020") => (RuleGroup::Preview, rules::ruff::rules::NeverUnion),
(Ruff, "021") => (RuleGroup::Preview, rules::ruff::rules::ParenthesizeChainedOperators),
(Ruff, "022") => (RuleGroup::Preview, rules::ruff::rules::UnsortedDunderAll),
(Ruff, "023") => (RuleGroup::Preview, rules::ruff::rules::UnsortedDunderSlots),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml),

Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ mod tests {
#[test_case(Rule::NeverUnion, Path::new("RUF020.py"))]
#[test_case(Rule::ParenthesizeChainedOperators, Path::new("RUF021.py"))]
#[test_case(Rule::UnsortedDunderAll, Path::new("RUF022.py"))]
#[test_case(Rule::UnsortedDunderSlots, Path::new("RUF023.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub(crate) use pairwise_over_zipped::*;
pub(crate) use parenthesize_logical_operators::*;
pub(crate) use quadratic_list_summation::*;
pub(crate) use sort_dunder_all::*;
pub(crate) use sort_dunder_slots::*;
pub(crate) use static_key_dict_comprehension::*;
pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
pub(crate) use unnecessary_key_check::*;
Expand All @@ -35,7 +36,9 @@ mod mutable_dataclass_default;
mod never_union;
mod pairwise_over_zipped;
mod parenthesize_logical_operators;
mod sequence_sorting;
mod sort_dunder_all;
mod sort_dunder_slots;
mod static_key_dict_comprehension;
mod unnecessary_iterable_allocation_for_first_element;
mod unnecessary_key_check;
Expand Down