Use dedicated AST nodes on MemberKind (#6374)

## Summary

This PR leverages the unified function definition node to add precise
AST node types to `MemberKind`, which is used to power our docstring
definition tracking (e.g., classes and functions, whether they're
methods or functions or nested functions and so on, whether they have a
docstring, etc.). It was painful to do this in the past because the
function variants needed to support a union anyway, but storing precise
nodes removes like a dozen panics.

No behavior changes -- purely a refactor.

## Test Plan

`cargo test`
This commit is contained in:
Charlie Marsh 2023-08-07 13:17:58 -04:00 committed by GitHub
parent daefa74e9a
commit c439435615
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 316 additions and 414 deletions

View file

@ -1,5 +1,4 @@
use ruff_diagnostics::Diagnostic;
use ruff_python_ast::cast;
use ruff_python_semantic::analyze::{branch_detection, visibility};
use ruff_python_semantic::{Binding, BindingKind, ScopeKind};
@ -172,18 +171,25 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
continue;
}
let Some(source) = shadowed.source else {
let Some(statement_id) = shadowed.source else {
continue;
};
// If this is an overloaded function, abort.
if shadowed.kind.is_function_definition()
&& visibility::is_overload(
cast::decorator_list(checker.semantic.statement(source)),
&checker.semantic,
)
{
continue;
if shadowed.kind.is_function_definition() {
if checker
.semantic
.statement(statement_id)
.as_function_def_stmt()
.is_some_and(|function| {
visibility::is_overload(
&function.decorator_list,
&checker.semantic,
)
})
{
continue;
}
}
} else {
// Only enforce cross-scope shadowing for imports.

View file

@ -455,14 +455,16 @@ where
// Step 2: Traversal
match stmt {
Stmt::FunctionDef(ast::StmtFunctionDef {
body,
parameters,
decorator_list,
returns,
type_params,
..
}) => {
Stmt::FunctionDef(
function_def @ ast::StmtFunctionDef {
body,
parameters,
decorator_list,
returns,
type_params,
..
},
) => {
// Visit the decorators and arguments, but avoid the body, which will be
// deferred.
for decorator in decorator_list {
@ -523,8 +525,7 @@ where
}
let definition = docstrings::extraction::extract_definition(
ExtractionTarget::Function,
stmt,
ExtractionTarget::Function(function_def),
self.semantic.definition_id,
&self.semantic.definitions,
);
@ -566,8 +567,7 @@ where
}
let definition = docstrings::extraction::extract_definition(
ExtractionTarget::Class,
stmt,
ExtractionTarget::Class(class_def),
self.semantic.definition_id,
&self.semantic.definitions,
);

View file

@ -1,7 +1,6 @@
//! Extract docstrings from an AST.
use ruff_python_ast::{self as ast, Constant, Expr, Stmt};
use ruff_python_semantic::{Definition, DefinitionId, Definitions, Member, MemberKind};
/// Extract a docstring from a function or class body.
@ -28,62 +27,48 @@ pub(crate) fn docstring_from(suite: &[Stmt]) -> Option<&Expr> {
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 Stmt::ClassDef(ast::StmtClassDef { body, .. })
| Stmt::FunctionDef(ast::StmtFunctionDef { body, .. }) = &member.stmt
{
docstring_from(body)
} else {
None
}
}
Definition::Member(member) => docstring_from(member.body()),
}
}
#[derive(Copy, Clone)]
pub(crate) enum ExtractionTarget {
Class,
Function,
pub(crate) enum ExtractionTarget<'a> {
Class(&'a ast::StmtClassDef),
Function(&'a ast::StmtFunctionDef),
}
/// Extract a `Definition` from the AST node defined by a `Stmt`.
pub(crate) fn extract_definition<'a>(
target: ExtractionTarget,
stmt: &'a Stmt,
target: ExtractionTarget<'a>,
parent: DefinitionId,
definitions: &Definitions<'a>,
) -> Member<'a> {
match target {
ExtractionTarget::Function => match &definitions[parent] {
ExtractionTarget::Function(function) => match &definitions[parent] {
Definition::Module(..) => Member {
parent,
kind: MemberKind::Function,
stmt,
kind: MemberKind::Function(function),
},
Definition::Member(Member {
kind: MemberKind::Class | MemberKind::NestedClass,
kind: MemberKind::Class(_) | MemberKind::NestedClass(_),
..
}) => Member {
parent,
kind: MemberKind::Method,
stmt,
kind: MemberKind::Method(function),
},
Definition::Member(..) => Member {
Definition::Member(_) => Member {
parent,
kind: MemberKind::NestedFunction,
stmt,
kind: MemberKind::NestedFunction(function),
},
},
ExtractionTarget::Class => match &definitions[parent] {
Definition::Module(..) => Member {
ExtractionTarget::Class(class) => match &definitions[parent] {
Definition::Module(_) => Member {
parent,
kind: MemberKind::Class,
stmt,
kind: MemberKind::Class(class),
},
Definition::Member(..) => Member {
Definition::Member(_) => Member {
parent,
kind: MemberKind::NestedClass,
stmt,
kind: MemberKind::NestedClass(class),
},
},
}

View file

@ -1,25 +1,25 @@
use anyhow::{bail, Result};
use ruff_python_ast::{PySourceType, Ranged, Stmt};
use ruff_python_ast::{PySourceType, Ranged};
use ruff_python_parser::{lexer, AsMode, Tok};
use ruff_diagnostics::Edit;
use ruff_source_file::Locator;
/// ANN204
pub(crate) fn add_return_annotation(
locator: &Locator,
stmt: &Stmt,
pub(crate) fn add_return_annotation<T: Ranged>(
statement: &T,
annotation: &str,
source_type: PySourceType,
locator: &Locator,
) -> Result<Edit> {
let contents = &locator.contents()[stmt.range()];
let contents = &locator.contents()[statement.range()];
// Find the colon (following the `def` keyword).
let mut seen_lpar = false;
let mut seen_rpar = false;
let mut count = 0u32;
for (tok, range) in
lexer::lex_starts_at(contents, source_type.as_mode(), stmt.start()).flatten()
lexer::lex_starts_at(contents, source_type.as_mode(), statement.start()).flatten()
{
if seen_lpar && seen_rpar {
if matches!(tok, Tok::Colon) {

View file

@ -1,45 +1,11 @@
use ruff_python_ast::{self as ast, Expr, Parameters, Stmt};
use ruff_python_ast::cast;
use ruff_python_semantic::analyze::visibility;
use ruff_python_semantic::{Definition, Member, MemberKind, SemanticModel};
pub(super) fn match_function_def(
stmt: &Stmt,
) -> (&str, &Parameters, Option<&Expr>, &[Stmt], &[ast::Decorator]) {
match stmt {
Stmt::FunctionDef(ast::StmtFunctionDef {
name,
parameters,
returns,
body,
decorator_list,
..
}) => (
name,
parameters,
returns.as_ref().map(AsRef::as_ref),
body,
decorator_list,
),
_ => panic!("Found non-FunctionDef in match_function_def"),
}
}
use ruff_python_semantic::{Definition, SemanticModel};
/// Return the name of the function, if it's overloaded.
pub(crate) fn overloaded_name(definition: &Definition, semantic: &SemanticModel) -> Option<String> {
if let Definition::Member(Member {
kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method,
stmt,
..
}) = definition
{
if visibility::is_overload(cast::decorator_list(stmt), semantic) {
let (name, ..) = match_function_def(stmt);
Some(name.to_string())
} else {
None
}
let function = definition.as_function_def()?;
if visibility::is_overload(&function.decorator_list, semantic) {
Some(function.name.to_string())
} else {
None
}
@ -52,19 +18,12 @@ pub(crate) fn is_overload_impl(
overloaded_name: &str,
semantic: &SemanticModel,
) -> bool {
if let Definition::Member(Member {
kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method,
stmt,
..
}) = definition
{
if visibility::is_overload(cast::decorator_list(stmt), semantic) {
false
} else {
let (name, ..) = match_function_def(stmt);
name == overloaded_name
}
} else {
let Some(function) = definition.as_function_def() else {
return false;
};
if visibility::is_overload(&function.decorator_list, semantic) {
false
} else {
function.name.as_str() == overloaded_name
}
}

View file

@ -1,23 +1,19 @@
use ruff_python_ast::{self as ast, Constant, Expr, ParameterWithDefault, Ranged, Stmt};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::cast;
use ruff_python_ast::helpers::ReturnStatementVisitor;
use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::statement_visitor::StatementVisitor;
use ruff_python_ast::{self as ast, Constant, Expr, ParameterWithDefault, Ranged, Stmt};
use ruff_python_parser::typing::parse_type_annotation;
use ruff_python_semantic::analyze::visibility;
use ruff_python_semantic::{Definition, Member, MemberKind};
use ruff_python_semantic::Definition;
use ruff_python_stdlib::typing::simple_magic_return_type;
use crate::checkers::ast::Checker;
use crate::registry::{AsRule, Rule};
use crate::rules::flake8_annotations::fixes;
use crate::rules::ruff::typing::type_hint_resolves_to_any;
use super::super::fixes;
use super::super::helpers::match_function_def;
/// ## What it does
/// Checks that function arguments have type annotations.
///
@ -498,20 +494,23 @@ pub(crate) fn definition(
definition: &Definition,
visibility: visibility::Visibility,
) -> Vec<Diagnostic> {
// 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.
let Definition::Member(Member { kind, stmt, .. }) = definition else {
let Some(function) = definition.as_function_def() else {
return vec![];
};
let is_method = match kind {
MemberKind::Method => true,
MemberKind::Function | MemberKind::NestedFunction => false,
_ => return vec![],
};
let ast::StmtFunctionDef {
range: _,
is_async: _,
decorator_list,
name,
type_params: _,
parameters,
returns,
body,
} = function;
let is_method = definition.is_method();
let (name, arguments, returns, body, decorator_list) = 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?
@ -528,20 +527,19 @@ pub(crate) fn definition(
parameter,
default: _,
range: _,
} in arguments
} in parameters
.posonlyargs
.iter()
.chain(&arguments.args)
.chain(&arguments.kwonlyargs)
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.skip(
// If this is a non-static method, skip `cls` or `self`.
usize::from(
is_method
&& !visibility::is_staticmethod(cast::decorator_list(stmt), checker.semantic()),
is_method && !visibility::is_staticmethod(decorator_list, checker.semantic()),
),
)
{
// ANN401 for dynamically typed arguments
// ANN401 for dynamically typed parameters
if let Some(annotation) = &parameter.annotation {
has_any_typed_arg = true;
if checker.enabled(Rule::AnyType) && !is_overridden {
@ -572,7 +570,7 @@ pub(crate) fn definition(
}
// ANN002, ANN401
if let Some(arg) = &arguments.vararg {
if let Some(arg) = &parameters.vararg {
if let Some(expr) = &arg.annotation {
has_any_typed_arg = true;
if !checker.settings.flake8_annotations.allow_star_arg_any {
@ -598,7 +596,7 @@ pub(crate) fn definition(
}
// ANN003, ANN401
if let Some(arg) = &arguments.kwarg {
if let Some(arg) = &parameters.kwarg {
if let Some(expr) = &arg.annotation {
has_any_typed_arg = true;
if !checker.settings.flake8_annotations.allow_star_arg_any {
@ -629,18 +627,18 @@ pub(crate) fn definition(
}
// ANN101, ANN102
if is_method && !visibility::is_staticmethod(cast::decorator_list(stmt), checker.semantic()) {
if is_method && !visibility::is_staticmethod(decorator_list, checker.semantic()) {
if let Some(ParameterWithDefault {
parameter,
default: _,
range: _,
}) = arguments
}) = parameters
.posonlyargs
.first()
.or_else(|| arguments.args.first())
.or_else(|| parameters.args.first())
{
if parameter.annotation.is_none() {
if visibility::is_classmethod(cast::decorator_list(stmt), checker.semantic()) {
if visibility::is_classmethod(decorator_list, checker.semantic()) {
if checker.enabled(Rule::MissingTypeCls) {
diagnostics.push(Diagnostic::new(
MissingTypeCls {
@ -676,24 +674,22 @@ pub(crate) fn definition(
// (explicitly or implicitly).
checker.settings.flake8_annotations.suppress_none_returning && is_none_returning(body)
) {
if is_method && visibility::is_classmethod(cast::decorator_list(stmt), checker.semantic()) {
if is_method && visibility::is_classmethod(decorator_list, checker.semantic()) {
if checker.enabled(Rule::MissingReturnTypeClassMethod) {
diagnostics.push(Diagnostic::new(
MissingReturnTypeClassMethod {
name: name.to_string(),
},
stmt.identifier(),
function.identifier(),
));
}
} else if is_method
&& visibility::is_staticmethod(cast::decorator_list(stmt), checker.semantic())
{
} else if is_method && visibility::is_staticmethod(decorator_list, checker.semantic()) {
if checker.enabled(Rule::MissingReturnTypeStaticMethod) {
diagnostics.push(Diagnostic::new(
MissingReturnTypeStaticMethod {
name: name.to_string(),
},
stmt.identifier(),
function.identifier(),
));
}
} else if is_method && visibility::is_init(name) {
@ -705,15 +701,15 @@ pub(crate) fn definition(
MissingReturnTypeSpecialMethod {
name: name.to_string(),
},
stmt.identifier(),
function.identifier(),
);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| {
fixes::add_return_annotation(
checker.locator(),
stmt,
function,
"None",
checker.source_type,
checker.locator(),
)
.map(Fix::suggested)
});
@ -727,16 +723,16 @@ pub(crate) fn definition(
MissingReturnTypeSpecialMethod {
name: name.to_string(),
},
stmt.identifier(),
function.identifier(),
);
if checker.patch(diagnostic.kind.rule()) {
if let Some(return_type) = simple_magic_return_type(name) {
diagnostic.try_set_fix(|| {
fixes::add_return_annotation(
checker.locator(),
stmt,
function,
return_type,
checker.source_type,
checker.locator(),
)
.map(Fix::suggested)
});
@ -752,7 +748,7 @@ pub(crate) fn definition(
MissingReturnTypeUndocumentedPublicFunction {
name: name.to_string(),
},
stmt.identifier(),
function.identifier(),
));
}
}
@ -762,7 +758,7 @@ pub(crate) fn definition(
MissingReturnTypePrivateFunction {
name: name.to_string(),
},
stmt.identifier(),
function.identifier(),
));
}
}

View file

@ -1,9 +1,9 @@
use ruff_python_ast as ast;
use ruff_python_ast::{Ranged, Stmt};
use ruff_python_ast::Ranged;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Expr;
use ruff_python_ast::helpers::map_subscript;
use ruff_python_semantic::{Definition, Member, MemberKind};
use crate::checkers::ast::Checker;
@ -67,39 +67,29 @@ impl Violation for IterMethodReturnIterable {
/// PYI045
pub(crate) fn iter_method_return_iterable(checker: &mut Checker, definition: &Definition) {
let Definition::Member(Member {
kind: MemberKind::Method,
stmt,
kind: MemberKind::Method(function),
..
}) = definition
else {
return;
};
let Stmt::FunctionDef(ast::StmtFunctionDef { name, returns, .. }) = stmt else {
let Some(returns) = function.returns.as_ref() else {
return;
};
let Some(returns) = returns else {
return;
};
let annotation = if let Expr::Subscript(ast::ExprSubscript { value, .. }) = returns.as_ref() {
// Ex) `Iterable[T]`
value
} else {
// Ex) `Iterable`, `typing.Iterable`
returns
};
let is_async = match name.as_str() {
let is_async = match function.name.as_str() {
"__iter__" => false,
"__aiter__" => true,
_ => return,
};
// Support both `Iterable` and `Iterable[T]`.
let annotation = map_subscript(returns);
if checker
.semantic()
.resolve_call_path(annotation)
.resolve_call_path(map_subscript(annotation))
.is_some_and(|call_path| {
if is_async {
matches!(

View file

@ -1,10 +1,9 @@
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::str::is_implicit_concatenation;
use ruff_python_semantic::{Definition, Member, MemberKind, SemanticModel};
use ruff_python_semantic::{Definition, SemanticModel};
use ruff_source_file::UniversalNewlines;
/// Return the index of the first logical line in a string.
@ -48,25 +47,19 @@ pub(crate) fn should_ignore_definition(
return false;
}
if let Definition::Member(Member {
kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method,
stmt,
..
}) = definition
{
for decorator in cast::decorator_list(stmt) {
if let Some(call_path) = semantic.resolve_call_path(map_callable(&decorator.expression))
{
if ignore_decorators
let Some(function) = definition.as_function_def() else {
return false;
};
function.decorator_list.iter().any(|decorator| {
semantic
.resolve_call_path(map_callable(&decorator.expression))
.is_some_and(|call_path| {
ignore_decorators
.iter()
.any(|decorator| from_qualified_name(decorator) == call_path)
{
return true;
}
}
}
}
false
})
})
}
/// Check if a docstring should be ignored.

View file

@ -1,11 +1,9 @@
use ruff_python_ast::Ranged;
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, Member, MemberKind};
use ruff_python_ast::Ranged;
use ruff_python_trivia::PythonWhitespace;
use ruff_source_file::{UniversalNewlineIterator, UniversalNewlines};
use ruff_text_size::{TextLen, TextRange};
use crate::checkers::ast::Checker;
use crate::docstrings::Docstring;
@ -155,12 +153,7 @@ impl AlwaysAutofixableViolation for BlankLineBeforeClass {
/// D203, D204, D211
pub(crate) fn blank_before_after_class(checker: &mut Checker, docstring: &Docstring) {
let Definition::Member(Member {
kind: MemberKind::Class | MemberKind::NestedClass,
stmt,
..
}) = docstring.definition
else {
let Some(class) = docstring.definition.as_class_def() else {
return;
};
@ -168,7 +161,7 @@ pub(crate) fn blank_before_after_class(checker: &mut Checker, docstring: &Docstr
// ```python
// class PhotoMetadata: """Metadata about a photo."""
// ```
let between_range = TextRange::new(stmt.start(), docstring.start());
let between_range = TextRange::new(class.start(), docstring.start());
if !checker.locator().contains_line_break(between_range) {
return;
}
@ -223,7 +216,7 @@ pub(crate) fn blank_before_after_class(checker: &mut Checker, docstring: &Docstr
}
if checker.enabled(Rule::OneBlankLineAfterClass) {
let after_range = TextRange::new(docstring.end(), stmt.end());
let after_range = TextRange::new(docstring.end(), class.end());
let after = checker.locator().slice(after_range);

View file

@ -1,13 +1,12 @@
use once_cell::sync::Lazy;
use regex::Regex;
use ruff_python_ast::Ranged;
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, Member, MemberKind};
use ruff_python_ast::Ranged;
use ruff_python_trivia::PythonWhitespace;
use ruff_source_file::{UniversalNewlineIterator, UniversalNewlines};
use ruff_text_size::{TextLen, TextRange};
use crate::checkers::ast::Checker;
use crate::docstrings::Docstring;
@ -104,21 +103,16 @@ static INNER_FUNCTION_OR_CLASS_REGEX: Lazy<Regex> =
/// D201, D202
pub(crate) fn blank_before_after_function(checker: &mut Checker, docstring: &Docstring) {
let Definition::Member(Member {
kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method,
stmt,
..
}) = docstring.definition
else {
let Some(function) = docstring.definition.as_function_def() else {
return;
};
if checker.enabled(Rule::NoBlankLineBeforeFunction) {
let before = checker
.locator()
.slice(TextRange::new(stmt.start(), docstring.start()));
.slice(TextRange::new(function.start(), docstring.start()));
let mut lines = UniversalNewlineIterator::with_offset(before, stmt.start()).rev();
let mut lines = UniversalNewlineIterator::with_offset(before, function.start()).rev();
let mut blank_lines_before = 0usize;
let mut blank_lines_start = lines.next().map(|l| l.end()).unwrap_or_default();
@ -152,7 +146,7 @@ pub(crate) fn blank_before_after_function(checker: &mut Checker, docstring: &Doc
if checker.enabled(Rule::NoBlankLineAfterFunction) {
let after = checker
.locator()
.slice(TextRange::new(docstring.end(), stmt.end()));
.slice(TextRange::new(docstring.end(), function.end()));
// If the docstring is only followed by blank and commented lines, abort.
let all_blank_after = after.universal_newlines().skip(1).all(|line| {

View file

@ -1,9 +1,7 @@
use ruff_python_ast::Ranged;
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, Member, MemberKind};
use ruff_python_ast::Ranged;
use ruff_text_size::{TextLen, TextRange};
use crate::checkers::ast::Checker;
use crate::docstrings::Docstring;
@ -57,13 +55,7 @@ impl AlwaysAutofixableViolation for FirstLineCapitalized {
/// D403
pub(crate) fn capitalized(checker: &mut Checker, docstring: &Docstring) {
if !matches!(
docstring.definition,
Definition::Member(Member {
kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method,
..
})
) {
if docstring.definition.as_function_def().is_none() {
return;
}

View file

@ -1,9 +1,7 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::cast;
use ruff_python_ast::identifier::Identifier;
use ruff_python_semantic::analyze::visibility::is_overload;
use ruff_python_semantic::{Definition, Member, MemberKind};
use crate::checkers::ast::Checker;
use crate::docstrings::Docstring;
@ -80,18 +78,13 @@ impl Violation for OverloadWithDocstring {
/// D418
pub(crate) fn if_needed(checker: &mut Checker, docstring: &Docstring) {
let Definition::Member(Member {
kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method,
stmt,
..
}) = docstring.definition
else {
let Some(function) = docstring.definition.as_function_def() else {
return;
};
if !is_overload(cast::decorator_list(stmt), checker.semantic()) {
return;
if is_overload(&function.decorator_list, checker.semantic()) {
checker.diagnostics.push(Diagnostic::new(
OverloadWithDocstring,
function.identifier(),
));
}
checker
.diagnostics
.push(Diagnostic::new(OverloadWithDocstring, stmt.identifier()));
}

View file

@ -4,7 +4,7 @@ use ruff_text_size::{TextRange, TextSize};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::str::{is_triple_quote, leading_quote};
use ruff_python_semantic::{Definition, Member};
use ruff_python_semantic::Definition;
use ruff_source_file::{NewlineWithTrailingNewline, UniversalNewlineIterator};
use crate::checkers::ast::Checker;
@ -164,11 +164,11 @@ pub(crate) fn multi_line_summary_start(checker: &mut Checker, docstring: &Docstr
// 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());
if let Definition::Member(member) = &docstring.definition {
let stmt_line_start = checker.locator().line_start(member.start());
let stmt_indentation = checker
.locator()
.slice(TextRange::new(stmt_line_start, stmt.start()));
.slice(TextRange::new(stmt_line_start, member.start()));
if stmt_indentation.chars().all(char::is_whitespace) {
indentation.clear();

View file

@ -1,8 +1,5 @@
use ruff_python_ast::{self as ast, Stmt};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::{Definition, Member, MemberKind};
use ruff_source_file::UniversalNewlines;
use crate::checkers::ast::Checker;
@ -54,15 +51,7 @@ impl Violation for NoSignature {
/// D402
pub(crate) fn no_signature(checker: &mut Checker, docstring: &Docstring) {
let Definition::Member(Member {
kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method,
stmt,
..
}) = docstring.definition
else {
return;
};
let Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) = stmt else {
let Some(function) = docstring.definition.as_function_def() else {
return;
};
@ -75,8 +64,8 @@ pub(crate) fn no_signature(checker: &mut Checker, docstring: &Docstring) {
// Search for occurrences of the function name followed by an open parenthesis (e.g., `foo(` for
// a function named `foo`).
if first_line
.match_indices(name.as_str())
.any(|(index, _)| first_line[index + name.len()..].starts_with('('))
.match_indices(function.name.as_str())
.any(|(index, _)| first_line[index + function.name.len()..].starts_with('('))
{
checker
.diagnostics

View file

@ -6,9 +6,7 @@ use once_cell::sync::Lazy;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::call_path::{from_qualified_name, CallPath};
use ruff_python_ast::cast;
use ruff_python_semantic::analyze::visibility::{is_property, is_test};
use ruff_python_semantic::{Definition, Member, MemberKind};
use ruff_source_file::UniversalNewlines;
use crate::checkers::ast::Checker;
@ -69,25 +67,18 @@ pub(crate) fn non_imperative_mood(
docstring: &Docstring,
property_decorators: &BTreeSet<String>,
) {
let Definition::Member(Member { kind, stmt, .. }) = &docstring.definition else {
let Some(function) = docstring.definition.as_function_def() 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::<Vec<CallPath>>();
if is_test(cast::name(stmt))
if is_test(&function.name)
|| is_property(
cast::decorator_list(stmt),
&function.decorator_list,
&property_decorators,
checker.semantic(),
)

View file

@ -1,13 +1,11 @@
use ruff_text_size::TextRange;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::cast;
use ruff_python_ast::identifier::Identifier;
use ruff_python_semantic::analyze::visibility::{
is_call, is_init, is_magic, is_new, is_overload, is_override, Visibility,
};
use ruff_python_semantic::{Definition, Member, MemberKind, Module, ModuleKind};
use ruff_text_size::TextRange;
use crate::checkers::ast::Checker;
use crate::registry::Rule;
@ -557,82 +555,82 @@ pub(crate) fn not_missing(
false
}
Definition::Member(Member {
kind: MemberKind::Class,
stmt,
kind: MemberKind::Class(class),
..
}) => {
if checker.enabled(Rule::UndocumentedPublicClass) {
checker
.diagnostics
.push(Diagnostic::new(UndocumentedPublicClass, stmt.identifier()));
.push(Diagnostic::new(UndocumentedPublicClass, class.identifier()));
}
false
}
Definition::Member(Member {
kind: MemberKind::NestedClass,
stmt,
kind: MemberKind::NestedClass(function),
..
}) => {
if checker.enabled(Rule::UndocumentedPublicNestedClass) {
checker.diagnostics.push(Diagnostic::new(
UndocumentedPublicNestedClass,
stmt.identifier(),
function.identifier(),
));
}
false
}
Definition::Member(Member {
kind: MemberKind::Function | MemberKind::NestedFunction,
stmt,
kind: MemberKind::Function(function) | MemberKind::NestedFunction(function),
..
}) => {
if is_overload(cast::decorator_list(stmt), checker.semantic()) {
if is_overload(&function.decorator_list, checker.semantic()) {
true
} else {
if checker.enabled(Rule::UndocumentedPublicFunction) {
checker.diagnostics.push(Diagnostic::new(
UndocumentedPublicFunction,
stmt.identifier(),
function.identifier(),
));
}
false
}
}
Definition::Member(Member {
kind: MemberKind::Method,
stmt,
kind: MemberKind::Method(function),
..
}) => {
if is_overload(cast::decorator_list(stmt), checker.semantic())
|| is_override(cast::decorator_list(stmt), checker.semantic())
if is_overload(&function.decorator_list, checker.semantic())
|| is_override(&function.decorator_list, checker.semantic())
{
true
} else if is_init(cast::name(stmt)) {
} else if is_init(&function.name) {
if checker.enabled(Rule::UndocumentedPublicInit) {
checker
.diagnostics
.push(Diagnostic::new(UndocumentedPublicInit, stmt.identifier()));
checker.diagnostics.push(Diagnostic::new(
UndocumentedPublicInit,
function.identifier(),
));
}
true
} else if is_new(cast::name(stmt)) || is_call(cast::name(stmt)) {
} else if is_new(&function.name) || is_call(&function.name) {
if checker.enabled(Rule::UndocumentedPublicMethod) {
checker
.diagnostics
.push(Diagnostic::new(UndocumentedPublicMethod, stmt.identifier()));
checker.diagnostics.push(Diagnostic::new(
UndocumentedPublicMethod,
function.identifier(),
));
}
true
} else if is_magic(cast::name(stmt)) {
} else if is_magic(&function.name) {
if checker.enabled(Rule::UndocumentedMagicMethod) {
checker
.diagnostics
.push(Diagnostic::new(UndocumentedMagicMethod, stmt.identifier()));
checker.diagnostics.push(Diagnostic::new(
UndocumentedMagicMethod,
function.identifier(),
));
}
true
} else {
if checker.enabled(Rule::UndocumentedPublicMethod) {
checker
.diagnostics
.push(Diagnostic::new(UndocumentedPublicMethod, stmt.identifier()));
checker.diagnostics.push(Diagnostic::new(
UndocumentedPublicMethod,
function.identifier(),
));
}
true
}

View file

@ -1,20 +1,18 @@
use itertools::Itertools;
use once_cell::sync::Lazy;
use regex::Regex;
use ruff_python_ast::{self as ast, ParameterWithDefault, Stmt};
use ruff_text_size::{TextLen, TextRange, TextSize};
use rustc_hash::FxHashSet;
use ruff_diagnostics::{AlwaysAutofixableViolation, Violation};
use ruff_diagnostics::{Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::cast;
use ruff_python_ast::docstrings::{clean_space, leading_space};
use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::ParameterWithDefault;
use ruff_python_semantic::analyze::visibility::is_staticmethod;
use ruff_python_semantic::{Definition, Member, MemberKind};
use ruff_python_trivia::{textwrap::dedent, PythonWhitespace};
use ruff_source_file::NewlineWithTrailingNewline;
use ruff_text_size::{TextLen, TextRange, TextSize};
use crate::checkers::ast::Checker;
use crate::docstrings::sections::{SectionContext, SectionContexts, SectionKind};
@ -1717,16 +1715,7 @@ fn common_section(
}
fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: &FxHashSet<String>) {
let Definition::Member(Member {
kind: MemberKind::Function | MemberKind::NestedFunction | MemberKind::Method,
stmt,
..
}) = docstring.definition
else {
return;
};
let Stmt::FunctionDef(ast::StmtFunctionDef { parameters, .. }) = stmt else {
let Some(function) = docstring.definition.as_function_def() else {
return;
};
@ -1736,16 +1725,17 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: &
parameter,
default: _,
range: _,
} in parameters
} in function
.parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.chain(&function.parameters.args)
.chain(&function.parameters.kwonlyargs)
.skip(
// If this is a non-static method, skip `cls` or `self`.
usize::from(
docstring.definition.is_method()
&& !is_staticmethod(cast::decorator_list(stmt), checker.semantic()),
&& !is_staticmethod(&function.decorator_list, checker.semantic()),
),
)
{
@ -1757,7 +1747,7 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: &
// Check specifically for `vararg` and `kwarg`, which can be prefixed with a
// single or double star, respectively.
if let Some(arg) = &parameters.vararg {
if let Some(arg) = function.parameters.vararg.as_ref() {
let arg_name = arg.name.as_str();
let starred_arg_name = format!("*{arg_name}");
if !arg_name.starts_with('_')
@ -1767,7 +1757,7 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: &
missing_arg_names.insert(starred_arg_name);
}
}
if let Some(arg) = &parameters.kwarg {
if let Some(arg) = function.parameters.kwarg.as_ref() {
let arg_name = arg.name.as_str();
let starred_arg_name = format!("**{arg_name}");
if !arg_name.starts_with('_')
@ -1786,7 +1776,7 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: &
definition: definition.to_string(),
names,
},
stmt.identifier(),
function.identifier(),
));
}
}

View file

@ -62,7 +62,7 @@ pub(crate) fn yield_outside_function(checker: &mut Checker, expr: &Expr) {
Expr::Yield(_) => DeferralKeyword::Yield,
Expr::YieldFrom(_) => DeferralKeyword::YieldFrom,
Expr::Await(_) => DeferralKeyword::Await,
_ => panic!("Expected Expr::Yield | Expr::YieldFrom | Expr::Await"),
_ => return,
};
checker.diagnostics.push(Diagnostic::new(
YieldOutsideFunction { keyword },

View file

@ -1,15 +0,0 @@
use crate::{nodes, Decorator, Stmt};
pub fn name(stmt: &Stmt) -> &str {
let Stmt::FunctionDef(nodes::StmtFunctionDef { name, .. }) = stmt else {
panic!("Expected Stmt::FunctionDef")
};
name.as_str()
}
pub fn decorator_list(stmt: &Stmt) -> &[Decorator] {
let Stmt::FunctionDef(nodes::StmtFunctionDef { decorator_list, .. }) = stmt else {
panic!("Expected Stmt::FunctionDef")
};
decorator_list
}

View file

@ -20,6 +20,32 @@ pub trait Identifier {
fn identifier(&self) -> TextRange;
}
impl Identifier for ast::StmtFunctionDef {
/// Return the [`TextRange`] of the identifier in the given function definition.
///
/// For example, return the range of `f` in:
/// ```python
/// def f():
/// ...
/// ```
fn identifier(&self) -> TextRange {
self.name.range()
}
}
impl Identifier for ast::StmtClassDef {
/// Return the [`TextRange`] of the identifier in the given class definition.
///
/// For example, return the range of `C` in:
/// ```python
/// class C():
/// ...
/// ```
fn identifier(&self) -> TextRange {
self.name.range()
}
}
impl Identifier for Stmt {
/// Return the [`TextRange`] of the identifier in the given statement.
///
@ -30,8 +56,8 @@ impl Identifier for Stmt {
/// ```
fn identifier(&self) -> TextRange {
match self {
Stmt::ClassDef(ast::StmtClassDef { name, .. })
| Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) => name.range(),
Stmt::ClassDef(class) => class.identifier(),
Stmt::FunctionDef(function) => function.identifier(),
_ => self.range(),
}
}

View file

@ -3,7 +3,6 @@ use std::path::Path;
pub mod all;
pub mod call_path;
pub mod cast;
pub mod comparable;
pub mod docstrings;
pub mod hashable;

View file

@ -1,6 +1,6 @@
use std::path::Path;
use ruff_python_ast::{self as ast, Decorator, Stmt};
use ruff_python_ast::{self as ast, Decorator};
use ruff_python_ast::call_path::{collect_call_path, CallPath};
use ruff_python_ast::helpers::map_callable;
@ -176,57 +176,40 @@ impl ModuleSource<'_> {
}
}
pub(crate) fn function_visibility(stmt: &Stmt) -> Visibility {
match stmt {
Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) => {
if name.starts_with('_') {
Visibility::Private
} else {
Visibility::Public
}
}
_ => panic!("Found non-FunctionDef in function_visibility"),
pub(crate) fn function_visibility(function: &ast::StmtFunctionDef) -> Visibility {
if function.name.starts_with('_') {
Visibility::Private
} else {
Visibility::Public
}
}
pub(crate) fn method_visibility(stmt: &Stmt) -> Visibility {
let Stmt::FunctionDef(ast::StmtFunctionDef {
name,
decorator_list,
..
}) = stmt
else {
panic!("Found non-FunctionDef in method_visibility")
};
pub(crate) fn method_visibility(function: &ast::StmtFunctionDef) -> Visibility {
// Is this a setter or deleter?
if decorator_list.iter().any(|decorator| {
if function.decorator_list.iter().any(|decorator| {
collect_call_path(&decorator.expression).is_some_and(|call_path| {
call_path.as_slice() == [name, "setter"] || call_path.as_slice() == [name, "deleter"]
call_path.as_slice() == [function.name.as_str(), "setter"]
|| call_path.as_slice() == [function.name.as_str(), "deleter"]
})
}) {
return Visibility::Private;
}
// Is the method non-private?
if !name.starts_with('_') {
if !function.name.starts_with('_') {
return Visibility::Public;
}
// Is this a magic method?
if name.starts_with("__") && name.ends_with("__") {
if function.name.starts_with("__") && function.name.ends_with("__") {
return Visibility::Public;
}
Visibility::Private
}
pub(crate) fn class_visibility(stmt: &Stmt) -> Visibility {
let Stmt::ClassDef(ast::StmtClassDef { name, .. }) = stmt else {
panic!("Found non-ClassDef in class_visibility")
};
if name.starts_with('_') {
pub(crate) fn class_visibility(class: &ast::StmtClassDef) -> Visibility {
if class.name.starts_with('_') {
Visibility::Private
} else {
Visibility::Public

View file

@ -5,7 +5,8 @@ use std::fmt::Debug;
use std::ops::Deref;
use ruff_index::{newtype_index, IndexSlice, IndexVec};
use ruff_python_ast::{self as ast, Stmt};
use ruff_python_ast::{self as ast, Ranged, Stmt};
use ruff_text_size::TextRange;
use crate::analyze::visibility::{
class_visibility, function_visibility, method_visibility, ModuleSource, Visibility,
@ -23,7 +24,7 @@ impl DefinitionId {
}
}
#[derive(Debug)]
#[derive(Debug, is_macro::Is)]
pub enum ModuleKind {
/// A Python file that represents a module within a package.
Module,
@ -57,35 +58,60 @@ impl<'a> Module<'a> {
}
}
#[derive(Debug, Copy, Clone)]
pub enum MemberKind {
#[derive(Debug, Copy, Clone, is_macro::Is)]
pub enum MemberKind<'a> {
/// A class definition within a program.
Class,
Class(&'a ast::StmtClassDef),
/// A nested class definition within a program.
NestedClass,
NestedClass(&'a ast::StmtClassDef),
/// A function definition within a program.
Function,
Function(&'a ast::StmtFunctionDef),
/// A nested function definition within a program.
NestedFunction,
NestedFunction(&'a ast::StmtFunctionDef),
/// A method definition within a program.
Method,
Method(&'a ast::StmtFunctionDef),
}
/// A member of a Python module.
#[derive(Debug)]
pub struct Member<'a> {
pub kind: MemberKind,
pub kind: MemberKind<'a>,
pub parent: DefinitionId,
pub stmt: &'a Stmt,
}
impl<'a> Member<'a> {
/// Return the name of the member.
pub fn name(&self) -> Option<&'a str> {
match &self.stmt {
Stmt::FunctionDef(ast::StmtFunctionDef { name, .. })
| Stmt::ClassDef(ast::StmtClassDef { name, .. }) => Some(name),
_ => None,
pub fn name(&self) -> &str {
match self.kind {
MemberKind::Class(class) => &class.name,
MemberKind::NestedClass(class) => &class.name,
MemberKind::Function(function) => &function.name,
MemberKind::NestedFunction(function) => &function.name,
MemberKind::Method(method) => &method.name,
}
}
/// Return the body of the member.
pub fn body(&self) -> &[Stmt] {
match self.kind {
MemberKind::Class(class) => &class.body,
MemberKind::NestedClass(class) => &class.body,
MemberKind::Function(function) => &function.body,
MemberKind::NestedFunction(function) => &function.body,
MemberKind::Method(method) => &method.body,
}
}
}
impl Ranged for Member<'_> {
/// Return the range of the member.
fn range(&self) -> TextRange {
match self.kind {
MemberKind::Class(class) => class.range(),
MemberKind::NestedClass(class) => class.range(),
MemberKind::Function(function) => function.range(),
MemberKind::NestedFunction(function) => function.range(),
MemberKind::Method(method) => method.range(),
}
}
}
@ -103,16 +129,42 @@ impl Definition<'_> {
matches!(
self,
Definition::Member(Member {
kind: MemberKind::Method,
kind: MemberKind::Method(_),
..
})
)
}
/// Return the name of the definition.
pub fn name(&self) -> Option<&str> {
match self {
Definition::Module(module) => module.name(),
Definition::Member(member) => member.name(),
Definition::Member(member) => Some(member.name()),
}
}
/// Return the [`ast::StmtFunctionDef`] of the definition, if it's a function definition.
pub fn as_function_def(&self) -> Option<&ast::StmtFunctionDef> {
match self {
Definition::Member(Member {
kind:
MemberKind::Function(function)
| MemberKind::NestedFunction(function)
| MemberKind::Method(function),
..
}) => Some(function),
_ => None,
}
}
/// Return the [`ast::StmtClassDef`] of the definition, if it's a class definition.
pub fn as_class_def(&self) -> Option<&ast::StmtClassDef> {
match self {
Definition::Member(Member {
kind: MemberKind::Class(class) | MemberKind::NestedClass(class),
..
}) => Some(class),
_ => None,
}
}
}
@ -146,55 +198,43 @@ impl<'a> Definitions<'a> {
match &definition {
Definition::Module(module) => module.source.to_visibility(),
Definition::Member(member) => match member.kind {
MemberKind::Class => {
MemberKind::Class(class) => {
let parent = &definitions[member.parent];
if parent.visibility.is_private()
|| exports.is_some_and(|exports| {
member.name().is_some_and(|name| !exports.contains(&name))
})
|| exports.is_some_and(|exports| !exports.contains(&member.name()))
{
Visibility::Private
} else {
class_visibility(member.stmt)
class_visibility(class)
}
}
MemberKind::NestedClass => {
MemberKind::NestedClass(class) => {
let parent = &definitions[member.parent];
if parent.visibility.is_private()
|| matches!(
parent.definition,
Definition::Member(Member {
kind: MemberKind::Function
| MemberKind::NestedFunction
| MemberKind::Method,
..
})
)
|| parent.definition.as_function_def().is_some()
{
Visibility::Private
} else {
class_visibility(member.stmt)
class_visibility(class)
}
}
MemberKind::Function => {
MemberKind::Function(function) => {
let parent = &definitions[member.parent];
if parent.visibility.is_private()
|| exports.is_some_and(|exports| {
member.name().is_some_and(|name| !exports.contains(&name))
})
|| exports.is_some_and(|exports| !exports.contains(&member.name()))
{
Visibility::Private
} else {
function_visibility(member.stmt)
function_visibility(function)
}
}
MemberKind::NestedFunction => Visibility::Private,
MemberKind::Method => {
MemberKind::NestedFunction(_) => Visibility::Private,
MemberKind::Method(function) => {
let parent = &definitions[member.parent];
if parent.visibility.is_private() {
Visibility::Private
} else {
method_visibility(member.stmt)
method_visibility(function)
}
}
},