Support glob patterns in pep8_naming ignore-names (#5024)

## Summary

 Support glob patterns in pep8_naming ignore-names.

Closes #2787

## Test Plan

Added new tests.
This commit is contained in:
Thomas de Zeeuw 2023-06-13 17:37:13 +02:00 committed by GitHub
parent 65312bad01
commit b0f89fa814
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
29 changed files with 482 additions and 22 deletions

1
Cargo.lock generated
View file

@ -1880,6 +1880,7 @@ name = "ruff_cache"
version = "0.0.0"
dependencies = [
"filetime",
"glob",
"globset",
"itertools",
"regex",

View file

@ -0,0 +1,14 @@
import unittest
def badAllowed():
pass
def stillBad():
pass
class Test(unittest.TestCase):
def badAllowed(self):
return super().tearDown()
def stillBad(self):
return super().tearDown()

View file

@ -0,0 +1,12 @@
def func(_, a, badAllowed):
return _, a, badAllowed
def func(_, a, stillBad):
return _, a, stillBad
class Class:
def method(self, _, a, badAllowed):
return _, a, badAllowed
def method(self, _, a, stillBad):
return _, a, stillBad

View file

@ -0,0 +1,22 @@
from abc import ABCMeta
class Class:
def __init_subclass__(self, default_name, **kwargs):
...
@classmethod
def badAllowed(self, x, /, other):
...
@classmethod
def stillBad(self, x, /, other):
...
class MetaClass(ABCMeta):
def badAllowed(self):
pass
def stillBad(self):
pass

View file

@ -0,0 +1,42 @@
from abc import ABCMeta
import pydantic
class Class:
def badAllowed(this):
pass
def stillBad(this):
pass
if False:
def badAllowed(this):
pass
def stillBad(this):
pass
@pydantic.validator
def badAllowed(cls, my_field: str) -> str:
pass
@pydantic.validator
def stillBad(cls, my_field: str) -> str:
pass
@pydantic.validator("my_field")
def badAllowed(cls, my_field: str) -> str:
pass
@pydantic.validator("my_field")
def stillBad(cls, my_field: str) -> str:
pass
class PosOnlyClass:
def badAllowed(this, blah, /, self, something: str):
pass
def stillBad(this, blah, /, self, something: str):
pass

View file

@ -0,0 +1,6 @@
def assign():
badAllowed = 0
stillBad = 0
BAD_ALLOWED = 0
STILL_BAD = 0

View file

@ -0,0 +1,19 @@
class C:
badAllowed = 0
stillBad = 0
_badAllowed = 0
_stillBad = 0
bad_Allowed = 0
still_Bad = 0
class D(TypedDict):
badAllowed: bool
stillBad: bool
_badAllowed: list
_stillBad: list
bad_Allowed: set
still_Bad: set

View file

@ -0,0 +1,8 @@
badAllowed = 0
stillBad = 0
_badAllowed = 0
_stillBad = 0
bad_Allowed = 0
still_Bad = 0

View file

@ -5,13 +5,14 @@ pub mod settings;
#[cfg(test)]
mod tests {
use std::path::Path;
use std::path::{Path, PathBuf};
use anyhow::Result;
use test_case::test_case;
use crate::registry::Rule;
use crate::rules::pep8_naming;
use crate::settings::types::IdentifierPattern;
use crate::test::test_path;
use crate::{assert_messages, settings};
@ -104,4 +105,30 @@ mod tests {
assert_messages!(diagnostics);
Ok(())
}
#[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::MixedCaseVariableInClassScope, "N815.py")]
#[test_case(Rule::MixedCaseVariableInGlobalScope, "N816.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(),
],
..Default::default()
},
..settings::Settings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
}

View file

