Add our own ignored-names abstractions (#9802)

## 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.
This commit is contained in:
Charlie Marsh 2024-02-03 06:48:07 -08:00 committed by GitHub
parent 2352de2277
commit c53aae0b6f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 248 additions and 185 deletions

View File

@ -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};
@ -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)

View File

@ -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.
@ -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(),

View File

@ -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.
@ -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(),

View File

@ -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.
@ -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(),

View File

@ -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
@ -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(),

View File

@ -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
@ -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;
@ -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()))
}

View File

@ -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.
@ -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;
@ -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;
}

View File

@ -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.
@ -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(),

View File

@ -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.
@ -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(),

View File

@ -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(

View File

@ -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(

View File

@ -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
@ -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;
@ -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(),

View File

@ -6,7 +6,7 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_stdlib::identifiers::{is_migration_name, is_module_name};
use ruff_text_size::TextRange;
use crate::settings::types::IdentifierPattern;
use crate::rules::pep8_naming::settings::IgnoreNames;
/// ## What it does
/// Checks for module names that do not follow the `snake_case` naming
@ -50,7 +50,7 @@ impl Violation for InvalidModuleName {
pub(crate) fn invalid_module_name(
path: &Path,
package: Option<&Path>,
ignore_names: &[IdentifierPattern],
ignore_names: &IgnoreNames,
) -> Option<Diagnostic> {
if !path
.extension()
@ -66,13 +66,6 @@ pub(crate) fn invalid_module_name(
path.file_stem().unwrap().to_string_lossy()
};
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(&module_name))
{
return None;
}
// As a special case, we allow files in `versions` and `migrations` directories to start
// with a digit (e.g., `0001_initial.py`), to support common conventions used by Django
// and other frameworks.
@ -81,7 +74,12 @@ pub(crate) fn invalid_module_name(
} else {
is_module_name(&module_name)
};
if !is_valid_module_name {
// Ignore any explicitly-allowed names.
if ignore_names.matches(&module_name) {
return None;
}
return Some(Diagnostic::new(
InvalidModuleName {
name: module_name.to_string(),

View File

@ -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 lowercase imports that are aliased to non-lowercase names.
@ -50,17 +49,14 @@ pub(crate) fn lowercase_imported_as_non_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 !str::is_cased_uppercase(name) && str::is_cased_lowercase(name) && !str::is_lowercase(asname)
{
// Ignore any explicitly-allowed names.
if ignore_names.matches(asname) {
return None;
}
let mut diagnostic = Diagnostic::new(
LowercaseImportedAsNonLowercase {
name: name.to_string(),

View File

@ -57,15 +57,6 @@ pub(crate) fn mixed_case_variable_in_class_scope(
name: &str,
arguments: Option<&Arguments>,
) {
if checker
.settings
.pep8_naming
.ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return;
}
if !helpers::is_mixed_case(name) {
return;
}
@ -78,6 +69,10 @@ pub(crate) fn mixed_case_variable_in_class_scope(
return;
}
if checker.settings.pep8_naming.ignore_names.matches(name) {
return;
}
checker.diagnostics.push(Diagnostic::new(
MixedCaseVariableInClassScope {
name: name.to_string(),

View File

@ -62,16 +62,6 @@ impl Violation for MixedCaseVariableInGlobalScope {
/// N816
pub(crate) fn mixed_case_variable_in_global_scope(checker: &mut Checker, expr: &Expr, name: &str) {
if checker
.settings
.pep8_naming
.ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return;
}
if !helpers::is_mixed_case(name) {
return;
}
@ -81,6 +71,10 @@ pub(crate) fn mixed_case_variable_in_global_scope(checker: &mut Checker, expr: &
return;
}
if checker.settings.pep8_naming.ignore_names.matches(name) {
return;
}
checker.diagnostics.push(Diagnostic::new(
MixedCaseVariableInGlobalScope {
name: name.to_string(),

View File

@ -66,17 +66,6 @@ pub(crate) fn non_lowercase_variable_in_function(checker: &mut Checker, expr: &E
return;
}
// Ignore explicitly-allowed names.
if checker
.settings
.pep8_naming
.ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return;
}
let parent = checker.semantic().current_statement();
if helpers::is_named_tuple_assignment(parent, checker.semantic())
|| helpers::is_typed_dict_assignment(parent, checker.semantic())
@ -87,6 +76,11 @@ pub(crate) fn non_lowercase_variable_in_function(checker: &mut Checker, expr: &E
return;
}
// Ignore explicitly-allowed names.
if checker.settings.pep8_naming.ignore_names.matches(name) {
return;
}
checker.diagnostics.push(Diagnostic::new(
NonLowercaseVariableInFunction {
name: name.to_string(),

View File

@ -4,42 +4,24 @@ use std::error::Error;
use std::fmt;
use std::fmt::Formatter;
use crate::display_settings;
use globset::{Glob, GlobSet, GlobSetBuilder};
use ruff_cache::{CacheKey, CacheKeyHasher};
use ruff_macros::CacheKey;
use crate::settings::types::IdentifierPattern;
use crate::display_settings;
#[derive(Debug, CacheKey)]
pub struct Settings {
pub ignore_names: Vec<IdentifierPattern>,
pub ignore_names: IgnoreNames,
pub classmethod_decorators: Vec<String>,
pub staticmethod_decorators: Vec<String>,
}
pub fn default_ignore_names() -> Vec<String> {
vec![
"setUp".to_string(),
"tearDown".to_string(),
"setUpClass".to_string(),
"tearDownClass".to_string(),
"setUpModule".to_string(),
"tearDownModule".to_string(),
"asyncSetUp".to_string(),
"asyncTearDown".to_string(),
"setUpTestData".to_string(),
"failureException".to_string(),
"longMessage".to_string(),
"maxDiff".to_string(),
]
}
impl Default for Settings {
fn default() -> Self {
Self {
ignore_names: default_ignore_names()
.into_iter()
.map(|name| IdentifierPattern::new(&name).unwrap())
.collect(),
ignore_names: IgnoreNames::Default,
classmethod_decorators: Vec::new(),
staticmethod_decorators: Vec::new(),
}
@ -52,7 +34,7 @@ impl fmt::Display for Settings {
formatter = f,
namespace = "linter.pep8_naming",
fields = [
self.ignore_names | array,
self.ignore_names,
self.classmethod_decorators | array,
self.staticmethod_decorators | array
]
@ -64,7 +46,7 @@ impl fmt::Display for Settings {
/// Error returned by the [`TryFrom`] implementation of [`Settings`].
#[derive(Debug)]
pub enum SettingsError {
InvalidIgnoreName(glob::PatternError),
InvalidIgnoreName(globset::Error),
}
impl fmt::Display for SettingsError {
@ -84,3 +66,152 @@ impl Error for SettingsError {
}
}
}
/// The default names to ignore.
///
/// Must be kept in-sync with the `matches!` macro in [`IgnoreNames::matches`].
static DEFAULTS: &[&str] = &[
"setUp",
"tearDown",
"setUpClass",
"tearDownClass",
"setUpModule",
"tearDownModule",
"asyncSetUp",
"asyncTearDown",
"setUpTestData",
"failureException",
"longMessage",
"maxDiff",
];
#[derive(Debug)]
pub enum IgnoreNames {
Default,
UserProvided {
matcher: GlobSet,
literals: Vec<String>,
},
}
impl IgnoreNames {
/// Create a new [`IgnoreNames`] from the given options.
pub fn from_options(
ignore_names: Option<Vec<String>>,
extend_ignore_names: Option<Vec<String>>,
) -> Result<Self, SettingsError> {
// If the user is not customizing the set of ignored names, use the default matcher,
// which is hard-coded to avoid expensive regex matching.
if ignore_names.is_none() && extend_ignore_names.as_ref().map_or(true, Vec::is_empty) {
return Ok(IgnoreNames::Default);
}
let mut builder = GlobSetBuilder::new();
let mut literals = Vec::new();
// Add the ignored names from the `ignore-names` option. If omitted entirely, use the
// defaults
if let Some(names) = ignore_names {
for name in names {
builder.add(Glob::new(&name).map_err(SettingsError::InvalidIgnoreName)?);
literals.push(name);
}
} else {
for name in DEFAULTS {
builder.add(Glob::new(name).unwrap());
literals.push((*name).to_string());
}
}
// Add the ignored names from the `extend-ignore-names` option.
if let Some(names) = extend_ignore_names {
for name in names {
builder.add(Glob::new(&name).map_err(SettingsError::InvalidIgnoreName)?);
literals.push(name);
}
}
let matcher = builder.build().map_err(SettingsError::InvalidIgnoreName)?;
Ok(IgnoreNames::UserProvided { matcher, literals })
}
/// Returns `true` if the given name matches any of the ignored patterns.
pub fn matches(&self, name: &str) -> bool {
match self {
IgnoreNames::Default => matches!(
name,
"setUp"
| "tearDown"
| "setUpClass"
| "tearDownClass"
| "setUpModule"
| "tearDownModule"
| "asyncSetUp"
| "asyncTearDown"
| "setUpTestData"
| "failureException"
| "longMessage"
| "maxDiff"
),
IgnoreNames::UserProvided { matcher, .. } => matcher.is_match(name),
}
}
/// Create a new [`IgnoreNames`] from the given patterns.
pub fn from_patterns(
patterns: impl IntoIterator<Item = String>,
) -> Result<Self, SettingsError> {
let mut builder = GlobSetBuilder::new();
let mut literals = Vec::new();
for name in patterns {
builder.add(Glob::new(&name).map_err(SettingsError::InvalidIgnoreName)?);
literals.push(name);
}
let matcher = builder.build().map_err(SettingsError::InvalidIgnoreName)?;
Ok(IgnoreNames::UserProvided { matcher, literals })
}
}
impl CacheKey for IgnoreNames {
fn cache_key(&self, state: &mut CacheKeyHasher) {
match self {
IgnoreNames::Default => {
"default".cache_key(state);
}
IgnoreNames::UserProvided {
literals: patterns, ..
} => {
"user-provided".cache_key(state);
patterns.cache_key(state);
}
}
}
}
impl fmt::Display for IgnoreNames {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
IgnoreNames::Default => {
writeln!(f, "[")?;
for elem in DEFAULTS {
writeln!(f, "\t{elem},")?;
}
write!(f, "]")?;
}
IgnoreNames::UserProvided {
literals: patterns, ..
} => {
writeln!(f, "[")?;
for elem in patterns {
writeln!(f, "\t{elem},")?;
}
write!(f, "]")?;
}
}
Ok(())
}
}

View File

@ -15,6 +15,7 @@ use ruff_linter::rules::flake8_quotes::settings::Quote;
use ruff_linter::rules::flake8_tidy_imports::settings::{ApiBan, Strictness};
use ruff_linter::rules::isort::settings::RelativeImportsOrder;
use ruff_linter::rules::isort::{ImportSection, ImportType};
use ruff_linter::rules::pep8_naming::settings::IgnoreNames;
use ruff_linter::rules::pydocstyle::settings::Convention;
use ruff_linter::rules::pylint::settings::ConstantType;
use ruff_linter::rules::{
@ -2511,16 +2512,7 @@ impl Pep8NamingOptions {
self,
) -> Result<pep8_naming::settings::Settings, pep8_naming::settings::SettingsError> {
Ok(pep8_naming::settings::Settings {
ignore_names: self
.ignore_names
.unwrap_or_else(pep8_naming::settings::default_ignore_names)
.into_iter()
.chain(self.extend_ignore_names.unwrap_or_default())
.map(|name| {
IdentifierPattern::new(&name)
.map_err(pep8_naming::settings::SettingsError::InvalidIgnoreName)
})
.collect::<Result<Vec<_>, pep8_naming::settings::SettingsError>>()?,
ignore_names: IgnoreNames::from_options(self.ignore_names, self.extend_ignore_names)?,
classmethod_decorators: self.classmethod_decorators.unwrap_or_default(),
staticmethod_decorators: self.staticmethod_decorators.unwrap_or_default(),
})