Skip to content

Comments

[refurb] Fix starred expressions fix (FURB161)#16550

Merged
ntBre merged 4 commits intoastral-sh:mainfrom
VascoSch92:FURB161
Mar 19, 2025
Merged

[refurb] Fix starred expressions fix (FURB161)#16550
ntBre merged 4 commits intoastral-sh:mainfrom
VascoSch92:FURB161

Conversation

@VascoSch92
Copy link
Contributor

@VascoSch92 VascoSch92 commented Mar 7, 2025

The PR partially solves issue #16457

Specifically, it solves the following problem:

$ cat >furb161_1.py <<'# EOF'
print(bin(*[123]).count("1"))
# EOF

$ python furb161_1.py
6

$ ruff --isolated check --target-version py310 --preview --select FURB161 furb161_1.py --diff 2>&1 | grep error:
error: Fix introduced a syntax error. Reverting all changes.

Now starred expressions are corrected handled.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2025 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@ntBre ntBre added bug Something isn't working fixes Related to suggested fixes for violations labels Mar 7, 2025
@MichaReiser
Copy link
Member

MichaReiser commented Mar 14, 2025

@ntBre could you take a look at this PR

@MichaReiser MichaReiser requested a review from ntBre March 14, 2025 08:27
ntBre
ntBre requested changes Mar 14, 2025
View reviewed changes
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! I wrote a longer comment below explaining why I think we should just return on any starred expression, but I'm interested to hear your thoughts.

@VascoSch92
Copy link
Contributor Author

VascoSch92 commented Mar 16, 2025

Thanks for doing this! I wrote a longer comment below explaining why I think we should just return on any starred expression, but I'm interested to hear your thoughts.

Thank you for the feedback. I don't have strong opinions on that matter. I agree that returning in the case of starred expressions makes perfect sense.

I have updated the code accordingly. Please let me know if it aligns with your suggestion

ntBre
ntBre approved these changes Mar 18, 2025
View reviewed changes
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thanks for following up! I have one more nit, but this otherwise looks good to me. If you push another commit or close and reopen the PR, it should trigger CI.

@VascoSch92
Copy link
Contributor Author

VascoSch92 commented Mar 19, 2025

Thanks for following up! I have one more nit, but this otherwise looks good to me. If you push another commit or close and reopen the PR, it should trigger CI.

is_starred_expr is the correct function ;-)
I changed.

@ntBre ntBre changed the title [refurb] Fix starred expressions fix (FURB161) [refurb] Fix starred expressions fix (FURB161) Mar 19, 2025
@ntBre ntBre merged commit 999fd4f into astral-sh:main Mar 19, 2025
22 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Mar 20, 2025
Merged
dcreager added a commit that referenced this pull request Mar 21, 2025
@dcreager
Merge branch 'main' into dcreager/two-phase-binding
01a5b3a 
* main: (26 commits)
  Use the common `OperatorPrecedence` for the parser (#16747)
  [red-knot] Check subtype relation between callable types (#16804)
  [red-knot] Check whether two callable types are equivalent (#16698)
  [red-knot] Ban most `Type::Instance` types in type expressions (#16872)
  Special-case value-expression inference of special form subscriptions (#16877)
  [syntax-errors] Fix star annotation before Python 3.11 (#16878)
  Recognize `SyntaxError:` as an error code for ecosystem checks (#16879)
  [red-knot] add test cases result in false positive errors (#16856)
  Bump 0.11.1 (#16871)
  Allow discovery of venv in VIRTUAL_ENV env variable (#16853)
  Split git pathspecs in change determination onto separate lines (#16869)
  Use the correct base commit for change determination (#16857)
  Separate `BitXorOr` into `BitXor` and `BitOr` precedence (#16844)
  Server: Allow `FixAll` action in presence of version-specific syntax errors (#16848)
  [`refurb`] Fix starred expressions fix (`FURB161`) (#16550)
  [`flake8-executable`] Add pytest and uv run to help message for `shebang-missing-python` (`EXE003`) (#16855)
  Show more precise messages in invalid type expressions (#16850)
  [`flake8-executables`] Allow `uv run` in shebang line for `shebang-missing-python` (`EXE003`) (#16849)
  Add `--exit-non-zero-on-format` (#16009)
  [red-knot] Ban list literals in most contexts in type expressions (#16847)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ntBre ntBre ntBre approved these changes

Assignees

No one assigned

Labels

bug Something isn't working fixes Related to suggested fixes for violations

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@VascoSch92 @MichaReiser @ntBre