N811 & N814: eliminate false positives for single-letter names (#14584)

Co-authored-by: Micha Reiser <micha@reiser.io>
Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
This commit is contained in:
Alexandra Valentine-Ketchum 2024-11-27 14:38:36 +00:00 committed by GitHub
parent ef0e2a6e1b
commit c40b37aa36
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 72 additions and 4 deletions

View file

@ -1,3 +1,9 @@
import mod.CONST as const
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

View file

@ -1,3 +1,7 @@
import mod.Camel as CAMEL
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

View file

@ -30,6 +30,20 @@ use crate::rules::pep8_naming::settings::IgnoreNames;
/// from example import MyClassName
/// ```
///
/// ## 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
/// 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
/// 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/
#[derive(ViolationMetadata)]
pub(crate) struct CamelcaseImportedAsConstant {
@ -53,6 +67,9 @@ pub(crate) fn camelcase_imported_as_constant(
stmt: &Stmt,
ignore_names: &IgnoreNames,
) -> Option<Diagnostic> {
// Single-character names are ambiguous. It could be a class or a constant.
asname.chars().nth(1)?;
if helpers::is_camelcase(name)
&& !str::is_cased_lowercase(asname)
&& str::is_cased_uppercase(asname)

View file

@ -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
@ -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 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
/// 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
/// 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/
#[derive(ViolationMetadata)]
pub(crate) struct ConstantImportedAsNonConstant {
@ -52,7 +65,13 @@ pub(crate) fn constant_imported_as_non_constant(
stmt: &Stmt,
ignore_names: &IgnoreNames,
) -> Option<Diagnostic> {
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;

View file

@ -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`
|
@ -16,6 +15,7 @@ N811.py:2:17: N811 Constant `CONSTANT` imported as non-constant `constant`
2 | from mod import CONSTANT as constant
| ^^^^^^^^^^^^^^^^^^^^ N811
3 | from mod import ANOTHER_CONSTANT as another_constant
4 | import mod.CON as c
|
N811.py:3:17: N811 Constant `ANOTHER_CONSTANT` imported as non-constant `another_constant`
@ -24,4 +24,25 @@ N811.py:3:17: N811 Constant `ANOTHER_CONSTANT` imported as non-constant `another
2 | from mod import CONSTANT as constant
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`
|
2 | from mod import CONSTANT as constant
3 | from mod import ANOTHER_CONSTANT as another_constant
4 | import mod.CON as c
| ^^^^^^^^^^^^ N811
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:
|

View file

@ -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`
|
@ -24,4 +23,6 @@ N814.py:3:17: N814 Camelcase `AnotherCamelCase` imported as constant `ANOTHER_CA
2 | from mod import CamelCase as CAMELCASE
3 | from mod import AnotherCamelCase as ANOTHER_CAMELCASE
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ N814
4 |
5 | # These are all OK:
|