Skip to content

Commit

Permalink
[flake8-naming]: Respect import conventions (N817) (#12922)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser authored Aug 16, 2024
1 parent c319414 commit aba0d83
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
import mod.CaMel as CM
from mod import CamelCase as CC


# OK depending on configured import convention
import xml.etree.ElementTree as ET
8 changes: 2 additions & 6 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,11 +707,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}
if checker.enabled(Rule::CamelcaseImportedAsAcronym) {
if let Some(diagnostic) = pep8_naming::rules::camelcase_imported_as_acronym(
name,
asname,
alias,
stmt,
&checker.settings.pep8_naming.ignore_names,
name, asname, alias, stmt, checker,
) {
checker.diagnostics.push(diagnostic);
}
Expand Down Expand Up @@ -1026,7 +1022,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
asname,
alias,
stmt,
&checker.settings.pep8_naming.ignore_names,
checker,
) {
checker.diagnostics.push(diagnostic);
}
Expand Down
22 changes: 21 additions & 1 deletion crates/ruff_linter/src/rules/pep8_naming/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ mod tests {
use std::path::{Path, PathBuf};

use anyhow::Result;
use rustc_hash::FxHashMap;
use test_case::test_case;

use crate::registry::Rule;
use crate::rules::pep8_naming;
use crate::rules::pep8_naming::settings::IgnoreNames;
use crate::rules::{flake8_import_conventions, pep8_naming};
use crate::test::test_path;
use crate::{assert_messages, settings};

Expand Down Expand Up @@ -87,6 +88,25 @@ mod tests {
Ok(())
}

#[test]
fn camelcase_imported_as_incorrect_convention() -> Result<()> {
let diagnostics = test_path(
Path::new("pep8_naming").join("N817.py").as_path(),
&settings::LinterSettings {
flake8_import_conventions: flake8_import_conventions::settings::Settings {
aliases: FxHashMap::from_iter([(
"xml.etree.ElementTree".to_string(),
"XET".to_string(),
)]),
..Default::default()
},
..settings::LinterSettings::for_rule(Rule::CamelcaseImportedAsAcronym)
},
)?;
assert_messages!(diagnostics);
Ok(())
}

#[test]
fn classmethod_decorators() -> Result<()> {
let diagnostics = test_path(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use ruff_python_ast::{Alias, Stmt};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Alias, Stmt};
use ruff_python_stdlib::str::{self};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::rules::pep8_naming::helpers;
use crate::rules::pep8_naming::settings::IgnoreNames;

/// ## What it does
/// Checks for `CamelCase` imports that are aliased as acronyms.
Expand All @@ -23,6 +22,10 @@ use crate::rules::pep8_naming::settings::IgnoreNames;
/// Note that this rule is distinct from `camelcase-imported-as-constant`
/// to accommodate selective enforcement.
///
/// Also note that import aliases following an import convention according to the
/// [`lint.flake8-boolean-trap.extend-allowed-calls`] option are allowed.
///
///
/// ## Example
/// ```python
/// from example import MyClassName as MCN
Expand All @@ -34,6 +37,9 @@ use crate::rules::pep8_naming::settings::IgnoreNames;
/// ```
///
/// [PEP 8]: https://peps.python.org/pep-0008/
///
/// ## Options
/// - `lint.flake8-import-conventions.banned-aliases`
#[violation]
pub struct CamelcaseImportedAsAcronym {
name: String,
Expand All @@ -54,17 +60,32 @@ pub(crate) fn camelcase_imported_as_acronym(
asname: &str,
alias: &Alias,
stmt: &Stmt,
ignore_names: &IgnoreNames,
checker: &Checker,
) -> Option<Diagnostic> {
if helpers::is_camelcase(name)
&& !str::is_cased_lowercase(asname)
&& str::is_cased_uppercase(asname)
&& helpers::is_acronym(name, asname)
{
let ignore_names = &checker.settings.pep8_naming.ignore_names;

// Ignore any explicitly-allowed names.
if ignore_names.matches(name) || ignore_names.matches(asname) {
return None;
}

// Ignore names that follow a community-agreed import convention.
if checker
.settings
.flake8_import_conventions
.aliases
.get(&*alias.name)
.map(String::as_str)
== Some(asname)
{
return None;
}

let mut diagnostic = Diagnostic::new(
CamelcaseImportedAsAcronym {
name: name.to_string(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
source: crates/ruff_linter/src/rules/pep8_naming/mod.rs
---
N817.py:1:8: N817 CamelCase `CaMel` imported as acronym `CM`
|
1 | import mod.CaMel as CM
| ^^^^^^^^^^^^^^^ N817
2 | from mod import CamelCase as CC
|

N817.py:2:17: N817 CamelCase `CamelCase` imported as acronym `CC`
|
1 | import mod.CaMel as CM
2 | from mod import CamelCase as CC
| ^^^^^^^^^^^^^^^ N817
|

N817.py:6:8: N817 CamelCase `ElementTree` imported as acronym `ET`
|
5 | # OK depending on configured import convention
6 | import xml.etree.ElementTree as ET
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ N817
|

0 comments on commit aba0d83

Please sign in to comment.