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

Skip LibCST parsing for standard dedent adjustments #9769

Merged
merged 2 commits into from
Feb 2, 2024
Merged

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Feb 2, 2024

Summary

Often, when fixing, we need to dedent a block of code (e.g., if we remove an if and dedent its body). Today, we use LibCST to parse and adjust the indentation, which is really expensive -- but this is only really necessary if the block contains a multiline string, since naively adjusting the indentation for such a string can change the whitespace within the string.

This PR uses a simple dedent implementation for cases in which the block doesn't intersect with a multi-line string (or an f-string, since we don't support tracking multi-line strings for f-strings right now).

We could improve this even further by using the ranges to guide the dedent function, such that we don't apply the dedent if the line starts within a multiline string. But that would also need to take f-strings into account, which is a little tricky.

Test Plan

cargo test

@charliermarsh charliermarsh added the performance Potential performance improvement label Feb 2, 2024
Copy link

codspeed-hq bot commented Feb 2, 2024

CodSpeed Performance Report

Merging #9769 will improve performances by 29.71%

Comparing charlie/p (43e972a) with main (4f7fb56)

Summary

⚡ 2 improvements
✅ 28 untouched benchmarks

Benchmarks breakdown

Benchmark main charlie/p Change
linter/all-with-preview-rules[numpy/ctypeslib.py] 34.6 ms 26.7 ms +29.71%
linter/all-with-preview-rules[pydantic/types.py] 63.8 ms 56.5 ms +13.07%

Copy link
Contributor

github-actions bot commented Feb 2, 2024

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check encountered linter errors. (no lint changes; 1 project error)

sphinx-doc/sphinx (error)

ruff failed
  Cause: Selection of unstable rules without the `--preview` flag is not allowed. Enable preview or remove selection of:
	- FURB113
	- FURB131
	- FURB132

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -530 violations, +0 -0 fixes in 2 projects; 41 projects unchanged)

