Add convert exit() to sys.exit() rule

[JonathanPlasse]

• Close Add rule to use sys.exit() instead of exit() #787
• Fix
  • It fixes when
    • import sys
    • import sys as sys2
    • import sys.exit does not work
    • from sys import exit as exit2
  • It does not fix when not it has to import sys. Is there an example of how to add an import while keeping it sorted if I is selected?
    • Do not fix
    • Open a new issue to add this feature.
• exit() is not detected inside functions. What did I do wrong? Is it current_scope() that is not enough? Do I need to iterate over every scope?

[charliermarsh]

It does not fix when not it has to import sys.

Hmm, no, we don't do that anywhere yet. Let's try for that later, as it would require fixes with multiple edits that change multiple parts of the tree.

exit() is not detected inside functions. What did I do wrong? Is it current_scope() that is not enough? Do I need to iterate over every scope?

Yeah, yeah you have to iterate over every scope in reverse, like:

for scope_index in self.scope_stack.iter().rev() {
    let scope = &mut self.scopes[*scope_index];
    ...
}

We might want to add a .current_scope()-like method to Checker to expose that iterate to plugins.

[JonathanPlasse]

@charliermarsh,

It does not fix when not it has to import sys.

Hmm, no, we don't do that anywhere yet. Let's try for that later, as it would require fixes with multiple edits that change multiple parts of the tree.

Let's open an issue once this PR is merged.

exit() is not detected inside functions. What did I do wrong? Is it current_scope() that is not enough? Do I need to iterate over every scope?

Yeah, yeah you have to iterate over every scope in reverse, like:

for scope_index in self.scope_stack.iter().rev() {
    let scope = &mut self.scopes[*scope_index];
    ...
}

We might want to add a .current_scope()-like method to Checker to expose that iterate to plugins.

I added Checker.current_scopes(). But, even if I iterate over every scope, it still does not work.
Or maybe I made an error.

[charliermarsh]

@JonathanPlasse - I think your code is actually right! But the test case is a bit off. Right now, you have this:

...

def main():
    exit(6)  # Is not detected

...

def exit(e):
    print(e)

...

So, you're re-defining exit at the bottom, and when the main() with exit(6) is evaluated, exit is no longer the built-in. If you remove the def exit, the error is picked up -- which matches Python's semantics.

I think you'll need to split into a few different test files to evaluate all of those cases.

[charliermarsh]

Probably a bit hard to get right since also needs to take aliases into account... but could be useful beyond this rule if robust.

[JonathanPlasse]

Do you mean alias as in from a import b as alias or import a as alias?
If yes, it already does.

[charliermarsh]

Yeah, that's what I meant! Oh, very nice.

[charliermarsh]

@JonathanPlasse - Feel free to mark as "ready for review" when you're happy with this!

[JonathanPlasse]

I am ready for review :)

[JonathanPlasse]

Is it possible to make a pre-commit release?

[JonathanPlasse]

Is it normal that I have to specify --fixable in ruff --fix --fixable RUF --select RUFF file.py for the fix to work?

[charliermarsh]

Great! Will review tonight!

@JonathanPlasse - No, --fixable shouldn't be necessary. That's a bug, will PR, one sec...

[charliermarsh]

(Fixed in #838.)