From 72e0ffc1acdbff9709ac3fc381e0cd2e96e05355 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 11 May 2023 13:14:29 -0400 Subject: [PATCH] Delay computation of `Definition` visibility (#4339) --- crates/ruff/src/checkers/ast/deferred.rs | 6 +- crates/ruff/src/checkers/ast/mod.rs | 471 ++++++++-------- crates/ruff/src/docstrings/definition.rs | 140 ----- crates/ruff/src/docstrings/extraction.rs | 105 ++-- crates/ruff/src/docstrings/mod.rs | 85 ++- crates/ruff/src/docstrings/sections.rs | 2 +- .../src/rules/flake8_annotations/helpers.rs | 18 +- .../src/rules/flake8_annotations/rules.rs | 504 +++++++++--------- crates/ruff/src/rules/pydocstyle/helpers.rs | 14 +- .../src/rules/pydocstyle/rules/backslashes.rs | 2 +- .../pydocstyle/rules/blank_after_summary.rs | 2 +- .../rules/blank_before_after_class.rs | 15 +- .../rules/blank_before_after_function.rs | 19 +- .../src/rules/pydocstyle/rules/capitalized.rs | 10 +- .../pydocstyle/rules/ends_with_period.rs | 2 +- .../pydocstyle/rules/ends_with_punctuation.rs | 2 +- .../src/rules/pydocstyle/rules/if_needed.rs | 15 +- .../ruff/src/rules/pydocstyle/rules/indent.rs | 2 +- .../rules/multi_line_summary_start.rs | 27 +- .../rules/newline_after_last_paragraph.rs | 2 +- .../rules/pydocstyle/rules/no_signature.rs | 15 +- .../rules/no_surrounding_whitespace.rs | 2 +- .../pydocstyle/rules/non_imperative_mood.rs | 20 +- .../src/rules/pydocstyle/rules/not_empty.rs | 2 +- .../src/rules/pydocstyle/rules/not_missing.rs | 41 +- .../src/rules/pydocstyle/rules/one_liner.rs | 2 +- .../src/rules/pydocstyle/rules/sections.rs | 22 +- .../pydocstyle/rules/starts_with_this.rs | 2 +- .../rules/pydocstyle/rules/triple_quotes.rs | 2 +- .../src/analyze/visibility.rs | 77 +-- crates/ruff_python_semantic/src/context.rs | 43 +- crates/ruff_python_semantic/src/definition.rs | 225 ++++++++ crates/ruff_python_semantic/src/lib.rs | 1 + 33 files changed, 1064 insertions(+), 833 deletions(-) delete mode 100644 crates/ruff/src/docstrings/definition.rs create mode 100644 crates/ruff_python_semantic/src/definition.rs diff --git a/crates/ruff/src/checkers/ast/deferred.rs b/crates/ruff/src/checkers/ast/deferred.rs index ae79c37542..cefc201419 100644 --- a/crates/ruff/src/checkers/ast/deferred.rs +++ b/crates/ruff/src/checkers/ast/deferred.rs @@ -1,20 +1,16 @@ use ruff_text_size::TextRange; use rustpython_parser::ast::Expr; -use ruff_python_semantic::analyze::visibility::{Visibility, VisibleScope}; use ruff_python_semantic::context::Snapshot; -use crate::docstrings::definition::Definition; - /// A collection of AST nodes that are deferred for later analysis. /// Used to, e.g., store functions, whose bodies shouldn't be analyzed until all /// module-level definitions have been analyzed. #[derive(Default)] pub struct Deferred<'a> { - pub definitions: Vec<(Definition<'a>, Visibility, Snapshot)>, pub string_type_definitions: Vec<(TextRange, &'a str, Snapshot)>, pub type_definitions: Vec<(&'a Expr, Snapshot)>, - pub functions: Vec<(Snapshot, VisibleScope)>, + pub functions: Vec, pub lambdas: Vec<(&'a Expr, Snapshot)>, pub for_loops: Vec, pub assignments: Vec, diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 632d03aa9d..879340a3aa 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -22,20 +22,21 @@ use ruff_python_ast::{cast, helpers, str, visitor}; use ruff_python_semantic::analyze; use ruff_python_semantic::analyze::branch_detection; use ruff_python_semantic::analyze::typing::{Callable, SubscriptKind}; +use ruff_python_semantic::analyze::visibility::ModuleSource; use ruff_python_semantic::binding::{ Binding, BindingId, BindingKind, Exceptions, ExecutionContext, Export, FromImportation, Importation, StarImportation, SubmoduleImportation, }; use ruff_python_semantic::context::Context; +use ruff_python_semantic::definition::{Module, ModuleKind}; use ruff_python_semantic::node::NodeId; use ruff_python_semantic::scope::{ClassDef, FunctionDef, Lambda, Scope, ScopeId, ScopeKind}; use ruff_python_stdlib::builtins::{BUILTINS, MAGIC_GLOBALS}; use ruff_python_stdlib::path::is_python_stub_file; use crate::checkers::ast::deferred::Deferred; -use crate::docstrings::definition::{ - transition_scope, Definition, DefinitionKind, Docstring, Documentable, -}; +use crate::docstrings::extraction::ExtractionTarget; +use crate::docstrings::Docstring; use crate::fs::relativize_path; use crate::importer::Importer; use crate::noqa::NoqaMapping; @@ -59,7 +60,7 @@ mod deferred; pub struct Checker<'a> { // Settings, static metadata, etc. path: &'a Path, - module_path: Option>, + module_path: Option<&'a [String]>, package: Option<&'a Path>, is_stub: bool, noqa: flags::Noqa, @@ -86,7 +87,7 @@ impl<'a> Checker<'a> { noqa: flags::Noqa, path: &'a Path, package: Option<&'a Path>, - module_path: Option>, + module: Module<'a>, locator: &'a Locator, stylist: &'a Stylist, indexer: &'a Indexer, @@ -98,13 +99,13 @@ impl<'a> Checker<'a> { noqa, path, package, - module_path: module_path.clone(), + module_path: module.path(), is_stub: is_python_stub_file(path), locator, stylist, indexer, importer, - ctx: Context::new(&settings.typing_modules, path, module_path), + ctx: Context::new(&settings.typing_modules, path, module), deferred: Deferred::default(), diagnostics: Vec::default(), deletions: FxHashSet::default(), @@ -996,7 +997,7 @@ where } if self.settings.rules.enabled(Rule::ImportSelf) { if let Some(diagnostic) = - pylint::rules::import_self(alias, self.module_path.as_deref()) + pylint::rules::import_self(alias, self.module_path) { self.diagnostics.push(diagnostic); } @@ -1166,11 +1167,9 @@ where } if self.settings.rules.enabled(Rule::BannedApi) { - if let Some(module) = helpers::resolve_imported_module_path( - level, - module, - self.module_path.as_deref(), - ) { + if let Some(module) = + helpers::resolve_imported_module_path(level, module, self.module_path) + { flake8_tidy_imports::banned_api::name_or_parent_is_banned( self, &module, stmt, ); @@ -1315,7 +1314,7 @@ where stmt, level, module, - self.module_path.as_deref(), + self.module_path, &self.settings.flake8_tidy_imports.ban_relative_imports, ) { @@ -1460,12 +1459,9 @@ where } if self.settings.rules.enabled(Rule::ImportSelf) { - if let Some(diagnostic) = pylint::rules::import_from_self( - level, - module, - names, - self.module_path.as_deref(), - ) { + if let Some(diagnostic) = + pylint::rules::import_from_self(level, module, names, self.module_path) + { self.diagnostics.push(diagnostic); } } @@ -1944,7 +1940,6 @@ where // Recurse. let prev_in_exception_handler = self.ctx.in_exception_handler; - let prev_visible_scope = self.ctx.visible_scope; match &stmt.node { StmtKind::FunctionDef(ast::StmtFunctionDef { body, @@ -1963,20 +1958,9 @@ where if self.settings.rules.enabled(Rule::FStringDocstring) { flake8_bugbear::rules::f_string_docstring(self, body); } - let definition = docstrings::extraction::extract( - self.ctx.visible_scope, - stmt, - body, - Documentable::Function, - ); if self.settings.rules.enabled(Rule::YieldInForLoop) { pyupgrade::rules::yield_in_for_loop(self, stmt); } - let scope = transition_scope(self.ctx.visible_scope, stmt, Documentable::Function); - self.deferred - .definitions - .push((definition, scope.visibility, self.ctx.snapshot())); - self.ctx.visible_scope = scope; // If any global bindings don't already exist in the global scope, add it. let globals = helpers::extract_globals(body); @@ -2001,6 +1985,14 @@ where } } + let definition = docstrings::extraction::extract_definition( + ExtractionTarget::Function, + stmt, + self.ctx.definition_id, + &self.ctx.definitions, + ); + self.ctx.push_definition(definition); + self.ctx.push_scope(ScopeKind::Function(FunctionDef { name, body, @@ -2010,9 +2002,7 @@ where globals, })); - self.deferred - .functions - .push((self.ctx.snapshot(), self.ctx.visible_scope)); + self.deferred.functions.push(self.ctx.snapshot()); } StmtKind::ClassDef(ast::StmtClassDef { body, @@ -2024,17 +2014,6 @@ where if self.settings.rules.enabled(Rule::FStringDocstring) { flake8_bugbear::rules::f_string_docstring(self, body); } - let definition = docstrings::extraction::extract( - self.ctx.visible_scope, - stmt, - body, - Documentable::Class, - ); - let scope = transition_scope(self.ctx.visible_scope, stmt, Documentable::Class); - self.deferred - .definitions - .push((definition, scope.visibility, self.ctx.snapshot())); - self.ctx.visible_scope = scope; // If any global bindings don't already exist in the global scope, add it. let globals = helpers::extract_globals(body); @@ -2059,6 +2038,14 @@ where } } + let definition = docstrings::extraction::extract_definition( + ExtractionTarget::Class, + stmt, + self.ctx.definition_id, + &self.ctx.definitions, + ); + self.ctx.push_definition(definition); + self.ctx.push_scope(ScopeKind::Class(ClassDef { name, bases, @@ -2195,15 +2182,16 @@ where } _ => visitor::walk_stmt(self, stmt), }; - self.ctx.visible_scope = prev_visible_scope; // Post-visit. match &stmt.node { StmtKind::FunctionDef(_) | StmtKind::AsyncFunctionDef(_) => { self.ctx.pop_scope(); + self.ctx.pop_definition(); } StmtKind::ClassDef(ast::StmtClassDef { name, .. }) => { self.ctx.pop_scope(); + self.ctx.pop_definition(); self.add_binding( name, Binding { @@ -4751,23 +4739,11 @@ impl<'a> Checker<'a> { )); } - fn visit_docstring(&mut self, python_ast: &'a Suite) -> bool { + fn visit_module(&mut self, python_ast: &'a Suite) -> bool { if self.settings.rules.enabled(Rule::FStringDocstring) { flake8_bugbear::rules::f_string_docstring(self, python_ast); } let docstring = docstrings::extraction::docstring_from(python_ast); - self.deferred.definitions.push(( - Definition { - kind: if self.path.ends_with("__init__.py") { - DefinitionKind::Package - } else { - DefinitionKind::Module - }, - docstring, - }, - self.ctx.visible_scope.visibility, - self.ctx.snapshot(), - )); docstring.is_some() } @@ -4794,8 +4770,6 @@ impl<'a> Checker<'a> { let expr = allocator.alloc(expr); self.ctx.restore(snapshot); - self.ctx.in_type_definition = true; - self.ctx.in_deferred_string_type_definition = Some(kind); if self.ctx.in_annotation && self.ctx.annotations_future_enabled { if self.settings.rules.enabled(Rule::QuotedAnnotation) { @@ -4807,6 +4781,9 @@ impl<'a> Checker<'a> { flake8_pyi::rules::quoted_annotation_in_stub(self, value, range); } } + + self.ctx.in_type_definition = true; + self.ctx.in_deferred_string_type_definition = Some(kind); self.visit_expr(expr); self.ctx.in_deferred_string_type_definition = None; @@ -4832,9 +4809,8 @@ impl<'a> Checker<'a> { fn check_deferred_functions(&mut self) { while !self.deferred.functions.is_empty() { let deferred_functions = std::mem::take(&mut self.deferred.functions); - for (snapshot, visibility) in deferred_functions { + for snapshot in deferred_functions { self.ctx.restore(snapshot); - self.ctx.visible_scope = visibility; match &self.ctx.stmt().node { StmtKind::FunctionDef(ast::StmtFunctionDef { body, args, .. }) @@ -5343,6 +5319,11 @@ impl<'a> Checker<'a> { self.diagnostics.extend(diagnostics); } + /// Visit all the [`Definition`] nodes in the AST. + /// + /// This phase is expected to run after the AST has been traversed in its entirety; as such, + /// it is expected that all [`Definition`] nodes have been visited by the time, and that this + /// method will not recurse into any other nodes. fn check_definitions(&mut self) { let enforce_annotations = self.settings.rules.any_enabled(&[ Rule::MissingTypeFunctionArgument, @@ -5357,6 +5338,8 @@ impl<'a> Checker<'a> { Rule::MissingReturnTypeClassMethod, Rule::AnyType, ]); + let enforce_stubs = + self.is_stub && self.settings.rules.any_enabled(&[Rule::DocstringInStub]); let enforce_docstrings = self.settings.rules.any_enabled(&[ Rule::UndocumentedPublicModule, Rule::UndocumentedPublicClass, @@ -5406,185 +5389,187 @@ impl<'a> Checker<'a> { Rule::EmptyDocstring, ]); + if !enforce_annotations && !enforce_docstrings && !enforce_stubs { + return; + } + let mut overloaded_name: Option = None; - while !self.deferred.definitions.is_empty() { - let definitions = std::mem::take(&mut self.deferred.definitions); - for (definition, visibility, snapshot) in definitions { - self.ctx.restore(snapshot); - // flake8-annotations - if enforce_annotations { - // TODO(charlie): This should be even stricter, in that an overload - // implementation should come immediately after the overloaded - // interfaces, without any AST nodes in between. Right now, we - // only error when traversing definition boundaries (functions, - // classes, etc.). - if !overloaded_name.map_or(false, |overloaded_name| { - flake8_annotations::helpers::is_overload_impl( - self, - &definition, - &overloaded_name, - ) - }) { - self.diagnostics - .extend(flake8_annotations::rules::definition( - self, - &definition, - visibility, - )); - } - overloaded_name = - flake8_annotations::helpers::overloaded_name(self, &definition); + let definitions = std::mem::take(&mut self.ctx.definitions); + for (definition, visibility) in definitions.iter() { + let docstring = docstrings::extraction::extract_docstring(definition); + + // flake8-annotations + if enforce_annotations { + // TODO(charlie): This should be even stricter, in that an overload + // implementation should come immediately after the overloaded + // interfaces, without any AST nodes in between. Right now, we + // only error when traversing definition boundaries (functions, + // classes, etc.). + if !overloaded_name.map_or(false, |overloaded_name| { + flake8_annotations::helpers::is_overload_impl( + self, + definition, + &overloaded_name, + ) + }) { + self.diagnostics + .extend(flake8_annotations::rules::definition( + self, definition, visibility, + )); } + overloaded_name = flake8_annotations::helpers::overloaded_name(self, definition); + } + // flake8-pyi + if enforce_stubs { if self.is_stub { if self.settings.rules.enabled(Rule::DocstringInStub) { - flake8_pyi::rules::docstring_in_stubs(self, definition.docstring); + flake8_pyi::rules::docstring_in_stubs(self, docstring); } } + } - // pydocstyle - if enforce_docstrings { - if pydocstyle::helpers::should_ignore_definition( + // pydocstyle + if enforce_docstrings { + if pydocstyle::helpers::should_ignore_definition( + self, + definition, + &self.settings.pydocstyle.ignore_decorators, + ) { + continue; + } + + // Extract a `Docstring` from a `Definition`. + let Some(expr) = docstring else { + pydocstyle::rules::not_missing(self, definition, visibility); + continue; + }; + + let contents = self.locator.slice(expr.range()); + + let indentation = self.locator.slice(TextRange::new( + self.locator.line_start(expr.start()), + expr.start(), + )); + + if pydocstyle::helpers::should_ignore_docstring(contents) { + #[allow(deprecated)] + let location = self.locator.compute_source_location(expr.start()); + warn_user!( + "Docstring at {}:{}:{} contains implicit string concatenation; ignoring...", + relativize_path(self.path), + location.row, + location.column + ); + continue; + } + + // SAFETY: Safe for docstrings that pass `should_ignore_docstring`. + let body_range = str::raw_contents_range(contents).unwrap(); + let docstring = Docstring { + definition, + expr, + contents, + body_range, + indentation, + }; + + if !pydocstyle::rules::not_empty(self, &docstring) { + continue; + } + + if self.settings.rules.enabled(Rule::FitsOnOneLine) { + pydocstyle::rules::one_liner(self, &docstring); + } + if self.settings.rules.any_enabled(&[ + Rule::NoBlankLineBeforeFunction, + Rule::NoBlankLineAfterFunction, + ]) { + pydocstyle::rules::blank_before_after_function(self, &docstring); + } + if self.settings.rules.any_enabled(&[ + Rule::OneBlankLineBeforeClass, + Rule::OneBlankLineAfterClass, + Rule::BlankLineBeforeClass, + ]) { + pydocstyle::rules::blank_before_after_class(self, &docstring); + } + if self.settings.rules.enabled(Rule::BlankLineAfterSummary) { + pydocstyle::rules::blank_after_summary(self, &docstring); + } + if self.settings.rules.any_enabled(&[ + Rule::IndentWithSpaces, + Rule::UnderIndentation, + Rule::OverIndentation, + ]) { + pydocstyle::rules::indent(self, &docstring); + } + if self.settings.rules.enabled(Rule::NewLineAfterLastParagraph) { + pydocstyle::rules::newline_after_last_paragraph(self, &docstring); + } + if self.settings.rules.enabled(Rule::SurroundingWhitespace) { + pydocstyle::rules::no_surrounding_whitespace(self, &docstring); + } + if self.settings.rules.any_enabled(&[ + Rule::MultiLineSummaryFirstLine, + Rule::MultiLineSummarySecondLine, + ]) { + pydocstyle::rules::multi_line_summary_start(self, &docstring); + } + if self.settings.rules.enabled(Rule::TripleSingleQuotes) { + pydocstyle::rules::triple_quotes(self, &docstring); + } + if self.settings.rules.enabled(Rule::EscapeSequenceInDocstring) { + pydocstyle::rules::backslashes(self, &docstring); + } + if self.settings.rules.enabled(Rule::EndsInPeriod) { + pydocstyle::rules::ends_with_period(self, &docstring); + } + if self.settings.rules.enabled(Rule::NonImperativeMood) { + pydocstyle::rules::non_imperative_mood( self, - &definition, - &self.settings.pydocstyle.ignore_decorators, - ) { - continue; - } - - if definition.docstring.is_none() { - pydocstyle::rules::not_missing(self, &definition, visibility); - continue; - } - - // Extract a `Docstring` from a `Definition`. - let expr = definition.docstring.unwrap(); - let contents = self.locator.slice(expr.range()); - - let indentation = self.locator.slice(TextRange::new( - self.locator.line_start(expr.start()), - expr.start(), - )); - - if pydocstyle::helpers::should_ignore_docstring(contents) { - #[allow(deprecated)] - let location = self.locator.compute_source_location(expr.start()); - warn_user!( - "Docstring at {}:{}:{} contains implicit string concatenation; ignoring...", - relativize_path(self.path), - location.row, - location.column - ); - continue; - } - - // SAFETY: Safe for docstrings that pass `should_ignore_docstring`. - let body_range = str::raw_contents_range(contents).unwrap(); - let docstring = Docstring { - kind: definition.kind, - expr, - contents, - indentation, - body_range, - }; - - if !pydocstyle::rules::not_empty(self, &docstring) { - continue; - } - - if self.settings.rules.enabled(Rule::FitsOnOneLine) { - pydocstyle::rules::one_liner(self, &docstring); - } - if self.settings.rules.any_enabled(&[ - Rule::NoBlankLineBeforeFunction, - Rule::NoBlankLineAfterFunction, - ]) { - pydocstyle::rules::blank_before_after_function(self, &docstring); - } - if self.settings.rules.any_enabled(&[ - Rule::OneBlankLineBeforeClass, - Rule::OneBlankLineAfterClass, - Rule::BlankLineBeforeClass, - ]) { - pydocstyle::rules::blank_before_after_class(self, &docstring); - } - if self.settings.rules.enabled(Rule::BlankLineAfterSummary) { - pydocstyle::rules::blank_after_summary(self, &docstring); - } - if self.settings.rules.any_enabled(&[ - Rule::IndentWithSpaces, - Rule::UnderIndentation, - Rule::OverIndentation, - ]) { - pydocstyle::rules::indent(self, &docstring); - } - if self.settings.rules.enabled(Rule::NewLineAfterLastParagraph) { - pydocstyle::rules::newline_after_last_paragraph(self, &docstring); - } - if self.settings.rules.enabled(Rule::SurroundingWhitespace) { - pydocstyle::rules::no_surrounding_whitespace(self, &docstring); - } - if self.settings.rules.any_enabled(&[ - Rule::MultiLineSummaryFirstLine, - Rule::MultiLineSummarySecondLine, - ]) { - pydocstyle::rules::multi_line_summary_start(self, &docstring); - } - if self.settings.rules.enabled(Rule::TripleSingleQuotes) { - pydocstyle::rules::triple_quotes(self, &docstring); - } - if self.settings.rules.enabled(Rule::EscapeSequenceInDocstring) { - pydocstyle::rules::backslashes(self, &docstring); - } - if self.settings.rules.enabled(Rule::EndsInPeriod) { - pydocstyle::rules::ends_with_period(self, &docstring); - } - if self.settings.rules.enabled(Rule::NonImperativeMood) { - pydocstyle::rules::non_imperative_mood( - self, - &docstring, - &self.settings.pydocstyle.property_decorators, - ); - } - if self.settings.rules.enabled(Rule::NoSignature) { - pydocstyle::rules::no_signature(self, &docstring); - } - if self.settings.rules.enabled(Rule::FirstLineCapitalized) { - pydocstyle::rules::capitalized(self, &docstring); - } - if self.settings.rules.enabled(Rule::DocstringStartsWithThis) { - pydocstyle::rules::starts_with_this(self, &docstring); - } - if self.settings.rules.enabled(Rule::EndsInPunctuation) { - pydocstyle::rules::ends_with_punctuation(self, &docstring); - } - if self.settings.rules.enabled(Rule::OverloadWithDocstring) { - pydocstyle::rules::if_needed(self, &docstring); - } - if self.settings.rules.any_enabled(&[ - Rule::MultiLineSummaryFirstLine, - Rule::SectionNotOverIndented, - Rule::SectionUnderlineNotOverIndented, - Rule::CapitalizeSectionName, - Rule::NewLineAfterSectionName, - Rule::DashedUnderlineAfterSection, - Rule::SectionUnderlineAfterName, - Rule::SectionUnderlineMatchesSectionLength, - Rule::NoBlankLineAfterSection, - Rule::NoBlankLineBeforeSection, - Rule::BlankLinesBetweenHeaderAndContent, - Rule::BlankLineAfterLastSection, - Rule::EmptyDocstringSection, - Rule::SectionNameEndsInColon, - Rule::UndocumentedParam, - ]) { - pydocstyle::rules::sections( - self, - &docstring, - self.settings.pydocstyle.convention.as_ref(), - ); - } + &docstring, + &self.settings.pydocstyle.property_decorators, + ); + } + if self.settings.rules.enabled(Rule::NoSignature) { + pydocstyle::rules::no_signature(self, &docstring); + } + if self.settings.rules.enabled(Rule::FirstLineCapitalized) { + pydocstyle::rules::capitalized(self, &docstring); + } + if self.settings.rules.enabled(Rule::DocstringStartsWithThis) { + pydocstyle::rules::starts_with_this(self, &docstring); + } + if self.settings.rules.enabled(Rule::EndsInPunctuation) { + pydocstyle::rules::ends_with_punctuation(self, &docstring); + } + if self.settings.rules.enabled(Rule::OverloadWithDocstring) { + pydocstyle::rules::if_needed(self, &docstring); + } + if self.settings.rules.any_enabled(&[ + Rule::MultiLineSummaryFirstLine, + Rule::SectionNotOverIndented, + Rule::SectionUnderlineNotOverIndented, + Rule::CapitalizeSectionName, + Rule::NewLineAfterSectionName, + Rule::DashedUnderlineAfterSection, + Rule::SectionUnderlineAfterName, + Rule::SectionUnderlineMatchesSectionLength, + Rule::NoBlankLineAfterSection, + Rule::NoBlankLineBeforeSection, + Rule::BlankLinesBetweenHeaderAndContent, + Rule::BlankLineAfterLastSection, + Rule::EmptyDocstringSection, + Rule::SectionNameEndsInColon, + Rule::UndocumentedParam, + ]) { + pydocstyle::rules::sections( + self, + &docstring, + self.settings.pydocstyle.convention.as_ref(), + ); } } } @@ -5647,13 +5632,28 @@ pub fn check_ast( path: &Path, package: Option<&Path>, ) -> Vec { + let module_path = package.and_then(|package| to_module_path(package, path)); + let module = Module { + kind: if path.ends_with("__init__.py") { + ModuleKind::Package + } else { + ModuleKind::Module + }, + source: if let Some(module_path) = module_path.as_ref() { + ModuleSource::Path(module_path) + } else { + ModuleSource::File(path) + }, + python_ast, + }; + let mut checker = Checker::new( settings, noqa_line_for, noqa, path, package, - package.and_then(|package| to_module_path(package, path)), + module, locator, stylist, indexer, @@ -5662,11 +5662,12 @@ pub fn check_ast( checker.bind_builtins(); // Check for module docstring. - let python_ast = if checker.visit_docstring(python_ast) { + let python_ast = if checker.visit_module(python_ast) { &python_ast[1..] } else { python_ast }; + // Iterate over the AST. checker.visit_body(python_ast); diff --git a/crates/ruff/src/docstrings/definition.rs b/crates/ruff/src/docstrings/definition.rs deleted file mode 100644 index cf338a364a..0000000000 --- a/crates/ruff/src/docstrings/definition.rs +++ /dev/null @@ -1,140 +0,0 @@ -use ruff_text_size::{TextRange, TextSize}; -use rustpython_parser::ast::{Expr, Stmt}; -use std::fmt::{Debug, Formatter}; -use std::ops::Deref; - -use ruff_python_semantic::analyze::visibility::{ - class_visibility, function_visibility, method_visibility, Modifier, Visibility, VisibleScope, -}; - -#[derive(Debug, Clone)] -pub enum DefinitionKind<'a> { - Module, - Package, - Class(&'a Stmt), - NestedClass(&'a Stmt), - Function(&'a Stmt), - NestedFunction(&'a Stmt), - Method(&'a Stmt), -} - -#[derive(Debug)] -pub struct Definition<'a> { - pub kind: DefinitionKind<'a>, - pub docstring: Option<&'a Expr>, -} - -#[derive(Debug)] -pub struct Docstring<'a> { - pub kind: DefinitionKind<'a>, - pub expr: &'a Expr, - /// The content of the docstring, including the leading and trailing quotes. - pub contents: &'a str, - - /// The range of the docstring body (without the quotes). The range is relative to [`Self::contents`]. - pub body_range: TextRange, - pub indentation: &'a str, -} - -impl<'a> Docstring<'a> { - pub fn body(&self) -> DocstringBody { - DocstringBody { docstring: self } - } - - pub const fn start(&self) -> TextSize { - self.expr.start() - } - - pub const fn end(&self) -> TextSize { - self.expr.end() - } - - pub const fn range(&self) -> TextRange { - self.expr.range() - } - - pub fn leading_quote(&self) -> &'a str { - &self.contents[TextRange::up_to(self.body_range.start())] - } -} - -#[derive(Copy, Clone)] -pub struct DocstringBody<'a> { - docstring: &'a Docstring<'a>, -} - -impl<'a> DocstringBody<'a> { - #[inline] - pub fn start(self) -> TextSize { - self.range().start() - } - - #[inline] - pub fn end(self) -> TextSize { - self.range().end() - } - - pub fn range(self) -> TextRange { - self.docstring.body_range + self.docstring.start() - } - - pub fn as_str(self) -> &'a str { - &self.docstring.contents[self.docstring.body_range] - } -} - -impl Deref for DocstringBody<'_> { - type Target = str; - - fn deref(&self) -> &Self::Target { - self.as_str() - } -} - -impl Debug for DocstringBody<'_> { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - f.debug_struct("DocstringBody") - .field("text", &self.as_str()) - .field("range", &self.range()) - .finish() - } -} - -#[derive(Copy, Clone)] -pub enum Documentable { - Class, - Function, -} - -pub fn transition_scope(scope: VisibleScope, stmt: &Stmt, kind: Documentable) -> VisibleScope { - match kind { - Documentable::Function => VisibleScope { - modifier: Modifier::Function, - visibility: match scope { - VisibleScope { - modifier: Modifier::Module, - visibility: Visibility::Public, - } => function_visibility(stmt), - VisibleScope { - modifier: Modifier::Class, - visibility: Visibility::Public, - } => method_visibility(stmt), - _ => Visibility::Private, - }, - }, - Documentable::Class => VisibleScope { - modifier: Modifier::Class, - visibility: match scope { - VisibleScope { - modifier: Modifier::Module, - visibility: Visibility::Public, - } => class_visibility(stmt), - VisibleScope { - modifier: Modifier::Class, - visibility: Visibility::Public, - } => class_visibility(stmt), - _ => Visibility::Private, - }, - }, - } -} diff --git a/crates/ruff/src/docstrings/extraction.rs b/crates/ruff/src/docstrings/extraction.rs index e9434cb5cb..f6574de642 100644 --- a/crates/ruff/src/docstrings/extraction.rs +++ b/crates/ruff/src/docstrings/extraction.rs @@ -2,12 +2,10 @@ use rustpython_parser::ast::{self, Constant, Expr, ExprKind, Stmt, StmtKind}; -use ruff_python_semantic::analyze::visibility; - -use crate::docstrings::definition::{Definition, DefinitionKind, Documentable}; +use ruff_python_semantic::definition::{Definition, DefinitionId, Definitions, Member, MemberKind}; /// Extract a docstring from a function or class body. -pub fn docstring_from(suite: &[Stmt]) -> Option<&Expr> { +pub(crate) fn docstring_from(suite: &[Stmt]) -> Option<&Expr> { let stmt = suite.first()?; // Require the docstring to be a standalone expression. let StmtKind::Expr(ast::StmtExpr { value }) = &stmt.node else { @@ -26,59 +24,68 @@ pub fn docstring_from(suite: &[Stmt]) -> Option<&Expr> { Some(value) } +/// Extract a docstring from a `Definition`. +pub(crate) fn extract_docstring<'a>(definition: &'a Definition<'a>) -> Option<&'a Expr> { + match definition { + Definition::Module(module) => docstring_from(module.python_ast), + Definition::Member(member) => { + if let StmtKind::ClassDef(ast::StmtClassDef { body, .. }) + | StmtKind::FunctionDef(ast::StmtFunctionDef { body, .. }) + | StmtKind::AsyncFunctionDef(ast::StmtAsyncFunctionDef { body, .. }) = + &member.stmt.node + { + docstring_from(body) + } else { + None + } + } + } +} + +#[derive(Copy, Clone)] +pub(crate) enum ExtractionTarget { + Class, + Function, +} + /// Extract a `Definition` from the AST node defined by a `Stmt`. -pub fn extract<'a>( - scope: visibility::VisibleScope, +pub(crate) fn extract_definition<'a>( + target: ExtractionTarget, stmt: &'a Stmt, - body: &'a [Stmt], - kind: Documentable, -) -> Definition<'a> { - let expr = docstring_from(body); - match kind { - Documentable::Function => match scope { - visibility::VisibleScope { - modifier: visibility::Modifier::Module, - .. - } => Definition { - kind: DefinitionKind::Function(stmt), - docstring: expr, + parent: DefinitionId, + definitions: &Definitions<'a>, +) -> Member<'a> { + match target { + ExtractionTarget::Function => match &definitions[parent] { + Definition::Module(..) => Member { + parent, + kind: MemberKind::Function, + stmt, }, - visibility::VisibleScope { - modifier: visibility::Modifier::Class, + Definition::Member(Member { + kind: MemberKind::Class | MemberKind::NestedClass, .. - } => Definition { - kind: DefinitionKind::Method(stmt), - docstring: expr, + }) => Member { + parent, + kind: MemberKind::Method, + stmt, }, - visibility::VisibleScope { - modifier: visibility::Modifier::Function, - .. - } => Definition { - kind: DefinitionKind::NestedFunction(stmt), - docstring: expr, + Definition::Member(..) => Member { + parent, + kind: MemberKind::NestedFunction, + stmt, }, }, - Documentable::Class => match scope { - visibility::VisibleScope { - modifier: visibility::Modifier::Module, - .. - } => Definition { - kind: DefinitionKind::Class(stmt), - docstring: expr, + ExtractionTarget::Class => match &definitions[parent] { + Definition::Module(..) => Member { + parent, + kind: MemberKind::Class, + stmt, }, - visibility::VisibleScope { - modifier: visibility::Modifier::Class, - .. - } => Definition { - kind: DefinitionKind::NestedClass(stmt), - docstring: expr, - }, - visibility::VisibleScope { - modifier: visibility::Modifier::Function, - .. - } => Definition { - kind: DefinitionKind::NestedClass(stmt), - docstring: expr, + Definition::Member(..) => Member { + parent, + kind: MemberKind::NestedClass, + stmt, }, }, } diff --git a/crates/ruff/src/docstrings/mod.rs b/crates/ruff/src/docstrings/mod.rs index 468f9f4608..aedde3ec2f 100644 --- a/crates/ruff/src/docstrings/mod.rs +++ b/crates/ruff/src/docstrings/mod.rs @@ -1,6 +1,89 @@ -pub mod definition; +use std::fmt::{Debug, Formatter}; +use std::ops::Deref; + +use ruff_text_size::{TextRange, TextSize}; +use rustpython_parser::ast::Expr; + +use ruff_python_semantic::definition::Definition; + pub mod extraction; pub mod google; pub mod numpy; pub mod sections; pub mod styles; + +#[derive(Debug)] +pub struct Docstring<'a> { + pub definition: &'a Definition<'a>, + pub expr: &'a Expr, + /// The content of the docstring, including the leading and trailing quotes. + pub contents: &'a str, + + /// The range of the docstring body (without the quotes). The range is relative to [`Self::contents`]. + pub body_range: TextRange, + pub indentation: &'a str, +} + +impl<'a> Docstring<'a> { + pub fn body(&self) -> DocstringBody { + DocstringBody { docstring: self } + } + + pub const fn start(&self) -> TextSize { + self.expr.start() + } + + pub const fn end(&self) -> TextSize { + self.expr.end() + } + + pub const fn range(&self) -> TextRange { + self.expr.range() + } + + pub fn leading_quote(&self) -> &'a str { + &self.contents[TextRange::up_to(self.body_range.start())] + } +} + +#[derive(Copy, Clone)] +pub struct DocstringBody<'a> { + docstring: &'a Docstring<'a>, +} + +impl<'a> DocstringBody<'a> { + #[inline] + pub fn start(self) -> TextSize { + self.range().start() + } + + #[inline] + pub fn end(self) -> TextSize { + self.range().end() + } + + pub fn range(self) -> TextRange { + self.docstring.body_range + self.docstring.start() + } + + pub fn as_str(self) -> &'a str { + &self.docstring.contents[self.docstring.body_range] + } +} + +impl Deref for DocstringBody<'_> { + type Target = str; + + fn deref(&self) -> &Self::Target { + self.as_str() + } +} + +impl Debug for DocstringBody<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("DocstringBody") + .field("text", &self.as_str()) + .field("range", &self.range()) + .finish() + } +} diff --git a/crates/ruff/src/docstrings/sections.rs b/crates/ruff/src/docstrings/sections.rs index 1685c7e761..7676b58c06 100644 --- a/crates/ruff/src/docstrings/sections.rs +++ b/crates/ruff/src/docstrings/sections.rs @@ -4,8 +4,8 @@ use std::fmt::{Debug, Formatter}; use std::iter::FusedIterator; use strum_macros::EnumIter; -use crate::docstrings::definition::{Docstring, DocstringBody}; use crate::docstrings::styles::SectionStyle; +use crate::docstrings::{Docstring, DocstringBody}; use ruff_python_ast::whitespace; #[derive(EnumIter, PartialEq, Eq, Debug, Clone, Copy)] diff --git a/crates/ruff/src/rules/flake8_annotations/helpers.rs b/crates/ruff/src/rules/flake8_annotations/helpers.rs index 5db8369193..166493cca7 100644 --- a/crates/ruff/src/rules/flake8_annotations/helpers.rs +++ b/crates/ruff/src/rules/flake8_annotations/helpers.rs @@ -2,9 +2,9 @@ use rustpython_parser::ast::{self, Arguments, Expr, Stmt, StmtKind}; use ruff_python_ast::cast; use ruff_python_semantic::analyze::visibility; +use ruff_python_semantic::definition::{Definition, Member, MemberKind}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{Definition, DefinitionKind}; pub(super) fn match_function_def(stmt: &Stmt) -> (&str, &Arguments, Option<&Expr>, &Vec) { match &stmt.node { @@ -28,9 +28,11 @@ pub(super) fn match_function_def(stmt: &Stmt) -> (&str, &Arguments, Option<&Expr /// Return the name of the function, if it's overloaded. pub fn overloaded_name(checker: &Checker, definition: &Definition) -> Option { - if let DefinitionKind::Function(stmt) - | DefinitionKind::NestedFunction(stmt) - | DefinitionKind::Method(stmt) = definition.kind + if let Definition::Member(Member { + kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method, + stmt, + .. + }) = definition { if visibility::is_overload(&checker.ctx, cast::decorator_list(stmt)) { let (name, ..) = match_function_def(stmt); @@ -46,9 +48,11 @@ pub fn overloaded_name(checker: &Checker, definition: &Definition) -> Option bool { - if let DefinitionKind::Function(stmt) - | DefinitionKind::NestedFunction(stmt) - | DefinitionKind::Method(stmt) = definition.kind + if let Definition::Member(Member { + kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method, + stmt, + .. + }) = definition { if visibility::is_overload(&checker.ctx, cast::decorator_list(stmt)) { false diff --git a/crates/ruff/src/rules/flake8_annotations/rules.rs b/crates/ruff/src/rules/flake8_annotations/rules.rs index ddca3e3aee..8f7dd9ce52 100644 --- a/crates/ruff/src/rules/flake8_annotations/rules.rs +++ b/crates/ruff/src/rules/flake8_annotations/rules.rs @@ -7,10 +7,10 @@ use ruff_python_ast::statement_visitor::StatementVisitor; use ruff_python_ast::{cast, helpers}; use ruff_python_semantic::analyze::visibility; use ruff_python_semantic::analyze::visibility::Visibility; +use ruff_python_semantic::definition::{Definition, Member, MemberKind}; use ruff_python_stdlib::typing::SIMPLE_MAGIC_RETURN_TYPES; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{Definition, DefinitionKind}; use crate::registry::{AsRule, Rule}; use super::fixes; @@ -454,287 +454,285 @@ pub fn definition( // TODO(charlie): Consider using the AST directly here rather than `Definition`. // We could adhere more closely to `flake8-annotations` by defining public // vs. secret vs. protected. - if let DefinitionKind::Function(stmt) - | DefinitionKind::NestedFunction(stmt) - | DefinitionKind::Method(stmt) = &definition.kind + let Definition::Member(Member { + kind, + stmt, + .. + }) = definition else { + return vec![]; + }; + + let is_method = match kind { + MemberKind::Method => true, + MemberKind::Function | MemberKind::NestedFunction => false, + _ => return vec![], + }; + + let (name, args, returns, body) = match_function_def(stmt); + // Keep track of whether we've seen any typed arguments or return values. + let mut has_any_typed_arg = false; // Any argument has been typed? + let mut has_typed_return = false; // Return value has been typed? + let mut has_typed_self_or_cls = false; // Has a typed `self` or `cls` argument? + + // Temporary storage for diagnostics; we emit them at the end + // unless configured to suppress ANN* for declarations that are fully untyped. + let mut diagnostics = Vec::new(); + + // ANN001, ANN401 + for arg in args + .posonlyargs + .iter() + .chain(args.args.iter()) + .chain(args.kwonlyargs.iter()) + .skip( + // If this is a non-static method, skip `cls` or `self`. + usize::from( + is_method && !visibility::is_staticmethod(&checker.ctx, cast::decorator_list(stmt)), + ), + ) { - let is_method = matches!(definition.kind, DefinitionKind::Method(_)); - let (name, args, returns, body) = match_function_def(stmt); - // Keep track of whether we've seen any typed arguments or return values. - let mut has_any_typed_arg = false; // Any argument has been typed? - let mut has_typed_return = false; // Return value has been typed? - let mut has_typed_self_or_cls = false; // Has a typed `self` or `cls` argument? - - // Temporary storage for diagnostics; we emit them at the end - // unless configured to suppress ANN* for declarations that are fully untyped. - let mut diagnostics = Vec::new(); - - // ANN001, ANN401 - for arg in args - .posonlyargs - .iter() - .chain(args.args.iter()) - .chain(args.kwonlyargs.iter()) - .skip( - // If this is a non-static method, skip `cls` or `self`. - usize::from( - is_method - && !visibility::is_staticmethod(&checker.ctx, cast::decorator_list(stmt)), - ), - ) - { - // ANN401 for dynamically typed arguments - if let Some(annotation) = &arg.node.annotation { - has_any_typed_arg = true; - if checker.settings.rules.enabled(Rule::AnyType) { - check_dynamically_typed( - checker, - annotation, - || arg.node.arg.to_string(), - &mut diagnostics, - ); - } - } else { - if !(checker.settings.flake8_annotations.suppress_dummy_args - && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) - { - if checker - .settings - .rules - .enabled(Rule::MissingTypeFunctionArgument) - { - diagnostics.push(Diagnostic::new( - MissingTypeFunctionArgument { - name: arg.node.arg.to_string(), - }, - arg.range(), - )); - } - } - } - } - - // ANN002, ANN401 - if let Some(arg) = &args.vararg { - if let Some(expr) = &arg.node.annotation { - has_any_typed_arg = true; - if !checker.settings.flake8_annotations.allow_star_arg_any { - if checker.settings.rules.enabled(Rule::AnyType) { - let name = &arg.node.arg; - check_dynamically_typed( - checker, - expr, - || format!("*{name}"), - &mut diagnostics, - ); - } - } - } else { - if !(checker.settings.flake8_annotations.suppress_dummy_args - && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) - { - if checker.settings.rules.enabled(Rule::MissingTypeArgs) { - diagnostics.push(Diagnostic::new( - MissingTypeArgs { - name: arg.node.arg.to_string(), - }, - arg.range(), - )); - } - } - } - } - - // ANN003, ANN401 - if let Some(arg) = &args.kwarg { - if let Some(expr) = &arg.node.annotation { - has_any_typed_arg = true; - if !checker.settings.flake8_annotations.allow_star_arg_any { - if checker.settings.rules.enabled(Rule::AnyType) { - let name = &arg.node.arg; - check_dynamically_typed( - checker, - expr, - || format!("**{name}"), - &mut diagnostics, - ); - } - } - } else { - if !(checker.settings.flake8_annotations.suppress_dummy_args - && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) - { - if checker.settings.rules.enabled(Rule::MissingTypeKwargs) { - diagnostics.push(Diagnostic::new( - MissingTypeKwargs { - name: arg.node.arg.to_string(), - }, - arg.range(), - )); - } - } - } - } - - // ANN101, ANN102 - if is_method && !visibility::is_staticmethod(&checker.ctx, cast::decorator_list(stmt)) { - if let Some(arg) = args.posonlyargs.first().or_else(|| args.args.first()) { - if arg.node.annotation.is_none() { - if visibility::is_classmethod(&checker.ctx, cast::decorator_list(stmt)) { - if checker.settings.rules.enabled(Rule::MissingTypeCls) { - diagnostics.push(Diagnostic::new( - MissingTypeCls { - name: arg.node.arg.to_string(), - }, - arg.range(), - )); - } - } else { - if checker.settings.rules.enabled(Rule::MissingTypeSelf) { - diagnostics.push(Diagnostic::new( - MissingTypeSelf { - name: arg.node.arg.to_string(), - }, - arg.range(), - )); - } - } - } else { - has_typed_self_or_cls = true; - } - } - } - - // ANN201, ANN202, ANN401 - if let Some(expr) = &returns { - has_typed_return = true; + // ANN401 for dynamically typed arguments + if let Some(annotation) = &arg.node.annotation { + has_any_typed_arg = true; if checker.settings.rules.enabled(Rule::AnyType) { - check_dynamically_typed(checker, expr, || name.to_string(), &mut diagnostics); + check_dynamically_typed( + checker, + annotation, + || arg.node.arg.to_string(), + &mut diagnostics, + ); } - } else if !( - // Allow omission of return annotation if the function only returns `None` - // (explicitly or implicitly). - checker.settings.flake8_annotations.suppress_none_returning && is_none_returning(body) - ) { - if is_method && visibility::is_classmethod(&checker.ctx, cast::decorator_list(stmt)) { - if checker - .settings - .rules - .enabled(Rule::MissingReturnTypeClassMethod) - { - diagnostics.push(Diagnostic::new( - MissingReturnTypeClassMethod { - name: name.to_string(), - }, - helpers::identifier_range(stmt, checker.locator), - )); - } - } else if is_method - && visibility::is_staticmethod(&checker.ctx, cast::decorator_list(stmt)) + } else { + if !(checker.settings.flake8_annotations.suppress_dummy_args + && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) { if checker .settings .rules - .enabled(Rule::MissingReturnTypeStaticMethod) + .enabled(Rule::MissingTypeFunctionArgument) { diagnostics.push(Diagnostic::new( - MissingReturnTypeStaticMethod { - name: name.to_string(), + MissingTypeFunctionArgument { + name: arg.node.arg.to_string(), }, - helpers::identifier_range(stmt, checker.locator), + arg.range(), )); } - } else if is_method && visibility::is_init(name) { - // Allow omission of return annotation in `__init__` functions, as long as at - // least one argument is typed. - if checker - .settings - .rules - .enabled(Rule::MissingReturnTypeSpecialMethod) - { - if !(checker.settings.flake8_annotations.mypy_init_return && has_any_typed_arg) - { - let mut diagnostic = Diagnostic::new( - MissingReturnTypeSpecialMethod { - name: name.to_string(), + } + } + } + + // ANN002, ANN401 + if let Some(arg) = &args.vararg { + if let Some(expr) = &arg.node.annotation { + has_any_typed_arg = true; + if !checker.settings.flake8_annotations.allow_star_arg_any { + if checker.settings.rules.enabled(Rule::AnyType) { + let name = &arg.node.arg; + check_dynamically_typed(checker, expr, || format!("*{name}"), &mut diagnostics); + } + } + } else { + if !(checker.settings.flake8_annotations.suppress_dummy_args + && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) + { + if checker.settings.rules.enabled(Rule::MissingTypeArgs) { + diagnostics.push(Diagnostic::new( + MissingTypeArgs { + name: arg.node.arg.to_string(), + }, + arg.range(), + )); + } + } + } + } + + // ANN003, ANN401 + if let Some(arg) = &args.kwarg { + if let Some(expr) = &arg.node.annotation { + has_any_typed_arg = true; + if !checker.settings.flake8_annotations.allow_star_arg_any { + if checker.settings.rules.enabled(Rule::AnyType) { + let name = &arg.node.arg; + check_dynamically_typed( + checker, + expr, + || format!("**{name}"), + &mut diagnostics, + ); + } + } + } else { + if !(checker.settings.flake8_annotations.suppress_dummy_args + && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) + { + if checker.settings.rules.enabled(Rule::MissingTypeKwargs) { + diagnostics.push(Diagnostic::new( + MissingTypeKwargs { + name: arg.node.arg.to_string(), + }, + arg.range(), + )); + } + } + } + } + + // ANN101, ANN102 + if is_method && !visibility::is_staticmethod(&checker.ctx, cast::decorator_list(stmt)) { + if let Some(arg) = args.posonlyargs.first().or_else(|| args.args.first()) { + if arg.node.annotation.is_none() { + if visibility::is_classmethod(&checker.ctx, cast::decorator_list(stmt)) { + if checker.settings.rules.enabled(Rule::MissingTypeCls) { + diagnostics.push(Diagnostic::new( + MissingTypeCls { + name: arg.node.arg.to_string(), }, - helpers::identifier_range(stmt, checker.locator), - ); - if checker.patch(diagnostic.kind.rule()) { - #[allow(deprecated)] - diagnostic.try_set_fix_from_edit(|| { - fixes::add_return_annotation(checker.locator, stmt, "None") - }); - } - diagnostics.push(diagnostic); + arg.range(), + )); + } + } else { + if checker.settings.rules.enabled(Rule::MissingTypeSelf) { + diagnostics.push(Diagnostic::new( + MissingTypeSelf { + name: arg.node.arg.to_string(), + }, + arg.range(), + )); } } - } else if is_method && visibility::is_magic(name) { - if checker - .settings - .rules - .enabled(Rule::MissingReturnTypeSpecialMethod) - { + } else { + has_typed_self_or_cls = true; + } + } + } + + // ANN201, ANN202, ANN401 + if let Some(expr) = &returns { + has_typed_return = true; + if checker.settings.rules.enabled(Rule::AnyType) { + check_dynamically_typed(checker, expr, || name.to_string(), &mut diagnostics); + } + } else if !( + // Allow omission of return annotation if the function only returns `None` + // (explicitly or implicitly). + checker.settings.flake8_annotations.suppress_none_returning && is_none_returning(body) + ) { + if is_method && visibility::is_classmethod(&checker.ctx, cast::decorator_list(stmt)) { + if checker + .settings + .rules + .enabled(Rule::MissingReturnTypeClassMethod) + { + diagnostics.push(Diagnostic::new( + MissingReturnTypeClassMethod { + name: name.to_string(), + }, + helpers::identifier_range(stmt, checker.locator), + )); + } + } else if is_method && visibility::is_staticmethod(&checker.ctx, cast::decorator_list(stmt)) + { + if checker + .settings + .rules + .enabled(Rule::MissingReturnTypeStaticMethod) + { + diagnostics.push(Diagnostic::new( + MissingReturnTypeStaticMethod { + name: name.to_string(), + }, + helpers::identifier_range(stmt, checker.locator), + )); + } + } else if is_method && visibility::is_init(name) { + // Allow omission of return annotation in `__init__` functions, as long as at + // least one argument is typed. + if checker + .settings + .rules + .enabled(Rule::MissingReturnTypeSpecialMethod) + { + if !(checker.settings.flake8_annotations.mypy_init_return && has_any_typed_arg) { let mut diagnostic = Diagnostic::new( MissingReturnTypeSpecialMethod { name: name.to_string(), }, helpers::identifier_range(stmt, checker.locator), ); - let return_type = SIMPLE_MAGIC_RETURN_TYPES.get(name); - if let Some(return_type) = return_type { - if checker.patch(diagnostic.kind.rule()) { - #[allow(deprecated)] - diagnostic.try_set_fix_from_edit(|| { - fixes::add_return_annotation(checker.locator, stmt, return_type) - }); - } + if checker.patch(diagnostic.kind.rule()) { + #[allow(deprecated)] + diagnostic.try_set_fix_from_edit(|| { + fixes::add_return_annotation(checker.locator, stmt, "None") + }); } diagnostics.push(diagnostic); } - } else { - match visibility { - Visibility::Public => { - if checker - .settings - .rules - .enabled(Rule::MissingReturnTypeUndocumentedPublicFunction) - { - diagnostics.push(Diagnostic::new( - MissingReturnTypeUndocumentedPublicFunction { - name: name.to_string(), - }, - helpers::identifier_range(stmt, checker.locator), - )); - } + } + } else if is_method && visibility::is_magic(name) { + if checker + .settings + .rules + .enabled(Rule::MissingReturnTypeSpecialMethod) + { + let mut diagnostic = Diagnostic::new( + MissingReturnTypeSpecialMethod { + name: name.to_string(), + }, + helpers::identifier_range(stmt, checker.locator), + ); + let return_type = SIMPLE_MAGIC_RETURN_TYPES.get(name); + if let Some(return_type) = return_type { + if checker.patch(diagnostic.kind.rule()) { + #[allow(deprecated)] + diagnostic.try_set_fix_from_edit(|| { + fixes::add_return_annotation(checker.locator, stmt, return_type) + }); } - Visibility::Private => { - if checker - .settings - .rules - .enabled(Rule::MissingReturnTypePrivateFunction) - { - diagnostics.push(Diagnostic::new( - MissingReturnTypePrivateFunction { - name: name.to_string(), - }, - helpers::identifier_range(stmt, checker.locator), - )); - } + } + diagnostics.push(diagnostic); + } + } else { + match visibility { + Visibility::Public => { + if checker + .settings + .rules + .enabled(Rule::MissingReturnTypeUndocumentedPublicFunction) + { + diagnostics.push(Diagnostic::new( + MissingReturnTypeUndocumentedPublicFunction { + name: name.to_string(), + }, + helpers::identifier_range(stmt, checker.locator), + )); + } + } + Visibility::Private => { + if checker + .settings + .rules + .enabled(Rule::MissingReturnTypePrivateFunction) + { + diagnostics.push(Diagnostic::new( + MissingReturnTypePrivateFunction { + name: name.to_string(), + }, + helpers::identifier_range(stmt, checker.locator), + )); } } } } - // If settings say so, don't report any of the - // diagnostics gathered here if there were no type annotations at all. - if checker.settings.flake8_annotations.ignore_fully_untyped - && !(has_any_typed_arg || has_typed_self_or_cls || has_typed_return) - { - vec![] - } else { - diagnostics - } - } else { + } + // If settings say so, don't report any of the + // diagnostics gathered here if there were no type annotations at all. + if checker.settings.flake8_annotations.ignore_fully_untyped + && !(has_any_typed_arg || has_typed_self_or_cls || has_typed_return) + { vec![] + } else { + diagnostics } } diff --git a/crates/ruff/src/rules/pydocstyle/helpers.rs b/crates/ruff/src/rules/pydocstyle/helpers.rs index 196fa42f2f..c63964ba19 100644 --- a/crates/ruff/src/rules/pydocstyle/helpers.rs +++ b/crates/ruff/src/rules/pydocstyle/helpers.rs @@ -1,13 +1,13 @@ -use ruff_python_ast::call_path::from_qualified_name; use std::collections::BTreeSet; +use ruff_python_ast::call_path::from_qualified_name; use ruff_python_ast::cast; use ruff_python_ast::helpers::map_callable; use ruff_python_ast::newlines::StrExt; use ruff_python_ast::str::is_implicit_concatenation; +use ruff_python_semantic::definition::{Definition, Member, MemberKind}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{Definition, DefinitionKind}; /// Return the index of the first logical line in a string. pub(crate) fn logical_line(content: &str) -> Option { @@ -45,11 +45,13 @@ pub(crate) fn should_ignore_definition( return false; } - if let DefinitionKind::Function(parent) - | DefinitionKind::NestedFunction(parent) - | DefinitionKind::Method(parent) = definition.kind + if let Definition::Member(Member { + kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method, + stmt, + .. + }) = definition { - for decorator in cast::decorator_list(parent) { + for decorator in cast::decorator_list(stmt) { if let Some(call_path) = checker.ctx.resolve_call_path(map_callable(decorator)) { if ignore_decorators .iter() diff --git a/crates/ruff/src/rules/pydocstyle/rules/backslashes.rs b/crates/ruff/src/rules/pydocstyle/rules/backslashes.rs index b29f3de2cc..c5de4b50f7 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/backslashes.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/backslashes.rs @@ -5,7 +5,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; +use crate::docstrings::Docstring; #[violation] pub struct EscapeSequenceInDocstring; diff --git a/crates/ruff/src/rules/pydocstyle/rules/blank_after_summary.rs b/crates/ruff/src/rules/pydocstyle/rules/blank_after_summary.rs index 19db784791..92d15382f5 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/blank_after_summary.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/blank_after_summary.rs @@ -3,7 +3,7 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::newlines::{StrExt, UniversalNewlineIterator}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; +use crate::docstrings::Docstring; use crate::registry::AsRule; #[violation] diff --git a/crates/ruff/src/rules/pydocstyle/rules/blank_before_after_class.rs b/crates/ruff/src/rules/pydocstyle/rules/blank_before_after_class.rs index 036b1929d0..4040e31131 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/blank_before_after_class.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/blank_before_after_class.rs @@ -1,10 +1,11 @@ use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::newlines::{StrExt, UniversalNewlineIterator}; +use ruff_python_semantic::definition::{Definition, Member, MemberKind}; use ruff_text_size::{TextLen, TextRange}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{DefinitionKind, Docstring}; +use crate::docstrings::Docstring; use crate::registry::{AsRule, Rule}; #[violation] @@ -57,7 +58,11 @@ impl AlwaysAutofixableViolation for BlankLineBeforeClass { /// D203, D204, D211 pub fn blank_before_after_class(checker: &mut Checker, docstring: &Docstring) { - let (DefinitionKind::Class(parent) | DefinitionKind::NestedClass(parent)) = &docstring.kind else { + let Definition::Member(Member { + kind: MemberKind::Class | MemberKind::NestedClass , + stmt, + .. + }) = docstring.definition else { return; }; @@ -69,10 +74,10 @@ pub fn blank_before_after_class(checker: &mut Checker, docstring: &Docstring) { { let before = checker .locator - .slice(TextRange::new(parent.start(), docstring.start())); + .slice(TextRange::new(stmt.start(), docstring.start())); let mut blank_lines_before = 0usize; - let mut lines = UniversalNewlineIterator::with_offset(before, parent.start()).rev(); + let mut lines = UniversalNewlineIterator::with_offset(before, stmt.start()).rev(); let mut blank_lines_start = lines.next().map(|line| line.start()).unwrap_or_default(); for line in lines { @@ -132,7 +137,7 @@ pub fn blank_before_after_class(checker: &mut Checker, docstring: &Docstring) { if checker.settings.rules.enabled(Rule::OneBlankLineAfterClass) { let after = checker .locator - .slice(TextRange::new(docstring.end(), parent.end())); + .slice(TextRange::new(docstring.end(), stmt.end())); let all_blank_after = after .universal_newlines() diff --git a/crates/ruff/src/rules/pydocstyle/rules/blank_before_after_function.rs b/crates/ruff/src/rules/pydocstyle/rules/blank_before_after_function.rs index 46e4261943..3b977000d0 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/blank_before_after_function.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/blank_before_after_function.rs @@ -5,9 +5,10 @@ use ruff_text_size::{TextLen, TextRange}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::newlines::{StrExt, UniversalNewlineIterator}; +use ruff_python_semantic::definition::{Definition, Member, MemberKind}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{DefinitionKind, Docstring}; +use crate::docstrings::Docstring; use crate::registry::{AsRule, Rule}; #[violation] @@ -49,11 +50,11 @@ static INNER_FUNCTION_OR_CLASS_REGEX: Lazy = /// D201, D202 pub fn blank_before_after_function(checker: &mut Checker, docstring: &Docstring) { - let ( - DefinitionKind::Function(parent) - | DefinitionKind::NestedFunction(parent) - | DefinitionKind::Method(parent) - ) = &docstring.kind else { + let Definition::Member(Member { + kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method, + stmt, + .. + }) = docstring.definition else { return; }; @@ -64,9 +65,9 @@ pub fn blank_before_after_function(checker: &mut Checker, docstring: &Docstring) { let before = checker .locator - .slice(TextRange::new(parent.start(), docstring.start())); + .slice(TextRange::new(stmt.start(), docstring.start())); - let mut lines = UniversalNewlineIterator::with_offset(before, parent.start()).rev(); + let mut lines = UniversalNewlineIterator::with_offset(before, stmt.start()).rev(); let mut blank_lines_before = 0usize; let mut blank_lines_start = lines.next().map(|l| l.end()).unwrap_or_default(); @@ -105,7 +106,7 @@ pub fn blank_before_after_function(checker: &mut Checker, docstring: &Docstring) { let after = checker .locator - .slice(TextRange::new(docstring.end(), parent.end())); + .slice(TextRange::new(docstring.end(), stmt.end())); // If the docstring is only followed by blank and commented lines, abort. let all_blank_after = after diff --git a/crates/ruff/src/rules/pydocstyle/rules/capitalized.rs b/crates/ruff/src/rules/pydocstyle/rules/capitalized.rs index 32c92d9d03..3c3c947c44 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/capitalized.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/capitalized.rs @@ -2,9 +2,10 @@ use ruff_text_size::{TextLen, TextRange}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_semantic::definition::{Definition, Member, MemberKind}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{DefinitionKind, Docstring}; +use crate::docstrings::Docstring; use crate::registry::AsRule; #[violation] @@ -33,8 +34,11 @@ impl AlwaysAutofixableViolation for FirstLineCapitalized { /// D403 pub fn capitalized(checker: &mut Checker, docstring: &Docstring) { if !matches!( - docstring.kind, - DefinitionKind::Function(_) | DefinitionKind::NestedFunction(_) | DefinitionKind::Method(_) + docstring.definition, + Definition::Member(Member { + kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method, + .. + }) ) { return; } diff --git a/crates/ruff/src/rules/pydocstyle/rules/ends_with_period.rs b/crates/ruff/src/rules/pydocstyle/rules/ends_with_period.rs index cf9c76f0a9..e861e8dfcd 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/ends_with_period.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/ends_with_period.rs @@ -6,8 +6,8 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::newlines::{StrExt, UniversalNewlineIterator}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; use crate::docstrings::sections::SectionKind; +use crate::docstrings::Docstring; use crate::registry::AsRule; use crate::rules::pydocstyle::helpers::logical_line; diff --git a/crates/ruff/src/rules/pydocstyle/rules/ends_with_punctuation.rs b/crates/ruff/src/rules/pydocstyle/rules/ends_with_punctuation.rs index 0c2c06661e..b1273da496 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/ends_with_punctuation.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/ends_with_punctuation.rs @@ -6,8 +6,8 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::newlines::{StrExt, UniversalNewlineIterator}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; use crate::docstrings::sections::SectionKind; +use crate::docstrings::Docstring; use crate::registry::AsRule; use crate::rules::pydocstyle::helpers::logical_line; diff --git a/crates/ruff/src/rules/pydocstyle/rules/if_needed.rs b/crates/ruff/src/rules/pydocstyle/rules/if_needed.rs index ecb2d9e299..cd63b09765 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/if_needed.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/if_needed.rs @@ -3,9 +3,10 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::cast; use ruff_python_ast::helpers::identifier_range; use ruff_python_semantic::analyze::visibility::is_overload; +use ruff_python_semantic::definition::{Definition, Member, MemberKind}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{DefinitionKind, Docstring}; +use crate::docstrings::Docstring; #[violation] pub struct OverloadWithDocstring; @@ -19,12 +20,12 @@ impl Violation for OverloadWithDocstring { /// D418 pub fn if_needed(checker: &mut Checker, docstring: &Docstring) { - let ( - DefinitionKind::Function(stmt) - | DefinitionKind::NestedFunction(stmt) - | DefinitionKind::Method(stmt) - ) = docstring.kind else { - return + let Definition::Member(Member { + kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method, + stmt, + .. + }) = docstring.definition else { + return; }; if !is_overload(&checker.ctx, cast::decorator_list(stmt)) { return; diff --git a/crates/ruff/src/rules/pydocstyle/rules/indent.rs b/crates/ruff/src/rules/pydocstyle/rules/indent.rs index 9b2ca5f414..e03ab155f0 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/indent.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/indent.rs @@ -6,7 +6,7 @@ use ruff_python_ast::whitespace; use ruff_text_size::{TextLen, TextRange}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; +use crate::docstrings::Docstring; use crate::registry::{AsRule, Rule}; #[violation] diff --git a/crates/ruff/src/rules/pydocstyle/rules/multi_line_summary_start.rs b/crates/ruff/src/rules/pydocstyle/rules/multi_line_summary_start.rs index 93c766e0a0..c179623714 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/multi_line_summary_start.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/multi_line_summary_start.rs @@ -1,11 +1,13 @@ +use ruff_text_size::{TextRange, TextSize}; + use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::newlines::{NewlineWithTrailingNewline, UniversalNewlineIterator}; use ruff_python_ast::str::{is_triple_quote, leading_quote}; -use ruff_text_size::{TextRange, TextSize}; +use ruff_python_semantic::definition::{Definition, Member}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{DefinitionKind, Docstring}; +use crate::docstrings::Docstring; use crate::registry::{AsRule, Rule}; #[violation] @@ -91,22 +93,17 @@ pub fn multi_line_summary_start(checker: &mut Checker, docstring: &Docstring) { if !indentation.chars().all(char::is_whitespace) { fixable = false; - // If the docstring isn't on its own line, look at the parent indentation, and - // add the default indentation to get the "right" level. - if let DefinitionKind::Class(parent) - | DefinitionKind::NestedClass(parent) - | DefinitionKind::Function(parent) - | DefinitionKind::NestedFunction(parent) - | DefinitionKind::Method(parent) = &docstring.kind - { - let parent_line_start = checker.locator.line_start(parent.start()); - let parent_indentation = checker + // If the docstring isn't on its own line, look at the statement indentation, + // and add the default indentation to get the "right" level. + if let Definition::Member(Member { stmt, .. }) = &docstring.definition { + let stmt_line_start = checker.locator.line_start(stmt.start()); + let stmt_indentation = checker .locator - .slice(TextRange::new(parent_line_start, parent.start())); + .slice(TextRange::new(stmt_line_start, stmt.start())); - if parent_indentation.chars().all(char::is_whitespace) { + if stmt_indentation.chars().all(char::is_whitespace) { indentation.clear(); - indentation.push_str(parent_indentation); + indentation.push_str(stmt_indentation); indentation.push_str(checker.stylist.indentation()); fixable = true; } diff --git a/crates/ruff/src/rules/pydocstyle/rules/newline_after_last_paragraph.rs b/crates/ruff/src/rules/pydocstyle/rules/newline_after_last_paragraph.rs index aa435172cc..9d5d518b45 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/newline_after_last_paragraph.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/newline_after_last_paragraph.rs @@ -5,7 +5,7 @@ use ruff_python_ast::whitespace; use ruff_text_size::{TextLen, TextSize}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; +use crate::docstrings::Docstring; use crate::registry::AsRule; #[violation] diff --git a/crates/ruff/src/rules/pydocstyle/rules/no_signature.rs b/crates/ruff/src/rules/pydocstyle/rules/no_signature.rs index 8ba3d3bbc3..a389bbc9b6 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/no_signature.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/no_signature.rs @@ -3,9 +3,10 @@ use rustpython_parser::ast::{self, StmtKind}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::newlines::StrExt; +use ruff_python_semantic::definition::{Definition, Member, MemberKind}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{DefinitionKind, Docstring}; +use crate::docstrings::Docstring; #[violation] pub struct NoSignature; @@ -19,14 +20,14 @@ impl Violation for NoSignature { /// D402 pub fn no_signature(checker: &mut Checker, docstring: &Docstring) { - let ( - DefinitionKind::Function(parent) - | DefinitionKind::NestedFunction(parent) - | DefinitionKind::Method(parent) - ) = docstring.kind else { + let Definition::Member(Member { + kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method, + stmt, + .. + }) = docstring.definition else { return; }; - let StmtKind::FunctionDef(ast::StmtFunctionDef { name, .. }) = &parent.node else { + let StmtKind::FunctionDef(ast::StmtFunctionDef { name, .. }) = &stmt.node else { return; }; diff --git a/crates/ruff/src/rules/pydocstyle/rules/no_surrounding_whitespace.rs b/crates/ruff/src/rules/pydocstyle/rules/no_surrounding_whitespace.rs index 26e3427643..2ef5eda776 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/no_surrounding_whitespace.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/no_surrounding_whitespace.rs @@ -4,7 +4,7 @@ use ruff_python_ast::newlines::NewlineWithTrailingNewline; use ruff_text_size::{TextLen, TextRange}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; +use crate::docstrings::Docstring; use crate::registry::AsRule; use crate::rules::pydocstyle::helpers::ends_with_backslash; diff --git a/crates/ruff/src/rules/pydocstyle/rules/non_imperative_mood.rs b/crates/ruff/src/rules/pydocstyle/rules/non_imperative_mood.rs index c2b5a63bfd..c4522142bf 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/non_imperative_mood.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/non_imperative_mood.rs @@ -9,9 +9,10 @@ use ruff_python_ast::call_path::{from_qualified_name, CallPath}; use ruff_python_ast::cast; use ruff_python_ast::newlines::StrExt; use ruff_python_semantic::analyze::visibility::{is_property, is_test}; +use ruff_python_semantic::definition::{Definition, Member, MemberKind}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{DefinitionKind, Docstring}; +use crate::docstrings::Docstring; use crate::rules::pydocstyle::helpers::normalize_word; static MOOD: Lazy = Lazy::new(Mood::new); @@ -22,23 +23,26 @@ pub fn non_imperative_mood( docstring: &Docstring, property_decorators: &BTreeSet, ) { - let ( - DefinitionKind::Function(parent) - | DefinitionKind::NestedFunction(parent) - | DefinitionKind::Method(parent) - ) = &docstring.kind else { + let Definition::Member(Member { kind, stmt, .. }) = &docstring.definition else { return; }; + if !matches!( + kind, + MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method, + ) { + return; + } + let property_decorators = property_decorators .iter() .map(|decorator| from_qualified_name(decorator)) .collect::>(); - if is_test(cast::name(parent)) + if is_test(cast::name(stmt)) || is_property( &checker.ctx, - cast::decorator_list(parent), + cast::decorator_list(stmt), &property_decorators, ) { diff --git a/crates/ruff/src/rules/pydocstyle/rules/not_empty.rs b/crates/ruff/src/rules/pydocstyle/rules/not_empty.rs index 1fcaa7a29b..410ccb4a21 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/not_empty.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/not_empty.rs @@ -2,7 +2,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; +use crate::docstrings::Docstring; use crate::registry::Rule; #[violation] diff --git a/crates/ruff/src/rules/pydocstyle/rules/not_missing.rs b/crates/ruff/src/rules/pydocstyle/rules/not_missing.rs index d5107b970f..74d7330daa 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/not_missing.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/not_missing.rs @@ -5,10 +5,11 @@ use ruff_python_ast::helpers::identifier_range; use ruff_python_semantic::analyze::visibility::{ is_call, is_init, is_magic, is_new, is_overload, is_override, Visibility, }; +use ruff_python_semantic::definition::{Definition, Member, MemberKind, Module, ModuleKind}; use ruff_text_size::TextRange; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{Definition, DefinitionKind}; + use crate::registry::Rule; #[violation] @@ -93,12 +94,15 @@ impl Violation for UndocumentedPublicInit { /// D100, D101, D102, D103, D104, D105, D106, D107 pub fn not_missing(checker: &mut Checker, definition: &Definition, visibility: Visibility) -> bool { - if matches!(visibility, Visibility::Private) { + if visibility.is_private() { return true; } - match definition.kind { - DefinitionKind::Module => { + match definition { + Definition::Module(Module { + kind: ModuleKind::Module, + .. + }) => { if checker .settings .rules @@ -111,7 +115,10 @@ pub fn not_missing(checker: &mut Checker, definition: &Definition, visibility: V } false } - DefinitionKind::Package => { + Definition::Module(Module { + kind: ModuleKind::Package, + .. + }) => { if checker .settings .rules @@ -124,7 +131,11 @@ pub fn not_missing(checker: &mut Checker, definition: &Definition, visibility: V } false } - DefinitionKind::Class(stmt) => { + Definition::Member(Member { + kind: MemberKind::Class, + stmt, + .. + }) => { if checker .settings .rules @@ -137,7 +148,11 @@ pub fn not_missing(checker: &mut Checker, definition: &Definition, visibility: V } false } - DefinitionKind::NestedClass(stmt) => { + Definition::Member(Member { + kind: MemberKind::NestedClass, + stmt, + .. + }) => { if checker .settings .rules @@ -150,7 +165,11 @@ pub fn not_missing(checker: &mut Checker, definition: &Definition, visibility: V } false } - DefinitionKind::Function(stmt) | DefinitionKind::NestedFunction(stmt) => { + Definition::Member(Member { + kind: MemberKind::Function | MemberKind::NestedFunction, + stmt, + .. + }) => { if is_overload(&checker.ctx, cast::decorator_list(stmt)) { true } else { @@ -167,7 +186,11 @@ pub fn not_missing(checker: &mut Checker, definition: &Definition, visibility: V false } } - DefinitionKind::Method(stmt) => { + Definition::Member(Member { + kind: MemberKind::Method, + stmt, + .. + }) => { if is_overload(&checker.ctx, cast::decorator_list(stmt)) || is_override(&checker.ctx, cast::decorator_list(stmt)) { diff --git a/crates/ruff/src/rules/pydocstyle/rules/one_liner.rs b/crates/ruff/src/rules/pydocstyle/rules/one_liner.rs index dc48bd3e3e..2a528c6f5f 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/one_liner.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/one_liner.rs @@ -4,7 +4,7 @@ use ruff_python_ast::newlines::NewlineWithTrailingNewline; use ruff_python_ast::str::{leading_quote, trailing_quote}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; +use crate::docstrings::Docstring; use crate::registry::AsRule; #[violation] diff --git a/crates/ruff/src/rules/pydocstyle/rules/sections.rs b/crates/ruff/src/rules/pydocstyle/rules/sections.rs index d241abfe74..b6172ccebe 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/sections.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/sections.rs @@ -12,11 +12,12 @@ use ruff_python_ast::helpers::identifier_range; use ruff_python_ast::newlines::NewlineWithTrailingNewline; use ruff_python_ast::{cast, whitespace}; use ruff_python_semantic::analyze::visibility::is_staticmethod; +use ruff_python_semantic::definition::{Definition, Member, MemberKind}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::{DefinitionKind, Docstring}; use crate::docstrings::sections::{SectionContext, SectionContexts, SectionKind}; use crate::docstrings::styles::SectionStyle; +use crate::docstrings::Docstring; use crate::registry::{AsRule, Rule}; use crate::rules::pydocstyle::settings::Convention; @@ -725,13 +726,14 @@ fn common_section( } fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: &FxHashSet) { - let ( - DefinitionKind::Function(parent) - | DefinitionKind::NestedFunction(parent) - | DefinitionKind::Method(parent) - ) = docstring.kind else { + let Definition::Member(Member { + kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method, + stmt, + .. + }) = docstring.definition else { return; }; + let ( StmtKind::FunctionDef(ast::StmtFunctionDef { args: arguments, .. @@ -739,7 +741,7 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: & | StmtKind::AsyncFunctionDef(ast::StmtAsyncFunctionDef { args: arguments, .. }) - ) = &parent.node else { + ) = &stmt.node else { return; }; @@ -753,8 +755,8 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: & .skip( // If this is a non-static method, skip `cls` or `self`. usize::from( - matches!(docstring.kind, DefinitionKind::Method(_)) - && !is_staticmethod(&checker.ctx, cast::decorator_list(parent)), + docstring.definition.is_method() + && !is_staticmethod(&checker.ctx, cast::decorator_list(stmt)), ), ) { @@ -791,7 +793,7 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: & let names = missing_arg_names.into_iter().sorted().collect(); checker.diagnostics.push(Diagnostic::new( UndocumentedParam { names }, - identifier_range(parent, checker.locator), + identifier_range(stmt, checker.locator), )); } } diff --git a/crates/ruff/src/rules/pydocstyle/rules/starts_with_this.rs b/crates/ruff/src/rules/pydocstyle/rules/starts_with_this.rs index 18a9b8c0c3..076eae502a 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/starts_with_this.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/starts_with_this.rs @@ -2,7 +2,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; +use crate::docstrings::Docstring; use crate::rules::pydocstyle::helpers::normalize_word; #[violation] diff --git a/crates/ruff/src/rules/pydocstyle/rules/triple_quotes.rs b/crates/ruff/src/rules/pydocstyle/rules/triple_quotes.rs index e5cd02e154..6576995579 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/triple_quotes.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/triple_quotes.rs @@ -2,7 +2,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; -use crate::docstrings::definition::Docstring; +use crate::docstrings::Docstring; #[violation] pub struct TripleSingleQuotes; diff --git a/crates/ruff_python_semantic/src/analyze/visibility.rs b/crates/ruff_python_semantic/src/analyze/visibility.rs index 9d241c5e57..e9d0b2d660 100644 --- a/crates/ruff_python_semantic/src/analyze/visibility.rs +++ b/crates/ruff_python_semantic/src/analyze/visibility.rs @@ -7,25 +7,12 @@ use ruff_python_ast::helpers::map_callable; use crate::context::Context; -#[derive(Debug, Clone, Copy)] -pub enum Modifier { - Module, - Class, - Function, -} - -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, is_macro::Is)] pub enum Visibility { Public, Private, } -#[derive(Debug, Clone, Copy)] -pub struct VisibleScope { - pub modifier: Modifier, - pub visibility: Visibility, -} - /// Returns `true` if a function is a "static method". pub fn is_staticmethod(ctx: &Context, decorator_list: &[Expr]) -> bool { decorator_list.iter().any(|expr| { @@ -138,29 +125,45 @@ fn stem(path: &str) -> &str { } } -/// Return the `Visibility` of the Python file at `Path` based on its name. -pub fn module_visibility(module_path: Option<&[String]>, path: &Path) -> Visibility { - if let Some(module_path) = module_path { - if module_path.iter().any(|m| is_private_module(m)) { - return Visibility::Private; - } - } else { - // When module_path is None, path is a script outside a package, so just - // check to see if the module name itself is private. - // Ex) `_foo.py` (but not `__init__.py`) - let mut components = path.iter().rev(); - if let Some(filename) = components.next() { - let module_name = filename.to_string_lossy(); - let module_name = stem(&module_name); - if is_private_module(module_name) { - return Visibility::Private; - } - } - } - Visibility::Public +/// A Python module can either be defined as a module path (i.e., the dot-separated path to the +/// module) or, if the module can't be resolved, as a file path (i.e., the path to the file defining +/// the module). +#[derive(Debug)] +pub enum ModuleSource<'a> { + /// A module path is a dot-separated path to the module. + Path(&'a [String]), + /// A file path is the path to the file defining the module, often a script outside of a + /// package. + File(&'a Path), } -pub fn function_visibility(stmt: &Stmt) -> Visibility { +impl ModuleSource<'_> { + /// Return the `Visibility` of the module. + pub(crate) fn to_visibility(&self) -> Visibility { + match self { + Self::Path(path) => { + if path.iter().any(|m| is_private_module(m)) { + return Visibility::Private; + } + } + Self::File(path) => { + // Check to see if the filename itself indicates private visibility. + // Ex) `_foo.py` (but not `__init__.py`) + let mut components = path.iter().rev(); + if let Some(filename) = components.next() { + let module_name = filename.to_string_lossy(); + let module_name = stem(&module_name); + if is_private_module(module_name) { + return Visibility::Private; + } + } + } + } + Visibility::Public + } +} + +pub(crate) fn function_visibility(stmt: &Stmt) -> Visibility { match &stmt.node { StmtKind::FunctionDef(ast::StmtFunctionDef { name, .. }) | StmtKind::AsyncFunctionDef(ast::StmtAsyncFunctionDef { name, .. }) => { @@ -174,7 +177,7 @@ pub fn function_visibility(stmt: &Stmt) -> Visibility { } } -pub fn method_visibility(stmt: &Stmt) -> Visibility { +pub(crate) fn method_visibility(stmt: &Stmt) -> Visibility { match &stmt.node { StmtKind::FunctionDef(ast::StmtFunctionDef { name, @@ -212,7 +215,7 @@ pub fn method_visibility(stmt: &Stmt) -> Visibility { } } -pub fn class_visibility(stmt: &Stmt) -> Visibility { +pub(crate) fn class_visibility(stmt: &Stmt) -> Visibility { match &stmt.node { StmtKind::ClassDef(ast::StmtClassDef { name, .. }) => { if name.starts_with('_') { diff --git a/crates/ruff_python_semantic/src/context.rs b/crates/ruff_python_semantic/src/context.rs index 8589e7d594..3b00f2e505 100644 --- a/crates/ruff_python_semantic/src/context.rs +++ b/crates/ruff_python_semantic/src/context.rs @@ -11,11 +11,11 @@ use ruff_python_ast::typing::AnnotationKind; use ruff_python_stdlib::path::is_python_stub_file; use ruff_python_stdlib::typing::TYPING_EXTENSIONS; -use crate::analyze::visibility::{module_visibility, Modifier, VisibleScope}; use crate::binding::{ Binding, BindingId, BindingKind, Bindings, Exceptions, ExecutionContext, FromImportation, Importation, SubmoduleImportation, }; +use crate::definition::{Definition, DefinitionId, Definitions, Member, Module}; use crate::node::{NodeId, Nodes}; use crate::scope::{Scope, ScopeId, ScopeKind, Scopes}; @@ -24,6 +24,7 @@ use crate::scope::{Scope, ScopeId, ScopeKind, Scopes}; pub struct Snapshot { scope_id: ScopeId, stmt_id: Option, + definition_id: DefinitionId, in_annotation: bool, in_type_checking_block: bool, } @@ -31,7 +32,7 @@ pub struct Snapshot { #[allow(clippy::struct_excessive_bools)] pub struct Context<'a> { pub typing_modules: &'a [String], - pub module_path: Option>, + pub module_path: Option<&'a [String]>, // Stack of all visited statements, along with the identifier of the current statement. pub stmts: Nodes<'a>, pub stmt_id: Option, @@ -41,6 +42,10 @@ pub struct Context<'a> { pub scopes: Scopes<'a>, pub scope_id: ScopeId, pub dead_scopes: Vec, + // Stack of all definitions created in any scope, at any point in execution, along with the + // identifier of the current definition. + pub definitions: Definitions<'a>, + pub definition_id: DefinitionId, // A stack of all bindings created in any scope, at any point in execution. pub bindings: Bindings<'a>, // Map from binding index to indexes of bindings that shadow it in other scopes. @@ -49,7 +54,6 @@ pub struct Context<'a> { pub body: &'a [Stmt], pub body_index: usize, // Internal, derivative state. - pub visible_scope: VisibleScope, pub in_annotation: bool, pub in_type_definition: bool, pub in_deferred_string_type_definition: Option, @@ -67,29 +71,22 @@ pub struct Context<'a> { } impl<'a> Context<'a> { - pub fn new( - typing_modules: &'a [String], - path: &'a Path, - module_path: Option>, - ) -> Self { - let visibility = module_visibility(module_path.as_deref(), path); + pub fn new(typing_modules: &'a [String], path: &'a Path, module: Module<'a>) -> Self { Self { typing_modules, - module_path, + module_path: module.path(), stmts: Nodes::default(), stmt_id: None, + exprs: Vec::default(), scopes: Scopes::default(), scope_id: ScopeId::global(), dead_scopes: Vec::default(), + definitions: Definitions::for_module(module), + definition_id: DefinitionId::module(), bindings: Bindings::default(), shadowed_bindings: IntMap::default(), - exprs: Vec::default(), body: &[], body_index: 0, - visible_scope: VisibleScope { - modifier: Modifier::Module, - visibility, - }, in_annotation: false, in_type_definition: false, in_deferred_string_type_definition: None, @@ -347,6 +344,19 @@ impl<'a> Context<'a> { .expect("Attempted to pop without scope"); } + /// Push a [`Member`] onto the stack. + pub fn push_definition(&mut self, definition: Member<'a>) { + self.definition_id = self.definitions.push_member(definition); + } + + /// Pop the current [`Member`] off the stack. + pub fn pop_definition(&mut self) { + let Definition::Member(member) = &self.definitions[self.definition_id] else { + panic!("Attempted to pop without member definition"); + }; + self.definition_id = member.parent; + } + /// Return the current `Stmt`. pub fn stmt(&self) -> &'a Stmt { let node_id = self.stmt_id.expect("No current statement"); @@ -450,6 +460,7 @@ impl<'a> Context<'a> { Snapshot { scope_id: self.scope_id, stmt_id: self.stmt_id, + definition_id: self.definition_id, in_annotation: self.in_annotation, in_type_checking_block: self.in_type_checking_block, } @@ -460,11 +471,13 @@ impl<'a> Context<'a> { let Snapshot { scope_id, stmt_id, + definition_id, in_annotation, in_type_checking_block, } = snapshot; self.scope_id = scope_id; self.stmt_id = stmt_id; + self.definition_id = definition_id; self.in_annotation = in_annotation; self.in_type_checking_block = in_type_checking_block; } diff --git a/crates/ruff_python_semantic/src/definition.rs b/crates/ruff_python_semantic/src/definition.rs new file mode 100644 index 0000000000..f899d2efd1 --- /dev/null +++ b/crates/ruff_python_semantic/src/definition.rs @@ -0,0 +1,225 @@ +//! Definitions within a Python program. In this context, a "definition" is any named entity that +//! can be documented, such as a module, class, or function. + +use std::fmt::Debug; +use std::iter::FusedIterator; +use std::num::TryFromIntError; +use std::ops::{Deref, Index}; + +use rustpython_parser::ast::Stmt; + +use crate::analyze::visibility::{ + class_visibility, function_visibility, method_visibility, ModuleSource, Visibility, +}; + +/// Id uniquely identifying a definition in a program. +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct DefinitionId(u32); + +impl DefinitionId { + /// Returns the ID for the module definition. + #[inline] + pub const fn module() -> Self { + DefinitionId(0) + } +} + +impl TryFrom for DefinitionId { + type Error = TryFromIntError; + + fn try_from(value: usize) -> Result { + Ok(Self(u32::try_from(value)?)) + } +} + +impl From for usize { + fn from(value: DefinitionId) -> Self { + value.0 as usize + } +} + +#[derive(Debug)] +pub enum ModuleKind { + /// A Python file that represents a module within a package. + Module, + /// A Python file that represents the root of a package (i.e., an `__init__.py` file). + Package, +} + +/// A Python module. +#[derive(Debug)] +pub struct Module<'a> { + pub kind: ModuleKind, + pub source: ModuleSource<'a>, + pub python_ast: &'a [Stmt], +} + +impl<'a> Module<'a> { + pub fn path(&self) -> Option<&'a [String]> { + if let ModuleSource::Path(path) = self.source { + Some(path) + } else { + None + } + } +} + +#[derive(Debug, Copy, Clone)] +pub enum MemberKind { + /// A class definition within a program. + Class, + /// A nested class definition within a program. + NestedClass, + /// A function definition within a program. + Function, + /// A nested function definition within a program. + NestedFunction, + /// A method definition within a program. + Method, +} + +/// A member of a Python module. +#[derive(Debug)] +pub struct Member<'a> { + pub kind: MemberKind, + pub parent: DefinitionId, + pub stmt: &'a Stmt, +} + +/// A definition within a Python program. +#[derive(Debug)] +pub enum Definition<'a> { + Module(Module<'a>), + Member(Member<'a>), +} + +impl Definition<'_> { + /// Returns `true` if the [`Definition`] is a method definition. + pub const fn is_method(&self) -> bool { + matches!( + self, + Definition::Member(Member { + kind: MemberKind::Method, + .. + }) + ) + } +} + +/// The definitions within a Python program indexed by [`DefinitionId`]. +#[derive(Debug, Default)] +pub struct Definitions<'a>(Vec>); + +impl<'a> Definitions<'a> { + pub fn for_module(definition: Module<'a>) -> Self { + Self(vec![Definition::Module(definition)]) + } + + /// Pushes a new member definition and returns its unique id. + /// + /// Members are assumed to be pushed in traversal order, such that parents are pushed before + /// their children. + pub fn push_member(&mut self, member: Member<'a>) -> DefinitionId { + let next_id = DefinitionId::try_from(self.0.len()).unwrap(); + self.0.push(Definition::Member(member)); + next_id + } + + /// Iterate over all definitions in a program, with their visibilities. + pub fn iter(&self) -> DefinitionsIter { + DefinitionsIter { + inner: self.0.iter(), + definitions: Vec::with_capacity(self.0.len()), + visibilities: Vec::with_capacity(self.0.len()), + } + } +} + +impl<'a> Index for Definitions<'a> { + type Output = Definition<'a>; + + fn index(&self, index: DefinitionId) -> &Self::Output { + &self.0[usize::from(index)] + } +} + +impl<'a> Deref for Definitions<'a> { + type Target = [Definition<'a>]; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +pub struct DefinitionsIter<'a> { + inner: std::slice::Iter<'a, Definition<'a>>, + definitions: Vec<&'a Definition<'a>>, + visibilities: Vec, +} + +impl<'a> Iterator for DefinitionsIter<'a> { + type Item = (&'a Definition<'a>, Visibility); + + fn next(&mut self) -> Option { + let definition = self.inner.next()?; + + // Determine the visibility of the next definition, taking into account its parent's + // visibility. + let visibility = { + match definition { + Definition::Module(module) => module.source.to_visibility(), + Definition::Member(member) => match member.kind { + MemberKind::Class => { + if self.visibilities[usize::from(member.parent)].is_private() { + Visibility::Private + } else { + class_visibility(member.stmt) + } + } + MemberKind::NestedClass => { + if self.visibilities[usize::from(member.parent)].is_private() + || matches!( + self.definitions[usize::from(member.parent)], + Definition::Member(Member { + kind: MemberKind::Function + | MemberKind::NestedFunction + | MemberKind::Method, + .. + }) + ) + { + Visibility::Private + } else { + class_visibility(member.stmt) + } + } + MemberKind::Function => { + if self.visibilities[usize::from(member.parent)].is_private() { + Visibility::Private + } else { + function_visibility(member.stmt) + } + } + MemberKind::NestedFunction => Visibility::Private, + MemberKind::Method => { + if self.visibilities[usize::from(member.parent)].is_private() { + Visibility::Private + } else { + method_visibility(member.stmt) + } + } + }, + } + }; + self.definitions.push(definition); + self.visibilities.push(visibility); + + Some((definition, visibility)) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } +} + +impl FusedIterator for DefinitionsIter<'_> {} +impl ExactSizeIterator for DefinitionsIter<'_> {} diff --git a/crates/ruff_python_semantic/src/lib.rs b/crates/ruff_python_semantic/src/lib.rs index e85172a6a1..e5f72a625b 100644 --- a/crates/ruff_python_semantic/src/lib.rs +++ b/crates/ruff_python_semantic/src/lib.rs @@ -1,5 +1,6 @@ pub mod analyze; pub mod binding; pub mod context; +pub mod definition; pub mod node; pub mod scope;