apache/airflow (+0 -342 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

- tests/utils/test_task_group.py:1001:9: ANN202 Missing return type annotation for private function `task_1`
- tests/utils/test_task_group.py:1003:9: T201 `print` found
- tests/utils/test_task_group.py:1006:9: ANN202 Missing return type annotation for private function `task_2`
- tests/utils/test_task_group.py:1008:9: T201 `print` found
- tests/utils/test_task_group.py:1011:9: ANN202 Missing return type annotation for private function `task_3`
- tests/utils/test_task_group.py:1013:9: T201 `print` found
- tests/utils/test_task_group.py:1016:9: ANN202 Missing return type annotation for private function `task_group1`
- tests/utils/test_task_group.py:1022:9: ANN202 Missing return type annotation for private function `task_group2`
- tests/utils/test_task_group.py:1026:9: ANN202 Missing return type annotation for private function `task_group3`
... 65 additional changes omitted for rule ANN202
- tests/utils/test_task_group.py:1050:5: S101 Use of `assert` detected
- tests/utils/test_task_group.py:1053:5: ANN201 Missing return type annotation for public function `test_call_taskgroup_twice`
- tests/utils/test_task_group.py:1055:5: PLC0415 `import` should be at the top-level of a file
- tests/utils/test_task_group.py:1060:9: T201 `print` found
- tests/utils/test_task_group.py:1065:9: T201 `print` found
- tests/utils/test_task_group.py:1071:9: T201 `print` found
... 15 additional changes omitted for rule T201
- tests/utils/test_task_group.py:1107:5: S101 Use of `assert` detected
- tests/utils/test_task_group.py:1110:5: ANN201 Missing return type annotation for public function `test_pass_taskgroup_output_to_task`
- tests/utils/test_task_group.py:1112:5: PLC0415 `import` should be at the top-level of a file
- tests/utils/test_task_group.py:1119:29: ANN001 Missing type annotation for function argument `num`
- tests/utils/test_task_group.py:1121:21: ANN001 Missing type annotation for function argument `i`
- tests/utils/test_task_group.py:1127:19: ANN001 Missing type annotation for function argument `num`
- tests/utils/test_task_group.py:1133:9: S101 Use of `assert` detected
- tests/utils/test_task_group.py:1135:9: S101 Use of `assert` detected
- tests/utils/test_task_group.py:1137:9: S101 Use of `assert` detected
- tests/utils/test_task_group.py:1142:5: ANN201 Missing return type annotation for public function `test_decorator_unknown_args`
- tests/utils/test_task_group.py:1151:5: ANN201 Missing return type annotation for public function `test_decorator_multiple_use_task`
- tests/utils/test_task_group.py:1152:5: PLC0415 `import` should be at the top-level of a file
- tests/utils/test_task_group.py:1168:5: S101 Use of `assert` detected
... 101 additional changes omitted for rule S101
- tests/utils/test_task_group.py:1177:5: ANN201 Missing return type annotation for public function `test_topological_sort1`
- tests/utils/test_task_group.py:1184:9: AIR001 Task variable name should match the `task_id`: "A"
- tests/utils/test_task_group.py:1185:9: AIR001 Task variable name should match the `task_id`: "B"
- tests/utils/test_task_group.py:1186:9: AIR001 Task variable name should match the `task_id`: "C"
... 310 additional changes omitted for project

zulip/zulip (+0 -188 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

- zerver/lib/test_helpers.py:100:44: FA100 Missing `from __future__ import annotations`, but uses `typing.Optional`
- zerver/lib/test_helpers.py:100:69: FA100 Missing `from __future__ import annotations`, but uses `typing.Optional`
- zerver/lib/test_helpers.py:100:78: FA100 Missing `from __future__ import annotations`, but uses `typing.Dict`
- zerver/lib/test_helpers.py:104:33: FA100 Missing `from __future__ import annotations`, but uses `typing.List`
- zerver/lib/test_helpers.py:104:56: FA100 Missing `from __future__ import annotations`, but uses `typing.Optional`
- zerver/lib/test_helpers.py:104:81: FA100 Missing `from __future__ import annotations`, but uses `typing.Dict`
... 50 additional changes omitted for rule FA100
- zerver/lib/test_helpers.py:113:5: D103 Missing docstring in public function
- zerver/lib/test_helpers.py:121:58: COM812 [*] Trailing comma missing
- zerver/lib/test_helpers.py:131:7: D101 Missing docstring in public class
- zerver/lib/test_helpers.py:138:39: FBT001 Boolean-typed positional argument in function definition
- zerver/lib/test_helpers.py:138:39: FBT002 Boolean default positional argument in function definition
- zerver/lib/test_helpers.py:138:5: FBT001 Boolean-typed positional argument in function definition
- zerver/lib/test_helpers.py:138:5: FBT002 Boolean default positional argument in function definition
- zerver/lib/test_helpers.py:138:68: COM812 [*] Trailing comma missing
- zerver/lib/test_helpers.py:140:5: D202 [*] No blank lines allowed after function docstring (found 1)
- zerver/lib/test_helpers.py:140:5: D205 1 blank line required between summary line and description
- zerver/lib/test_helpers.py:140:5: D212 [*] Multi-line docstring summary should start at the first line
... 171 additional changes omitted for project

Changes by rule (53 rules affected)

code total + violation - violation + fix - fix
S101 117 0 117 0 0
ANN202 70 0 70 0 0
FA100 55 0 55 0 0
AIR001 40 0 40 0 0
ANN201 38 0 38 0 0
ANN001 28 0 28 0 0
D103 24 0 24 0 0
T201 23 0 23 0 0
PLC0415 18 0 18 0 0
COM812 14 0 14 0 0
SIM117 12 0 12 0 0
FBT001 6 0 6 0 0
FBT002 5 0 5 0 0
PT012 5 0 5 0 0
PTH118 5 0 5 0 0
PTH123 4 0 4 0 0
FIX002 4 0 4 0 0
TD002 4 0 4 0 0
TD003 4 0 4 0 0
D106 4 0 4 0 0
RET505 3 0 3 0 0
D101 3 0 3 0 0
D205 3 0 3 0 0
ANN401 3 0 3 0 0
B015 2 0 2 0 0
CPY001 2 0 2 0 0
D202 2 0 2 0 0
D212 2 0 2 0 0
PLW1514 2 0 2 0 0
D107 2 0 2 0 0
D400 2 0 2 0 0
D415 2 0 2 0 0
C408 2 0 2 0 0
PERF401 1 0 1 0 0
A002 1 0 1 0 0
A001 1 0 1 0 0
D100 1 0 1 0 0
D401 1 0 1 0 0
D404 1 0 1 0 0
PTH100 1 0 1 0 0
PTH120 1 0 1 0 0
D209 1 0 1 0 0
PLR0913 1 0 1 0 0
PLR0917 1 0 1 0 0
C901 1 0 1 0 0
PLR0915 1 0 1 0 0
PLR1702 1 0 1 0 0
PLR6201 1 0 1 0 0
PLR2004 1 0 1 0 0
TD004 1 0 1 0 0
RET504 1 0 1 0 0
PLR0914 1 0 1 0 0
ARG001 1 0 1 0 0

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 2 project errors)

sphinx-doc/sphinx (error)

ruff format --no-preview --exclude tests/roots/test-pycode/cp_1251_coded.py

ruff failed
  Cause: Selection of unstable rules without the `--preview` flag is not allowed. Enable preview or remove selection of:
	- FURB113
	- FURB131
	- FURB132

openai/openai-cookbook (error)

warning: Detected debug build without --no-cache.
error: Failed to parse examples/dalle/Image_generations_edits_and_variations_with_DALL-E.ipynb:3:7:8: Unexpected token 'prompt'

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to parse examples/dalle/Image_generations_edits_and_variations_with_DALL-E.ipynb:3:7:8: Unexpected token 'prompt'

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat

crates/ruff_python_index/src/multiline_ranges.rs Outdated Show resolved Hide resolved
Comment on lines +141 to +145
// Look at the indentation of the first line, to determine the "baseline" indentation.
let existing_indent_len = text
.universal_newlines()
.next()
.map_or(0, |line| line.len() - line.trim_start().len());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Why not indentation_at_offset?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the text here is the "full line", not the start of the content.

crates/ruff_python_trivia/src/textwrap.rs Show resolved Hide resolved
@charliermarsh charliermarsh enabled auto-merge (squash) February 2, 2024 18:06
@charliermarsh charliermarsh merged commit c3ca345 into main Feb 2, 2024
17 checks passed
@charliermarsh charliermarsh deleted the charlie/p branch February 2, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Potential performance improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants