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 range for TRIO115 fix #8933

Merged
merged 1 commit into from
Dec 1, 2023
Merged

Use correct range for TRIO115 fix #8933

merged 1 commit into from
Dec 1, 2023

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Dec 1, 2023

Summary

This PR fixes the bug where the autofix for TRIO115 was taking the entire arguments range for the fix which included the parenthesis as well. This means that the fix would remove the arguments and the parenthesis. The fix is to use the correct range.

fixes: #8713

Test Plan

Update existing snapshots :)

@dhruvmanila
Copy link
Member Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@dhruvmanila dhruvmanila added the bug Something isn't working label Dec 1, 2023
@dhruvmanila dhruvmanila enabled auto-merge (squash) December 1, 2023 01:42
@dhruvmanila dhruvmanila merged commit f06c5dc into main Dec 1, 2023
16 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/fix-trio115 branch December 1, 2023 01:42
Copy link
Contributor

github-actions bot commented Dec 1, 2023

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@@ -103,7 +103,7 @@ pub(crate) fn zero_sleep_call(checker: &mut Checker, call: &ExprCall) {
)?;
let reference_edit =
Edit::range_replacement(format!("{binding}.checkpoint"), call.func.range());
let arg_edit = Edit::range_deletion(call.arguments.range);
let arg_edit = Edit::range_deletion(arg.range());
Copy link
Member

Choose a reason for hiding this comment

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

Will this range be incorrect if the user uses a keyword argument?

Copy link
Member

Choose a reason for hiding this comment

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

We probably want "replace call.arguments.range with ()".

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm out now but I think that should still work as arg is either a positional or keyword argument.

Copy link
Member

Choose a reason for hiding this comment

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

But arg is just the value, I think.

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in #8936.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support multi-level member imports in fixes
3 participants