Skip to content

[pylint] Fix convert a code keyword argument to a positional argument (PLR1722) - #16424

Merged
ntBre merged 8 commits into
astral-sh:mainfrom
VascoSch92:PLR1722
Mar 3, 2025
Merged

ntBre merged 8 commits into
astral-sh:mainfrom
VascoSch92:PLR1722

Conversation

@VascoSch92

Copy link
Copy Markdown
Contributor

The PR addresses issue #16396 .

Specifically:

  • If the exit statement contains a code keyword argument, it is converted into a positional argument.
  • If retrieving the code from the exit statement is not possible, a violation is raised without suggesting a fix.

@github-actions

github-actions Bot commented Feb 27, 2025 •

Copy link
Copy Markdown
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre added the bug An issue describing something that isn't working, or a PR that fixes a bug label Feb 27, 2025
@ntBre ntBre linked an issue Feb 27, 2025 that may be closed by this pull request
@MichaReiser
MichaReiser requested a review from ntBre February 28, 2025 08:11

@ntBre ntBre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! I think we can simplify the implementation a bit here and also support more use cases at the same time.

Comment thread crates/ruff_linter/src/rules/pylint/rules/sys_exit_alias.rs Outdated
Comment thread crates/ruff_linter/src/rules/pylint/rules/sys_exit_alias.rs Outdated
VascoSch92 and others added 3 commits March 1, 2025 13:34
@VascoSch92

Copy link
Copy Markdown
Contributor Author

Hey @ntBre

thank you very much for the feedback. Yes, my solution was pretty convoluted. I suspected there it was a better way to do it but I could not find how ;-)

Everything works fine.

I also added the test cases that you suggested and it seems that the output it is what expected.

@VascoSch92
VascoSch92 requested a review from ntBre March 1, 2025 12:44
Comment thread crates/ruff_linter/src/rules/pylint/rules/sys_exit_alias.rs
@VascoSch92
VascoSch92 requested a review from ntBre March 2, 2025 18:58

@ntBre ntBre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks great, thank you!

@ntBre
ntBre merged commit 4d92e20 into astral-sh:main Mar 3, 2025
@VascoSch92
VascoSch92 deleted the PLR1722 branch March 3, 2025 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue describing something that isn't working, or a PR that fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PLR1722 should convert a code keyword argument to a positional argument

2 participants