forked from astral-sh/ruff
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Implement pygrep-hook's Mock-mistake diagnostic (astral-sh#4366)
- Loading branch information
1 parent
572adf7
commit 865205d
Showing
11 changed files
with
231 additions
and
7 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
19 changes: 19 additions & 0 deletions
19
crates/ruff/resources/test/fixtures/pygrep-hooks/PGH005_0.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # Errors | ||
| assert my_mock.not_called() | ||
| assert my_mock.called_once_with() | ||
| assert my_mock.not_called | ||
| assert my_mock.called_once_with | ||
| my_mock.assert_not_called | ||
| my_mock.assert_called | ||
| my_mock.assert_called_once_with | ||
| my_mock.assert_called_once_with | ||
| MyMock.assert_called_once_with | ||
|
|
||
| # OK | ||
| assert my_mock.call_count == 1 | ||
| assert my_mock.called | ||
| my_mock.assert_not_called() | ||
| my_mock.assert_called() | ||
| my_mock.assert_called_once_with() | ||
| """like :meth:`Mock.assert_called_once_with`""" | ||
| """like :meth:`MagicMock.assert_called_once_with`""" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
99 changes: 99 additions & 0 deletions
99
crates/ruff/src/rules/pygrep_hooks/rules/invalid_mock_access.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| use rustpython_parser::ast::{Expr, ExprKind}; | ||
|
|
||
| use ruff_diagnostics::{Diagnostic, Violation}; | ||
| use ruff_macros::{derive_message_formats, violation}; | ||
|
|
||
| use crate::checkers::ast::Checker; | ||
|
|
||
| #[derive(Debug, PartialEq, Eq)] | ||
| enum Reason { | ||
| UncalledMethod(String), | ||
| NonExistentMethod(String), | ||
| } | ||
|
|
||
| /// ## What it does | ||
| /// Checks for common mistakes when using mock objects. | ||
| /// | ||
| /// ## Why is this bad? | ||
| /// The `mock` module exposes an assertion API that can be used to verify that | ||
| /// mock objects undergo expected interactions. This rule checks for common | ||
| /// mistakes when using this API. | ||
| /// | ||
| /// For example, it checks for mock attribute accesses that should be replaced | ||
| /// with mock method calls. | ||
| /// | ||
| /// ## Example | ||
| /// ```python | ||
| /// my_mock.assert_called | ||
| /// ``` | ||
| /// | ||
| /// Use instead: | ||
| /// ```python | ||
| /// my_mock.assert_called() | ||
| /// ``` | ||
| #[violation] | ||
| pub struct InvalidMockAccess { | ||
| reason: Reason, | ||
| } | ||
|
|
||
| impl Violation for InvalidMockAccess { | ||
| #[derive_message_formats] | ||
| fn message(&self) -> String { | ||
| let InvalidMockAccess { reason } = self; | ||
| match reason { | ||
| Reason::UncalledMethod(name) => format!("Mock method should be called: `{name}`"), | ||
| Reason::NonExistentMethod(name) => format!("Non-existent mock method: `{name}`"), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// PGH005 | ||
| pub fn uncalled_mock_method(checker: &mut Checker, expr: &Expr) { | ||
| if let ExprKind::Attribute { attr, .. } = &expr.node { | ||
| if matches!( | ||
| attr.as_str(), | ||
| "assert_any_call" | ||
| | "assert_called" | ||
| | "assert_called_once" | ||
| | "assert_called_once_with" | ||
| | "assert_called_with" | ||
| | "assert_has_calls" | ||
| | "assert_not_called" | ||
| ) { | ||
| checker.diagnostics.push(Diagnostic::new( | ||
| InvalidMockAccess { | ||
| reason: Reason::UncalledMethod(attr.to_string()), | ||
| }, | ||
| expr.range(), | ||
| )); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// PGH005 | ||
| pub fn non_existent_mock_method(checker: &mut Checker, test: &Expr) { | ||
| let attr = match &test.node { | ||
| ExprKind::Attribute { attr, .. } => attr, | ||
| ExprKind::Call { func, .. } => match &func.node { | ||
| ExprKind::Attribute { attr, .. } => attr, | ||
| _ => return, | ||
| }, | ||
| _ => return, | ||
| }; | ||
| if matches!( | ||
| attr.as_str(), | ||
| "any_call" | ||
| | "called_once" | ||
| | "called_once_with" | ||
| | "called_with" | ||
| | "has_calls" | ||
| | "not_called" | ||
| ) { | ||
| checker.diagnostics.push(Diagnostic::new( | ||
| InvalidMockAccess { | ||
| reason: Reason::NonExistentMethod(attr.to_string()), | ||
| }, | ||
| test.range(), | ||
| )); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,13 @@ | ||
| pub(crate) use blanket_noqa::{blanket_noqa, BlanketNOQA}; | ||
| pub(crate) use blanket_type_ignore::{blanket_type_ignore, BlanketTypeIgnore}; | ||
| pub use deprecated_log_warn::{deprecated_log_warn, DeprecatedLogWarn}; | ||
| pub use no_eval::{no_eval, Eval}; | ||
| pub(crate) use deprecated_log_warn::{deprecated_log_warn, DeprecatedLogWarn}; | ||
| pub(crate) use invalid_mock_access::{ | ||
| non_existent_mock_method, uncalled_mock_method, InvalidMockAccess, | ||
| }; | ||
| pub(crate) use no_eval::{no_eval, Eval}; | ||
|
|
||
| mod blanket_noqa; | ||
| mod blanket_type_ignore; | ||
| mod deprecated_log_warn; | ||
| mod invalid_mock_access; | ||
| mod no_eval; |
92 changes: 92 additions & 0 deletions
92
...rc/rules/pygrep_hooks/snapshots/ruff__rules__pygrep_hooks__tests__PGH005_PGH005_0.py.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| --- | ||
| source: crates/ruff/src/rules/pygrep_hooks/mod.rs | ||
| --- | ||
| PGH005_0.py:2:8: PGH005 Non-existent mock method: `not_called` | ||
| | | ||
| 2 | # Errors | ||
| 3 | assert my_mock.not_called() | ||
| | ^^^^^^^^^^^^^^^^^^^^ PGH005 | ||
| 4 | assert my_mock.called_once_with() | ||
| 5 | assert my_mock.not_called | ||
| | | ||
|
|
||
| PGH005_0.py:3:8: PGH005 Non-existent mock method: `called_once_with` | ||
| | | ||
| 3 | # Errors | ||
| 4 | assert my_mock.not_called() | ||
| 5 | assert my_mock.called_once_with() | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^ PGH005 | ||
| 6 | assert my_mock.not_called | ||
| 7 | assert my_mock.called_once_with | ||
| | | ||
|
|
||
| PGH005_0.py:4:8: PGH005 Non-existent mock method: `not_called` | ||
| | | ||
| 4 | assert my_mock.not_called() | ||
| 5 | assert my_mock.called_once_with() | ||
| 6 | assert my_mock.not_called | ||
| | ^^^^^^^^^^^^^^^^^^ PGH005 | ||
| 7 | assert my_mock.called_once_with | ||
| 8 | my_mock.assert_not_called | ||
| | | ||
|
|
||
| PGH005_0.py:5:8: PGH005 Non-existent mock method: `called_once_with` | ||
| | | ||
| 5 | assert my_mock.called_once_with() | ||
| 6 | assert my_mock.not_called | ||
| 7 | assert my_mock.called_once_with | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^ PGH005 | ||
| 8 | my_mock.assert_not_called | ||
| 9 | my_mock.assert_called | ||
| | | ||
|
|
||
| PGH005_0.py:6:1: PGH005 Mock method should be called: `assert_not_called` | ||
| | | ||
| 6 | assert my_mock.not_called | ||
| 7 | assert my_mock.called_once_with | ||
| 8 | my_mock.assert_not_called | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^ PGH005 | ||
| 9 | my_mock.assert_called | ||
| 10 | my_mock.assert_called_once_with | ||
| | | ||
|
|
||
| PGH005_0.py:7:1: PGH005 Mock method should be called: `assert_called` | ||
| | | ||
| 7 | assert my_mock.called_once_with | ||
| 8 | my_mock.assert_not_called | ||
| 9 | my_mock.assert_called | ||
| | ^^^^^^^^^^^^^^^^^^^^^ PGH005 | ||
| 10 | my_mock.assert_called_once_with | ||
| 11 | my_mock.assert_called_once_with | ||
| | | ||
|
|
||
| PGH005_0.py:8:1: PGH005 Mock method should be called: `assert_called_once_with` | ||
| | | ||
| 8 | my_mock.assert_not_called | ||
| 9 | my_mock.assert_called | ||
| 10 | my_mock.assert_called_once_with | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PGH005 | ||
| 11 | my_mock.assert_called_once_with | ||
| 12 | MyMock.assert_called_once_with | ||
| | | ||
|
|
||
| PGH005_0.py:9:1: PGH005 Mock method should be called: `assert_called_once_with` | ||
| | | ||
| 9 | my_mock.assert_called | ||
| 10 | my_mock.assert_called_once_with | ||
| 11 | my_mock.assert_called_once_with | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PGH005 | ||
| 12 | MyMock.assert_called_once_with | ||
| | | ||
|
|
||
| PGH005_0.py:10:1: PGH005 Mock method should be called: `assert_called_once_with` | ||
| | | ||
| 10 | my_mock.assert_called_once_with | ||
| 11 | my_mock.assert_called_once_with | ||
| 12 | MyMock.assert_called_once_with | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PGH005 | ||
| 13 | | ||
| 14 | # OK | ||
| | | ||
|
|
||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.