Skip to content

Commit

Permalink
Implement E0308 - invalid-bytes-returned
Browse files Browse the repository at this point in the history
  • Loading branch information
tibor-reiss committed Apr 15, 2024
1 parent 6dccbd2 commit 0317d41
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# These testcases should raise errors

class Float:
def __bytes__(self):
return 3.05 # [invalid-bytes-return]

class Int:
def __bytes__(self):
return 0 # [invalid-bytes-return]

class Str:
def __bytes__(self):
return "some bytes" # [invalid-bytes-return]

# TODO: Once Ruff has better type checking
def return_bytes():
return "some string"

class ComplexReturn:
def __bytes__(self):
return return_bytes() # [invalid-bytes-return]



# These testcases should NOT raise errors

class Bytes:
def __bytes__(self):
return b"some bytes"


class Bytes2:
def __bytes__(self):
x = b"some bytes"
return x
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::InvalidBoolReturnType) {
pylint::rules::invalid_bool_return(checker, name, body);
}
if checker.enabled(Rule::InvalidBytesReturnType) {
pylint::rules::invalid_bytes_return(checker, name, body);
}
if checker.enabled(Rule::InvalidStrReturnType) {
pylint::rules::invalid_str_return(checker, name, body);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "E0302") => (RuleGroup::Stable, rules::pylint::rules::UnexpectedSpecialMethodSignature),
(Pylint, "E0304") => (RuleGroup::Preview, rules::pylint::rules::InvalidBoolReturnType),
(Pylint, "E0307") => (RuleGroup::Stable, rules::pylint::rules::InvalidStrReturnType),
(Pylint, "E0308") => (RuleGroup::Preview, rules::pylint::rules::InvalidBytesReturnType),
(Pylint, "E0604") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllObject),
(Pylint, "E0605") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllFormat),
(Pylint, "E0643") => (RuleGroup::Preview, rules::pylint::rules::PotentialIndexError),
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ mod tests {
#[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.py"))]
#[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"))]
#[test_case(Rule::InvalidBoolReturnType, Path::new("invalid_return_type_bool.py"))]
#[test_case(
Rule::InvalidBytesReturnType,
Path::new("invalid_return_type_bytes.py")
)]
#[test_case(Rule::InvalidStrReturnType, Path::new("invalid_return_type_str.py"))]
#[test_case(Rule::DuplicateBases, Path::new("duplicate_bases.py"))]
#[test_case(Rule::InvalidCharacterBackspace, Path::new("invalid_characters.py"))]
Expand Down
77 changes: 77 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/invalid_bytes_return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::ReturnStatementVisitor;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::Stmt;
use ruff_python_semantic::analyze::type_inference::{PythonType, ResolvedPythonType};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `__bytes__` implementations that return a type other than `bytes`.
///
/// ## Why is this bad?
/// The `__bytes__` method should return a `bytes` object. Returning a different
/// type may cause unexpected behavior.
///
/// ## Example
/// ```python
/// class Foo:
/// def __bytes__(self):
/// return 2
/// ```
///
/// Use instead:
/// ```python
/// class Foo:
/// def __bytes__(self):
/// return b"2"
/// ```
///
/// ## References
/// - [Python documentation: The `__bytes__` method](https://docs.python.org/3/reference/datamodel.html#object.__bytes__)
#[violation]
pub struct InvalidBytesReturnType;

impl Violation for InvalidBytesReturnType {
#[derive_message_formats]
fn message(&self) -> String {
format!("`__bytes__` does not return `bytes`")
}
}

/// E0308
pub(crate) fn invalid_bytes_return(checker: &mut Checker, name: &str, body: &[Stmt]) {
if name != "__bytes__" {
return;
}

if !checker.semantic().current_scope().kind.is_class() {
return;
}

let returns = {
let mut visitor = ReturnStatementVisitor::default();
visitor.visit_body(body);
visitor.returns
};

for stmt in returns {
if let Some(value) = stmt.value.as_deref() {
if !matches!(
ResolvedPythonType::from(value),
ResolvedPythonType::Unknown | ResolvedPythonType::Atom(PythonType::Bytes)
) {
checker
.diagnostics
.push(Diagnostic::new(InvalidBytesReturnType, value.range()));
}
} else {
// Disallow implicit `None`.
checker
.diagnostics
.push(Diagnostic::new(InvalidBytesReturnType, stmt.range()));
}
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub(crate) use import_self::*;
pub(crate) use invalid_all_format::*;
pub(crate) use invalid_all_object::*;
pub(crate) use invalid_bool_return::*;
pub(crate) use invalid_bytes_return::*;
pub(crate) use invalid_envvar_default::*;
pub(crate) use invalid_envvar_value::*;
pub(crate) use invalid_str_return::*;
Expand Down Expand Up @@ -126,6 +127,7 @@ mod import_self;
mod invalid_all_format;
mod invalid_all_object;
mod invalid_bool_return;
mod invalid_bytes_return;
mod invalid_envvar_default;
mod invalid_envvar_value;
mod invalid_str_return;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
invalid_return_type_bytes.py:5:16: PLE0308 `__bytes__` does not return `bytes`
|
3 | class Float:
4 | def __bytes__(self):
5 | return 3.05 # [invalid-bytes-return]
| ^^^^ PLE0308
6 |
7 | class Int:
|

invalid_return_type_bytes.py:9:16: PLE0308 `__bytes__` does not return `bytes`
|
7 | class Int:
8 | def __bytes__(self):
9 | return 0 # [invalid-bytes-return]
| ^ PLE0308
10 |
11 | class Str:
|

invalid_return_type_bytes.py:13:16: PLE0308 `__bytes__` does not return `bytes`
|
11 | class Str:
12 | def __bytes__(self):
13 | return "some bytes" # [invalid-bytes-return]
| ^^^^^^^^^^^^ PLE0308
14 |
15 | # TODO: Once Ruff has better type checking
|
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 0317d41

Please sign in to comment.