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

Use correct start location for class/function clause header #7802

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Oct 4, 2023

Summary

This PR fixes the bug where the formatter would panic if a class/function with
decorators had a suppression comment.

The fix is to use to correct start location to find the async/def/class
keyword when decorators are present which is the end of the last decorator.

Test Plan

Add test cases for the fix and update the snapshots.

@dhruvmanila
Copy link
Member Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@dhruvmanila dhruvmanila added bug Something isn't working formatter Related to the formatter labels Oct 4, 2023
@dhruvmanila dhruvmanila requested a review from konstin October 4, 2023 05:36
@dhruvmanila dhruvmanila force-pushed the dhruv/start-location branch from 26fb68e to 4bed0d5 Compare October 4, 2023 06:43
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 4, 2023

CodSpeed Performance Report

Merging #7802 will degrade performances by 2.37%

Comparing dhruv/start-location (b79b73d) with main (7b4fb4f)

Summary

⚡ 1 improvements
❌ 1 regressions
✅ 23 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main dhruv/start-location Change
linter/default-rules[pydantic/types.py] 38.9 ms 38 ms +2.48%
linter/all-rules[numpy/ctypeslib.py] 34.6 ms 35.5 ms -2.37%

@konstin
Copy link
Member

konstin commented Oct 4, 2023

What about fmt_skip/decorators.py for the tests?

@dhruvmanila dhruvmanila enabled auto-merge (squash) October 4, 2023 07:47
@dhruvmanila dhruvmanila merged commit a1509df into main Oct 4, 2023
14 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/start-location branch October 4, 2023 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working formatter Related to the formatter
Projects
None yet
2 participants