From 3bd7fd2631a070f23ae35b311ab5f30c40db0fc6 Mon Sep 17 00:00:00 2001 From: snowdrop4 <82846066+snowdrop4@users.noreply.github.com> Date: Mon, 25 Nov 2024 14:39:04 +0000 Subject: [PATCH 01/11] reduce false positives with single-letter names --- .../test/fixtures/pep8_naming/N811.py | 5 ++++ .../test/fixtures/pep8_naming/N814.py | 5 ++++ .../rules/camelcase_imported_as_constant.rs | 5 ++++ .../constant_imported_as_non_constant.rs | 5 ++++ ...les__pep8_naming__tests__N811_N811.py.snap | 28 +++++++++++-------- ...les__pep8_naming__tests__N814_N814.py.snap | 28 +++++++++++-------- 6 files changed, 52 insertions(+), 24 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py index 0ef646edc9461..bf62e2de1840e 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py @@ -1,3 +1,8 @@ +# Error import mod.CONST as const from mod import CONSTANT as constant from mod import ANOTHER_CONSTANT as another_constant + +# Ok +import django.db.models.Q as Query1 +from django.db.models import Q as Query2 diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py index 65a03383a5a28..82f0971c4ac7e 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py @@ -1,3 +1,8 @@ +# Error import mod.Camel as CAMEL from mod import CamelCase as CAMELCASE from mod import AnotherCamelCase as ANOTHER_CAMELCASE + +# Ok +import mod.AppleFruit as A +from mod import BananaFruit as B diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs index f805bd2915e91..e41ac229c4044 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs @@ -53,6 +53,11 @@ pub(crate) fn camelcase_imported_as_constant( stmt: &Stmt, ignore_names: &IgnoreNames, ) -> Option { + // Single-character names are ambiguous. It could be a class or a constant. + if asname.chars().count() == 1 { + return None; + } + if helpers::is_camelcase(name) && !str::is_cased_lowercase(asname) && str::is_cased_uppercase(asname) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs index 0f81043f60200..f71c5abd634f3 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs @@ -52,6 +52,11 @@ pub(crate) fn constant_imported_as_non_constant( stmt: &Stmt, ignore_names: &IgnoreNames, ) -> Option { + // Single-character names are ambiguous. It could be a class or a constant. + if name.chars().count() == 1 { + return None; + } + if str::is_cased_uppercase(name) && !str::is_cased_uppercase(asname) { // Ignore any explicitly-allowed names. if ignore_names.matches(name) || ignore_names.matches(asname) { diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap index fce6f320a792a..694ae6022ea9c 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap @@ -2,26 +2,30 @@ source: crates/ruff_linter/src/rules/pep8_naming/mod.rs snapshot_kind: text --- -N811.py:1:8: N811 Constant `CONST` imported as non-constant `const` +N811.py:2:8: N811 Constant `CONST` imported as non-constant `const` | -1 | import mod.CONST as const +1 | # Error +2 | import mod.CONST as const | ^^^^^^^^^^^^^^^^^^ N811 -2 | from mod import CONSTANT as constant -3 | from mod import ANOTHER_CONSTANT as another_constant +3 | from mod import CONSTANT as constant +4 | from mod import ANOTHER_CONSTANT as another_constant | -N811.py:2:17: N811 Constant `CONSTANT` imported as non-constant `constant` +N811.py:3:17: N811 Constant `CONSTANT` imported as non-constant `constant` | -1 | import mod.CONST as const -2 | from mod import CONSTANT as constant +1 | # Error +2 | import mod.CONST as const +3 | from mod import CONSTANT as constant | ^^^^^^^^^^^^^^^^^^^^ N811 -3 | from mod import ANOTHER_CONSTANT as another_constant +4 | from mod import ANOTHER_CONSTANT as another_constant | -N811.py:3:17: N811 Constant `ANOTHER_CONSTANT` imported as non-constant `another_constant` +N811.py:4:17: N811 Constant `ANOTHER_CONSTANT` imported as non-constant `another_constant` | -1 | import mod.CONST as const -2 | from mod import CONSTANT as constant -3 | from mod import ANOTHER_CONSTANT as another_constant +2 | import mod.CONST as const +3 | from mod import CONSTANT as constant +4 | from mod import ANOTHER_CONSTANT as another_constant | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ N811 +5 | +6 | # Ok | diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap index d14b3e35951d6..37d05d2ca626b 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap @@ -2,26 +2,30 @@ source: crates/ruff_linter/src/rules/pep8_naming/mod.rs snapshot_kind: text --- -N814.py:1:8: N814 Camelcase `Camel` imported as constant `CAMEL` +N814.py:2:8: N814 Camelcase `Camel` imported as constant `CAMEL` | -1 | import mod.Camel as CAMEL +1 | # Error +2 | import mod.Camel as CAMEL | ^^^^^^^^^^^^^^^^^^ N814 -2 | from mod import CamelCase as CAMELCASE -3 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE +3 | from mod import CamelCase as CAMELCASE +4 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE | -N814.py:2:17: N814 Camelcase `CamelCase` imported as constant `CAMELCASE` +N814.py:3:17: N814 Camelcase `CamelCase` imported as constant `CAMELCASE` | -1 | import mod.Camel as CAMEL -2 | from mod import CamelCase as CAMELCASE +1 | # Error +2 | import mod.Camel as CAMEL +3 | from mod import CamelCase as CAMELCASE | ^^^^^^^^^^^^^^^^^^^^^^ N814 -3 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE +4 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE | -N814.py:3:17: N814 Camelcase `AnotherCamelCase` imported as constant `ANOTHER_CAMELCASE` +N814.py:4:17: N814 Camelcase `AnotherCamelCase` imported as constant `ANOTHER_CAMELCASE` | -1 | import mod.Camel as CAMEL -2 | from mod import CamelCase as CAMELCASE -3 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE +2 | import mod.Camel as CAMEL +3 | from mod import CamelCase as CAMELCASE +4 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ N814 +5 | +6 | # Ok | From 3da4c284f93770d299849f8dd3579865eb4b800e Mon Sep 17 00:00:00 2001 From: snowdrop4 <82846066+snowdrop4@users.noreply.github.com> Date: Mon, 25 Nov 2024 23:06:15 +0000 Subject: [PATCH 02/11] don't rely on compiler optimisation --- .../rules/pep8_naming/rules/camelcase_imported_as_constant.rs | 4 +--- .../pep8_naming/rules/constant_imported_as_non_constant.rs | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs index e41ac229c4044..64e17bca616d6 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs @@ -54,9 +54,7 @@ pub(crate) fn camelcase_imported_as_constant( ignore_names: &IgnoreNames, ) -> Option { // Single-character names are ambiguous. It could be a class or a constant. - if asname.chars().count() == 1 { - return None; - } + asname.chars().skip(1).next()?; if helpers::is_camelcase(name) && !str::is_cased_lowercase(asname) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs index f71c5abd634f3..132a873c857ba 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs @@ -53,9 +53,7 @@ pub(crate) fn constant_imported_as_non_constant( ignore_names: &IgnoreNames, ) -> Option { // Single-character names are ambiguous. It could be a class or a constant. - if name.chars().count() == 1 { - return None; - } + name.chars().skip(1).next()?; if str::is_cased_uppercase(name) && !str::is_cased_uppercase(asname) { // Ignore any explicitly-allowed names. From b56d9334b12d36271e5d1af614e8ac18eba5ab55 Mon Sep 17 00:00:00 2001 From: snowdrop4 <82846066+snowdrop4@users.noreply.github.com> Date: Mon, 25 Nov 2024 23:35:58 +0000 Subject: [PATCH 03/11] add one more test case --- .../test/fixtures/pep8_naming/N811.py | 1 + ...les__pep8_naming__tests__N811_N811.py.snap | 40 ++++++++++++------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py index bf62e2de1840e..e67e3afd9e8e0 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py @@ -1,4 +1,5 @@ # Error +import mod.CON as c import mod.CONST as const from mod import CONSTANT as constant from mod import ANOTHER_CONSTANT as another_constant diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap index 694ae6022ea9c..0dcde0cfd21e2 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap @@ -2,30 +2,40 @@ source: crates/ruff_linter/src/rules/pep8_naming/mod.rs snapshot_kind: text --- -N811.py:2:8: N811 Constant `CONST` imported as non-constant `const` +N811.py:2:8: N811 Constant `CON` imported as non-constant `c` | 1 | # Error -2 | import mod.CONST as const - | ^^^^^^^^^^^^^^^^^^ N811 -3 | from mod import CONSTANT as constant -4 | from mod import ANOTHER_CONSTANT as another_constant +2 | import mod.CON as c + | ^^^^^^^^^^^^ N811 +3 | import mod.CONST as const +4 | from mod import CONSTANT as constant | -N811.py:3:17: N811 Constant `CONSTANT` imported as non-constant `constant` +N811.py:3:8: N811 Constant `CONST` imported as non-constant `const` | 1 | # Error -2 | import mod.CONST as const -3 | from mod import CONSTANT as constant +2 | import mod.CON as c +3 | import mod.CONST as const + | ^^^^^^^^^^^^^^^^^^ N811 +4 | from mod import CONSTANT as constant +5 | from mod import ANOTHER_CONSTANT as another_constant + | + +N811.py:4:17: N811 Constant `CONSTANT` imported as non-constant `constant` + | +2 | import mod.CON as c +3 | import mod.CONST as const +4 | from mod import CONSTANT as constant | ^^^^^^^^^^^^^^^^^^^^ N811 -4 | from mod import ANOTHER_CONSTANT as another_constant +5 | from mod import ANOTHER_CONSTANT as another_constant | -N811.py:4:17: N811 Constant `ANOTHER_CONSTANT` imported as non-constant `another_constant` +N811.py:5:17: N811 Constant `ANOTHER_CONSTANT` imported as non-constant `another_constant` | -2 | import mod.CONST as const -3 | from mod import CONSTANT as constant -4 | from mod import ANOTHER_CONSTANT as another_constant +3 | import mod.CONST as const +4 | from mod import CONSTANT as constant +5 | from mod import ANOTHER_CONSTANT as another_constant | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ N811 -5 | -6 | # Ok +6 | +7 | # Ok | From 9396f38e60c0d4285d1ee8d6197ea087c6067e4a Mon Sep 17 00:00:00 2001 From: snowdrop4 <82846066+snowdrop4@users.noreply.github.com> Date: Mon, 25 Nov 2024 23:36:09 +0000 Subject: [PATCH 04/11] add documentation about the ambiguity --- .../rules/camelcase_imported_as_constant.rs | 14 ++++++++++++++ .../rules/constant_imported_as_non_constant.rs | 13 +++++++++++++ 2 files changed, 27 insertions(+) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs index 64e17bca616d6..9257ae632166f 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs @@ -30,6 +30,20 @@ use crate::rules::pep8_naming::settings::IgnoreNames; /// from example import MyClassName /// ``` /// +/// ## Note +/// Identifiers consisting of a single uppercase character are ambigous under +/// the rules of PEP8, which specifies PascalCase for classes and +/// ALL_CAPS_SNAKE_CASE for constants. Without a second character, it is not +/// possible to reliably guess whether the identifier is intended to be part +/// of a PascalCase string for a class or an ALL_CAPS_SNAKE_CASE string for +/// a constant, since both conventions will produce the same output when given +/// a single input character. Therefore, this lint rule does not apply to cases +/// where the alias for the imported identifier consists of a single uppercase +/// character. +/// +/// A common example of a single uppercase character being used for a class +/// name can be found in Django's `django.db.models.Q` class. +/// /// [PEP 8]: https://peps.python.org/pep-0008/ #[violation] pub struct CamelcaseImportedAsConstant { diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs index 132a873c857ba..0bf33f6681e0a 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs @@ -29,6 +29,19 @@ use crate::rules::pep8_naming::settings::IgnoreNames; /// from example import CONSTANT_VALUE /// ``` /// +/// ## Note +/// Identifiers consisting of a single uppercase character are ambigous under +/// the rules of PEP8, which specifies PascalCase for classes and +/// ALL_CAPS_SNAKE_CASE for constants. Without a second character, it is not +/// possible to reliably guess whether the identifier is intended to be part +/// of a PascalCase string for a class or an ALL_CAPS_SNAKE_CASE string for +/// a constant, since both conventions will produce the same output when given +/// a single input character. Therefore, this lint rule does not apply to cases +/// where the imported identifier consists of a single uppercase character. +/// +/// A common example of a single uppercase character being used for a class +/// name can be found in Django's `django.db.models.Q` class. +/// /// [PEP 8]: https://peps.python.org/pep-0008/ #[violation] pub struct ConstantImportedAsNonConstant { From c811971d7a1e03a88ca624133dcf0a031310997f Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 26 Nov 2024 09:08:39 +0100 Subject: [PATCH 05/11] Clippy and typo --- .../rules/pep8_naming/rules/camelcase_imported_as_constant.rs | 4 ++-- .../pep8_naming/rules/constant_imported_as_non_constant.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs index 9257ae632166f..017dcd120481f 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs @@ -31,7 +31,7 @@ use crate::rules::pep8_naming::settings::IgnoreNames; /// ``` /// /// ## Note -/// Identifiers consisting of a single uppercase character are ambigous under +/// Identifiers consisting of a single uppercase character are ambiguous under /// the rules of PEP8, which specifies PascalCase for classes and /// ALL_CAPS_SNAKE_CASE for constants. Without a second character, it is not /// possible to reliably guess whether the identifier is intended to be part @@ -68,7 +68,7 @@ pub(crate) fn camelcase_imported_as_constant( ignore_names: &IgnoreNames, ) -> Option { // Single-character names are ambiguous. It could be a class or a constant. - asname.chars().skip(1).next()?; + asname.chars().nth(1)?; if helpers::is_camelcase(name) && !str::is_cased_lowercase(asname) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs index 0bf33f6681e0a..aeeb166ff6acf 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs @@ -30,7 +30,7 @@ use crate::rules::pep8_naming::settings::IgnoreNames; /// ``` /// /// ## Note -/// Identifiers consisting of a single uppercase character are ambigous under +/// Identifiers consisting of a single uppercase character are ambiguous under /// the rules of PEP8, which specifies PascalCase for classes and /// ALL_CAPS_SNAKE_CASE for constants. Without a second character, it is not /// possible to reliably guess whether the identifier is intended to be part @@ -66,7 +66,7 @@ pub(crate) fn constant_imported_as_non_constant( ignore_names: &IgnoreNames, ) -> Option { // Single-character names are ambiguous. It could be a class or a constant. - name.chars().skip(1).next()?; + name.chars().nth(1)?; if str::is_cased_uppercase(name) && !str::is_cased_uppercase(asname) { // Ignore any explicitly-allowed names. From 1732d6b4e448ce76745b739a6206af846bfcd834 Mon Sep 17 00:00:00 2001 From: snowdrop4 <82846066+snowdrop4@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:57:44 +0000 Subject: [PATCH 06/11] remove comments from test files --- crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py | 2 -- crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py | 2 -- 2 files changed, 4 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py index e67e3afd9e8e0..f71ae67dee81e 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py @@ -1,9 +1,7 @@ -# Error import mod.CON as c import mod.CONST as const from mod import CONSTANT as constant from mod import ANOTHER_CONSTANT as another_constant -# Ok import django.db.models.Q as Query1 from django.db.models import Q as Query2 diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py index 82f0971c4ac7e..0d47a4d0347fb 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py @@ -1,8 +1,6 @@ -# Error import mod.Camel as CAMEL from mod import CamelCase as CAMELCASE from mod import AnotherCamelCase as ANOTHER_CAMELCASE -# Ok import mod.AppleFruit as A from mod import BananaFruit as B From cfc70bb152cd289a55ff637061aabf4b82674e75 Mon Sep 17 00:00:00 2001 From: snowdrop4 <82846066+snowdrop4@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:58:46 +0000 Subject: [PATCH 07/11] change ordering of test file --- crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py index f71ae67dee81e..859edcc29b323 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py @@ -1,7 +1,7 @@ -import mod.CON as c import mod.CONST as const from mod import CONSTANT as constant from mod import ANOTHER_CONSTANT as another_constant +import mod.CON as c import django.db.models.Q as Query1 from django.db.models import Q as Query2 From 3de5771e656430be123b76e83cb0ce1d47929fd0 Mon Sep 17 00:00:00 2001 From: snowdrop4 <82846066+snowdrop4@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:03:36 +0000 Subject: [PATCH 08/11] docs --- .../rules/pep8_naming/rules/camelcase_imported_as_constant.rs | 4 ++-- .../pep8_naming/rules/constant_imported_as_non_constant.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs index 017dcd120481f..49bda02d0ab8f 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs @@ -32,10 +32,10 @@ use crate::rules::pep8_naming::settings::IgnoreNames; /// /// ## Note /// Identifiers consisting of a single uppercase character are ambiguous under -/// the rules of PEP8, which specifies PascalCase for classes and +/// the rules of [PEP 8], which specifies CamelCase for classes and /// ALL_CAPS_SNAKE_CASE for constants. Without a second character, it is not /// possible to reliably guess whether the identifier is intended to be part -/// of a PascalCase string for a class or an ALL_CAPS_SNAKE_CASE string for +/// of a CamelCase string for a class or an ALL_CAPS_SNAKE_CASE string for /// a constant, since both conventions will produce the same output when given /// a single input character. Therefore, this lint rule does not apply to cases /// where the alias for the imported identifier consists of a single uppercase diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs index aeeb166ff6acf..e2dae630d19ea 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs @@ -31,10 +31,10 @@ use crate::rules::pep8_naming::settings::IgnoreNames; /// /// ## Note /// Identifiers consisting of a single uppercase character are ambiguous under -/// the rules of PEP8, which specifies PascalCase for classes and +/// the rules of [PEP 8], which specifies CamelCase for classes and /// ALL_CAPS_SNAKE_CASE for constants. Without a second character, it is not /// possible to reliably guess whether the identifier is intended to be part -/// of a PascalCase string for a class or an ALL_CAPS_SNAKE_CASE string for +/// of a CamelCase string for a class or an ALL_CAPS_SNAKE_CASE string for /// a constant, since both conventions will produce the same output when given /// a single input character. Therefore, this lint rule does not apply to cases /// where the imported identifier consists of a single uppercase character. From d261a0e04c4e39948519e7bccf3f911cee35e4c0 Mon Sep 17 00:00:00 2001 From: snowdrop4 <82846066+snowdrop4@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:05:34 +0000 Subject: [PATCH 09/11] new snaps --- ...les__pep8_naming__tests__N811_N811.py.snap | 50 +++++++++---------- ...les__pep8_naming__tests__N814_N814.py.snap | 30 ++++++----- 2 files changed, 38 insertions(+), 42 deletions(-) diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap index 0dcde0cfd21e2..f4df43bc5b953 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap @@ -2,40 +2,38 @@ source: crates/ruff_linter/src/rules/pep8_naming/mod.rs snapshot_kind: text --- -N811.py:2:8: N811 Constant `CON` imported as non-constant `c` +N811.py:1:8: N811 Constant `CONST` imported as non-constant `const` | -1 | # Error -2 | import mod.CON as c - | ^^^^^^^^^^^^ N811 -3 | import mod.CONST as const -4 | from mod import CONSTANT as constant - | - -N811.py:3:8: N811 Constant `CONST` imported as non-constant `const` - | -1 | # Error -2 | import mod.CON as c -3 | import mod.CONST as const +1 | import mod.CONST as const | ^^^^^^^^^^^^^^^^^^ N811 -4 | from mod import CONSTANT as constant -5 | from mod import ANOTHER_CONSTANT as another_constant +2 | from mod import CONSTANT as constant +3 | from mod import ANOTHER_CONSTANT as another_constant | -N811.py:4:17: N811 Constant `CONSTANT` imported as non-constant `constant` +N811.py:2:17: N811 Constant `CONSTANT` imported as non-constant `constant` | -2 | import mod.CON as c -3 | import mod.CONST as const -4 | from mod import CONSTANT as constant +1 | import mod.CONST as const +2 | from mod import CONSTANT as constant | ^^^^^^^^^^^^^^^^^^^^ N811 -5 | from mod import ANOTHER_CONSTANT as another_constant +3 | from mod import ANOTHER_CONSTANT as another_constant +4 | import mod.CON as c | -N811.py:5:17: N811 Constant `ANOTHER_CONSTANT` imported as non-constant `another_constant` +N811.py:3:17: N811 Constant `ANOTHER_CONSTANT` imported as non-constant `another_constant` | -3 | import mod.CONST as const -4 | from mod import CONSTANT as constant -5 | from mod import ANOTHER_CONSTANT as another_constant +1 | import mod.CONST as const +2 | from mod import CONSTANT as constant +3 | from mod import ANOTHER_CONSTANT as another_constant | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ N811 -6 | -7 | # Ok +4 | import mod.CON as c + | + +N811.py:4:8: N811 Constant `CON` imported as non-constant `c` + | +2 | from mod import CONSTANT as constant +3 | from mod import ANOTHER_CONSTANT as another_constant +4 | import mod.CON as c + | ^^^^^^^^^^^^ N811 +5 | +6 | import django.db.models.Q as Query1 | diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap index 37d05d2ca626b..df0d4f983f684 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap @@ -2,30 +2,28 @@ source: crates/ruff_linter/src/rules/pep8_naming/mod.rs snapshot_kind: text --- -N814.py:2:8: N814 Camelcase `Camel` imported as constant `CAMEL` +N814.py:1:8: N814 Camelcase `Camel` imported as constant `CAMEL` | -1 | # Error -2 | import mod.Camel as CAMEL +1 | import mod.Camel as CAMEL | ^^^^^^^^^^^^^^^^^^ N814 -3 | from mod import CamelCase as CAMELCASE -4 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE +2 | from mod import CamelCase as CAMELCASE +3 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE | -N814.py:3:17: N814 Camelcase `CamelCase` imported as constant `CAMELCASE` +N814.py:2:17: N814 Camelcase `CamelCase` imported as constant `CAMELCASE` | -1 | # Error -2 | import mod.Camel as CAMEL -3 | from mod import CamelCase as CAMELCASE +1 | import mod.Camel as CAMEL +2 | from mod import CamelCase as CAMELCASE | ^^^^^^^^^^^^^^^^^^^^^^ N814 -4 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE +3 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE | -N814.py:4:17: N814 Camelcase `AnotherCamelCase` imported as constant `ANOTHER_CAMELCASE` +N814.py:3:17: N814 Camelcase `AnotherCamelCase` imported as constant `ANOTHER_CAMELCASE` | -2 | import mod.Camel as CAMEL -3 | from mod import CamelCase as CAMELCASE -4 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE +1 | import mod.Camel as CAMEL +2 | from mod import CamelCase as CAMELCASE +3 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ N814 -5 | -6 | # Ok +4 | +5 | import mod.AppleFruit as A | From e9af7fac2a389fa43f12e220499a19784330210b Mon Sep 17 00:00:00 2001 From: snowdrop4 <82846066+snowdrop4@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:13:36 +0000 Subject: [PATCH 10/11] backticks in docs to appease clippy --- .../pep8_naming/rules/camelcase_imported_as_constant.rs | 6 +++--- .../pep8_naming/rules/constant_imported_as_non_constant.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs index 49bda02d0ab8f..e4fdce67bc931 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/camelcase_imported_as_constant.rs @@ -32,10 +32,10 @@ use crate::rules::pep8_naming::settings::IgnoreNames; /// /// ## Note /// Identifiers consisting of a single uppercase character are ambiguous under -/// the rules of [PEP 8], which specifies CamelCase for classes and -/// ALL_CAPS_SNAKE_CASE for constants. Without a second character, it is not +/// the rules of [PEP 8], which specifies `CamelCase` for classes and +/// `ALL_CAPS_SNAKE_CASE` for constants. Without a second character, it is not /// possible to reliably guess whether the identifier is intended to be part -/// of a CamelCase string for a class or an ALL_CAPS_SNAKE_CASE string for +/// of a `CamelCase` string for a class or an `ALL_CAPS_SNAKE_CASE` string for /// a constant, since both conventions will produce the same output when given /// a single input character. Therefore, this lint rule does not apply to cases /// where the alias for the imported identifier consists of a single uppercase diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs index e2dae630d19ea..92e6b135a6596 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs @@ -31,10 +31,10 @@ use crate::rules::pep8_naming::settings::IgnoreNames; /// /// ## Note /// Identifiers consisting of a single uppercase character are ambiguous under -/// the rules of [PEP 8], which specifies CamelCase for classes and -/// ALL_CAPS_SNAKE_CASE for constants. Without a second character, it is not +/// the rules of [PEP 8], which specifies `CamelCase` for classes and +/// `ALL_CAPS_SNAKE_CASE` for constants. Without a second character, it is not /// possible to reliably guess whether the identifier is intended to be part -/// of a CamelCase string for a class or an ALL_CAPS_SNAKE_CASE string for +/// of a `CamelCase` string for a class or an `ALL_CAPS_SNAKE_CASE` string for /// a constant, since both conventions will produce the same output when given /// a single input character. Therefore, this lint rule does not apply to cases /// where the imported identifier consists of a single uppercase character. From 7ae363ccd236e9cfff26bd514a60ccf728e02632 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 27 Nov 2024 14:20:12 +0000 Subject: [PATCH 11/11] Ensure N811 flags single-character constants imported as lowercase --- .../resources/test/fixtures/pep8_naming/N811.py | 2 ++ .../resources/test/fixtures/pep8_naming/N814.py | 1 + .../rules/constant_imported_as_non_constant.rs | 13 ++++++++----- ...__rules__pep8_naming__tests__N811_N811.py.snap | 15 ++++++++++++--- ...__rules__pep8_naming__tests__N814_N814.py.snap | 3 +-- 5 files changed, 24 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py index 859edcc29b323..c18b8f992d859 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N811.py @@ -2,6 +2,8 @@ from mod import CONSTANT as constant from mod import ANOTHER_CONSTANT as another_constant import mod.CON as c +from mod import C as c +# These are all OK: import django.db.models.Q as Query1 from django.db.models import Q as Query2 diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py index 0d47a4d0347fb..4a6809043000c 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N814.py @@ -2,5 +2,6 @@ from mod import CamelCase as CAMELCASE from mod import AnotherCamelCase as ANOTHER_CAMELCASE +# These are all OK: import mod.AppleFruit as A from mod import BananaFruit as B diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs index 92e6b135a6596..ef6197aa08f8a 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/constant_imported_as_non_constant.rs @@ -4,7 +4,7 @@ use ruff_python_ast::{Alias, Stmt}; use ruff_python_stdlib::str; use ruff_text_size::Ranged; -use crate::rules::pep8_naming::settings::IgnoreNames; +use crate::rules::pep8_naming::{helpers, settings::IgnoreNames}; /// ## What it does /// Checks for constant imports that are aliased to non-constant-style @@ -65,10 +65,13 @@ pub(crate) fn constant_imported_as_non_constant( stmt: &Stmt, ignore_names: &IgnoreNames, ) -> Option { - // Single-character names are ambiguous. It could be a class or a constant. - name.chars().nth(1)?; - - if str::is_cased_uppercase(name) && !str::is_cased_uppercase(asname) { + if str::is_cased_uppercase(name) + && !(str::is_cased_uppercase(asname) + // Single-character names are ambiguous. + // It could be a class or a constant, so allow it to be imported + // as `SCREAMING_SNAKE_CASE` *or* `CamelCase`. + || (name.chars().nth(1).is_none() && helpers::is_camelcase(asname))) + { // Ignore any explicitly-allowed names. if ignore_names.matches(name) || ignore_names.matches(asname) { return None; diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap index f4df43bc5b953..680195dbffe53 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N811_N811.py.snap @@ -1,6 +1,5 @@ --- source: crates/ruff_linter/src/rules/pep8_naming/mod.rs -snapshot_kind: text --- N811.py:1:8: N811 Constant `CONST` imported as non-constant `const` | @@ -26,6 +25,7 @@ N811.py:3:17: N811 Constant `ANOTHER_CONSTANT` imported as non-constant `another 3 | from mod import ANOTHER_CONSTANT as another_constant | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ N811 4 | import mod.CON as c +5 | from mod import C as c | N811.py:4:8: N811 Constant `CON` imported as non-constant `c` @@ -34,6 +34,15 @@ N811.py:4:8: N811 Constant `CON` imported as non-constant `c` 3 | from mod import ANOTHER_CONSTANT as another_constant 4 | import mod.CON as c | ^^^^^^^^^^^^ N811 -5 | -6 | import django.db.models.Q as Query1 +5 | from mod import C as c + | + +N811.py:5:17: N811 Constant `C` imported as non-constant `c` + | +3 | from mod import ANOTHER_CONSTANT as another_constant +4 | import mod.CON as c +5 | from mod import C as c + | ^^^^^^ N811 +6 | +7 | # These are all OK: | diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap index df0d4f983f684..57da8ddfaeed8 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N814_N814.py.snap @@ -1,6 +1,5 @@ --- source: crates/ruff_linter/src/rules/pep8_naming/mod.rs -snapshot_kind: text --- N814.py:1:8: N814 Camelcase `Camel` imported as constant `CAMEL` | @@ -25,5 +24,5 @@ N814.py:3:17: N814 Camelcase `AnotherCamelCase` imported as constant `ANOTHER_CA 3 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ N814 4 | -5 | import mod.AppleFruit as A +5 | # These are all OK: |