Use more precise ranges for function and class checks (#1247)

This commit is contained in:
Charlie Marsh 2022-12-14 22:40:00 -05:00 committed by GitHub
parent ba85eb846c
commit 6be910ae07
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 337 additions and 206 deletions

View file

@ -1,9 +1,12 @@
use log::error;
use once_cell::sync::Lazy; use once_cell::sync::Lazy;
use regex::Regex; use regex::Regex;
use rustc_hash::{FxHashMap, FxHashSet}; use rustc_hash::{FxHashMap, FxHashSet};
use rustpython_ast::{ use rustpython_ast::{
Arguments, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Location, Stmt, StmtKind, Arguments, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Location, Stmt, StmtKind,
}; };
use rustpython_parser::lexer;
use rustpython_parser::lexer::Tok;
use crate::ast::types::Range; use crate::ast::types::Range;
use crate::SourceCodeLocator; use crate::SourceCodeLocator;
@ -311,13 +314,42 @@ pub fn count_trailing_lines(stmt: &Stmt, locator: &SourceCodeLocator) -> usize {
.count() .count()
} }
/// Return the appropriate visual `Range` for any message that spans a `Stmt`.
/// Specifically, this method returns the range of a function or class name,
/// rather than that of the entire function or class body.
pub fn identifier_range(stmt: &Stmt, locator: &SourceCodeLocator) -> Range {
if matches!(
stmt.node,
StmtKind::ClassDef { .. }
| StmtKind::FunctionDef { .. }
| StmtKind::AsyncFunctionDef { .. }
) {
let contents = locator.slice_source_code_range(&Range::from_located(stmt));
for (start, tok, end) in lexer::make_tokenizer(&contents).flatten() {
if matches!(tok, Tok::Name { .. }) {
let start = to_absolute(start, stmt.location);
let end = to_absolute(end, stmt.location);
return Range {
location: start,
end_location: end,
};
}
}
error!("Failed to find identifier for {:?}", stmt);
}
Range::from_located(stmt)
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use anyhow::Result; use anyhow::Result;
use rustc_hash::{FxHashMap, FxHashSet}; use rustc_hash::{FxHashMap, FxHashSet};
use rustpython_ast::Location;
use rustpython_parser::parser; use rustpython_parser::parser;
use crate::ast::helpers::match_module_member; use crate::ast::helpers::{identifier_range, match_module_member};
use crate::ast::types::Range;
use crate::source_code_locator::SourceCodeLocator;
#[test] #[test]
fn builtin() -> Result<()> { fn builtin() -> Result<()> {
@ -461,4 +493,91 @@ mod tests {
)); ));
Ok(()) Ok(())
} }
#[test]
fn extract_identifier_range() -> Result<()> {
let contents = "def f(): pass".trim();
let program = parser::parse_program(contents, "<filename>")?;
let stmt = program.first().unwrap();
let locator = SourceCodeLocator::new(contents);
assert_eq!(
identifier_range(stmt, &locator),
Range {
location: Location::new(1, 4),
end_location: Location::new(1, 5),
}
);
let contents = r#"
def \
f():
pass
"#
.trim();
let program = parser::parse_program(contents, "<filename>")?;
let stmt = program.first().unwrap();
let locator = SourceCodeLocator::new(contents);
assert_eq!(
identifier_range(stmt, &locator),
Range {
location: Location::new(2, 2),
end_location: Location::new(2, 3),
}
);
let contents = "class Class(): pass".trim();
let program = parser::parse_program(contents, "<filename>")?;
let stmt = program.first().unwrap();
let locator = SourceCodeLocator::new(contents);
assert_eq!(
identifier_range(stmt, &locator),
Range {
location: Location::new(1, 6),
end_location: Location::new(1, 11),
}
);
let contents = "class Class: pass".trim();
let program = parser::parse_program(contents, "<filename>")?;
let stmt = program.first().unwrap();
let locator = SourceCodeLocator::new(contents);
assert_eq!(
identifier_range(stmt, &locator),
Range {
location: Location::new(1, 6),
end_location: Location::new(1, 11),
}
);
let contents = r#"
@decorator()
class Class():
pass
"#
.trim();
let program = parser::parse_program(contents, "<filename>")?;
let stmt = program.first().unwrap();
let locator = SourceCodeLocator::new(contents);
assert_eq!(
identifier_range(stmt, &locator),
Range {
location: Location::new(2, 6),
end_location: Location::new(2, 11),
}
);
let contents = r#"x = y + 1"#.trim();
let program = parser::parse_program(contents, "<filename>")?;
let stmt = program.first().unwrap();
let locator = SourceCodeLocator::new(contents);
assert_eq!(
identifier_range(stmt, &locator),
Range {
location: Location::new(1, 0),
end_location: Location::new(1, 9),
}
);
Ok(())
}
} }

