diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI021_1.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI021_1.pyi new file mode 100644 index 0000000000000..59dceece73ba1 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI021_1.pyi @@ -0,0 +1,18 @@ +from contextlib import nullcontext + + +def check_isolation_level(mode: int) -> None: + """Will report both, but only fix the first.""" # ERROR PYI021 + ... # ERROR PIE790 + + +with nullcontext(): + """Should not report.""" + # add something thats not a pass here + # to not get a remove unnecessary pass err + x = 0 + +if True: + """Should not report.""" + # same as above + y = 1 \ No newline at end of file diff --git a/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs b/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs index 9938ba4b33070..e3a2567bbfbe2 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs @@ -145,9 +145,6 @@ pub(crate) fn definitions(checker: &mut Checker) { } // flake8-pyi - if enforce_stubs { - flake8_pyi::rules::docstring_in_stubs(checker, definition, docstring); - } if enforce_stubs_and_runtime { flake8_pyi::rules::iter_method_return_iterable(checker, definition); } diff --git a/crates/ruff_linter/src/checkers/ast/analyze/suite.rs b/crates/ruff_linter/src/checkers/ast/analyze/suite.rs index 503a864259471..697e485018981 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/suite.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/suite.rs @@ -2,14 +2,17 @@ use ruff_python_ast::Stmt; use crate::checkers::ast::Checker; use crate::codes::Rule; -use crate::rules::flake8_pie; use crate::rules::refurb; +use crate::rules::{flake8_pie, flake8_pyi}; /// Run lint rules over a suite of [`Stmt`] syntax nodes. pub(crate) fn suite(suite: &[Stmt], checker: &Checker) { if checker.is_rule_enabled(Rule::UnnecessaryPlaceholder) { flake8_pie::rules::unnecessary_placeholder(checker, suite); } + if checker.source_type.is_stub() && checker.is_rule_enabled(Rule::DocstringInStub) { + flake8_pyi::rules::docstring_in_stubs(checker, suite); + } if checker.is_rule_enabled(Rule::RepeatedGlobal) { refurb::rules::repeated_global(checker, suite); } diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index a38086137e30d..446287f6b8438 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -86,7 +86,7 @@ mod deferred; /// State representing whether a docstring is expected or not for the next statement. #[derive(Debug, Copy, Clone, PartialEq)] -enum DocstringState { +pub(crate) enum DocstringState { /// The next statement is expected to be a docstring, but not necessarily so. /// /// For example, in the following code: @@ -127,7 +127,7 @@ impl DocstringState { /// The kind of an expected docstring. #[derive(Debug, Copy, Clone, PartialEq)] -enum ExpectedDocstringKind { +pub(crate) enum ExpectedDocstringKind { /// A module-level docstring. /// /// For example, @@ -602,6 +602,11 @@ impl<'a> Checker<'a> { pub(crate) const fn context(&self) -> &'a LintContext<'a> { self.context } + + /// Return the current [`DocstringState`]. + pub(crate) fn docstring_state(&self) -> DocstringState { + self.docstring_state + } } pub(crate) struct TypingImporter<'a, 'b> { diff --git a/crates/ruff_linter/src/rules/flake8_pyi/mod.rs b/crates/ruff_linter/src/rules/flake8_pyi/mod.rs index a065946483ae6..1248e3c029124 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/mod.rs @@ -192,4 +192,17 @@ mod tests { assert_diagnostics!(snapshot, diagnostics); Ok(()) } + + #[test_case(Path::new("PYI021_1.pyi"))] + fn pyi021_pie790_isolation_check(path: &Path) -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_pyi").join(path).as_path(), + &settings::LinterSettings::for_rules([ + Rule::DocstringInStub, + Rule::UnnecessaryPlaceholder, + ]), + )?; + assert_diagnostics!(diagnostics); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/docstring_in_stubs.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/docstring_in_stubs.rs index 5921058fbec35..6db0029643b53 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/docstring_in_stubs.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/docstring_in_stubs.rs @@ -1,9 +1,9 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprStringLiteral; -use ruff_python_semantic::Definition; +use ruff_python_ast::{ExprStringLiteral, Stmt}; use ruff_text_size::Ranged; -use crate::checkers::ast::Checker; +use crate::checkers::ast::{Checker, DocstringState, ExpectedDocstringKind}; +use crate::docstrings::extraction::docstring_from; use crate::{AlwaysFixableViolation, Edit, Fix}; /// ## What it does @@ -41,26 +41,34 @@ impl AlwaysFixableViolation for DocstringInStub { } /// PYI021 -pub(crate) fn docstring_in_stubs( - checker: &Checker, - definition: &Definition, - docstring: Option<&ExprStringLiteral>, -) { - let Some(docstring_range) = docstring.map(ExprStringLiteral::range) else { +pub(crate) fn docstring_in_stubs(checker: &Checker, body: &[Stmt]) { + if !matches!( + checker.docstring_state(), + DocstringState::Expected( + ExpectedDocstringKind::Module + | ExpectedDocstringKind::Class + | ExpectedDocstringKind::Function + ) + ) { return; - }; + } + + let docstring = docstring_from(body); - let statements = match definition { - Definition::Module(module) => module.python_ast, - Definition::Member(member) => member.body(), + let Some(docstring_range) = docstring.map(ExprStringLiteral::range) else { + return; }; - let edit = if statements.len() == 1 { + let edit = if body.len() == 1 { Edit::range_replacement("...".to_string(), docstring_range) } else { Edit::range_deletion(docstring_range) }; - let mut diagnostic = checker.report_diagnostic(DocstringInStub, docstring_range); - diagnostic.set_fix(Fix::unsafe_edit(edit)); + let isolation_level = Checker::isolation(checker.semantic().current_statement_id()); + let fix = Fix::unsafe_edit(edit).isolate(isolation_level); + + checker + .report_diagnostic(DocstringInStub, docstring_range) + .set_fix(fix); } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__pyi021_pie790_isolation_check.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__pyi021_pie790_isolation_check.snap new file mode 100644 index 0000000000000..dc2d6688d9d7c --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__pyi021_pie790_isolation_check.snap @@ -0,0 +1,39 @@ +--- +source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs +--- +PYI021 [*] Docstrings should not be included in stubs + --> PYI021_1.pyi:5:5 + | +4 | def check_isolation_level(mode: int) -> None: +5 | """Will report both, but only fix the first.""" # ERROR PYI021 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +6 | ... # ERROR PIE790 + | +help: Remove docstring +2 | +3 | +4 | def check_isolation_level(mode: int) -> None: + - """Will report both, but only fix the first.""" # ERROR PYI021 +5 + # ERROR PYI021 +6 | ... # ERROR PIE790 +7 | +8 | +note: This is an unsafe fix and may change runtime behavior + +PIE790 [*] Unnecessary `...` literal + --> PYI021_1.pyi:6:5 + | +4 | def check_isolation_level(mode: int) -> None: +5 | """Will report both, but only fix the first.""" # ERROR PYI021 +6 | ... # ERROR PIE790 + | ^^^ + | +help: Remove unnecessary `...` +3 | +4 | def check_isolation_level(mode: int) -> None: +5 | """Will report both, but only fix the first.""" # ERROR PYI021 + - ... # ERROR PIE790 +6 + # ERROR PIE790 +7 | +8 | +9 | with nullcontext():