Skip to content

Commit

Permalink
Consider ignore-names in all pep8 naming rules (#5079)
Browse files Browse the repository at this point in the history
## Summary

This changes all remaining pep8 naming rules to consider the
`ingore-names` argument.

Closes #5050

## Test Plan

Added new tests.
  • Loading branch information
Thomas de Zeeuw committed Jun 14, 2023
1 parent 6f10aee commit e7316c1
Show file tree
Hide file tree
Showing 32 changed files with 400 additions and 12 deletions.
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ fail_fast: true
exclude: |
(?x)^(
crates/ruff/resources/.*|
crates/ruff/src/rules/.*/snapshots/.*|
crates/ruff_python_formatter/resources/.*|
crates/ruff_python_formatter/src/snapshots/.*
)$
Expand Down
1 change: 1 addition & 0 deletions _typos.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ whos = "whos"
spawnve = "spawnve"
ned = "ned"
poit = "poit"
BA = "BA" # acronym for "Bad Allowed", used in testing.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class badAllowed:
pass

class stillBad:
pass

class BAD_ALLOWED:
pass

class STILL_BAD:
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
def __badAllowed__():
pass

def __stillBad__():
pass


def nested():
def __badAllowed__():
pass

def __stillBad__():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import mod.BAD_ALLOWED as badAllowed
import mod.STILL_BAD as stillBad

from mod import BAD_ALLOWED as badAllowed
from mod import STILL_BAD as stillBad
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import mod.badallowed as badAllowed
import mod.stillbad as stillBad

from mod import badallowed as BadAllowed
from mod import stillbad as StillBad
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import mod.BadAllowed as badallowed
import mod.stillBad as stillbad

from mod import BadAllowed as badallowed
from mod import StillBad as stillbad

from mod import BadAllowed as bad_allowed
from mod import StillBad as still_bad
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import mod.BadAllowed as BADALLOWED
import mod.StillBad as STILLBAD

from mod import BadAllowed as BADALLOWED
from mod import StillBad as STILLBAD

from mod import BadAllowed as BAD_ALLOWED
from mod import StillBad as STILL_BAD
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import mod.BadAllowed as BA
import mod.StillBad as SB

from mod import BadAllowed as BA
from mod import StillBad as SB
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class BadAllowed(Exception):
pass

class StillBad(Exception):
pass

class BadAllowed(AnotherError):
pass

class StillBad(AnotherError):
pass
Empty file.
46 changes: 38 additions & 8 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ where
self.semantic_model.scope(),
stmt,
name,
&self.settings.pep8_naming.ignore_names,
self.locator,
) {
self.diagnostics.push(diagnostic);
Expand Down Expand Up @@ -703,9 +704,12 @@ where
}
}
if self.enabled(Rule::InvalidClassName) {
if let Some(diagnostic) =
pep8_naming::rules::invalid_class_name(stmt, name, self.locator)
{
if let Some(diagnostic) = pep8_naming::rules::invalid_class_name(
stmt,
name,
&self.settings.pep8_naming.ignore_names,
self.locator,
) {
self.diagnostics.push(diagnostic);
}
}
Expand All @@ -715,6 +719,7 @@ where
bases,
name,
self.locator,
&self.settings.pep8_naming.ignore_names,
) {
self.diagnostics.push(diagnostic);
}
Expand Down Expand Up @@ -898,7 +903,11 @@ where
if self.enabled(Rule::ConstantImportedAsNonConstant) {
if let Some(diagnostic) =
pep8_naming::rules::constant_imported_as_non_constant(
name, asname, alias, stmt,
name,
asname,
alias,
stmt,
&self.settings.pep8_naming.ignore_names,
)
{
self.diagnostics.push(diagnostic);
Expand All @@ -908,7 +917,11 @@ where
if self.enabled(Rule::LowercaseImportedAsNonLowercase) {
if let Some(diagnostic) =
pep8_naming::rules::lowercase_imported_as_non_lowercase(
name, asname, alias, stmt,
name,
asname,
alias,
stmt,
&self.settings.pep8_naming.ignore_names,
)
{
self.diagnostics.push(diagnostic);
Expand All @@ -918,7 +931,11 @@ where
if self.enabled(Rule::CamelcaseImportedAsLowercase) {
if let Some(diagnostic) =
pep8_naming::rules::camelcase_imported_as_lowercase(
name, asname, alias, stmt,
name,
asname,
alias,
stmt,
&self.settings.pep8_naming.ignore_names,
)
{
self.diagnostics.push(diagnostic);
Expand All @@ -928,7 +945,11 @@ where
if self.enabled(Rule::CamelcaseImportedAsConstant) {
if let Some(diagnostic) =
pep8_naming::rules::camelcase_imported_as_constant(
name, asname, alias, stmt,
name,
asname,
alias,
stmt,
&self.settings.pep8_naming.ignore_names,
)
{
self.diagnostics.push(diagnostic);
Expand All @@ -938,7 +959,11 @@ where
if self.enabled(Rule::CamelcaseImportedAsAcronym) {
if let Some(diagnostic) =
pep8_naming::rules::camelcase_imported_as_acronym(
name, asname, alias, stmt,
name,
asname,
alias,
stmt,
&self.settings.pep8_naming.ignore_names,
)
{
self.diagnostics.push(diagnostic);
Expand Down Expand Up @@ -1197,6 +1222,7 @@ where
asname,
alias,
stmt,
&self.settings.pep8_naming.ignore_names,
)
{
self.diagnostics.push(diagnostic);
Expand All @@ -1210,6 +1236,7 @@ where
asname,
alias,
stmt,
&self.settings.pep8_naming.ignore_names,
)
{
self.diagnostics.push(diagnostic);
Expand All @@ -1223,6 +1250,7 @@ where
asname,
alias,
stmt,
&self.settings.pep8_naming.ignore_names,
)
{
self.diagnostics.push(diagnostic);
Expand All @@ -1236,6 +1264,7 @@ where
asname,
alias,
stmt,
&self.settings.pep8_naming.ignore_names,
)
{
self.diagnostics.push(diagnostic);
Expand All @@ -1249,6 +1278,7 @@ where
asname,
alias,
stmt,
&self.settings.pep8_naming.ignore_names,
)
{
self.diagnostics.push(diagnostic);
Expand Down
4 changes: 3 additions & 1 deletion crates/ruff/src/checkers/filesystem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ pub(crate) fn check_file_path(

// pep8-naming
if settings.rules.enabled(Rule::InvalidModuleName) {
if let Some(diagnostic) = invalid_module_name(path, package) {
if let Some(diagnostic) =
invalid_module_name(path, package, &settings.pep8_naming.ignore_names)
{
diagnostics.push(diagnostic);
}
}
Expand Down
15 changes: 13 additions & 2 deletions crates/ruff/src/rules/pep8_naming/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,22 +106,33 @@ mod tests {
Ok(())
}

#[test_case(Rule::InvalidClassName, "N801.py")]
#[test_case(Rule::InvalidFunctionName, "N802.py")]
#[test_case(Rule::InvalidArgumentName, "N803.py")]
#[test_case(Rule::InvalidFirstArgumentNameForClassMethod, "N804.py")]
#[test_case(Rule::InvalidFirstArgumentNameForMethod, "N805.py")]
#[test_case(Rule::NonLowercaseVariableInFunction, "N806.py")]
#[test_case(Rule::DunderFunctionName, "N807.py")]
#[test_case(Rule::ConstantImportedAsNonConstant, "N811.py")]
#[test_case(Rule::LowercaseImportedAsNonLowercase, "N812.py")]
#[test_case(Rule::CamelcaseImportedAsLowercase, "N813.py")]
#[test_case(Rule::CamelcaseImportedAsConstant, "N814.py")]
#[test_case(Rule::MixedCaseVariableInClassScope, "N815.py")]
#[test_case(Rule::MixedCaseVariableInGlobalScope, "N816.py")]
#[test_case(Rule::CamelcaseImportedAsAcronym, "N817.py")]
#[test_case(Rule::ErrorSuffixOnExceptionName, "N818.py")]
#[test_case(Rule::InvalidModuleName, "N999/badAllowed/__init__.py")]
fn ignore_names(rule_code: Rule, path: &str) -> Result<()> {
let snapshot = format!("ignore_names_{}_{path}", rule_code.noqa_code());
let diagnostics = test_path(
PathBuf::from_iter(["pep8_naming", "ignore_names", path]).as_path(),
&settings::Settings {
pep8_naming: pep8_naming::settings::Settings {
ignore_names: vec![
IdentifierPattern::new("*Allowed").unwrap(),
IdentifierPattern::new("*ALLOWED").unwrap(),
IdentifierPattern::new("*allowed*").unwrap(),
IdentifierPattern::new("*Allowed*").unwrap(),
IdentifierPattern::new("*ALLOWED*").unwrap(),
IdentifierPattern::new("BA").unwrap(), // For N817.
],
..Default::default()
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_stdlib::str::{self};

use crate::rules::pep8_naming::helpers;
use crate::settings::types::IdentifierPattern;

/// ## What it does
/// Checks for `CamelCase` imports that are aliased as acronyms.
Expand Down Expand Up @@ -52,7 +53,15 @@ pub(crate) fn camelcase_imported_as_acronym(
asname: &str,
alias: &Alias,
stmt: &Stmt,
ignore_names: &[IdentifierPattern],
) -> Option<Diagnostic> {
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(asname))
{
return None;
}

if helpers::is_camelcase(name)
&& !str::is_lower(asname)
&& str::is_upper(asname)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_stdlib::str::{self};

use crate::rules::pep8_naming::helpers;
use crate::settings::types::IdentifierPattern;

/// ## What it does
/// Checks for `CamelCase` imports that are aliased to constant-style names.
Expand Down Expand Up @@ -49,7 +50,15 @@ pub(crate) fn camelcase_imported_as_constant(
asname: &str,
alias: &Alias,
stmt: &Stmt,
ignore_names: &[IdentifierPattern],
) -> Option<Diagnostic> {
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return None;
}

if helpers::is_camelcase(name)
&& !str::is_lower(asname)
&& str::is_upper(asname)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

use crate::rules::pep8_naming::helpers;
use crate::settings::types::IdentifierPattern;

/// ## What it does
/// Checks for `CamelCase` imports that are aliased to lowercase names.
Expand Down Expand Up @@ -48,7 +49,15 @@ pub(crate) fn camelcase_imported_as_lowercase(
asname: &str,
alias: &Alias,
stmt: &Stmt,
ignore_names: &[IdentifierPattern],
) -> Option<Diagnostic> {
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(asname))
{
return None;
}

if helpers::is_camelcase(name) && ruff_python_stdlib::str::is_lower(asname) {
let mut diagnostic = Diagnostic::new(
CamelcaseImportedAsLowercase {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_stdlib::str;

use crate::settings::types::IdentifierPattern;

/// ## What it does
/// Checks for constant imports that are aliased to non-constant-style
/// names.
Expand Down Expand Up @@ -48,7 +50,15 @@ pub(crate) fn constant_imported_as_non_constant(
asname: &str,
alias: &Alias,
stmt: &Stmt,
ignore_names: &[IdentifierPattern],
) -> Option<Diagnostic> {
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return None;
}

if str::is_upper(name) && !str::is_upper(asname) {
let mut diagnostic = Diagnostic::new(
ConstantImportedAsNonConstant {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use ruff_python_ast::helpers::identifier_range;
use ruff_python_ast::source_code::Locator;
use ruff_python_semantic::scope::{Scope, ScopeKind};

use crate::settings::types::IdentifierPattern;

/// ## What it does
/// Checks for functions with "dunder" names (that is, names with two
/// leading and trailing underscores) that are not documented.
Expand Down Expand Up @@ -45,6 +47,7 @@ pub(crate) fn dunder_function_name(
scope: &Scope,
stmt: &Stmt,
name: &str,
ignore_names: &[IdentifierPattern],
locator: &Locator,
) -> Option<Diagnostic> {
if matches!(scope.kind, ScopeKind::Class(_)) {
Expand All @@ -57,6 +60,12 @@ pub(crate) fn dunder_function_name(
if matches!(scope.kind, ScopeKind::Module) && (name == "__getattr__" || name == "__dir__") {
return None;
}
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return None;
}

Some(Diagnostic::new(
DunderFunctionName,
Expand Down
Loading

0 comments on commit e7316c1

Please sign in to comment.