Skip to content

Commit

Permalink
Add rule and autofix to sort the contents of __all__ (#9474)
Browse files Browse the repository at this point in the history
## Summary

This implements the rule proposed in #1198 (though it doesn't close the
issue, as there are some open questions about configuration that might
merit some further discussion).

## Test Plan

`cargo test` / `cargo insta review`. I also ran this PR branch on the CPython
codebase with `--fix --select=RUF022 --preview `, and the results looked
pretty good to me.

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
Co-authored-by: Andrew Gallant <andrew@astral.sh>
  • Loading branch information
3 people committed Jan 16, 2024
1 parent 3064312 commit 3aae16f
Show file tree
Hide file tree
Showing 9 changed files with 2,248 additions and 1 deletion.
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

0 comments on commit 3aae16f

Please sign in to comment.