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

[perflint] fix invalid hoist in perf401 #14369

Merged
merged 22 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
aafb689
fix invalid hoist
w0nder1ng Nov 15, 2024
fd71328
tentative fix
w0nder1ng Nov 15, 2024
f30118e
don't use a comprehension when the list is referenced
w0nder1ng Nov 15, 2024
238d17a
check for external references to the for loop variable
w0nder1ng Nov 18, 2024
46bd851
stop duplicating comments inside the fixed append
w0nder1ng Nov 18, 2024
9a336fd
fix annotated returns, but lose type info
w0nder1ng Nov 18, 2024
63b0e1e
apply fix to type-annotated lists
w0nder1ng Nov 18, 2024
527a050
fix async extends, implicit tuples, and comments in iterator
w0nder1ng Dec 3, 2024
f339d70
stop deleting semicolon statements in the same line as binding
w0nder1ng Dec 6, 2024
f6ccb39
move manual list comprehension to deferred check
w0nder1ng Dec 6, 2024
6a98542
add debugging code to show binding
w0nder1ng Dec 6, 2024
e2eec2e
find the right binding for the loop variable
w0nder1ng Dec 9, 2024
9d41fb7
check that the correct for loop is picked
w0nder1ng Dec 9, 2024
104b69e
don't stop fix if a reference to target is actually another binding
w0nder1ng Dec 9, 2024
e44fcaa
allow shadowed bindings from any scope
w0nder1ng Dec 9, 2024
4696878
simplify check for loop variable usages
w0nder1ng Dec 10, 2024
74afdb0
cargo fmt
w0nder1ng Dec 10, 2024
db8d0d6
Few simplifications
MichaReiser Dec 11, 2024
77a0290
Merge branch 'main' into perf401_invalid_hoist
w0nder1ng Dec 11, 2024
c9b394c
fix merge
w0nder1ng Dec 11, 2024
9c55f1b
replace tokenization with simple character iterator
w0nder1ng Dec 11, 2024
523808d
Use simple tokenizer
MichaReiser Dec 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 122 additions & 10 deletions crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,49 +74,57 @@ def f():
result.append(i) # Ok


def f():
async def f():
items = [1, 2, 3, 4]
result = []
async for i in items:
if i % 2:
result.append(i) # PERF401


def f():
async def f():
items = [1, 2, 3, 4]
result = []
async for i in items:
result.append(i) # PERF401


async def f():
items = [1, 2, 3, 4]
result = [1, 2]
async for i in items:
result.append(i) # PERF401


def f():
result, _ = [1,2,3,4], ...
result, _ = [1, 2, 3, 4], ...
for i in range(10):
result.append(i*2) # PERF401
result.append(i * 2) # PERF401


def f():
result = []
if True:
for i in range(10): # single-line comment 1 should be protected
# single-line comment 2 should be protected
if i % 2: # single-line comment 3 should be protected
result.append(i) # PERF401
if i % 2: # single-line comment 3 should be protected
result.append(i) # PERF401


def f():
result = [] # comment after assignment should be protected
result = [] # comment after assignment should be protected
for i in range(10): # single-line comment 1 should be protected
# single-line comment 2 should be protected
if i % 2: # single-line comment 3 should be protected
result.append(i) # PERF401
if i % 2: # single-line comment 3 should be protected
result.append(i) # PERF401


def f():
result = []
for i in range(10):
"""block comment stops the fix"""
result.append(i*2) # Ok
result.append(i * 2) # Ok


def f(param):
# PERF401
Expand All @@ -125,3 +133,107 @@ def f(param):
new_layers = []
for value in param:
new_layers.append(value * 3)


def f():
result = []
var = 1
for _ in range(10):
result.append(var + 1) # PERF401


def f():
# make sure that `tmp` is not deleted
tmp = 1; result = [] # commment should be protected
for i in range(10):
result.append(i + 1) # PERF401


def f():
# make sure that `tmp` is not deleted
result = []; tmp = 1 # commment should be protected
for i in range(10):
result.append(i + 1) # PERF401


def f():
result = [] # comment should be protected
for i in range(10):
result.append(i * 2) # PERF401


def f():
result = []
result.append(1)
for i in range(10):
result.append(i * 2) # PERF401


def f():
result = []
result += [1]
for i in range(10):
result.append(i * 2) # PERF401


def f():
result = []
for val in range(5):
result.append(val * 2) # Ok
print(val)


def f():
result = []
for val in range(5):
result.append(val * 2) # PERF401
val = 1
print(val)


def f():
i = [1, 2, 3]
result = []
for i in i:
result.append(i + 1) # PERF401


def f():
result = []
for i in range( # Comment 1 should not be duplicated
(
2 # Comment 2
+ 1
)
): # Comment 3
if i % 2: # Comment 4
result.append(
(
i + 1,
# Comment 5
2,
)
) # PERF401


def f():
result: list[int] = []
for i in range(10):
result.append(i * 2) # PERF401


def f():
a, b = [1, 2, 3], [4, 5, 6]
result = []
for i in a, b:
result.append(i[0] + i[1]) # PERF401
return result


def f():
values = [1, 2, 3]
result = []
for a in values:
print(a)
for a in values:
result.append(a + 1) # PERF401
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) {
let Stmt::For(stmt_for) = checker.semantic.current_statement() else {
unreachable!("Expected Stmt::For");
};

if checker.enabled(Rule::UnusedLoopControlVariable) {
flake8_bugbear::rules::unused_loop_control_variable(checker, stmt_for);
}
Expand All @@ -36,6 +35,9 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) {
if checker.enabled(Rule::DictIndexMissingItems) {
pylint::rules::dict_index_missing_items(checker, stmt_for);
}
if checker.enabled(Rule::ManualListComprehension) {
perflint::rules::manual_list_comprehension(checker, stmt_for);
}
}
}
}
4 changes: 1 addition & 3 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1379,6 +1379,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
Rule::UnnecessaryEnumerate,
Rule::UnusedLoopControlVariable,
Rule::YieldInForLoop,
Rule::ManualListComprehension,
]) {
checker.analyze.for_loops.push(checker.semantic.snapshot());
}
Expand All @@ -1403,9 +1404,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::DictIterMissingItems) {
pylint::rules::dict_iter_missing_items(checker, target, iter);
}
if checker.enabled(Rule::ManualListComprehension) {
perflint::rules::manual_list_comprehension(checker, for_stmt);
}
if checker.enabled(Rule::ManualListCopy) {
perflint::rules::manual_list_copy(checker, for_stmt);
}
Expand Down
Loading
Loading