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

Remove special pre-visit for module docstrings #9261

Merged
merged 1 commit into from
Dec 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import a

"""Some other docstring."""

import b

"""Some other docstring."""

import c
5 changes: 5 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyflakes/F404_1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
"""Docstring"""

"""Non-docstring"""

from __future__ import absolute_import
36 changes: 17 additions & 19 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP025.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,28 @@
# These should change
x = u"Hello"
u"Hello"

u'world'
x = u"Hello" # UP025

print(u"Hello")
u'world' # UP025

print(u'world')
print(u"Hello") # UP025

import foo

foo(u"Hello", U"world", a=u"Hello", b=u"world")
print(u'world') # UP025

# These should stay quoted they way they are
import foo

x = u'hello'
x = u"""hello"""
x = u'''hello'''
x = u'Hello "World"'
foo(u"Hello", U"world", a=u"Hello", b=u"world") # UP025

# These should not change
u = "Hello"
# Retain quotes when fixing.
x = u'hello' # UP025
x = u"""hello""" # UP025
x = u'''hello''' # UP025
x = u'Hello "World"' # UP025

u = u
u = "Hello" # OK
u = u # OK

def hello():
return"Hello"
return"Hello" # OK

f"foo"u"bar"
f"foo" u"bar"
f"foo"u"bar" # OK
f"foo" u"bar" # OK
26 changes: 15 additions & 11 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,18 @@ where