@ -3,6 +3,8 @@ use rustpython_parser::ast::{Arg, Ranged};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use crate::settings::types::IdentifierPattern;
/// ## What it does
/// Checks for argument names that do not follow the `snake_case` convention.
///
@ -48,9 +50,12 @@ impl Violation for InvalidArgumentName {
pub(crate) fn invalid_argument_name(
name: &str,
arg: &Arg,
ignore_names: &[String],
ignore_names: &[IdentifierPattern],
) -> Option<Diagnostic> {
if ignore_names.iter().any(|ignore_name| ignore_name == name) {
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return None;
}
if name.to_lowercase() != name {

View file

@ -82,7 +82,7 @@ pub(crate) fn invalid_first_argument_name_for_class_method(
.pep8_naming
.ignore_names
.iter()
.any(|ignore_name| ignore_name == name)
.any(|ignore_name| ignore_name.matches(name))
{
return None;
}

View file

@ -81,7 +81,7 @@ pub(crate) fn invalid_first_argument_name_for_method(
.pep8_naming
.ignore_names
.iter()
.any(|ignore_name| ignore_name == name)
.any(|ignore_name| ignore_name.matches(name))
{
return None;
}

View file

@ -7,6 +7,8 @@ use ruff_python_ast::source_code::Locator;
use ruff_python_semantic::analyze::visibility;
use ruff_python_semantic::model::SemanticModel;
use crate::settings::types::IdentifierPattern;
/// ## What it does
/// Checks for functions names that do not follow the `snake_case` naming
/// convention.
@ -52,12 +54,15 @@ pub(crate) fn invalid_function_name(
stmt: &Stmt,
name: &str,
decorator_list: &[Decorator],
ignore_names: &[String],
ignore_names: &[IdentifierPattern],
model: &SemanticModel,
locator: &Locator,
) -> Option<Diagnostic> {
// Ignore any explicitly-ignored function names.
if ignore_names.iter().any(|ignore_name| ignore_name == name) {
if ignore_names
.iter()
.any(|ignore_name| ignore_name.matches(name))
{
return None;
}

View file

@ -62,7 +62,7 @@ pub(crate) fn mixed_case_variable_in_class_scope(
.pep8_naming
.ignore_names
.iter()
.any(|ignore_name| ignore_name == name)
.any(|ignore_name| ignore_name.matches(name))
{
return;
}

View file

@ -71,7 +71,7 @@ pub(crate) fn mixed_case_variable_in_global_scope(
.pep8_naming
.ignore_names
.iter()
.any(|ignore_name| ignore_name == name)
.any(|ignore_name| ignore_name.matches(name))
{
return;
}

View file

@ -60,7 +60,7 @@ pub(crate) fn non_lowercase_variable_in_function(
.pep8_naming
.ignore_names
.iter()
.any(|ignore_name| ignore_name == name)
.any(|ignore_name| ignore_name.matches(name))
{
return;
}

View file

@ -1,9 +1,14 @@
//! Settings for the `pep8-naming` plugin.
use std::error::Error;
use std::fmt;
use serde::{Deserialize, Serialize};
use ruff_macros::{CacheKey, CombineOptions, ConfigurationOptions};
use crate::settings::types::IdentifierPattern;
const IGNORE_NAMES: [&str; 12] = [
"setUp",
"tearDown",
@ -36,7 +41,7 @@ pub struct Options {
ignore-names = ["callMethod"]
"#
)]
/// A list of names to ignore when considering `pep8-naming` violations.
/// A list of names (or patterns) to ignore when considering `pep8-naming` violations.
pub ignore_names: Option<Vec<String>>,
#[option(
default = r#"[]"#,
@ -72,7 +77,7 @@ pub struct Options {
#[derive(Debug, CacheKey)]
pub struct Settings {
pub ignore_names: Vec<String>,
pub ignore_names: Vec<IdentifierPattern>,
pub classmethod_decorators: Vec<String>,
pub staticmethod_decorators: Vec<String>,
}
@ -80,21 +85,59 @@ pub struct Settings {
impl Default for Settings {
fn default() -> Self {
Self {
ignore_names: IGNORE_NAMES.map(String::from).to_vec(),
ignore_names: IGNORE_NAMES
.iter()
.map(|name| IdentifierPattern::new(name).unwrap())
.collect(),
classmethod_decorators: Vec::new(),
staticmethod_decorators: Vec::new(),
}
}
}
impl From<Options> for Settings {
fn from(options: Options) -> Self {
Self {
ignore_names: options
.ignore_names
.unwrap_or_else(|| IGNORE_NAMES.map(String::from).to_vec()),
impl TryFrom<Options> for Settings {
type Error = SettingsError;
fn try_from(options: Options) -> Result<Self, Self::Error> {
Ok(Self {
ignore_names: match options.ignore_names {
Some(names) => names
.into_iter()
.map(|name| {
IdentifierPattern::new(&name).map_err(SettingsError::InvalidIgnoreName)
})
.collect::<Result<Vec<_>, Self::Error>>()?,
None => IGNORE_NAMES
.into_iter()
.map(|name| IdentifierPattern::new(name).unwrap())
.collect(),
},
classmethod_decorators: options.classmethod_decorators.unwrap_or_default(),
staticmethod_decorators: options.staticmethod_decorators.unwrap_or_default(),
})
}
}
/// Error returned by the [`TryFrom`] implementation of [`Settings`].
#[derive(Debug)]
pub enum SettingsError {
InvalidIgnoreName(glob::PatternError),
}
impl fmt::Display for SettingsError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
SettingsError::InvalidIgnoreName(err) => {
write!(f, "Invalid pattern in ignore-names: {err}")
}
}
}
}
impl Error for SettingsError {
fn source(&self) -> Option<&(dyn Error + 'static)> {
match self {
SettingsError::InvalidIgnoreName(err) => Some(err),
}
}
}
@ -102,7 +145,13 @@ impl From<Options> for Settings {
impl From<Settings> for Options {
fn from(settings: Settings) -> Self {
Self {
ignore_names: Some(settings.ignore_names),
ignore_names: Some(
settings
.ignore_names
.into_iter()
.map(|pattern| pattern.as_str().to_owned())
.collect(),
),
classmethod_decorators: Some(settings.classmethod_decorators),
staticmethod_decorators: Some(settings.staticmethod_decorators),
}

View file

@ -0,0 +1,22 @@
---
source: crates/ruff/src/rules/pep8_naming/mod.rs
---
N802.py:6:5: N802 Function name `stillBad` should be lowercase
|
4 | pass
5 |
6 | def stillBad():
| ^^^^^^^^ N802
7 | pass
|
N802.py:13:9: N802 Function name `stillBad` should be lowercase
|
11 | return super().tearDown()
12 |
13 | def stillBad(self):
| ^^^^^^^^ N802
14 | return super().tearDown()
|

View file

@ -0,0 +1,22 @@
---
source: crates/ruff/src/rules/pep8_naming/mod.rs
---
N803.py:4:16: N803 Argument name `stillBad` should be lowercase
|
2 | return _, a, badAllowed
3 |
4 | def func(_, a, stillBad):
| ^^^^^^^^ N803
5 | return _, a, stillBad
|
N803.py:11:28: N803 Argument name `stillBad` should be lowercase
|
9 | return _, a, badAllowed
10 |
11 | def method(self, _, a, stillBad):
| ^^^^^^^^ N803
12 | return _, a, stillBad
|

View file

@ -0,0 +1,29 @@
---
source: crates/ruff/src/rules/pep8_naming/mod.rs
---
N804.py:5:27: N804 First argument of a class method should be named `cls`
|
4 | class Class:
5 | def __init_subclass__(self, default_name, **kwargs):
| ^^^^ N804
6 | ...
|
N804.py:13:18: N804 First argument of a class method should be named `cls`
|
12 | @classmethod
13 | def stillBad(self, x, /, other):
| ^^^^ N804
14 | ...
|
N804.py:21:18: N804 First argument of a class method should be named `cls`
|
19 | pass
20 |
21 | def stillBad(self):
| ^^^^ N804
22 | pass
|

View file

@ -0,0 +1,47 @@
---
source: crates/ruff/src/rules/pep8_naming/mod.rs
---
N805.py:10:18: N805 First argument of a method should be named `self`
|
8 | pass
9 |
10 | def stillBad(this):
| ^^^^ N805
11 | pass
|
N805.py:18:22: N805 First argument of a method should be named `self`
|
16 | pass
17 |
18 | def stillBad(this):
| ^^^^ N805
19 | pass
|
N805.py:26:18: N805 First argument of a method should be named `self`
|
25 | @pydantic.validator
26 | def stillBad(cls, my_field: str) -> str:
| ^^^ N805
27 | pass
|
N805.py:34:18: N805 First argument of a method should be named `self`
|
33 | @pydantic.validator("my_field")
34 | def stillBad(cls, my_field: str) -> str:
| ^^^ N805
35 | pass
|
N805.py:41:18: N805 First argument of a method should be named `self`
|
39 | pass
40 |
41 | def stillBad(this, blah, /, self, something: str):
| ^^^^ N805
42 | pass
|

View file

@ -0,0 +1,21 @@
---
source: crates/ruff/src/rules/pep8_naming/mod.rs
---
N806.py:3:5: N806 Variable `stillBad` in function should be lowercase
|
1 | def assign():
2 | badAllowed = 0
3 | stillBad = 0
| ^^^^^^^^ N806
4 |
5 | BAD_ALLOWED = 0
|
N806.py:6:5: N806 Variable `STILL_BAD` in function should be lowercase
|
5 | BAD_ALLOWED = 0
6 | STILL_BAD = 0
| ^^^^^^^^^ N806
|

View file

@ -0,0 +1,58 @@
---
source: crates/ruff/src/rules/pep8_naming/mod.rs
---
N815.py:3:5: N815 Variable `stillBad` in class scope should not be mixedCase
|
1 | class C:
2 | badAllowed = 0
3 | stillBad = 0
| ^^^^^^^^ N815
4 |
5 | _badAllowed = 0
|
N815.py:6:5: N815 Variable `_stillBad` in class scope should not be mixedCase
|
5 | _badAllowed = 0
6 | _stillBad = 0
| ^^^^^^^^^ N815
7 |
8 | bad_Allowed = 0
|
N815.py:9:5: N815 Variable `still_Bad` in class scope should not be mixedCase
|
8 | bad_Allowed = 0
9 | still_Bad = 0
| ^^^^^^^^^ N815
10 |
11 | class D(TypedDict):
|
N815.py:13:5: N815 Variable `stillBad` in class scope should not be mixedCase
|
11 | class D(TypedDict):
12 | badAllowed: bool
13 | stillBad: bool
| ^^^^^^^^ N815
14 |
15 | _badAllowed: list
|
N815.py:16:5: N815 Variable `_stillBad` in class scope should not be mixedCase
|
15 | _badAllowed: list
16 | _stillBad: list
| ^^^^^^^^^ N815
17 |
18 | bad_Allowed: set
|
N815.py:19:5: N815 Variable `still_Bad` in class scope should not be mixedCase
|
18 | bad_Allowed: set
19 | still_Bad: set
| ^^^^^^^^^ N815
|

View file

@ -0,0 +1,29 @@
---
source: crates/ruff/src/rules/pep8_naming/mod.rs
---
N816.py:2:1: N816 Variable `stillBad` in global scope should not be mixedCase
|
1 | badAllowed = 0
2 | stillBad = 0
| ^^^^^^^^ N816
3 |
4 | _badAllowed = 0
|
N816.py:5:1: N816 Variable `_stillBad` in global scope should not be mixedCase
|
4 | _badAllowed = 0
5 | _stillBad = 0
| ^^^^^^^^^ N816
6 |
7 | bad_Allowed = 0
|
N816.py:8:1: N816 Variable `still_Bad` in global scope should not be mixedCase
|
7 | bad_Allowed = 0
8 | still_Bad = 0
| ^^^^^^^^^ N816
|

View file

@ -265,7 +265,8 @@ impl Settings {
.unwrap_or_default(),
pep8_naming: config
.pep8_naming
.map(pep8_naming::settings::Settings::from)
.map(pep8_naming::settings::Settings::try_from)
.transpose()?
.unwrap_or_default(),
pycodestyle: config
.pycodestyle

View file

@ -249,3 +249,16 @@ impl Deref for Version {
&self.0
}
}
/// Pattern to match an identifier.
///
/// # Notes
///
/// [`glob::Pattern`] matches a little differently than we ideally want to.
/// Specifically it uses `**` to match an arbitrary number of subdirectories,
/// luckily this not relevant since identifiers don't contains slashes.
///
/// For reference pep8-naming uses
/// [`fnmatch`](https://docs.python.org/3.11/library/fnmatch.html) for
/// pattern matching.
pub type IdentifierPattern = glob::Pattern;

View file

@ -12,6 +12,7 @@ license = { workspace = true }
[dependencies]
itertools = { workspace = true }
glob = { workspace = true }
globset = { workspace = true }
regex = { workspace = true }
filetime = { workspace = true }

View file

@ -5,6 +5,7 @@ use std::hash::{Hash, Hasher};
use std::ops::{Deref, DerefMut};
use std::path::{Path, PathBuf};
use glob::Pattern;
use itertools::Itertools;
use regex::Regex;
@ -375,3 +376,9 @@ impl CacheKey for Regex {
self.as_str().cache_key(state);
}
}
impl CacheKey for Pattern {
fn cache_key(&self, state: &mut CacheKeyHasher) {
self.as_str().cache_key(state);
}
}

2
ruff.schema.json generated
View file

@ -1397,7 +1397,7 @@
}
},
"ignore-names": {
"description": "A list of names to ignore when considering `pep8-naming` violations.",
"description": "A list of names (or patterns) to ignore when considering `pep8-naming` violations.",
"type": [
"array",
"null"