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

Ensure that B006 autofixes are inserted after imports #7629

Merged
merged 22 commits into from Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,5 @@
# Docstring followed by whitespace with no newline
# Regression test for https://github.com/astral-sh/ruff/issues/7155

def foobar(foor, bar={}):
import os
@@ -0,0 +1,5 @@
# Docstring with no newline


def foobar(foor, bar={}):
import os
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
Expand Up @@ -38,6 +38,8 @@ mod tests {
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_3.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_4.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_5.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_6.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_7.py"))]
#[test_case(Rule::NoExplicitStacklevel, Path::new("B028.py"))]
#[test_case(Rule::RaiseLiteral, Path::new("B016.py"))]
#[test_case(Rule::RaiseWithoutFromInsideExcept, Path::new("B904.py"))]
Expand Down
Expand Up @@ -192,6 +192,12 @@ fn move_initialization(
} else if statement.is_import_stmt() || statement.is_import_from_stmt() {
// If the statement in the function is an import, insert _after_ it.
Copy link
Member

Choose a reason for hiding this comment

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

Do you also need to handle the locator.full_line_end(statement.end()) == locator.text_len() case from above here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will take a look at it later today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should now be handled.

pos = locator.full_line_end(statement.end());
if pos == locator.text_len() {
// If the statement is at the end of the file, without a trailing newline, insert
// _after_ it with an extra newline.
content = format!("{}{}", stylist.line_ending().as_str(), content);
break;
}
} else {
break;
};
Expand Down
Expand Up @@ -151,7 +151,8 @@ B006_5.py:35:59: B006 [*] Do not use mutable data structures for argument defaul
35 |+def import_docstring_module_wrong(value: dict[str, str] = None):
36 36 | """Docstring"""
37 37 | import os
38 |+ if value is None:
39 |+ value = {}
38 |+
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line was added when implementing the locator.full_line_end(statement.end()) == locator.text_len() case.

I can see something similar happens when doing it with docstrings on 0.0.291.

This will:

def t(foo=[]):
    """Docstring"""

a = 2

will be fixed to

def t(foo=None):
    """Docstring"""
    if foo is None:
        foo = []

a = 2

Whereas

def t(foo=[]):
    """Docstring"""

will be fixed to

def t(foo=None):
    """Docstring"""

    if foo is None:
        foo = []

39 |+ if value is None:
40 |+ value = {}


@@ -0,0 +1,25 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
B006_6.py:4:22: B006 [*] Do not use mutable data structures for argument defaults
|
2 | # Regression test for https://github.com/astral-sh/ruff/issues/7155
3 |
4 | def foobar(foor, bar={}):
| ^^ B006
5 | import os
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
1 1 | # Docstring followed by whitespace with no newline
2 2 | # Regression test for https://github.com/astral-sh/ruff/issues/7155
3 3 |
4 |-def foobar(foor, bar={}):
5 |- import os
4 |+def foobar(foor, bar=None):
5 |+ import os
6 |+ if bar is None:
7 |+ bar = {}


@@ -0,0 +1,23 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
B006_7.py:4:22: B006 [*] Do not use mutable data structures for argument defaults
|
4 | def foobar(foor, bar={}):
| ^^ B006
5 | import os
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
1 1 | # Docstring with no newline
2 2 |
3 3 |
4 |-def foobar(foor, bar={}):
5 |- import os
4 |+def foobar(foor, bar=None):
5 |+ import os
6 |+ if bar is None:
7 |+ bar = {}