// Track whether we've seen docstrings, non-imports, etc.
match stmt {
Stmt::Expr(ast::StmtExpr { value, .. })
if !self
.semantic
.flags
.intersects(SemanticModelFlags::MODULE_DOCSTRING)
&& value.is_string_literal_expr() =>
{
self.semantic.flags |= SemanticModelFlags::MODULE_DOCSTRING;
}
Stmt::ImportFrom(ast::StmtImportFrom { module, names, .. }) => {
self.semantic.flags |= SemanticModelFlags::MODULE_DOCSTRING;

// Allow __future__ imports until we see a non-__future__ import.
if let Some("__future__") = module.as_deref() {
if names
Expand All @@ -301,9 +312,11 @@ where
}
}
Stmt::Import(_) => {
self.semantic.flags |= SemanticModelFlags::MODULE_DOCSTRING;
self.semantic.flags |= SemanticModelFlags::FUTURES_BOUNDARY;
}
_ => {
self.semantic.flags |= SemanticModelFlags::MODULE_DOCSTRING;
self.semantic.flags |= SemanticModelFlags::FUTURES_BOUNDARY;
if !(self.semantic.seen_import_boundary()
|| helpers::is_assignment_to_a_dunder(stmt)
Expand Down Expand Up @@ -1435,11 +1448,8 @@ where

impl<'a> Checker<'a> {
/// Visit a [`Module`]. Returns `true` if the module contains a module-level docstring.
fn visit_module(&mut self, python_ast: &'a Suite) -> bool {
fn visit_module(&mut self, python_ast: &'a Suite) {
analyze::module(python_ast, self);

let docstring = docstrings::extraction::docstring_from(python_ast);
docstring.is_some()
}

/// Visit a list of [`Comprehension`] nodes, assumed to be the comprehensions that compose a
Expand Down Expand Up @@ -2006,14 +2016,8 @@ pub(crate) fn check_ast(
);
checker.bind_builtins();

// Check for module docstring.
let python_ast = if checker.visit_module(python_ast) {
&python_ast[1..]
} else {
python_ast
};

// Iterate over the AST.
checker.visit_module(python_ast);
checker.visit_body(python_ast);

// Visit any deferred syntax nodes. Take care to visit in order, such that we avoid adding
Expand Down
5 changes: 3 additions & 2 deletions crates/ruff_linter/src/rules/pycodestyle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ mod tests {
#[test_case(Rule::LineTooLong, Path::new("E501_3.py"))]
#[test_case(Rule::MixedSpacesAndTabs, Path::new("E101.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E40.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_0.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_1.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.ipynb"))]
#[test_case(Rule::MultipleImportsOnOneLine, Path::new("E40.py"))]
#[test_case(Rule::MultipleStatementsOnOneLineColon, Path::new("E70.py"))]
Expand Down Expand Up @@ -65,7 +66,7 @@ mod tests {
}

#[test_case(Rule::IsLiteral, Path::new("constant_literals.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_0.py"))]
#[test_case(Rule::TypeComparison, Path::new("E721.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E402.py:25:1: E402 Module level import not at top of file
E402_0.py:25:1: E402 Module level import not at top of file
|
23 | sys.path.insert(0, "some/path")
24 |
Expand All @@ -11,7 +11,7 @@ E402.py:25:1: E402 Module level import not at top of file
27 | import matplotlib
|

E402.py:27:1: E402 Module level import not at top of file
E402_0.py:27:1: E402 Module level import not at top of file
|
25 | import f
26 |
Expand All @@ -21,7 +21,7 @@ E402.py:27:1: E402 Module level import not at top of file
29 | matplotlib.use("Agg")
|

E402.py:31:1: E402 Module level import not at top of file
E402_0.py:31:1: E402 Module level import not at top of file
|
29 | matplotlib.use("Agg")
30 |
Expand All @@ -31,23 +31,23 @@ E402.py:31:1: E402 Module level import not at top of file
33 | __some__magic = 1
|

E402.py:35:1: E402 Module level import not at top of file
E402_0.py:35:1: E402 Module level import not at top of file
|
33 | __some__magic = 1
34 |
35 | import h
| ^^^^^^^^ E402
|

E402.py:45:1: E402 Module level import not at top of file
E402_0.py:45:1: E402 Module level import not at top of file
|
43 | import j
44 |
45 | import k; import l
| ^^^^^^^^ E402
|

E402.py:45:11: E402 Module level import not at top of file
E402_0.py:45:11: E402 Module level import not at top of file
|
43 | import j
44 |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E402_1.py:5:1: E402 Module level import not at top of file
|
3 | """Some other docstring."""
4 |
5 | import b
| ^^^^^^^^ E402
6 |
7 | """Some other docstring."""
|

E402_1.py:9:1: E402 Module level import not at top of file
|
7 | """Some other docstring."""
8 |
9 | import c
| ^^^^^^^^ E402
|


Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E402.py:35:1: E402 Module level import not at top of file
E402_0.py:35:1: E402 Module level import not at top of file
|
33 | __some__magic = 1
34 |
35 | import h
| ^^^^^^^^ E402
|

E402.py:45:1: E402 Module level import not at top of file
E402_0.py:45:1: E402 Module level import not at top of file
|
43 | import j
44 |
45 | import k; import l
| ^^^^^^^^ E402
|

E402.py:45:11: E402 Module level import not at top of file
E402_0.py:45:11: E402 Module level import not at top of file
|
43 | import j
44 |
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff_linter/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ mod tests {
#[test_case(Rule::UnusedImport, Path::new("F401_20.py"))]
#[test_case(Rule::ImportShadowedByLoopVar, Path::new("F402.py"))]
#[test_case(Rule::UndefinedLocalWithImportStar, Path::new("F403.py"))]
#[test_case(Rule::LateFutureImport, Path::new("F404.py"))]
#[test_case(Rule::LateFutureImport, Path::new("F404_0.py"))]
#[test_case(Rule::LateFutureImport, Path::new("F404_1.py"))]
#[test_case(Rule::UndefinedLocalWithImportStarUsage, Path::new("F405.py"))]
#[test_case(Rule::UndefinedLocalWithNestedImportStarUsage, Path::new("F406.py"))]
#[test_case(Rule::FutureFeatureNotDefined, Path::new("F407.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F404.py:6:1: F404 `from __future__` imports must occur at the beginning of the file
F404_0.py:6:1: F404 `from __future__` imports must occur at the beginning of the file
|
4 | from collections import namedtuple
5 |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F404_1.py:5:1: F404 `from __future__` imports must occur at the beginning of the file
|
3 | """Non-docstring"""
4 |
5 | from __future__ import absolute_import
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F404
|