View file

@ -6,7 +6,7 @@ use itertools::Itertools;
use log::error; use log::error;
use nohash_hasher::IntMap; use nohash_hasher::IntMap;
use rustc_hash::{FxHashMap, FxHashSet}; use rustc_hash::{FxHashMap, FxHashSet};
use rustpython_ast::Location; use rustpython_ast::{Located, Location};
use rustpython_parser::ast::{ use rustpython_parser::ast::{
Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind,
KeywordData, Operator, Stmt, StmtKind, Suite, KeywordData, Operator, Stmt, StmtKind, Suite,
@ -257,12 +257,11 @@ where
} }
if self.settings.enabled.contains(&CheckCode::E741) { if self.settings.enabled.contains(&CheckCode::E741) {
let location = Range::from_located(stmt);
self.add_checks( self.add_checks(
names names
.iter() .iter()
.filter_map(|name| { .filter_map(|name| {
pycodestyle::checks::ambiguous_variable_name(name, location) pycodestyle::checks::ambiguous_variable_name(name, stmt)
}) })
.into_iter(), .into_iter(),
); );
@ -309,12 +308,11 @@ where
} }
if self.settings.enabled.contains(&CheckCode::E741) { if self.settings.enabled.contains(&CheckCode::E741) {
let location = Range::from_located(stmt);
self.add_checks( self.add_checks(
names names
.iter() .iter()
.filter_map(|name| { .filter_map(|name| {
pycodestyle::checks::ambiguous_variable_name(name, location) pycodestyle::checks::ambiguous_variable_name(name, stmt)
}) })
.into_iter(), .into_iter(),
); );
@ -369,7 +367,8 @@ where
if let Some(check) = pep8_naming::checks::invalid_function_name( if let Some(check) = pep8_naming::checks::invalid_function_name(
stmt, stmt,
name, name,
&self.settings.pep8_naming, &self.settings.pep8_naming.ignore_names,
self.locator,
) { ) {
self.add_check(check); self.add_check(check);
} }
@ -406,9 +405,12 @@ where
} }
if self.settings.enabled.contains(&CheckCode::N807) { if self.settings.enabled.contains(&CheckCode::N807) {
if let Some(check) = if let Some(check) = pep8_naming::checks::dunder_function_name(
pep8_naming::checks::dunder_function_name(self.current_scope(), stmt, name) self.current_scope(),
{ stmt,
name,
self.locator,
) {
self.add_check(check); self.add_check(check);
} }
} }
@ -445,6 +447,7 @@ where
name, name,
body, body,
self.settings.mccabe.max_complexity, self.settings.mccabe.max_complexity,
self.locator,
) { ) {
self.add_check(check); self.add_check(check);
} }
@ -460,7 +463,7 @@ where
pylint::plugins::property_with_parameters(self, stmt, decorator_list, args); pylint::plugins::property_with_parameters(self, stmt, decorator_list, args);
} }
self.check_builtin_shadowing(name, Range::from_located(stmt), true); self.check_builtin_shadowing(name, stmt, true);
// Visit the decorators and arguments, but avoid the body, which will be // Visit the decorators and arguments, but avoid the body, which will be
// deferred. // deferred.
@ -548,7 +551,9 @@ where
} }
if self.settings.enabled.contains(&CheckCode::N801) { if self.settings.enabled.contains(&CheckCode::N801) {
if let Some(check) = pep8_naming::checks::invalid_class_name(stmt, name) { if let Some(check) =
pep8_naming::checks::invalid_class_name(stmt, name, self.locator)
{
self.add_check(check); self.add_check(check);
} }
} }
@ -573,7 +578,7 @@ where
); );
} }
self.check_builtin_shadowing(name, Range::from_located(stmt), false); self.check_builtin_shadowing(name, stmt, false);
for expr in bases { for expr in bases {
self.visit_expr(expr); self.visit_expr(expr);
@ -615,7 +620,7 @@ where
); );
} else { } else {
if let Some(asname) = &alias.node.asname { if let Some(asname) = &alias.node.asname {
self.check_builtin_shadowing(asname, Range::from_located(stmt), false); self.check_builtin_shadowing(asname, stmt, false);
} }
// Given `import foo`, `name` and `full_name` would both be `foo`. // Given `import foo`, `name` and `full_name` would both be `foo`.
@ -683,7 +688,10 @@ where
if self.settings.enabled.contains(&CheckCode::N811) { if self.settings.enabled.contains(&CheckCode::N811) {
if let Some(check) = if let Some(check) =
pep8_naming::checks::constant_imported_as_non_constant( pep8_naming::checks::constant_imported_as_non_constant(
stmt, name, asname, stmt,
name,
asname,
self.locator,
) )
{ {
self.add_check(check); self.add_check(check);
@ -693,7 +701,10 @@ where
if self.settings.enabled.contains(&CheckCode::N812) { if self.settings.enabled.contains(&CheckCode::N812) {
if let Some(check) = if let Some(check) =
pep8_naming::checks::lowercase_imported_as_non_lowercase( pep8_naming::checks::lowercase_imported_as_non_lowercase(
stmt, name, asname, stmt,
name,
asname,
self.locator,
) )
{ {
self.add_check(check); self.add_check(check);
@ -703,7 +714,10 @@ where
if self.settings.enabled.contains(&CheckCode::N813) { if self.settings.enabled.contains(&CheckCode::N813) {
if let Some(check) = if let Some(check) =
pep8_naming::checks::camelcase_imported_as_lowercase( pep8_naming::checks::camelcase_imported_as_lowercase(
stmt, name, asname, stmt,
name,
asname,
self.locator,
) )
{ {
self.add_check(check); self.add_check(check);
@ -712,7 +726,10 @@ where
if self.settings.enabled.contains(&CheckCode::N814) { if self.settings.enabled.contains(&CheckCode::N814) {
if let Some(check) = pep8_naming::checks::camelcase_imported_as_constant( if let Some(check) = pep8_naming::checks::camelcase_imported_as_constant(
stmt, name, asname, stmt,
name,
asname,
self.locator,
) { ) {
self.add_check(check); self.add_check(check);
} }
@ -720,7 +737,10 @@ where
if self.settings.enabled.contains(&CheckCode::N817) { if self.settings.enabled.contains(&CheckCode::N817) {
if let Some(check) = pep8_naming::checks::camelcase_imported_as_acronym( if let Some(check) = pep8_naming::checks::camelcase_imported_as_acronym(
stmt, name, asname, stmt,
name,
asname,
self.locator,
) { ) {
self.add_check(check); self.add_check(check);
} }
@ -858,7 +878,7 @@ where
scope.import_starred = true; scope.import_starred = true;
} else { } else {
if let Some(asname) = &alias.node.asname { if let Some(asname) = &alias.node.asname {
self.check_builtin_shadowing(asname, Range::from_located(stmt), false); self.check_builtin_shadowing(asname, stmt, false);
} }
// Given `from foo import bar`, `name` would be "bar" and `full_name` would // Given `from foo import bar`, `name` would be "bar" and `full_name` would
@ -927,6 +947,7 @@ where
stmt, stmt,
&alias.node.name, &alias.node.name,
asname, asname,
self.locator,
) )
{ {
self.add_check(check); self.add_check(check);
@ -939,6 +960,7 @@ where
stmt, stmt,
&alias.node.name, &alias.node.name,
asname, asname,
self.locator,
) )
{ {
self.add_check(check); self.add_check(check);
@ -951,6 +973,7 @@ where
stmt, stmt,
&alias.node.name, &alias.node.name,
asname, asname,
self.locator,
) )
{ {
self.add_check(check); self.add_check(check);
@ -962,6 +985,7 @@ where
stmt, stmt,
&alias.node.name, &alias.node.name,
asname, asname,
self.locator,
) { ) {
self.add_check(check); self.add_check(check);
} }
@ -972,6 +996,7 @@ where
stmt, stmt,
&alias.node.name, &alias.node.name,
asname, asname,
self.locator,
) { ) {
self.add_check(check); self.add_check(check);
} }
@ -1422,15 +1447,14 @@ where
} }
ExprContext::Store => { ExprContext::Store => {
if self.settings.enabled.contains(&CheckCode::E741) { if self.settings.enabled.contains(&CheckCode::E741) {
if let Some(check) = pycodestyle::checks::ambiguous_variable_name( if let Some(check) =
id, pycodestyle::checks::ambiguous_variable_name(id, expr)
Range::from_located(expr), {
) {
self.add_check(check); self.add_check(check);
} }
} }
self.check_builtin_shadowing(id, Range::from_located(expr), true); self.check_builtin_shadowing(id, expr, true);
self.handle_node_store(id, expr); self.handle_node_store(id, expr);
} }
@ -2413,19 +2437,14 @@ where
match name { match name {
Some(name) => { Some(name) => {
if self.settings.enabled.contains(&CheckCode::E741) { if self.settings.enabled.contains(&CheckCode::E741) {
if let Some(check) = pycodestyle::checks::ambiguous_variable_name( if let Some(check) =
name, pycodestyle::checks::ambiguous_variable_name(name, excepthandler)
Range::from_located(excepthandler), {
) {
self.add_check(check); self.add_check(check);
} }
} }
self.check_builtin_shadowing( self.check_builtin_shadowing(name, excepthandler, false);
name,
Range::from_located(excepthandler),
false,
);
if self.current_scope().values.contains_key(&name.as_str()) { if self.current_scope().values.contains_key(&name.as_str()) {
self.handle_node_store( self.handle_node_store(
@ -2538,23 +2557,18 @@ where
); );
if self.settings.enabled.contains(&CheckCode::E741) { if self.settings.enabled.contains(&CheckCode::E741) {
if let Some(check) = pycodestyle::checks::ambiguous_variable_name( if let Some(check) = pycodestyle::checks::ambiguous_variable_name(&arg.node.arg, arg) {
&arg.node.arg,
Range::from_located(arg),
) {
self.add_check(check); self.add_check(check);
} }
} }
if self.settings.enabled.contains(&CheckCode::N803) { if self.settings.enabled.contains(&CheckCode::N803) {
if let Some(check) = if let Some(check) = pep8_naming::checks::invalid_argument_name(&arg.node.arg, arg) {
pep8_naming::checks::invalid_argument_name(&arg.node.arg, Range::from_located(arg))
{
self.add_check(check); self.add_check(check);
} }
} }
self.check_builtin_arg_shadowing(&arg.node.arg, Range::from_located(arg)); self.check_builtin_arg_shadowing(&arg.node.arg, arg);
} }
} }
@ -3584,12 +3598,12 @@ impl<'a> Checker<'a> {
} }
} }
fn check_builtin_shadowing(&mut self, name: &str, location: Range, is_attribute: bool) { fn check_builtin_shadowing<T>(&mut self, name: &str, located: &Located<T>, is_attribute: bool) {
if is_attribute && matches!(self.current_scope().kind, ScopeKind::Class(_)) { if is_attribute && matches!(self.current_scope().kind, ScopeKind::Class(_)) {
if self.settings.enabled.contains(&CheckCode::A003) { if self.settings.enabled.contains(&CheckCode::A003) {
if let Some(check) = flake8_builtins::checks::builtin_shadowing( if let Some(check) = flake8_builtins::checks::builtin_shadowing(
name, name,
location, located,
flake8_builtins::types::ShadowingType::Attribute, flake8_builtins::types::ShadowingType::Attribute,
) { ) {
self.add_check(check); self.add_check(check);
@ -3599,7 +3613,7 @@ impl<'a> Checker<'a> {
if self.settings.enabled.contains(&CheckCode::A001) { if self.settings.enabled.contains(&CheckCode::A001) {
if let Some(check) = flake8_builtins::checks::builtin_shadowing( if let Some(check) = flake8_builtins::checks::builtin_shadowing(
name, name,
location, located,
flake8_builtins::types::ShadowingType::Variable, flake8_builtins::types::ShadowingType::Variable,
) { ) {
self.add_check(check); self.add_check(check);
@ -3608,11 +3622,11 @@ impl<'a> Checker<'a> {
} }
} }
fn check_builtin_arg_shadowing(&mut self, name: &str, location: Range) { fn check_builtin_arg_shadowing(&mut self, name: &str, arg: &Arg) {
if self.settings.enabled.contains(&CheckCode::A002) { if self.settings.enabled.contains(&CheckCode::A002) {
if let Some(check) = flake8_builtins::checks::builtin_shadowing( if let Some(check) = flake8_builtins::checks::builtin_shadowing(
name, name,
location, arg,
flake8_builtins::types::ShadowingType::Argument, flake8_builtins::types::ShadowingType::Argument,
) { ) {
self.add_check(check); self.add_check(check);

View file

@ -1,10 +1,16 @@
use rustpython_ast::{Located};
use crate::ast::types::Range; use crate::ast::types::Range;
use crate::checks::{Check, CheckKind}; use crate::checks::{Check, CheckKind};
use crate::flake8_builtins::types::ShadowingType; use crate::flake8_builtins::types::ShadowingType;
use crate::python::builtins::BUILTINS; use crate::python::builtins::BUILTINS;
/// Check builtin name shadowing. /// Check builtin name shadowing.
pub fn builtin_shadowing(name: &str, location: Range, node_type: ShadowingType) -> Option<Check> { pub fn builtin_shadowing<T>(
name: &str,
located: &Located<T>,
node_type: ShadowingType,
) -> Option<Check> {
if BUILTINS.contains(&name) { if BUILTINS.contains(&name) {
Some(Check::new( Some(Check::new(
match node_type { match node_type {
@ -12,7 +18,7 @@ pub fn builtin_shadowing(name: &str, location: Range, node_type: ShadowingType)
ShadowingType::Argument => CheckKind::BuiltinArgumentShadowing(name.to_string()), ShadowingType::Argument => CheckKind::BuiltinArgumentShadowing(name.to_string()),
ShadowingType::Attribute => CheckKind::BuiltinAttributeShadowing(name.to_string()), ShadowingType::Attribute => CheckKind::BuiltinAttributeShadowing(name.to_string()),
}, },
location, Range::from_located(located),
)) ))
} else { } else {
None None

View file

@ -1,7 +1,9 @@
use rustpython_ast::{ExcepthandlerKind, ExprKind, Stmt, StmtKind}; use rustpython_ast::{ExcepthandlerKind, ExprKind, Stmt, StmtKind};
use crate::ast::types::Range; use crate::ast::helpers::identifier_range;
use crate::checks::{Check, CheckKind}; use crate::checks::{Check, CheckKind};
use crate::source_code_locator::SourceCodeLocator;
fn get_complexity_number(stmts: &[Stmt]) -> usize { fn get_complexity_number(stmts: &[Stmt]) -> usize {
let mut complexity = 0; let mut complexity = 0;
@ -59,12 +61,13 @@ pub fn function_is_too_complex(
name: &str, name: &str,
body: &[Stmt], body: &[Stmt],
max_complexity: usize, max_complexity: usize,
locator: &SourceCodeLocator,
) -> Option<Check> { ) -> Option<Check> {
let complexity = get_complexity_number(body) + 1; let complexity = get_complexity_number(body) + 1;
if complexity > max_complexity { if complexity > max_complexity {
Some(Check::new( Some(Check::new(
CheckKind::FunctionIsTooComplex(name.to_string(), complexity), CheckKind::FunctionIsTooComplex(name.to_string(), complexity),
Range::from_located(stmt), identifier_range(stmt, locator),
)) ))
} else { } else {
None None

View file

@ -8,10 +8,10 @@ expression: checks
- 1 - 1
location: location:
row: 2 row: 2
column: 0 column: 4
end_location: end_location:
row: 3 row: 2
column: 8 column: 11
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -19,10 +19,10 @@ expression: checks
- 1 - 1
location: location:
row: 7 row: 7
column: 0 column: 4
end_location: end_location:
row: 8 row: 7
column: 10 column: 21
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -30,10 +30,10 @@ expression: checks
- 1 - 1
location: location:
row: 12 row: 12
column: 0 column: 4
end_location: end_location:
row: 15 row: 12
column: 12 column: 14
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -41,10 +41,10 @@ expression: checks
- 3 - 3
location: location:
row: 19 row: 19
column: 0 column: 4
end_location: end_location:
row: 25 row: 19
column: 47 column: 26
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -52,10 +52,10 @@ expression: checks
- 3 - 3
location: location:
row: 29 row: 29
column: 0 column: 4
end_location: end_location:
row: 36 row: 29
column: 47 column: 14
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -63,10 +63,10 @@ expression: checks
- 2 - 2
location: location:
row: 40 row: 40
column: 0 column: 4
end_location: end_location:
row: 42 row: 40
column: 16 column: 12
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -74,10 +74,10 @@ expression: checks
- 2 - 2
location: location:
row: 46 row: 46
column: 0 column: 4
end_location: end_location:
row: 50 row: 46
column: 19 column: 12
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -85,10 +85,10 @@ expression: checks
- 2 - 2
location: location:
row: 54 row: 54
column: 0 column: 4
end_location: end_location:
row: 58 row: 54
column: 16 column: 13
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -96,10 +96,10 @@ expression: checks
- 3 - 3
location: location:
row: 62 row: 62
column: 0 column: 4
end_location: end_location:
row: 69 row: 62
column: 7 column: 20
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -107,10 +107,10 @@ expression: checks
- 2 - 2
location: location:
row: 63 row: 63
column: 4 column: 8
end_location: end_location:
row: 67 row: 63
column: 11 column: 9
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -118,10 +118,10 @@ expression: checks
- 1 - 1
location: location:
row: 64 row: 64
column: 8 column: 12
end_location: end_location:
row: 65 row: 64
column: 16 column: 13
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -129,10 +129,10 @@ expression: checks
- 4 - 4
location: location:
row: 73 row: 73
column: 0 column: 4
end_location: end_location:
row: 81 row: 73
column: 16 column: 12
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -140,10 +140,10 @@ expression: checks
- 3 - 3
location: location:
row: 85 row: 85
column: 0 column: 4
end_location: end_location:
row: 92 row: 85
column: 16 column: 22
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -151,10 +151,10 @@ expression: checks
- 3 - 3
location: location:
row: 96 row: 96
column: 0 column: 10
end_location: end_location:
row: 103 row: 96
column: 12 column: 16
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -162,10 +162,10 @@ expression: checks
- 1 - 1
location: location:
row: 107 row: 107
column: 0 column: 4
end_location: end_location:
row: 108 row: 107
column: 17 column: 20
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -173,10 +173,10 @@ expression: checks
- 9 - 9
location: location:
row: 113 row: 113
column: 4 column: 8
end_location: end_location:
row: 138 row: 113
column: 40 column: 14
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -184,10 +184,10 @@ expression: checks
- 1 - 1
location: location:
row: 118 row: 118
column: 12 column: 16
end_location: end_location:
row: 119 row: 118
column: 20 column: 17
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -195,10 +195,10 @@ expression: checks
- 2 - 2
location: location:
row: 121 row: 121
column: 12 column: 16
end_location: end_location:
row: 123 row: 121
column: 24 column: 17
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -206,10 +206,10 @@ expression: checks
- 1 - 1
location: location:
row: 126 row: 126
column: 12 column: 16
end_location: end_location:
row: 127 row: 126
column: 20 column: 17
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -217,10 +217,10 @@ expression: checks
- 1 - 1
location: location:
row: 129 row: 129
column: 12 column: 16
end_location: end_location:
row: 130 row: 129
column: 20 column: 21
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -228,9 +228,9 @@ expression: checks
- 1 - 1
location: location:
row: 132 row: 132
column: 12 column: 16
end_location: end_location:
row: 133 row: 132
column: 20 column: 20
fix: ~ fix: ~
- kind: - kind:
@ -239,9 +239,9 @@ expression: checks
- 1 - 1
location: location:
row: 135 row: 135
column: 12 column: 16
end_location: end_location:
row: 136 row: 135
column: 20 column: 25
fix: ~ fix: ~

View file

@ -8,10 +8,10 @@ expression: checks
- 4 - 4
location: location:
row: 73 row: 73
column: 0 column: 4
end_location: end_location:
row: 81 row: 73
column: 16 column: 12
fix: ~ fix: ~
- kind: - kind:
FunctionIsTooComplex: FunctionIsTooComplex:
@ -19,9 +19,9 @@ expression: checks
- 9 - 9
location: location:
row: 113 row: 113
column: 4 column: 8
end_location: end_location:
row: 138 row: 113
column: 40 column: 14
fix: ~ fix: ~

View file

@ -1,47 +1,53 @@
use rustc_hash::{FxHashMap, FxHashSet}; use rustc_hash::{FxHashMap, FxHashSet};
use rustpython_ast::{Arguments, Expr, ExprKind, Stmt}; use rustpython_ast::{Arg, Arguments, Expr, ExprKind, Stmt};
use crate::ast::function_type; use crate::ast::function_type;
use crate::ast::helpers::identifier_range;
use crate::ast::types::{Range, Scope, ScopeKind}; use crate::ast::types::{Range, Scope, ScopeKind};
use crate::checks::{Check, CheckKind}; use crate::checks::{Check, CheckKind};
use crate::pep8_naming::helpers; use crate::pep8_naming::helpers;
use crate::pep8_naming::settings::Settings; use crate::pep8_naming::settings::Settings;
use crate::python::string::{self}; use crate::python::string::{self};
use crate::source_code_locator::SourceCodeLocator;
/// N801 /// N801
pub fn invalid_class_name(class_def: &Stmt, name: &str) -> Option<Check> { pub fn invalid_class_name(
class_def: &Stmt,
name: &str,
locator: &SourceCodeLocator,
) -> Option<Check> {
let stripped = name.strip_prefix('_').unwrap_or(name); let stripped = name.strip_prefix('_').unwrap_or(name);
if !stripped.chars().next().map_or(false, char::is_uppercase) || stripped.contains('_') { if !stripped.chars().next().map_or(false, char::is_uppercase) || stripped.contains('_') {
return Some(Check::new( return Some(Check::new(
CheckKind::InvalidClassName(name.to_string()), CheckKind::InvalidClassName(name.to_string()),
Range::from_located(class_def), identifier_range(class_def, locator),
)); ));
} }
None None
} }
/// N802 /// N802
pub fn invalid_function_name(func_def: &Stmt, name: &str, settings: &Settings) -> Option<Check> { pub fn invalid_function_name(
if name.to_lowercase() != name func_def: &Stmt,
&& !settings name: &str,
.ignore_names ignore_names: &[String],
.iter() locator: &SourceCodeLocator,
.any(|ignore_name| ignore_name == name) ) -> Option<Check> {
{ if name.to_lowercase() != name && ignore_names.iter().any(|ignore_name| ignore_name == name) {
return Some(Check::new( return Some(Check::new(
CheckKind::InvalidFunctionName(name.to_string()), CheckKind::InvalidFunctionName(name.to_string()),
Range::from_located(func_def), identifier_range(func_def, locator),
)); ));
} }
None None
} }
/// N803 /// N803
pub fn invalid_argument_name(name: &str, location: Range) -> Option<Check> { pub fn invalid_argument_name(name: &str, arg: &Arg) -> Option<Check> {
if name.to_lowercase() != name { if name.to_lowercase() != name {
return Some(Check::new( return Some(Check::new(
CheckKind::InvalidArgumentName(name.to_string()), CheckKind::InvalidArgumentName(name.to_string()),
location, Range::from_located(arg),
)); ));
} }
None None
@ -124,7 +130,12 @@ pub fn invalid_first_argument_name_for_method(
} }
/// N807 /// N807
pub fn dunder_function_name(scope: &Scope, stmt: &Stmt, name: &str) -> Option<Check> { pub fn dunder_function_name(
scope: &Scope,
stmt: &Stmt,
name: &str,
locator: &SourceCodeLocator,
) -> Option<Check> {
if matches!(scope.kind, ScopeKind::Class(_)) { if matches!(scope.kind, ScopeKind::Class(_)) {
return None; return None;
} }
@ -138,7 +149,7 @@ pub fn dunder_function_name(scope: &Scope, stmt: &Stmt, name: &str) -> Option<Ch
Some(Check::new( Some(Check::new(
CheckKind::DunderFunctionName, CheckKind::DunderFunctionName,
Range::from_located(stmt), identifier_range(stmt, locator),
)) ))
} }
@ -147,11 +158,12 @@ pub fn constant_imported_as_non_constant(
import_from: &Stmt, import_from: &Stmt,
name: &str, name: &str,
asname: &str, asname: &str,
locator: &SourceCodeLocator,
) -> Option<Check> { ) -> Option<Check> {
if string::is_upper(name) && !string::is_upper(asname) { if string::is_upper(name) && !string::is_upper(asname) {
return Some(Check::new( return Some(Check::new(
CheckKind::ConstantImportedAsNonConstant(name.to_string(), asname.to_string()), CheckKind::ConstantImportedAsNonConstant(name.to_string(), asname.to_string()),
Range::from_located(import_from), identifier_range(import_from, locator),
)); ));
} }
None None
@ -162,11 +174,12 @@ pub fn lowercase_imported_as_non_lowercase(
import_from: &Stmt, import_from: &Stmt,
name: &str, name: &str,
asname: &str, asname: &str,
locator: &SourceCodeLocator,
) -> Option<Check> { ) -> Option<Check> {
if !string::is_upper(name) && string::is_lower(name) && asname.to_lowercase() != asname { if !string::is_upper(name) && string::is_lower(name) && asname.to_lowercase() != asname {
return Some(Check::new( return Some(Check::new(
CheckKind::LowercaseImportedAsNonLowercase(name.to_string(), asname.to_string()), CheckKind::LowercaseImportedAsNonLowercase(name.to_string(), asname.to_string()),
Range::from_located(import_from), identifier_range(import_from, locator),
)); ));
} }
None None
@ -177,11 +190,12 @@ pub fn camelcase_imported_as_lowercase(
import_from: &Stmt, import_from: &Stmt,
name: &str, name: &str,
asname: &str, asname: &str,
locator: &SourceCodeLocator,
) -> Option<Check> { ) -> Option<Check> {
if helpers::is_camelcase(name) && string::is_lower(asname) { if helpers::is_camelcase(name) && string::is_lower(asname) {
return Some(Check::new( return Some(Check::new(
CheckKind::CamelcaseImportedAsLowercase(name.to_string(), asname.to_string()), CheckKind::CamelcaseImportedAsLowercase(name.to_string(), asname.to_string()),
Range::from_located(import_from), identifier_range(import_from, locator),
)); ));
} }
None None
@ -192,6 +206,7 @@ pub fn camelcase_imported_as_constant(
import_from: &Stmt, import_from: &Stmt,
name: &str, name: &str,
asname: &str, asname: &str,
locator: &SourceCodeLocator,
) -> Option<Check> { ) -> Option<Check> {
if helpers::is_camelcase(name) if helpers::is_camelcase(name)
&& !string::is_lower(asname) && !string::is_lower(asname)
@ -200,7 +215,7 @@ pub fn camelcase_imported_as_constant(
{ {
return Some(Check::new( return Some(Check::new(
CheckKind::CamelcaseImportedAsConstant(name.to_string(), asname.to_string()), CheckKind::CamelcaseImportedAsConstant(name.to_string(), asname.to_string()),
Range::from_located(import_from), identifier_range(import_from, locator),
)); ));
} }
None None
@ -211,6 +226,7 @@ pub fn camelcase_imported_as_acronym(
import_from: &Stmt, import_from: &Stmt,
name: &str, name: &str,
asname: &str, asname: &str,
locator: &SourceCodeLocator,
) -> Option<Check> { ) -> Option<Check> {
if helpers::is_camelcase(name) if helpers::is_camelcase(name)
&& !string::is_lower(asname) && !string::is_lower(asname)
@ -219,7 +235,7 @@ pub fn camelcase_imported_as_acronym(
{ {
return Some(Check::new( return Some(Check::new(
CheckKind::CamelcaseImportedAsAcronym(name.to_string(), asname.to_string()), CheckKind::CamelcaseImportedAsAcronym(name.to_string(), asname.to_string()),
Range::from_located(import_from), identifier_range(import_from, locator),
)); ));
} }
None None

View file

@ -6,45 +6,45 @@ expression: checks
InvalidClassName: bad InvalidClassName: bad
location: location:
row: 1 row: 1
column: 0 column: 6
end_location: end_location:
row: 2 row: 1
column: 8 column: 9
fix: ~ fix: ~
- kind: - kind:
InvalidClassName: _bad InvalidClassName: _bad
location: location:
row: 5 row: 5
column: 0 column: 6
end_location: end_location:
row: 6 row: 5
column: 8 column: 10
fix: ~ fix: ~
- kind: - kind:
InvalidClassName: bad_class InvalidClassName: bad_class
location: location:
row: 9 row: 9
column: 0 column: 6
end_location: end_location:
row: 10 row: 9
column: 8 column: 15
fix: ~ fix: ~
- kind: - kind:
InvalidClassName: Bad_Class InvalidClassName: Bad_Class
location: location:
row: 13 row: 13
column: 0 column: 6
end_location: end_location:
row: 14 row: 13
column: 8 column: 15
fix: ~ fix: ~
- kind: - kind:
InvalidClassName: BAD_CLASS InvalidClassName: BAD_CLASS
location: location:
row: 17 row: 17
column: 0 column: 6
end_location: end_location:
row: 18 row: 17
column: 8 column: 15
fix: ~ fix: ~

View file

@ -3,48 +3,21 @@ source: src/pep8_naming/mod.rs
expression: checks expression: checks
--- ---
- kind: - kind:
InvalidFunctionName: Bad InvalidFunctionName: tearDownModule
location: location:
row: 4 row: 32
column: 0
end_location:
row: 5
column: 8
fix: ~
- kind:
InvalidFunctionName: _Bad
location:
row: 8
column: 0
end_location:
row: 9
column: 8
fix: ~
- kind:
InvalidFunctionName: BAD
location:
row: 12
column: 0
end_location:
row: 13
column: 8
fix: ~
- kind:
InvalidFunctionName: BAD_FUNC
location:
row: 16
column: 0
end_location:
row: 17
column: 8
fix: ~
- kind:
InvalidFunctionName: testTest
location:
row: 40
column: 4 column: 4
end_location: end_location:
row: 41 row: 32
column: 19 column: 18
fix: ~
- kind:
InvalidFunctionName: tearDown
location:
row: 37
column: 8
end_location:
row: 37
column: 16
fix: ~ fix: ~

View file

@ -5,17 +5,17 @@ expression: checks
- kind: DunderFunctionName - kind: DunderFunctionName
location: location:
row: 1 row: 1
column: 0 column: 4
end_location: end_location:
row: 2 row: 1
column: 8 column: 11
fix: ~ fix: ~
- kind: DunderFunctionName - kind: DunderFunctionName
location: location:
row: 14 row: 14
column: 4 column: 8
end_location: end_location:
row: 15 row: 14
column: 12 column: 15
fix: ~ fix: ~

View file

@ -1,5 +1,5 @@
use itertools::izip; use itertools::izip;
use rustpython_ast::{Location, Stmt, StmtKind}; use rustpython_ast::{Located, Location, Stmt, StmtKind};
use rustpython_parser::ast::{Cmpop, Expr, ExprKind}; use rustpython_parser::ast::{Cmpop, Expr, ExprKind};
use crate::ast::types::Range; use crate::ast::types::Range;
@ -65,11 +65,11 @@ fn is_ambiguous_name(name: &str) -> bool {
} }
/// E741 /// E741
pub fn ambiguous_variable_name(name: &str, location: Range) -> Option<Check> { pub fn ambiguous_variable_name<T>(name: &str, located: &Located<T>) -> Option<Check> {
if is_ambiguous_name(name) { if is_ambiguous_name(name) {
Some(Check::new( Some(Check::new(
CheckKind::AmbiguousVariableName(name.to_string()), CheckKind::AmbiguousVariableName(name.to_string()),
location, Range::from_located(located),
)) ))
} else { } else {
None None