diff --git a/crates/ruff_linter/resources/mdtest/flake8-bugbear/useless-expression.md b/crates/ruff_linter/resources/mdtest/flake8-bugbear/useless-expression.md new file mode 100644 index 0000000000000..a91803a705fab --- /dev/null +++ b/crates/ruff_linter/resources/mdtest/flake8-bugbear/useless-expression.md @@ -0,0 +1,158 @@ +# `useless-expression` (`B018`) + +```toml +lint.preview = true +lint.select = ["B018"] +``` + +## String literals + +In preview, string literals and side-effect-free f-strings are useless +expressions unless Ruff recognizes them as docstrings. + +```py +"""Module docstring.""" + +"Standalone module string." # error: [useless-expression] + +f"Useless module f-string: {1}" # error: [useless-expression] + + +class Class: + """Class docstring.""" + + "Standalone class string." # error: [useless-expression] + f"Useless class f-string: {1}" # error: [useless-expression] + + def method(self): + """Method docstring.""" + + "Standalone function string." # error: [useless-expression] + f"Useless function f-string: {1}" # error: [useless-expression] +``` + +## F-string formatting side effects + +Interpolating a value can call a user-defined `__format__` method, including +when the value appears inside another interpolation's format specification. +These f-strings are not useless expressions because formatting the value has +an observable side effect. + +```py +class Formatted: + def __format__(self, spec: str) -> str: + print("formatted") + return "1" + + +value = Formatted() +f"{value}" +f"{1:{value}}" +``` + +## Strings after overloads + +A string after an overload is not a docstring for the preceding function. + +```py +from typing import overload + + +@overload +def f(value: None) -> str: ... + + +"None overload documentation." # error: [useless-expression] + + +@overload +def f(value: list[str]) -> int: ... + + +"List overload documentation." # error: [useless-expression] + + +def f(value): + return value +``` + +## Section separators + +A standalone string used as a visual separator is still a useless expression. + +```py +def main(): + pass + + +"""MAIN""" # error: [useless-expression] + +if __name__ == "__main__": + main() +``` + +## Attribute docstrings + +Ruff recognizes strings immediately following simple assignments, annotated +assignments, and `type` statements at module or class scope as attribute +docstrings. A second string is not part of the attribute docstring. + +```py +module_attribute = 1 +"Module attribute docstring." + +annotated_module_attribute: int +"Annotated module attribute docstring." + +type ModuleAlias = int +"Module type alias docstring." + + +class Class: + attribute = 1 + "Class attribute docstring." + + annotated_attribute: int + "Annotated class attribute docstring." + + type ClassAlias = str + "Class type alias docstring." + + "Not an attribute docstring." # error: [useless-expression] + + +def function(): + type LocalAlias = bytes + "Not an attribute docstring." # error: [useless-expression] +``` + +## Instance attribute docstrings + +Strings following instance attribute assignments directly in `__init__` are +attribute docstrings. Local assignments, nested assignments, and assignments in +other methods do not introduce attribute docstrings. + +```py +class Class: + def __init__(this): + this.attribute = 1 + "Instance attribute docstring." + + this.annotated_attribute: int + "Annotated instance attribute docstring." + + local = 1 + "Not an instance attribute docstring." # error: [useless-expression] + + if local: + this.nested_attribute = 1 + "Not a top-level assignment." # error: [useless-expression] + + this = object() + this.rebound_attribute = 1 + "Not an instance attribute docstring." # error: [useless-expression] + + def method(self): + self.attribute = 1 + "Not in an `__init__` method." # error: [useless-expression] +``` diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 029ac85d08f1b..04cb6efa32373 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -53,7 +53,7 @@ use ruff_python_parser::semantic_errors::{ use ruff_python_parser::typing::{AnnotationKind, ParsedAnnotation, parse_type_annotation}; use ruff_python_parser::{ParseError, Parsed}; use ruff_python_semantic::all::{DunderAllDefinition, DunderAllFlags}; -use ruff_python_semantic::analyze::{imports, typing}; +use ruff_python_semantic::analyze::{imports, typing, visibility}; use ruff_python_semantic::{ BindingFlags, BindingId, BindingKind, Exceptions, Export, FromImport, GeneratorKind, Globals, Import, Module, ModuleKind, ModuleSource, NodeId, ScopeId, ScopeKind, SemanticModel, @@ -178,6 +178,10 @@ pub(crate) enum ExpectedDocstringKind { /// class Foo: /// b = 1 /// """This is the docstring for `Foo.b` class variable.""" + /// + /// def __init__(self) -> None: + /// self.c = 1 + /// """This is the docstring for the `c` instance variable.""" /// ``` Attribute, } @@ -666,6 +670,59 @@ impl<'a> Checker<'a> { self.context } + /// Return `true` if the statement assigns an instance attribute directly in an `__init__` + /// method. + fn is_init_instance_attribute_assignment(&self, stmt: &Stmt) -> bool { + let semantic = self.semantic(); + let scope = semantic.current_scope(); + let ScopeKind::Function(function) = scope.kind else { + return false; + }; + + if !visibility::is_init(&function.name) + || !semantic + .first_non_type_parent_scope(scope) + .is_some_and(|parent| parent.kind.is_class()) + || !matches!( + semantic.current_statement_parent(), + Some(Stmt::FunctionDef(parent)) if std::ptr::eq(parent, function) + ) + { + return false; + } + + let Some(instance) = function + .parameters + .posonlyargs + .first() + .or_else(|| function.parameters.args.first()) + else { + return false; + }; + + let is_instance_attribute = |target: &Expr| { + let Some(name) = target + .as_attribute_expr() + .and_then(|attribute| attribute.value.as_name_expr()) + else { + return false; + }; + let Some(binding_id) = semantic.resolve_name(name) else { + return false; + }; + let binding = semantic.binding(binding_id); + binding.kind.is_argument() && binding.range == instance.parameter.name.range() + }; + + match stmt { + Stmt::Assign(ast::StmtAssign { targets, .. }) => { + matches!(targets.as_slice(), [target] if is_instance_attribute(target)) + } + Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => is_instance_attribute(target), + _ => false, + } + } + /// Return the current [`DocstringState`]. pub(crate) fn docstring_state(&self) -> DocstringState { self.docstring_state @@ -1648,10 +1705,20 @@ impl<'a> Visitor<'a> for Checker<'a> { self.docstring_state = DocstringState::Expected(ExpectedDocstringKind::Attribute); } + // Follow Pyright's convention for documenting PEP 695 type aliases: + // https://discuss.python.org/t/docstrings-for-new-type-aliases-as-defined-in-pep-695/39816 + Stmt::TypeAlias(_) => { + self.docstring_state = + DocstringState::Expected(ExpectedDocstringKind::Attribute); + } _ => {} } } + if self.is_init_instance_attribute_assignment(stmt) { + self.docstring_state = DocstringState::Expected(ExpectedDocstringKind::Attribute); + } + // Step 3: Clean-up // Step 4: Analysis diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index 853063ec23dfe..844eb77270a16 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -26,6 +26,11 @@ pub(crate) const fn is_s103_extended_dangerous_bits_enabled(settings: &LinterSet settings.preview.is_enabled() } +// https://github.com/astral-sh/ruff/issues/11292 +pub(crate) const fn is_useless_string_expression_enabled(settings: &LinterSettings) -> bool { + settings.preview.is_enabled() +} + // https://github.com/astral-sh/ruff/pull/21382 pub(crate) const fn is_custom_exception_checking_enabled(settings: &LinterSettings) -> bool { settings.preview.is_enabled() diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/useless_expression.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/useless_expression.rs index 1bcd6f638b819..49ae591fb0cb0 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/useless_expression.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/useless_expression.rs @@ -1,10 +1,11 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::Expr; -use ruff_python_ast::helpers::contains_effect; +use ruff_python_ast::helpers::side_effect; use ruff_text_size::Ranged; use crate::Violation; use crate::checkers::ast::Checker; +use crate::preview::is_useless_string_expression_enabled; use crate::rules::flake8_bugbear::helpers::at_last_top_level_expression_in_cell; @@ -26,6 +27,14 @@ use crate::rules::flake8_bugbear::helpers::at_last_top_level_expression_in_cell; /// foo = 1 + 1 /// ``` /// +/// ## Preview +/// When [preview] is enabled, this rule also flags standalone string literals +/// and side-effect-free f-strings that are not docstrings or attribute docstrings. +/// +/// Strings following PEP 695 `type` statements are also treated as attribute docstrings. +/// +/// [preview]: https://docs.astral.sh/ruff/preview/ +/// /// ## Notebook behavior /// For Jupyter Notebooks, this rule is not applied to the last top-level expression in a cell. /// This is because it's common to have a notebook cell that ends with an expression, @@ -78,11 +87,17 @@ pub(crate) fn useless_expression(checker: &Checker, value: &Expr) { return; } - // Ignore strings, to avoid false positives with docstrings. - if matches!( - value, - Expr::FString(_) | Expr::StringLiteral(_) | Expr::EllipsisLiteral(_) - ) { + // In stable mode, ignore strings to avoid false positives with docstrings. In preview mode, + // only ignore strings that Ruff recognizes as PEP 257 or attribute docstrings. + if matches!(value, Expr::FString(_) | Expr::StringLiteral(_)) + && (!is_useless_string_expression_enabled(checker.settings()) + || checker.semantic().in_pep_257_docstring() + || checker.semantic().in_attribute_docstring()) + { + return; + } + + if value.is_ellipsis_literal_expr() { return; } @@ -96,8 +111,9 @@ pub(crate) fn useless_expression(checker: &Checker, value: &Expr) { return; } - // Ignore statements that have side effects. - if contains_effect(value, |id| checker.semantic().has_builtin_binding(id)) { + // Formatting an interpolated value can invoke user-defined `__format__` or `__str__` methods. + let effect = side_effect(value, |id| checker.semantic().has_builtin_binding(id)); + if effect.is_present() || (value.is_f_string_expr() && !effect.is_absent()) { // Flag attributes as useless expressions, even if they're attached to calls or other // expressions. if value.is_attribute_expr() {