From 84531d16446f363a8e1c45f492567b743565e787 Mon Sep 17 00:00:00 2001 From: Nicolas Jeker Date: Tue, 21 May 2024 21:12:18 +0200 Subject: [PATCH] Clarify motivation for E713 and E714 (#11483) 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 --- .../src/rules/pycodestyle/rules/not_tests.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/not_tests.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/not_tests.rs index bcd0dc3a33..6990d66f76 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/not_tests.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/not_tests.rs @@ -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 @@ -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 @@ -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;