diff --git a/crates/ruff/src/rules/pylint/rules/collapsible_else_if.rs b/crates/ruff/src/rules/pylint/rules/collapsible_else_if.rs index b99de7e37a..d53fd72869 100644 --- a/crates/ruff/src/rules/pylint/rules/collapsible_else_if.rs +++ b/crates/ruff/src/rules/pylint/rules/collapsible_else_if.rs @@ -4,13 +4,45 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::source_code::Locator; +/// ## What it does +/// Checks for `else` blocks that consist of a single `if` statement. +/// +/// ## Why is this bad? +/// If an `else` block contains a single `if` statement, it can be collapsed +/// into an `elif`, thus reducing the indentation level. +/// +/// ## Example +/// ```python +/// def check_sign(value: int) -> None: +/// if value > 0: +/// print("Number is positive.") +/// else: +/// if value < 0: +/// print("Number is negative.") +/// else: +/// print("Number is zero.") +/// ``` +/// +/// Use instead: +/// ```python +/// def check_sign(value: int) -> None: +/// if value > 0: +/// print("Number is positive.") +/// elif value < 0: +/// print("Number is negative.") +/// else: +/// print("Number is zero.") +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/tutorial/controlflow.html#if-statements) #[violation] pub struct CollapsibleElseIf; impl Violation for CollapsibleElseIf { #[derive_message_formats] fn message(&self) -> String { - format!("Consider using `elif` instead of `else` then `if` to remove one indentation level") + format!("Use `elif` instead of `else` then `if`, to reduce indentation") } } diff --git a/crates/ruff/src/rules/pylint/rules/comparison_of_constant.rs b/crates/ruff/src/rules/pylint/rules/comparison_of_constant.rs index 5897200301..76d4468f7f 100644 --- a/crates/ruff/src/rules/pylint/rules/comparison_of_constant.rs +++ b/crates/ruff/src/rules/pylint/rules/comparison_of_constant.rs @@ -57,6 +57,26 @@ impl fmt::Display for ViolationsCmpop { } } +/// ## What it does +/// Checks for comparisons between constants. +/// +/// ## Why is this bad? +/// Comparing two constants will always resolve to the same value, so the +/// comparison is redundant. Instead, the expression should be replaced +/// with the result of the comparison. +/// +/// ## Example +/// ```python +/// foo = 1 == 1 +/// ``` +/// +/// Use instead: +/// ```python +/// foo = True +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#comparisons) #[violation] pub struct ComparisonOfConstant { left_constant: String, diff --git a/crates/ruff/src/rules/pylint/rules/duplicate_bases.rs b/crates/ruff/src/rules/pylint/rules/duplicate_bases.rs index c49517b749..6d6256f472 100644 --- a/crates/ruff/src/rules/pylint/rules/duplicate_bases.rs +++ b/crates/ruff/src/rules/pylint/rules/duplicate_bases.rs @@ -8,6 +8,34 @@ use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for duplicate base classes in class definitions. +/// +/// ## Why is this bad? +/// Including duplicate base classes will raise a `TypeError` at runtime. +/// +/// ## Example +/// ```python +/// class Foo: +/// pass +/// +/// +/// class Bar(Foo, Foo): +/// pass +/// ``` +/// +/// Use instead: +/// ```python +/// class Foo: +/// pass +/// +/// +/// class Bar(Foo): +/// pass +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/reference/compound_stmts.html#class-definitions) #[violation] pub struct DuplicateBases { base: String, diff --git a/crates/ruff/src/rules/pylint/rules/duplicate_value.rs b/crates/ruff/src/rules/pylint/rules/duplicate_value.rs index 70652536b2..46ba673cc8 100644 --- a/crates/ruff/src/rules/pylint/rules/duplicate_value.rs +++ b/crates/ruff/src/rules/pylint/rules/duplicate_value.rs @@ -7,6 +7,23 @@ use ruff_python_ast::comparable::ComparableExpr; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for set literals that contain duplicate values. +/// +/// ## Why is this bad? +/// In Python, sets are unordered collections of unique elements. Including a +/// duplicate value in a set literal is redundant, and may be indicative of a +/// mistake. +/// +/// ## Example +/// ```python +/// {1, 2, 3, 1} +/// ``` +/// +/// Use instead: +/// ```python +/// {1, 2, 3} +/// ``` #[violation] pub struct DuplicateValue { value: String, diff --git a/crates/ruff/src/rules/pylint/rules/magic_value_comparison.rs b/crates/ruff/src/rules/pylint/rules/magic_value_comparison.rs index 3128777707..60a6a8e4c2 100644 --- a/crates/ruff/src/rules/pylint/rules/magic_value_comparison.rs +++ b/crates/ruff/src/rules/pylint/rules/magic_value_comparison.rs @@ -7,6 +7,35 @@ use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; use crate::rules::pylint::settings::ConstantType; +/// ## What it does +/// Checks for the use of unnamed numerical constants ("magic") values in +/// comparisons. +/// +/// ## Why is this bad? +/// The use of "magic" can make code harder to read and maintain, as readers +/// will have to infer the meaning of the value from the context. +/// +/// For convenience, this rule excludes a variety of common values from the +/// "magic" value definition, such as `0`, `1`, `""`, and `"__main__"`. +/// +/// ## Example +/// ```python +/// def calculate_discount(price: float) -> float: +/// return price * (1 - 0.2) +/// ``` +/// +/// Use instead: +/// ```python +/// DISCOUNT_RATE = 0.2 +/// +/// +/// def calculate_discount(price: float) -> float: +/// return price * (1 - DISCOUNT_RATE) +/// ``` +/// +/// ## References +/// - [Wikipedia](https://en.wikipedia.org/wiki/Magic_number_(programming)#Unnamed_numerical_constants) +/// - [PEP 8](https://peps.python.org/pep-0008/#constants) #[violation] pub struct MagicValueComparison { value: String, diff --git a/crates/ruff/src/rules/pylint/rules/manual_import_from.rs b/crates/ruff/src/rules/pylint/rules/manual_import_from.rs index 30f8e218e0..2274a3e435 100644 --- a/crates/ruff/src/rules/pylint/rules/manual_import_from.rs +++ b/crates/ruff/src/rules/pylint/rules/manual_import_from.rs @@ -7,6 +7,25 @@ use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; use crate::registry::AsRule; +/// ## What it does +/// Checks for submodule imports that are aliased to the submodule name. +/// +/// ## Why is this bad? +/// Using the `from` keyword to import the submodule is more concise and +/// readable. +/// +/// ## Example +/// ```python +/// import concurrent.futures as futures +/// ``` +/// +/// Use instead: +/// ```python +/// from concurrent import futures +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/reference/import.html#submodules) #[violation] pub struct ManualFromImport { module: String, diff --git a/crates/ruff/src/rules/pylint/rules/property_with_parameters.rs b/crates/ruff/src/rules/pylint/rules/property_with_parameters.rs index 7d28d1cfb7..4471a4a73d 100644 --- a/crates/ruff/src/rules/pylint/rules/property_with_parameters.rs +++ b/crates/ruff/src/rules/pylint/rules/property_with_parameters.rs @@ -6,6 +6,36 @@ use ruff_python_ast::helpers::identifier_range; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for property definitions that accept function parameters. +/// +/// ## Why is this bad? +/// Properties cannot be called with parameters. +/// +/// If you need to pass parameters to a property, create a method with the +/// desired parameters and call that method instead. +/// +/// ## Example +/// ```python +/// class Cat: +/// @property +/// def purr(self, volume): +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// class Cat: +/// @property +/// def purr(self): +/// ... +/// +/// def purr_volume(self, volume): +/// ... +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/library/functions.html#property) #[violation] pub struct PropertyWithParameters; diff --git a/crates/ruff/src/rules/pylint/rules/repeated_isinstance_calls.rs b/crates/ruff/src/rules/pylint/rules/repeated_isinstance_calls.rs index f8bb082c83..c3b5289b61 100644 --- a/crates/ruff/src/rules/pylint/rules/repeated_isinstance_calls.rs +++ b/crates/ruff/src/rules/pylint/rules/repeated_isinstance_calls.rs @@ -8,6 +8,34 @@ use ruff_python_ast::hashable::HashableExpr; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for repeated `isinstance` calls on the same object. +/// +/// ## Why is this bad? +/// Repeated `isinstance` calls on the same object can be merged into a +/// single call. +/// +/// ## Example +/// ```python +/// def is_number(x): +/// return isinstance(x, int) or isinstance(x, float) or isinstance(x, complex) +/// ``` +/// +/// Use instead: +/// ```python +/// def is_number(x): +/// return isinstance(x, (int, float, complex)) +/// ``` +/// +/// Or, for Python 3.10 and later: +/// +/// ```python +/// def is_number(x): +/// return isinstance(x, int | float | complex) +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/library/functions.html#isinstance) #[violation] pub struct RepeatedIsinstanceCalls { obj: String, diff --git a/crates/ruff/src/rules/pylint/rules/sys_exit_alias.rs b/crates/ruff/src/rules/pylint/rules/sys_exit_alias.rs index 83eeefa0fb..9bc28642ad 100644 --- a/crates/ruff/src/rules/pylint/rules/sys_exit_alias.rs +++ b/crates/ruff/src/rules/pylint/rules/sys_exit_alias.rs @@ -7,6 +7,35 @@ use crate::autofix::actions::get_or_import_symbol; use crate::checkers::ast::Checker; use crate::registry::AsRule; +/// ## What it does +/// Checks for uses of the `exit()` and `quit()`. +/// +/// ## Why is this bad? +/// `exit` and `quit` come from the `site` module, which is typically imported +/// automatically during startup. However, it is not _guaranteed_ to be +/// imported, and so using these functions may result in a `NameError` at +/// runtime. Generally, these constants are intended to be used in an interactive +/// interpreter, and not in programs. +/// +/// Prefer `sys.exit()`, as the `sys` module is guaranteed to exist in all +/// contexts. +/// +/// ## Example +/// ```python +/// if __name__ == "__main__": +/// exit() +/// ``` +/// +/// Use instead: +/// ```python +/// import sys +/// +/// if __name__ == "__main__": +/// sys.exit() +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/library/constants.html#constants-added-by-the-site-module) #[violation] pub struct SysExitAlias { name: String, diff --git a/crates/ruff/src/rules/pylint/rules/too_many_arguments.rs b/crates/ruff/src/rules/pylint/rules/too_many_arguments.rs index 8266aa290e..bd06b69c99 100644 --- a/crates/ruff/src/rules/pylint/rules/too_many_arguments.rs +++ b/crates/ruff/src/rules/pylint/rules/too_many_arguments.rs @@ -6,6 +6,43 @@ use ruff_python_ast::helpers::identifier_range; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for function definitions that include too many arguments. +/// +/// By default, this rule allows up to five arguments, as configured by the +/// `pylint.max-args` option. +/// +/// ## Why is this bad? +/// Functions with many arguments are harder to understand, maintain, and call. +/// Consider refactoring functions with many arguments into smaller functions +/// with fewer arguments, or using objects to group related arguments. +/// +/// ## Example +/// ```python +/// def calculate_position(x_pos, y_pos, z_pos, x_vel, y_vel, z_vel, time): +/// new_x = x_pos + x_vel * time +/// new_y = y_pos + y_vel * time +/// new_z = z_pos + z_vel * time +/// return new_x, new_y, new_z +/// ``` +/// +/// Use instead: +/// ```python +/// from typing import NamedTuple +/// +/// +/// class Vector(NamedTuple): +/// x: float +/// y: float +/// z: float +/// +/// +/// def calculate_position(pos: Vector, vel: Vector, time: float) -> Vector: +/// return Vector(*(p + v * time for p, v in zip(pos, vel))) +/// ``` +/// +/// ## Options +/// - `pylint.max-args` #[violation] pub struct TooManyArguments { c_args: usize, diff --git a/crates/ruff/src/rules/pylint/rules/too_many_branches.rs b/crates/ruff/src/rules/pylint/rules/too_many_branches.rs index 8edede1586..b597cbeae6 100644 --- a/crates/ruff/src/rules/pylint/rules/too_many_branches.rs +++ b/crates/ruff/src/rules/pylint/rules/too_many_branches.rs @@ -5,6 +5,70 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::identifier_range; use ruff_python_ast::source_code::Locator; +/// ## What it does +/// Checks for functions or methods with too many branches. +/// +/// By default, this rule allows up to 12 branches. This can be configured +/// using the `max-branches` option. +/// +/// ## Why is this bad? +/// Functions or methods with many branches are harder to understand +/// and maintain than functions or methods with fewer branches. +/// +/// ## Example +/// ```python +/// def capital(country): +/// if country == "Australia": +/// return "Canberra" +/// elif country == "Brazil": +/// return "Brasilia" +/// elif country == "Canada": +/// return "Ottawa" +/// elif country == "England": +/// return "London" +/// elif country == "France": +/// return "Paris" +/// elif country == "Germany": +/// return "Berlin" +/// elif country == "Poland": +/// return "Warsaw" +/// elif country == "Romania": +/// return "Bucharest" +/// elif country == "Spain": +/// return "Madrid" +/// elif country == "Thailand": +/// return "Bangkok" +/// elif country == "Turkey": +/// return "Ankara" +/// elif country == "United States": +/// return "Washington" +/// else: +/// return "Unknown" # 13th branch +/// ``` +/// +/// Use instead: +/// ```python +/// def capital(country): +/// capitals = { +/// "Australia": "Canberra", +/// "Brazil": "Brasilia", +/// "Canada": "Ottawa", +/// "England": "London", +/// "France": "Paris", +/// "Germany": "Berlin", +/// "Poland": "Warsaw", +/// "Romania": "Bucharest", +/// "Spain": "Madrid", +/// "Thailand": "Bangkok", +/// "Turkey": "Ankara", +/// "United States": "Washington", +/// } +/// city = capitals.get(country, "Unknown") +/// return city +/// ``` +/// +/// ## References +/// - [Ruff configuration documentation](https://beta.ruff.rs/docs/settings/#max-branches) #[violation] pub struct TooManyBranches { branches: usize, diff --git a/crates/ruff/src/rules/pylint/rules/too_many_return_statements.rs b/crates/ruff/src/rules/pylint/rules/too_many_return_statements.rs index bdb59212dd..9ba5dd2202 100644 --- a/crates/ruff/src/rules/pylint/rules/too_many_return_statements.rs +++ b/crates/ruff/src/rules/pylint/rules/too_many_return_statements.rs @@ -6,6 +6,51 @@ use ruff_python_ast::helpers::{identifier_range, ReturnStatementVisitor}; use ruff_python_ast::source_code::Locator; use ruff_python_ast::statement_visitor::StatementVisitor; +/// ## What it does +/// Checks for functions or methods with too many return statements. +/// +/// By default, this rule allows up to six return statements, as configured by +/// the `pylint.max-returns` option. +/// +/// ## Why is this bad? +/// Functions or methods with many return statements are harder to understand +/// and maintain, and often indicative of complex logic. +/// +/// ## Example +/// ```python +/// def capital(country: str) -> str | None: +/// if country == "England": +/// return "London" +/// elif country == "France": +/// return "Paris" +/// elif country == "Poland": +/// return "Warsaw" +/// elif country == "Romania": +/// return "Bucharest" +/// elif country == "Spain": +/// return "Madrid" +/// elif country == "Thailand": +/// return "Bangkok" +/// else: +/// return None +/// ``` +/// +/// Use instead: +/// ```python +/// def capital(country: str) -> str | None: +/// capitals = { +/// "England": "London", +/// "France": "Paris", +/// "Poland": "Warsaw", +/// "Romania": "Bucharest", +/// "Spain": "Madrid", +/// "Thailand": "Bangkok", +/// } +/// return capitals.get(country) +/// ``` +/// +/// ## Options +/// - `pylint.max-returns` #[violation] pub struct TooManyReturnStatements { returns: usize, diff --git a/crates/ruff/src/rules/pylint/rules/too_many_statements.rs b/crates/ruff/src/rules/pylint/rules/too_many_statements.rs index 829f85e471..6912ca0c10 100644 --- a/crates/ruff/src/rules/pylint/rules/too_many_statements.rs +++ b/crates/ruff/src/rules/pylint/rules/too_many_statements.rs @@ -5,6 +5,47 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::identifier_range; use ruff_python_ast::source_code::Locator; +/// ## What it does +/// Checks for functions or methods with too many statements. +/// +/// By default, this rule allows up to 50 statements, as configured by the +/// `pylint.max-statements` option. +/// +/// ## Why is this bad? +/// Functions or methods with many statements are harder to understand +/// and maintain. +/// +/// Instead, consider refactoring the function or method into smaller +/// functions or methods, or identifying generalizable patterns and +/// replacing them with generic logic or abstractions. +/// +/// ## Example +/// ```python +/// def is_even(number: int) -> bool: +/// if number == 0: +/// return True +/// elif number == 1: +/// return False +/// elif number == 2: +/// return True +/// elif number == 3: +/// return False +/// elif number == 4: +/// return True +/// elif number == 5: +/// return False +/// else: +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// def is_even(number: int) -> bool: +/// return number % 2 == 0 +/// ``` +/// +/// ## Options +/// - `pylint.max-statements` #[violation] pub struct TooManyStatements { statements: usize, diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap index 59d16efdea..567c03d257 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff/src/rules/pylint/mod.rs --- -collapsible_else_if.py:38:9: PLR5501 Consider using `elif` instead of `else` then `if` to remove one indentation level +collapsible_else_if.py:38:9: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation | 38 | pass 39 | else: @@ -11,7 +11,7 @@ collapsible_else_if.py:38:9: PLR5501 Consider using `elif` instead of `else` the | |________________^ PLR5501 | -collapsible_else_if.py:46:9: PLR5501 Consider using `elif` instead of `else` then `if` to remove one indentation level +collapsible_else_if.py:46:9: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation | 46 | pass 47 | else: