Skip to content

Commit

Permalink
[PLE0101] error when __init__ returns a value (#3007)
Browse files Browse the repository at this point in the history
  • Loading branch information
r3m0t committed Feb 19, 2023
1 parent 87422ba commit 4d3d04e
Show file tree
Hide file tree
Showing 11 changed files with 192 additions and 39 deletions.
41 changes: 41 additions & 0 deletions crates/ruff/resources/test/fixtures/pylint/return_in_init.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
def a():
return

def __init__():
return

class A:
def __init__(self):
return


class B:
def __init__(self):
return 3

def gen(self):
return 5

class MyClass:

def __init__(self):
return 1

class MyClass2:
"""dummy class"""

def __init__(self):
return


class MyClass3:
"""dummy class"""

def __init__(self):
return None

class MyClass5:
"""dummy class"""

def __init__(self):
self.callable = lambda: (yield None)
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,9 @@ where
if self.settings.rules.enabled(&Rule::ReturnOutsideFunction) {
pyflakes::rules::return_outside_function(self, stmt);
}
if self.settings.rules.enabled(&Rule::ReturnInInit) {
pylint::rules::return_in_init(self, stmt);
}
}
StmtKind::ClassDef {
name,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {

// pylint
(Pylint, "E0100") => Rule::YieldInInit,
(Pylint, "E0101") => Rule::ReturnInInit,
(Pylint, "E0604") => Rule::InvalidAllObject,
(Pylint, "E0605") => Rule::InvalidAllFormat,
(Pylint, "E1307") => Rule::BadStringFormatType,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ ruff_macros::register_rules!(
rules::pylint::rules::UsedPriorGlobalDeclaration,
rules::pylint::rules::AwaitOutsideAsync,
rules::pylint::rules::PropertyWithParameters,
rules::pylint::rules::ReturnInInit,
rules::pylint::rules::ConsiderUsingFromImport,
rules::pylint::rules::ComparisonOfConstant,
rules::pylint::rules::ConsiderMergingIsinstance,
Expand Down
38 changes: 38 additions & 0 deletions crates/ruff/src/rules/pylint/helpers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
use crate::ast::function_type;
use crate::ast::function_type::FunctionType;
use crate::{
ast::types::{FunctionDef, ScopeKind},
checkers::ast::Checker,
};

pub fn in_dunder_init(checker: &mut Checker) -> bool {
let scope = checker.current_scope();
let ScopeKind::Function(FunctionDef {
name,
decorator_list,
..
}) = &scope.kind else {
return false;
};
if *name != "__init__" {
return false;
}
let Some(parent) = checker.current_scope_parent() else {
return false;
};

if !matches!(
function_type::classify(
checker,
parent,
name,
decorator_list,
&checker.settings.pep8_naming.classmethod_decorators,
&checker.settings.pep8_naming.staticmethod_decorators,
),
FunctionType::Method
) {
return false;
}
true
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//! Rules from [Pylint](https://pypi.org/project/pylint/).
mod helpers;
pub(crate) mod rules;
pub mod settings;

Expand All @@ -16,6 +17,7 @@ mod tests {
use crate::settings::Settings;
use crate::test::test_path;

#[test_case(Rule::ReturnInInit, Path::new("return_in_init.py"); "PLE0101")]
#[test_case(Rule::UselessImportAlias, Path::new("import_aliasing.py"); "PLC0414")]
#[test_case(Rule::UnnecessaryDirectLambdaCall, Path::new("unnecessary_direct_lambda_call.py"); "PLC3002")]
#[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.py"); "PLE0117")]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub use magic_value_comparison::{magic_value_comparison, MagicValueComparison};
pub use merge_isinstance::{merge_isinstance, ConsiderMergingIsinstance};
pub use nonlocal_without_binding::NonlocalWithoutBinding;
pub use property_with_parameters::{property_with_parameters, PropertyWithParameters};
pub use return_in_init::{return_in_init, ReturnInInit};
pub use too_many_arguments::{too_many_arguments, TooManyArguments};
pub use too_many_branches::{too_many_branches, TooManyBranches};
pub use too_many_return_statements::{too_many_return_statements, TooManyReturnStatements};
Expand Down Expand Up @@ -39,6 +40,7 @@ mod magic_value_comparison;
mod merge_isinstance;
mod nonlocal_without_binding;
mod property_with_parameters;
mod return_in_init;
mod too_many_arguments;
mod too_many_branches;
mod too_many_return_statements;
Expand Down
72 changes: 72 additions & 0 deletions crates/ruff/src/rules/pylint/rules/return_in_init.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
use rustpython_parser::ast::{Constant, ExprKind, Stmt, StmtKind};

use ruff_macros::{define_violation, derive_message_formats};

use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::registry::Diagnostic;
use crate::rules::pylint::helpers::in_dunder_init;
use crate::violation::Violation;

define_violation!(
/// ## What it does
/// Checks for `__init__` methods that return values.
///
/// ## Why is this bad?
/// The `__init__` method is the constructor for a given Python class,
/// responsible for initializing, rather than creating, new objects.
///
/// The `__init__` method has to return `None`. Returning any value from
/// an `__init__` method will result in a runtime error.
///
/// ## Example
/// ```python
/// class Example:
/// def __init__(self):
/// return []
/// ```
///
/// Use instead:
/// ```python
/// class Example:
/// def __init__(self):
/// self.value = []
/// ```
///
/// ## References
/// * [CodeQL: `py-explicit-return-in-init`](https://codeql.github.com/codeql-query-help/python/py-explicit-return-in-init/)
pub struct ReturnInInit;
);
impl Violation for ReturnInInit {
#[derive_message_formats]
fn message(&self) -> String {
format!("Explicit return in `__init__`")
}
}

/// PLE0101
pub fn return_in_init(checker: &mut Checker, stmt: &Stmt) {
if let StmtKind::Return { value } = &stmt.node {
if let Some(expr) = value {
if matches!(
expr.node,
ExprKind::Constant {
value: Constant::None,
..
}
) {
// Explicit `return None`.
return;
}
} else {
// Implicit `return`.
return;
}
}

if in_dunder_init(checker) {
checker
.diagnostics
.push(Diagnostic::new(ReturnInInit, Range::from_located(stmt)));
}
}
45 changes: 6 additions & 39 deletions crates/ruff/src/rules/pylint/rules/yield_in_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,9 @@ use rustpython_parser::ast::Expr;

use ruff_macros::{define_violation, derive_message_formats};

use crate::ast::function_type;
use crate::ast::function_type::FunctionType;
use crate::rules::pylint::helpers::in_dunder_init;
use crate::{
ast::types::{FunctionDef, Range, ScopeKind},
checkers::ast::Checker,
registry::Diagnostic,
violation::Violation,
ast::types::Range, checkers::ast::Checker, registry::Diagnostic, violation::Violation,
};

define_violation!(
Expand Down Expand Up @@ -45,38 +41,9 @@ impl Violation for YieldInInit {

/// PLE0100
pub fn yield_in_init(checker: &mut Checker, expr: &Expr) {
let scope = checker.current_scope();
let ScopeKind::Function(FunctionDef {
name,
decorator_list,
..
}) = &scope.kind else {
return;
};

if *name != "__init__" {
return;
if in_dunder_init(checker) {
checker
.diagnostics
.push(Diagnostic::new(YieldInInit, Range::from_located(expr)));
}

let Some(parent) = checker.current_scope_parent() else {
return;
};

if !matches!(
function_type::classify(
checker,
parent,
name,
decorator_list,
&checker.settings.pep8_naming.classmethod_decorators,
&checker.settings.pep8_naming.staticmethod_decorators,
),
FunctionType::Method
) {
return;
}

checker
.diagnostics
.push(Diagnostic::new(YieldInInit, Range::from_located(expr)));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
source: crates/ruff/src/rules/pylint/mod.rs
expression: diagnostics
---
- kind:
ReturnInInit: ~
location:
row: 14
column: 8
end_location:
row: 14
column: 16
fix: ~
parent: ~
- kind:
ReturnInInit: ~
location:
row: 22
column: 8
end_location:
row: 22
column: 16
fix: ~
parent: ~

1 change: 1 addition & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1778,6 +1778,7 @@
"PLE01",
"PLE010",
"PLE0100",
"PLE0101",
"PLE011",
"PLE0117",
"PLE0118",
Expand Down

0 comments on commit 4d3d04e

Please sign in to comment.