Skip to content

Commit

Permalink
Add our own ignored-names abstractions (#9802)
Browse files Browse the repository at this point in the history
## Summary

These run over nearly every identifier. It's rare to override them, so
when not provided, we can just use a match against the hardcoded default
set.
  • Loading branch information
charliermarsh committed Feb 3, 2024
1 parent 2352de2 commit c53aae0
Show file tree
Hide file tree
Showing 19 changed files with 248 additions and 185 deletions.
15 changes: 8 additions & 7 deletions crates/ruff_linter/src/rules/pep8_naming/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ mod tests {

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

Expand Down Expand Up @@ -148,12 +148,13 @@ mod tests {
PathBuf::from_iter(["pep8_naming", "ignore_names", path]).as_path(),
&settings::LinterSettings {
pep8_naming: pep8_naming::settings::Settings {
ignore_names: vec![
IdentifierPattern::new("*allowed*").unwrap(),
IdentifierPattern::new("*Allowed*").unwrap(),
IdentifierPattern::new("*ALLOWED*").unwrap(),
IdentifierPattern::new("BA").unwrap(), // For N817.
],
ignore_names: IgnoreNames::from_patterns([
"*allowed*".to_string(),
"*Allowed*".to_string(),
"*ALLOWED*".to_string(),
"BA".to_string(), // For N817.
])
.unwrap(),
..Default::default()
},
..settings::LinterSettings::for_rule(rule_code)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ruff_python_stdlib::str::{self};
use ruff_text_size::Ranged;

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

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

if helpers::is_camelcase(name)
&& !str::is_cased_lowercase(asname)
&& str::is_cased_uppercase(asname)
&& helpers::is_acronym(name, asname)
{
// Ignore any explicitly-allowed names.
if ignore_names.matches(name) {
return None;
}
let mut diagnostic = Diagnostic::new(
CamelcaseImportedAsAcronym {
name: name.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ruff_python_stdlib::str::{self};
use ruff_text_size::Ranged;

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

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

if helpers::is_camelcase(name)
&& !str::is_cased_lowercase(asname)
&& str::is_cased_uppercase(asname)
&& !helpers::is_acronym(name, asname)
{
// Ignore any explicitly-allowed names.
if ignore_names.matches(asname) {
return None;
}
let mut diagnostic = Diagnostic::new(
CamelcaseImportedAsConstant {
name: name.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_text_size::Ranged;

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

/// ## What it does
/// Checks for `CamelCase` imports that are aliased to lowercase names.
Expand Down Expand Up @@ -50,16 +50,13 @@ pub(crate) fn camelcase_imported_as_lowercase(
asname: &str,
alias: &Alias,
stmt: &Stmt,
ignore_names: &[IdentifierPattern],
ignore_names: &IgnoreNames,
) -> 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_cased_lowercase(asname) {
// Ignore any explicitly-allowed names.
if ignore_names.matches(asname) {
return None;
}
let mut diagnostic = Diagnostic::new(
CamelcaseImportedAsLowercase {
name: name.to_string(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
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;
use ruff_text_size::Ranged;

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

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

if str::is_cased_uppercase(name) && !str::is_cased_uppercase(asname) {
// Ignore any explicitly-allowed names.
if ignore_names.matches(asname) {
return None;
}
let mut diagnostic = Diagnostic::new(
ConstantImportedAsNonConstant {
name: name.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ruff_python_ast::identifier::Identifier;
use ruff_python_semantic::analyze::visibility;
use ruff_python_semantic::{Scope, ScopeKind};

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

/// ## What it does
/// Checks for functions with "dunder" names (that is, names with two
Expand Down Expand Up @@ -47,7 +47,7 @@ pub(crate) fn dunder_function_name(
scope: &Scope,
stmt: &Stmt,
name: &str,
ignore_names: &[IdentifierPattern],
ignore_names: &IgnoreNames,
) -> Option<Diagnostic> {
if matches!(scope.kind, ScopeKind::Class(_)) {
return None;
Expand All @@ -59,12 +59,9 @@ 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))
{
// Ignore any explicitly-allowed names.
if ignore_names.matches(name) {
return None;
}

Some(Diagnostic::new(DunderFunctionName, stmt.identifier()))
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::identifier::Identifier;

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

/// ## What it does
/// Checks for custom exception definitions that omit the `Error` suffix.
Expand Down Expand Up @@ -47,7 +47,7 @@ pub(crate) fn error_suffix_on_exception_name(
class_def: &Stmt,
arguments: Option<&Arguments>,
name: &str,
ignore_names: &[IdentifierPattern],
ignore_names: &IgnoreNames,
) -> Option<Diagnostic> {
if name.ends_with("Error") {
return None;
Expand All @@ -65,10 +65,8 @@ pub(crate) fn error_suffix_on_exception_name(
return None;
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_stdlib::str;
use ruff_text_size::Ranged;

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

/// ## What it does
/// Checks for argument names that do not follow the `snake_case` convention.
Expand Down Expand Up @@ -52,15 +52,13 @@ impl Violation for InvalidArgumentName {
pub(crate) fn invalid_argument_name(
name: &str,
parameter: &Parameter,
ignore_names: &[IdentifierPattern],
ignore_names: &IgnoreNames,
) -> Option<Diagnostic> {
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return None;
}
if !str::is_lowercase(name) {
// Ignore any explicitly-allowed names.
if ignore_names.matches(name) {
return None;
}
return Some(Diagnostic::new(
InvalidArgumentName {
name: name.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::identifier::Identifier;

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

/// ## What it does
/// Checks for class names that do not follow the `CamelCase` convention.
Expand Down Expand Up @@ -52,17 +52,14 @@ impl Violation for InvalidClassName {
pub(crate) fn invalid_class_name(
class_def: &Stmt,
name: &str,
ignore_names: &[IdentifierPattern],
ignore_names: &IgnoreNames,
) -> Option<Diagnostic> {
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return None;
}

let stripped = name.strip_prefix('_').unwrap_or(name);
if !stripped.chars().next().is_some_and(char::is_uppercase) || stripped.contains('_') {
// Ignore any explicitly-allowed names.
if ignore_names.matches(name) {
return None;
}
return Some(Diagnostic::new(
InvalidClassName {
name: name.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,7 @@ pub(crate) fn invalid_first_argument_name_for_class_method(
.or_else(|| parameters.args.first())
{
if &parameter.name != "cls" {
if checker
.settings
.pep8_naming
.ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
if checker.settings.pep8_naming.ignore_names.matches(name) {
return None;
}
return Some(Diagnostic::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,7 @@ pub(crate) fn invalid_first_argument_name_for_method(
if &arg.parameter.name == "self" {
return None;
}
if checker
.settings
.pep8_naming
.ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
if checker.settings.pep8_naming.ignore_names.matches(name) {
return None;
}
Some(Diagnostic::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use ruff_python_semantic::analyze::visibility;
use ruff_python_semantic::SemanticModel;
use ruff_python_stdlib::str;

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

/// ## What it does
/// Checks for functions names that do not follow the `snake_case` naming
Expand Down Expand Up @@ -60,17 +60,9 @@ pub(crate) fn invalid_function_name(
stmt: &Stmt,
name: &str,
decorator_list: &[Decorator],
ignore_names: &[IdentifierPattern],
ignore_names: &IgnoreNames,
semantic: &SemanticModel,
) -> Option<Diagnostic> {
// Ignore any explicitly-ignored function names.
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return None;
}

// Ignore any function names that are already lowercase.
if str::is_lowercase(name) {
return None;
Expand All @@ -85,6 +77,11 @@ pub(crate) fn invalid_function_name(
return None;
}

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

Some(Diagnostic::new(
InvalidFunctionName {
name: name.to_string(),
Expand Down
Loading

0 comments on commit c53aae0

Please sign in to comment.