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 rule and autofix to sort the contents of __all__ #9474

Merged
merged 85 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 83 commits
Commits
Show all changes
85 commits
Select commit Hold shift + click to select a range
da6e3c1
Get it working for single-line definitions
AlexWaygood Jan 10, 2024
47fa963
Get it working for multiline `__all__` definitions
AlexWaygood Jan 10, 2024
f15c0c4
All tests passing 🥳🥳
AlexWaygood Jan 11, 2024
5d2335b
clean
AlexWaygood Jan 11, 2024
265b590
Document; adjust when it is marked as safe
AlexWaygood Jan 11, 2024
8619dad
More comments; misc nits
AlexWaygood Jan 11, 2024
5bacac0
make mkdocs happy
AlexWaygood Jan 11, 2024
e4842bd
Don't add trailing comma where unnecessary; fix misc nits
AlexWaygood Jan 11, 2024
a7dc871
I'm in rustland where the enums are cool
AlexWaygood Jan 11, 2024
4238b60
Merge branch 'main' into sort-dunder-all-2
AlexWaygood Jan 11, 2024
cc42bf9
Make it work for `AnnAssign` nodes, and use more type-safe hooks in `…
AlexWaygood Jan 11, 2024
65e2f7b
Merge branch 'main' into sort-dunder-all-2
AlexWaygood Jan 11, 2024
454e730
Attempt to clarify invariants relied upon in `construct_sorted_all()`
AlexWaygood Jan 11, 2024
e1c1f35
misc Charlie remarks
AlexWaygood Jan 11, 2024
4e405fb
remove unnecessary assignment
AlexWaygood Jan 11, 2024
26a2848
More misc Charlie comments
AlexWaygood Jan 12, 2024
b0c5d66
Update crates/ruff_linter/src/rules/ruff/rules/sort_dunder_all.rs
AlexWaygood Jan 12, 2024
e35f088
Update crates/ruff_linter/src/rules/ruff/rules/sort_dunder_all.rs
AlexWaygood Jan 12, 2024
8d4a9f6
Update crates/ruff_linter/src/rules/ruff/rules/sort_dunder_all.rs
AlexWaygood Jan 12, 2024
47ff9b2
Revert "Update crates/ruff_linter/src/rules/ruff/rules/sort_dunder_al…
AlexWaygood Jan 12, 2024
047e274
Get rid of one use of `.clone()`
AlexWaygood Jan 12, 2024
9904db4
Remove all uses of `.clone()`
AlexWaygood Jan 12, 2024
d86ca1d
Use absolute ranges everywhere; get rid of more cloning
AlexWaygood Jan 12, 2024
f02c635
Update crates/ruff_linter/src/rules/ruff/rules/sort_dunder_all.rs
AlexWaygood Jan 12, 2024
00121c7
Rename `construct_sorted_all()`; make it consume self rather than tak…
AlexWaygood Jan 12, 2024
61c57b0
suggestion by micha to use one `panic` instead of two `.expect()`s
AlexWaygood Jan 12, 2024
fd1dc54
fix bug relating to multiline comments
AlexWaygood Jan 12, 2024
5e129ad
Much bigger comment explaining `prelude` and `postlude`
AlexWaygood Jan 12, 2024
382aed2
Use a natural sort rather than an alphabetical sort
AlexWaygood Jan 12, 2024
6f5f558
Don't add a trailing comma if it didn't already exist
AlexWaygood Jan 12, 2024
655bf67
Don't insist on two spaces before an inline comment
AlexWaygood Jan 12, 2024
9e2bae2
rename `additional_comments` -> `end_of_line_comments`
AlexWaygood Jan 12, 2024
af6ca05
add comment explaining why comments are grouped with elements into items
AlexWaygood Jan 12, 2024
e8481b3
rename `this_range` -> `preceding_comment_range`
AlexWaygood Jan 12, 2024
572bb66
Hugely overhaul docs and comments
AlexWaygood Jan 12, 2024
baa6ce5
Merge branch 'main' into sort-dunder-all-2
AlexWaygood Jan 12, 2024
768a364
Postpone an allocation until we actually need it
AlexWaygood Jan 12, 2024
ea5270c
more clarifications and typo fixes in comments/docs
AlexWaygood Jan 12, 2024
229fec1
Get rid of `.as_ref()`
AlexWaygood Jan 12, 2024
f116550
Also sort tuples/lists passed to `__all__.extend()`
AlexWaygood Jan 13, 2024
bec4a1b
use a 🐄
AlexWaygood Jan 13, 2024
301bcd5
Huge cleanup by changing the way we calculate leading indentation
AlexWaygood Jan 13, 2024
444f7b5
Merge branch 'main' into sort-dunder-all-2
AlexWaygood Jan 13, 2024
de871a4
nits
AlexWaygood Jan 13, 2024
306cfd5
Fix an edge case that caused weird indentation for a string element f…
AlexWaygood Jan 13, 2024
35f2ee5
fix an edge case that caused weird indentation for the closing bracket
AlexWaygood Jan 13, 2024
f5cdcf2
more edge cases
AlexWaygood Jan 13, 2024
cdf7845
Use an 'isort-style' sort, not a natural sort
AlexWaygood Jan 13, 2024
559ce7a
`DunderAllItem` should not be clonable
AlexWaygood Jan 13, 2024
e752531
`panic()` -> `unreachable()`
AlexWaygood Jan 13, 2024
29de09b
Remove the need for an `unreachable!()` call
AlexWaygood Jan 13, 2024
f10bfc4
nits
AlexWaygood Jan 13, 2024
c6701b4
Remove remaining `unreachable!()` and `.expect()` calls
AlexWaygood Jan 14, 2024
f7ffad9
Merge branch 'main' into sort-dunder-all-2
AlexWaygood Jan 14, 2024
c74cb48
nit
AlexWaygood Jan 14, 2024
e298e1d
Improve clarity of `collect_dunder_all_lines()`
AlexWaygood Jan 14, 2024
256c37e
Use more ruff-specific idioms
AlexWaygood Jan 14, 2024
5290f93
deduplicate the different hook functions
AlexWaygood Jan 14, 2024
4677cb2
remove some unnecessary uses of `ref`
AlexWaygood Jan 14, 2024
bcfeae9
simplify slightly
AlexWaygood Jan 15, 2024
5dced4a
don't be so judgemental
AlexWaygood Jan 15, 2024
dd9370e
Rename `receive_*` methods for clarity
AlexWaygood Jan 15, 2024
0281f93
Add a test for `__all__` definitions with duplicate elements
AlexWaygood Jan 15, 2024
33cab1b
Get rid of `original_index`
AlexWaygood Jan 15, 2024
c8e4b63
Big overhaul addressing Micha's review
AlexWaygood Jan 15, 2024
fe10c1b
More Micha comments
AlexWaygood Jan 15, 2024
d185a4e
Merge branch 'main' into sort-dunder-all-2
AlexWaygood Jan 15, 2024
d5d5ce1
Deduplicate function that now exists on `main`
AlexWaygood Jan 15, 2024
93da9c2
fix comment following refactor
AlexWaygood Jan 15, 2024
d78e931
Make some comments more precise
AlexWaygood Jan 15, 2024
6ca35fe
More docs
AlexWaygood Jan 16, 2024
bbafca9
Merge branch 'sort-dunder-all-2' of https://github.com/AlexWaygood/ru…
AlexWaygood Jan 16, 2024
dd707aa
Address Micha's easy comments
AlexWaygood Jan 16, 2024
4489179
Merge branch 'main' into sort-dunder-all-2
AlexWaygood Jan 16, 2024
2e7b570
typos
AlexWaygood Jan 16, 2024
2413c04
Make `AllItemSortKey` generic over a lifetime
AlexWaygood Jan 16, 2024
ecb33d4
Improve handling of prelude/postlude newlines
AlexWaygood Jan 16, 2024
34b81e3
fix formatting, add another comment
AlexWaygood Jan 16, 2024
ad6a507
.
AlexWaygood Jan 16, 2024
24616f3
Mark fix as always safe
AlexWaygood Jan 16, 2024
497a94b
Final round of comment fixups
AlexWaygood Jan 16, 2024
16d9905
One more Micha comment I missed
AlexWaygood Jan 16, 2024
d23cded
Address one final remark by Micha
AlexWaygood Jan 16, 2024
0192488
Rename enum variant; clean up code slightly
AlexWaygood Jan 16, 2024
c89556c
another comment
AlexWaygood Jan 16, 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
308 changes: 308 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF022.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,308 @@
##################################################
# Single-line __all__ definitions (nice 'n' easy!)
##################################################

__all__ = ["d", "c", "b", "a"] # a comment that is untouched
__all__ += ["foo", "bar", "antipasti"]
__all__ = ("d", "c", "b", "a")

# Quoting style is retained,
# but unnecessary parens are not
__all__: list = ['b', "c", ((('a')))]
# Trailing commas are also not retained in single-line `__all__` definitions
# (but they are in multiline `__all__` definitions)
__all__: tuple = ("b", "c", "a",)

if bool():
__all__ += ("x", "m", "a", "s")
else:
__all__ += "foo3", "foo2", "foo1" # NB: an implicit tuple (without parens)

__all__: list[str] = ["the", "three", "little", "pigs"]

__all__ = ("parenthesized_item"), "in", ("an_unparenthesized_tuple")
__all__.extend(["foo", "bar"])
__all__.extend(("foo", "bar"))
__all__.extend((((["foo", "bar"]))))

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

__all__ = (
"d0",
"c0", # a comment regarding 'c0'
"b0",
# a comment regarding 'a0':
"a0"
)

__all__ = [
"d",
"c", # a comment regarding 'c'
"b",
# a comment regarding 'a':
"a"
]

# we implement an "isort-style sort":
# SCEAMING_CASE constants first,
# then CamelCase classes,
# then anything thats lowercase_snake_case.
# This (which is currently alphabetically sorted)
# should get reordered accordingly:
__all__ = [
"APRIL",
"AUGUST",
"Calendar",
"DECEMBER",
"Day",
"FEBRUARY",
"FRIDAY",
"HTMLCalendar",
"IllegalMonthError",
"JANUARY",
"JULY",
"JUNE",
"LocaleHTMLCalendar",
"MARCH",
"MAY",
"MONDAY",
"Month",
"NOVEMBER",
"OCTOBER",
"SATURDAY",
"SEPTEMBER",
"SUNDAY",
"THURSDAY",
"TUESDAY",
"TextCalendar",
"WEDNESDAY",
"calendar",
"timegm",
"weekday",
"weekheader"]

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

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

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

__all__ = ["register", "lookup", "open", "EncodedFile", "BOM", "BOM_BE",
"BOM_LE", "BOM32_BE", "BOM32_LE", "BOM64_BE", "BOM64_LE",
"BOM_UTF8", "BOM_UTF16", "BOM_UTF16_LE", "BOM_UTF16_BE",
"BOM_UTF32", "BOM_UTF32_LE", "BOM_UTF32_BE",
"CodecInfo", "Codec", "IncrementalEncoder", "IncrementalDecoder",
"StreamReader", "StreamWriter",
"StreamReaderWriter", "StreamRecoder",
"getencoder", "getdecoder", "getincrementalencoder",
"getincrementaldecoder", "getreader", "getwriter",
"encode", "decode", "iterencode", "iterdecode",
"strict_errors", "ignore_errors", "replace_errors",
"xmlcharrefreplace_errors",
"backslashreplace_errors", "namereplace_errors",
"register_error", "lookup_error"]

__all__: tuple[str, ...] = ( # a comment about the opening paren
# multiline comment about "bbb" part 1
# multiline comment about "bbb" part 2
"bbb",
# multiline comment about "aaa" part 1
# multiline comment about "aaa" part 2
"aaa",
)

# we use natural sort for `__all__`,
# not alphabetical sort.
# Also, this doesn't end with a trailing comma,
# so the autofix shouldn't introduce one:
__all__ = (
"aadvark237",
"aadvark10092",
"aadvark174", # the very long whitespace span before this comment is retained
"aadvark532" # the even longer whitespace span before this comment is retained
)

__all__.extend(( # comment0
# comment about foo
"foo", # comment about foo
# comment about bar
"bar" # comment about bar
# comment1
)) # comment2

__all__.extend( # comment0
# comment1
( # comment2
# comment about foo
"foo", # comment about foo
# comment about bar
"bar" # comment about bar
# comment3
) # comment4
) # comment2

__all__.extend([ # comment0
# comment about foo
"foo", # comment about foo
# comment about bar
"bar" # comment about bar
# comment1
]) # comment2

__all__.extend( # comment0
# comment1
[ # comment2
# comment about foo
"foo", # comment about foo
# comment about bar
"bar" # comment about bar
# comment3
] # comment4
) # comment2

__all__ = ["Style", "Treeview",
# Extensions
"LabeledScale", "OptionMenu",
]

__all__ = ["Awaitable", "Coroutine",
"AsyncIterable", "AsyncIterator", "AsyncGenerator",
]

__all__ = [
"foo",
"bar",
"baz",
]

#########################################################################
# These should be flagged, but not fixed:
# - Parenthesized items in multiline definitions are out of scope
# - The same goes for any `__all__` definitions with concatenated strings
#########################################################################

__all__ = (
"look",
(
"a_veeeeeeeeeeeeeeeeeeery_long_parenthesized_item"
),
)

__all__ = (
"b",
((
"c"
)),
"a"
)

__all__ = ("don't" "care" "about", "__all__" "with", "concatenated" "strings")

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

__all__ = ()
__all__ = []
__all__ = ("single_item",)
__all__ = (
"single_item_multiline",
)
__all__ = ["single_item",]
__all__ = ["single_item_no_trailing_comma"]
__all__ = [
"single_item_multiline_no_trailing_comma"
]
__all__ = ("not_a_tuple_just_a_string")
__all__ = ["a", "b", "c", "d"]
__all__ += ["e", "f", "g"]
__all__ = ("a", "b", "c", "d")

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

class IntroducesNonModuleScope:
__all__ = ("b", "a", "e", "d")
__all__ = ["b", "a", "e", "d"]
__all__ += ["foo", "bar", "antipasti"]
__all__.extend(["zebra", "giraffe", "antelope"])

__all__ = {"look", "a", "set"}
__all__ = {"very": "strange", "not": "sorted", "we don't": "care"}
["not", "an", "assignment", "just", "an", "expression"]
__all__ = (9, 8, 7)
__all__ = ( # This is just an empty tuple,
# but,
# it's very well
) # documented

__all__.append("foo")
__all__.extend(["bar", "foo"])
__all__.extend([
"bar", # comment0
"foo" # comment1
])
__all__.extend(("bar", "foo"))
__all__.extend(
(
"bar",
"foo"
)
)

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

__all__ =[
[]
]
__all__ [
()
]
__all__ = (
()
)
__all__ = (
[]
)
__all__ = (
(),
)
__all__ = (
[],
)
__all__ = (
"foo", [], "bar"
)
__all__ = [
"foo", (), "bar"
]
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -974,6 +974,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::SslInsecureVersion) {
flake8_bandit::rules::ssl_insecure_version(checker, call);
}
if checker.enabled(Rule::UnsortedDunderAll) {
ruff::rules::sort_dunder_all_extend_call(checker, call);
}
}
Expr::Dict(dict) => {
if checker.any_enabled(&[
Expand Down
11 changes: 10 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1059,12 +1059,15 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pylint::rules::misplaced_bare_raise(checker, raise);
}
}
Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => {
Stmt::AugAssign(aug_assign @ ast::StmtAugAssign { target, .. }) => {
if checker.enabled(Rule::GlobalStatement) {
if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
pylint::rules::global_statement(checker, id);
}
}
if checker.enabled(Rule::UnsortedDunderAll) {
ruff::rules::sort_dunder_all_aug_assign(checker, aug_assign);
}
}
Stmt::If(
if_ @ ast::StmtIf {
Expand Down Expand Up @@ -1452,6 +1455,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.settings.rules.enabled(Rule::TypeBivariance) {
pylint::rules::type_bivariance(checker, value);
}
if checker.settings.rules.enabled(Rule::UnsortedDunderAll) {
ruff::rules::sort_dunder_all_assign(checker, assign);
}
if checker.source_type.is_stub() {
if checker.any_enabled(&[
Rule::UnprefixedTypeParam,
Expand Down Expand Up @@ -1522,6 +1528,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::NonPEP695TypeAlias) {
pyupgrade::rules::non_pep695_type_alias(checker, assign_stmt);
}
if checker.settings.rules.enabled(Rule::UnsortedDunderAll) {
ruff::rules::sort_dunder_all_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 @@ -923,6 +923,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "019") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryKeyCheck),
(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, "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 @@ -42,6 +42,7 @@ mod tests {
#[test_case(Rule::UnnecessaryKeyCheck, Path::new("RUF019.py"))]
#[test_case(Rule::NeverUnion, Path::new("RUF020.py"))]
#[test_case(Rule::ParenthesizeChainedOperators, Path::new("RUF021.py"))]
#[test_case(Rule::UnsortedDunderAll, Path::new("RUF022.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
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub(crate) use never_union::*;
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 static_key_dict_comprehension::*;
pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
pub(crate) use unnecessary_key_check::*;
Expand All @@ -34,6 +35,7 @@ mod mutable_dataclass_default;
mod never_union;
mod pairwise_over_zipped;
mod parenthesize_logical_operators;
mod sort_dunder_all;
mod static_key_dict_comprehension;
mod unnecessary_iterable_allocation_for_first_element;
mod unnecessary_key_check;
Expand Down