Skip to content

Commit

Permalink
Clarify motivation for E713 and E714 (#11483)
Browse files Browse the repository at this point in the history
The wording 'negative comparison' is a rather vague description of the
'is not' operation and does not describe what the 'not in' operation
does (potentially copied from 'is not'). This was replaced with more
precise language to describe the operators taken from the official
python docs[1].

Both rules didn't have a strong reasoning besides 'it's bad, use the
other'. The origin of these rules seems to be PEP8[2] which prefers 'is
not' over 'not ... is' for readability. This is now reflected in the
description.

[1]:
https://docs.python.org/3/reference/expressions.html#membership-test-operations
[2]: https://peps.python.org/pep-0008/#programming-recommendations
  • Loading branch information
mnme authored May 21, 2024
1 parent 83b8b62 commit 84531d1
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions crates/ruff_linter/src/rules/pycodestyle/rules/not_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ use crate::checkers::ast::Checker;
use crate::registry::Rule;

/// ## What it does
/// Checks for negative comparison using `not {foo} in {bar}`.
/// Checks for membership tests using `not {element} in {collection}`.
///
/// ## Why is this bad?
/// Negative comparison should be done using `not in`.
/// Testing membership with `{element} not in {collection}` is more readable.
///
/// ## Example
/// ```python
Expand Down Expand Up @@ -42,10 +42,11 @@ impl AlwaysFixableViolation for NotInTest {
}

/// ## What it does
/// Checks for negative comparison using `not {foo} is {bar}`.
/// Checks for identity comparisons using `not {foo} is {bar}`.
///
/// ## Why is this bad?
/// Negative comparison should be done using `is not`.
/// According to [PEP8], testing for an object's identity with `is not` is more
/// readable.
///
/// ## Example
/// ```python
Expand All @@ -60,6 +61,8 @@ impl AlwaysFixableViolation for NotInTest {
/// pass
/// Z = X.B is not Y
/// ```
///
/// [PEP8]: https://peps.python.org/pep-0008/#programming-recommendations
#[violation]
pub struct NotIsTest;

Expand Down

0 comments on commit 84531d1

Please sign in to comment.