diff --git a/src/ast.rs b/src/ast.rs index d0fc905411..3b9a2fcd32 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1,4 +1,3 @@ -pub mod checkers; pub mod helpers; pub mod operations; pub mod relocate; diff --git a/src/ast/checkers.rs b/src/ast/checkers.rs deleted file mode 100644 index 1d28cbef2d..0000000000 --- a/src/ast/checkers.rs +++ /dev/null @@ -1,1424 +0,0 @@ -use std::collections::BTreeSet; - -use itertools::izip; -use num_bigint::BigInt; -use regex::Regex; -use rustpython_parser::ast::{ - Arg, ArgData, Arguments, Cmpop, Comprehension, Constant, Excepthandler, ExcepthandlerKind, - Expr, ExprKind, KeywordData, Located, Stmt, StmtKind, Unaryop, -}; -use serde::{Deserialize, Serialize}; - -use crate::ast::helpers; -use crate::ast::types::{ - Binding, BindingKind, CheckLocator, FunctionScope, Range, Scope, ScopeKind, -}; -use crate::checks::{Check, CheckKind, RejectedCmpop}; -use crate::python::builtins::BUILTINS; - -/// Check IfTuple compliance. -pub fn if_tuple(test: &Expr, location: Range) -> Option { - if let ExprKind::Tuple { elts, .. } = &test.node { - if !elts.is_empty() { - return Some(Check::new(CheckKind::IfTuple, location)); - } - } - None -} - -/// Check AssertTuple compliance. -pub fn assert_tuple(test: &Expr, location: Range) -> Option { - if let ExprKind::Tuple { elts, .. } = &test.node { - if !elts.is_empty() { - return Some(Check::new(CheckKind::AssertTuple, location)); - } - } - None -} - -/// Check NotInTest and NotIsTest compliance. -pub fn not_tests( - op: &Unaryop, - operand: &Expr, - check_not_in: bool, - check_not_is: bool, - locator: &dyn CheckLocator, -) -> Vec { - let mut checks: Vec = vec![]; - - if matches!(op, Unaryop::Not) { - if let ExprKind::Compare { ops, .. } = &operand.node { - for op in ops { - match op { - Cmpop::In => { - if check_not_in { - checks.push(Check::new( - CheckKind::NotInTest, - locator.locate_check(Range::from_located(operand)), - )); - } - } - Cmpop::Is => { - if check_not_is { - checks.push(Check::new( - CheckKind::NotIsTest, - locator.locate_check(Range::from_located(operand)), - )); - } - } - _ => {} - } - } - } - } - - checks -} - -/// Check UnusedVariable compliance. -pub fn unused_variables( - scope: &Scope, - locator: &dyn CheckLocator, - dummy_variable_rgx: &Regex, -) -> Vec { - let mut checks: Vec = vec![]; - - if matches!( - scope.kind, - ScopeKind::Function(FunctionScope { uses_locals: true }) - ) { - return checks; - } - - for (name, binding) in scope.values.iter() { - if binding.used.is_none() - && matches!(binding.kind, BindingKind::Assignment) - && !dummy_variable_rgx.is_match(name) - && name != "__tracebackhide__" - && name != "__traceback_info__" - && name != "__traceback_supplement__" - { - checks.push(Check::new( - CheckKind::UnusedVariable(name.to_string()), - locator.locate_check(binding.range), - )); - } - } - - checks -} - -/// Check DoNotAssignLambda compliance. -pub fn do_not_assign_lambda(value: &Expr, location: Range) -> Option { - if let ExprKind::Lambda { .. } = &value.node { - Some(Check::new(CheckKind::DoNotAssignLambda, location)) - } else { - None - } -} - -/// Check UselessMetaclassType compliance. -pub fn useless_metaclass_type(targets: &[Expr], value: &Expr, location: Range) -> Option { - if targets.len() == 1 { - if let ExprKind::Name { id, .. } = targets.first().map(|expr| &expr.node).unwrap() { - if id == "__metaclass__" { - if let ExprKind::Name { id, .. } = &value.node { - if id == "type" { - return Some(Check::new(CheckKind::UselessMetaclassType, location)); - } - } - } - } - } - None -} - -/// Check UnnecessaryAbspath compliance. -pub fn unnecessary_abspath(func: &Expr, args: &[Expr], location: Range) -> Option { - // Validate the arguments. - if args.len() == 1 { - if let ExprKind::Name { id, .. } = &args[0].node { - if id == "__file__" { - match &func.node { - ExprKind::Attribute { attr: id, .. } | ExprKind::Name { id, .. } => { - if id == "abspath" { - return Some(Check::new(CheckKind::UnnecessaryAbspath, location)); - } - } - _ => {} - } - } - } - } - - None -} - -#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] -pub enum Primitive { - Bool, - Str, - Bytes, - Int, - Float, - Complex, -} - -impl Primitive { - fn from_constant(constant: &Constant) -> Option { - match constant { - Constant::Bool(_) => Some(Primitive::Bool), - Constant::Str(_) => Some(Primitive::Str), - Constant::Bytes(_) => Some(Primitive::Bytes), - Constant::Int(_) => Some(Primitive::Int), - Constant::Float(_) => Some(Primitive::Float), - Constant::Complex { .. } => Some(Primitive::Complex), - _ => None, - } - } - - pub fn builtin(&self) -> String { - match self { - Primitive::Bool => "bool".to_string(), - Primitive::Str => "str".to_string(), - Primitive::Bytes => "bytes".to_string(), - Primitive::Int => "int".to_string(), - Primitive::Float => "float".to_string(), - Primitive::Complex => "complex".to_string(), - } - } -} - -/// Check TypeOfPrimitive compliance. -pub fn type_of_primitive(func: &Expr, args: &[Expr], location: Range) -> Option { - // Validate the arguments. - if args.len() == 1 { - match &func.node { - ExprKind::Attribute { attr: id, .. } | ExprKind::Name { id, .. } => { - if id == "type" { - if let ExprKind::Constant { value, .. } = &args[0].node { - if let Some(primitive) = Primitive::from_constant(value) { - return Some(Check::new( - CheckKind::TypeOfPrimitive(primitive), - location, - )); - } - } - } - } - _ => {} - } - } - - None -} - -fn is_ambiguous_name(name: &str) -> bool { - name == "l" || name == "I" || name == "O" -} - -/// Check AmbiguousVariableName compliance. -pub fn ambiguous_variable_name(name: &str, location: Range) -> Option { - if is_ambiguous_name(name) { - Some(Check::new( - CheckKind::AmbiguousVariableName(name.to_string()), - location, - )) - } else { - None - } -} - -/// Check AmbiguousClassName compliance. -pub fn ambiguous_class_name(name: &str, location: Range) -> Option { - if is_ambiguous_name(name) { - Some(Check::new( - CheckKind::AmbiguousClassName(name.to_string()), - location, - )) - } else { - None - } -} - -/// Check AmbiguousFunctionName compliance. -pub fn ambiguous_function_name(name: &str, location: Range) -> Option { - if is_ambiguous_name(name) { - Some(Check::new( - CheckKind::AmbiguousFunctionName(name.to_string()), - location, - )) - } else { - None - } -} - -/// Check UselessObjectInheritance compliance. -pub fn useless_object_inheritance(name: &str, bases: &[Expr], scope: &Scope) -> Option { - for expr in bases { - if let ExprKind::Name { id, .. } = &expr.node { - if id == "object" { - match scope.values.get(id) { - None - | Some(Binding { - kind: BindingKind::Builtin, - .. - }) => { - return Some(Check::new( - CheckKind::UselessObjectInheritance(name.to_string()), - Range::from_located(expr), - )); - } - _ => {} - } - } - } - } - - None -} - -/// Check DefaultExceptNotLast compliance. -pub fn default_except_not_last(handlers: &[Excepthandler]) -> Option { - for (idx, handler) in handlers.iter().enumerate() { - let ExcepthandlerKind::ExceptHandler { type_, .. } = &handler.node; - if type_.is_none() && idx < handlers.len() - 1 { - return Some(Check::new( - CheckKind::DefaultExceptNotLast, - Range::from_located(handler), - )); - } - } - - None -} - -/// Check RaiseNotImplemented compliance. -pub fn raise_not_implemented(expr: &Expr) -> Option { - match &expr.node { - ExprKind::Call { func, .. } => { - if let ExprKind::Name { id, .. } = &func.node { - if id == "NotImplemented" { - return Some(Check::new( - CheckKind::RaiseNotImplemented, - Range::from_located(expr), - )); - } - } - } - ExprKind::Name { id, .. } => { - if id == "NotImplemented" { - return Some(Check::new( - CheckKind::RaiseNotImplemented, - Range::from_located(expr), - )); - } - } - _ => {} - } - - None -} - -/// Check DuplicateArgumentName compliance. -pub fn duplicate_arguments(arguments: &Arguments) -> Vec { - let mut checks: Vec = vec![]; - - // Collect all the arguments into a single vector. - let mut all_arguments: Vec<&Arg> = arguments - .args - .iter() - .chain(arguments.posonlyargs.iter()) - .chain(arguments.kwonlyargs.iter()) - .collect(); - if let Some(arg) = &arguments.vararg { - all_arguments.push(arg); - } - if let Some(arg) = &arguments.kwarg { - all_arguments.push(arg); - } - - // Search for duplicates. - let mut idents: BTreeSet<&str> = BTreeSet::new(); - for arg in all_arguments { - let ident = &arg.node.arg; - if idents.contains(ident.as_str()) { - checks.push(Check::new( - CheckKind::DuplicateArgumentName, - Range::from_located(arg), - )); - } - idents.insert(ident); - } - - checks -} - -#[derive(Debug, PartialEq)] -enum DictionaryKey<'a> { - Constant(&'a Constant), - Variable(&'a String), -} - -fn convert_to_value(expr: &Expr) -> Option { - match &expr.node { - ExprKind::Constant { value, .. } => Some(DictionaryKey::Constant(value)), - ExprKind::Name { id, .. } => Some(DictionaryKey::Variable(id)), - _ => None, - } -} - -/// Check MultiValueRepeatedKeyLiteral and MultiValueRepeatedKeyVariable compliance. -pub fn repeated_keys( - keys: &[Expr], - check_repeated_literals: bool, - check_repeated_variables: bool, - locator: &dyn CheckLocator, -) -> Vec { - let mut checks: Vec = vec![]; - - let num_keys = keys.len(); - for i in 0..num_keys { - let k1 = &keys[i]; - let v1 = convert_to_value(k1); - for k2 in keys.iter().take(num_keys).skip(i + 1) { - let v2 = convert_to_value(k2); - match (&v1, &v2) { - (Some(DictionaryKey::Constant(v1)), Some(DictionaryKey::Constant(v2))) => { - if check_repeated_literals && v1 == v2 { - checks.push(Check::new( - CheckKind::MultiValueRepeatedKeyLiteral, - locator.locate_check(Range::from_located(k2)), - )) - } - } - (Some(DictionaryKey::Variable(v1)), Some(DictionaryKey::Variable(v2))) => { - if check_repeated_variables && v1 == v2 { - checks.push(Check::new( - CheckKind::MultiValueRepeatedKeyVariable((*v2).to_string()), - locator.locate_check(Range::from_located(k2)), - )) - } - } - _ => {} - } - } - } - - checks -} - -/// Check TrueFalseComparison and NoneComparison compliance. -pub fn literal_comparisons( - left: &Expr, - ops: &[Cmpop], - comparators: &[Expr], - check_none_comparisons: bool, - check_true_false_comparisons: bool, - locator: &dyn CheckLocator, -) -> Vec { - let mut checks: Vec = vec![]; - - let op = ops.first().unwrap(); - let comparator = left; - - // Check `left`. - if check_none_comparisons - && matches!( - comparator.node, - ExprKind::Constant { - value: Constant::None, - kind: None - } - ) - { - if matches!(op, Cmpop::Eq) { - checks.push(Check::new( - CheckKind::NoneComparison(RejectedCmpop::Eq), - locator.locate_check(Range::from_located(comparator)), - )); - } - if matches!(op, Cmpop::NotEq) { - checks.push(Check::new( - CheckKind::NoneComparison(RejectedCmpop::NotEq), - locator.locate_check(Range::from_located(comparator)), - )); - } - } - - if check_true_false_comparisons { - if let ExprKind::Constant { - value: Constant::Bool(value), - kind: None, - } = comparator.node - { - if matches!(op, Cmpop::Eq) { - checks.push(Check::new( - CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq), - locator.locate_check(Range::from_located(comparator)), - )); - } - if matches!(op, Cmpop::NotEq) { - checks.push(Check::new( - CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq), - locator.locate_check(Range::from_located(comparator)), - )); - } - } - } - - // Check each comparator in order. - for (op, comparator) in izip!(ops, comparators) { - if check_none_comparisons - && matches!( - comparator.node, - ExprKind::Constant { - value: Constant::None, - kind: None - } - ) - { - if matches!(op, Cmpop::Eq) { - checks.push(Check::new( - CheckKind::NoneComparison(RejectedCmpop::Eq), - locator.locate_check(Range::from_located(comparator)), - )); - } - if matches!(op, Cmpop::NotEq) { - checks.push(Check::new( - CheckKind::NoneComparison(RejectedCmpop::NotEq), - locator.locate_check(Range::from_located(comparator)), - )); - } - } - - if check_true_false_comparisons { - if let ExprKind::Constant { - value: Constant::Bool(value), - kind: None, - } = comparator.node - { - if matches!(op, Cmpop::Eq) { - checks.push(Check::new( - CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq), - locator.locate_check(Range::from_located(comparator)), - )); - } - if matches!(op, Cmpop::NotEq) { - checks.push(Check::new( - CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq), - locator.locate_check(Range::from_located(comparator)), - )); - } - } - } - } - - checks -} - -fn is_constant(expr: &Expr) -> bool { - match &expr.node { - ExprKind::Constant { .. } => true, - ExprKind::Tuple { elts, .. } => elts.iter().all(is_constant), - _ => false, - } -} - -fn is_singleton(expr: &Expr) -> bool { - matches!( - expr.node, - ExprKind::Constant { - value: Constant::None | Constant::Bool(_) | Constant::Ellipsis, - .. - } - ) -} - -fn is_constant_non_singleton(expr: &Expr) -> bool { - is_constant(expr) && !is_singleton(expr) -} - -/// Check IsLiteral compliance. -pub fn is_literal(left: &Expr, ops: &[Cmpop], comparators: &[Expr], location: Range) -> Vec { - let mut checks: Vec = vec![]; - - let mut left = left; - for (op, right) in izip!(ops, comparators) { - if matches!(op, Cmpop::Is | Cmpop::IsNot) - && (is_constant_non_singleton(left) || is_constant_non_singleton(right)) - { - checks.push(Check::new(CheckKind::IsLiteral, location)); - } - left = right; - } - - checks -} - -/// Check TypeComparison compliance. -pub fn type_comparison(ops: &[Cmpop], comparators: &[Expr], location: Range) -> Vec { - let mut checks: Vec = vec![]; - - for (op, right) in izip!(ops, comparators) { - if matches!(op, Cmpop::Is | Cmpop::IsNot | Cmpop::Eq | Cmpop::NotEq) { - match &right.node { - ExprKind::Call { func, args, .. } => { - if let ExprKind::Name { id, .. } = &func.node { - // Ex) type(False) - if id == "type" { - if let Some(arg) = args.first() { - // Allow comparison for types which are not obvious. - if !matches!(arg.node, ExprKind::Name { .. }) { - checks.push(Check::new(CheckKind::TypeComparison, location)); - } - } - } - } - } - ExprKind::Attribute { value, .. } => { - if let ExprKind::Name { id, .. } = &value.node { - // Ex) types.IntType - if id == "types" { - checks.push(Check::new(CheckKind::TypeComparison, location)); - } - } - } - _ => {} - } - } - } - - checks -} - -/// Check TwoStarredExpressions and TooManyExpressionsInStarredAssignment compliance. -pub fn starred_expressions( - elts: &[Expr], - check_too_many_expressions: bool, - check_two_starred_expressions: bool, - location: Range, -) -> Option { - let mut has_starred: bool = false; - let mut starred_index: Option = None; - for (index, elt) in elts.iter().enumerate() { - if matches!(elt.node, ExprKind::Starred { .. }) { - if has_starred && check_two_starred_expressions { - return Some(Check::new(CheckKind::TwoStarredExpressions, location)); - } - has_starred = true; - starred_index = Some(index); - } - } - - if check_too_many_expressions { - if let Some(starred_index) = starred_index { - if starred_index >= 1 << 8 || elts.len() - starred_index > 1 << 24 { - return Some(Check::new(CheckKind::ExpressionsInStarAssignment, location)); - } - } - } - - None -} - -/// Check BreakOutsideLoop compliance. -pub fn break_outside_loop( - stmt: &Stmt, - parents: &[&Stmt], - parent_stack: &[usize], - locator: &dyn CheckLocator, -) -> Option { - let mut allowed: bool = false; - let mut parent = stmt; - for index in parent_stack.iter().rev() { - let child = parent; - parent = parents[*index]; - match &parent.node { - StmtKind::For { orelse, .. } - | StmtKind::AsyncFor { orelse, .. } - | StmtKind::While { orelse, .. } => { - if !orelse.contains(child) { - allowed = true; - break; - } - } - - StmtKind::FunctionDef { .. } - | StmtKind::AsyncFunctionDef { .. } - | StmtKind::ClassDef { .. } => { - break; - } - _ => {} - } - } - - if !allowed { - Some(Check::new( - CheckKind::BreakOutsideLoop, - locator.locate_check(Range::from_located(stmt)), - )) - } else { - None - } -} - -/// Check ContinueOutsideLoop compliance. -pub fn continue_outside_loop( - stmt: &Stmt, - parents: &[&Stmt], - parent_stack: &[usize], - locator: &dyn CheckLocator, -) -> Option { - let mut allowed: bool = false; - let mut parent = stmt; - for index in parent_stack.iter().rev() { - let child = parent; - parent = parents[*index]; - match &parent.node { - StmtKind::For { orelse, .. } - | StmtKind::AsyncFor { orelse, .. } - | StmtKind::While { orelse, .. } => { - if !orelse.contains(child) { - allowed = true; - break; - } - } - - StmtKind::FunctionDef { .. } - | StmtKind::AsyncFunctionDef { .. } - | StmtKind::ClassDef { .. } => { - break; - } - _ => {} - } - } - - if !allowed { - Some(Check::new( - CheckKind::ContinueOutsideLoop, - locator.locate_check(Range::from_located(stmt)), - )) - } else { - None - } -} - -// flake8-builtins -pub enum ShadowingType { - Variable, - Argument, - Attribute, -} - -/// Check builtin name shadowing -pub fn builtin_shadowing(name: &str, location: Range, node_type: ShadowingType) -> Option { - if BUILTINS.contains(&name) { - Some(Check::new( - match node_type { - ShadowingType::Variable => CheckKind::BuiltinVariableShadowing(name.to_string()), - ShadowingType::Argument => CheckKind::BuiltinArgumentShadowing(name.to_string()), - ShadowingType::Attribute => CheckKind::BuiltinAttributeShadowing(name.to_string()), - }, - location, - )) - } else { - None - } -} - -// flake8-comprehensions -/// Check `list(generator)` compliance. -pub fn unnecessary_generator_list(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { - if args.len() == 1 { - if let ExprKind::Name { id, .. } = &func.node { - if id == "list" { - if let ExprKind::GeneratorExp { .. } = &args[0].node { - return Some(Check::new( - CheckKind::UnnecessaryGeneratorList, - Range::from_located(expr), - )); - } - } - } - } - None -} - -/// Check `set(generator)` compliance. -pub fn unnecessary_generator_set(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { - if args.len() == 1 { - if let ExprKind::Name { id, .. } = &func.node { - if id == "set" { - if let ExprKind::GeneratorExp { .. } = &args[0].node { - return Some(Check::new( - CheckKind::UnnecessaryGeneratorSet, - Range::from_located(expr), - )); - } - } - } - } - None -} - -/// Check `dict((x, y) for x, y in iterable)` compliance. -pub fn unnecessary_generator_dict(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { - if args.len() == 1 { - if let ExprKind::Name { id, .. } = &func.node { - if id == "dict" { - if let ExprKind::GeneratorExp { elt, .. } = &args[0].node { - match &elt.node { - ExprKind::Tuple { elts, .. } if elts.len() == 2 => { - return Some(Check::new( - CheckKind::UnnecessaryGeneratorDict, - Range::from_located(expr), - )); - } - _ => {} - } - } - } - } - } - None -} - -/// Check `set([...])` compliance. -pub fn unnecessary_list_comprehension_set( - expr: &Expr, - func: &Expr, - args: &[Expr], -) -> Option { - if args.len() == 1 { - if let ExprKind::Name { id, .. } = &func.node { - if id == "set" { - if let ExprKind::ListComp { .. } = &args[0].node { - return Some(Check::new( - CheckKind::UnnecessaryListComprehensionSet, - Range::from_located(expr), - )); - } - } - } - } - None -} - -/// Check `dict([...])` compliance. -pub fn unnecessary_list_comprehension_dict( - expr: &Expr, - func: &Expr, - args: &[Expr], -) -> Option { - if args.len() == 1 { - if let ExprKind::Name { id, .. } = &func.node { - if id == "dict" { - if let ExprKind::ListComp { elt, .. } = &args[0].node { - match &elt.node { - ExprKind::Tuple { elts, .. } if elts.len() == 2 => { - return Some(Check::new( - CheckKind::UnnecessaryListComprehensionDict, - Range::from_located(expr), - )); - } - _ => {} - } - } - } - } - } - None -} - -/// Check `set([1, 2])` compliance. -pub fn unnecessary_literal_set(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { - if args.len() == 1 { - if let ExprKind::Name { id, .. } = &func.node { - if id == "set" { - match &args[0].node { - ExprKind::List { .. } => { - return Some(Check::new( - CheckKind::UnnecessaryLiteralSet("list".to_string()), - Range::from_located(expr), - )); - } - ExprKind::Tuple { .. } => { - return Some(Check::new( - CheckKind::UnnecessaryLiteralSet("tuple".to_string()), - Range::from_located(expr), - )); - } - _ => {} - } - } - } - } - None -} - -/// Check `dict([(1, 2)])` compliance. -pub fn unnecessary_literal_dict(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { - if args.len() == 1 { - if let ExprKind::Name { id, .. } = &func.node { - if id == "dict" { - match &args[0].node { - ExprKind::Tuple { elts, .. } => { - if let Some(elt) = elts.first() { - match &elt.node { - // dict((1, 2), ...)) - ExprKind::Tuple { elts, .. } if elts.len() == 2 => { - return Some(Check::new( - CheckKind::UnnecessaryLiteralDict("tuple".to_string()), - Range::from_located(expr), - )); - } - _ => {} - } - } else { - // dict(()) - return Some(Check::new( - CheckKind::UnnecessaryLiteralDict("tuple".to_string()), - Range::from_located(expr), - )); - } - } - ExprKind::List { elts, .. } => { - if let Some(elt) = elts.first() { - match &elt.node { - // dict([(1, 2), ...]) - ExprKind::Tuple { elts, .. } if elts.len() == 2 => { - return Some(Check::new( - CheckKind::UnnecessaryLiteralDict("list".to_string()), - Range::from_located(expr), - )); - } - _ => {} - } - } else { - // dict([]) - return Some(Check::new( - CheckKind::UnnecessaryLiteralDict("list".to_string()), - Range::from_located(expr), - )); - } - } - _ => {} - } - } - } - } - None -} - -pub fn unnecessary_collection_call( - expr: &Expr, - func: &Expr, - args: &[Expr], - keywords: &[Located], -) -> Option { - if args.is_empty() { - if let ExprKind::Name { id, .. } = &func.node { - if id == "list" || id == "tuple" { - // list() or tuple() - return Some(Check::new( - CheckKind::UnnecessaryCollectionCall(id.to_string()), - Range::from_located(expr), - )); - } else if id == "dict" { - // dict() or dict(a=1) - if keywords.is_empty() || keywords.iter().all(|kw| kw.node.arg.is_some()) { - return Some(Check::new( - CheckKind::UnnecessaryCollectionCall(id.to_string()), - Range::from_located(expr), - )); - } - } - } - } - None -} - -pub fn unnecessary_literal_within_tuple_call( - expr: &Expr, - func: &Expr, - args: &[Expr], -) -> Option { - if let ExprKind::Name { id, .. } = &func.node { - if id == "tuple" { - if let Some(arg) = args.first() { - match &arg.node { - ExprKind::Tuple { .. } => { - return Some(Check::new( - CheckKind::UnnecessaryLiteralWithinTupleCall("tuple".to_string()), - Range::from_located(expr), - )); - } - ExprKind::List { .. } => { - return Some(Check::new( - CheckKind::UnnecessaryLiteralWithinTupleCall("list".to_string()), - Range::from_located(expr), - )); - } - _ => {} - } - } - } - } - - None -} - -pub fn unnecessary_literal_within_list_call( - expr: &Expr, - func: &Expr, - args: &[Expr], -) -> Option { - if let ExprKind::Name { id, .. } = &func.node { - if id == "list" { - if let Some(arg) = args.first() { - match &arg.node { - ExprKind::Tuple { .. } => { - return Some(Check::new( - CheckKind::UnnecessaryLiteralWithinListCall("tuple".to_string()), - Range::from_located(expr), - )); - } - ExprKind::List { .. } => { - return Some(Check::new( - CheckKind::UnnecessaryLiteralWithinListCall("list".to_string()), - Range::from_located(expr), - )); - } - _ => {} - } - } - } - } - - None -} - -pub fn unnecessary_list_call(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { - if let ExprKind::Name { id, .. } = &func.node { - if id == "list" { - if let Some(arg) = args.first() { - if let ExprKind::ListComp { .. } = &arg.node { - return Some(Check::new( - CheckKind::UnnecessaryListCall, - Range::from_located(expr), - )); - } - } - } - } - None -} - -pub fn unnecessary_call_around_sorted(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { - if let ExprKind::Name { id: outer, .. } = &func.node { - if outer == "list" || outer == "reversed" { - if let Some(arg) = args.first() { - if let ExprKind::Call { func, .. } = &arg.node { - if let ExprKind::Name { id: inner, .. } = &func.node { - if inner == "sorted" { - return Some(Check::new( - CheckKind::UnnecessaryCallAroundSorted(outer.to_string()), - Range::from_located(expr), - )); - } - } - } - } - } - } - None -} - -pub fn unnecessary_double_cast_or_process( - expr: &Expr, - func: &Expr, - args: &[Expr], -) -> Option { - if let ExprKind::Name { id: outer, .. } = &func.node { - if outer == "list" - || outer == "tuple" - || outer == "set" - || outer == "reversed" - || outer == "sorted" - { - if let Some(arg) = args.first() { - if let ExprKind::Call { func, .. } = &arg.node { - if let ExprKind::Name { id: inner, .. } = &func.node { - // Ex) set(tuple(...)) - if (outer == "set" || outer == "sorted") - && (inner == "list" - || inner == "tuple" - || inner == "reversed" - || inner == "sorted") - { - return Some(Check::new( - CheckKind::UnnecessaryDoubleCastOrProcess( - inner.to_string(), - outer.to_string(), - ), - Range::from_located(expr), - )); - } - - // Ex) list(tuple(...)) - if (outer == "list" || outer == "tuple") - && (inner == "list" || inner == "tuple") - { - return Some(Check::new( - CheckKind::UnnecessaryDoubleCastOrProcess( - inner.to_string(), - outer.to_string(), - ), - Range::from_located(expr), - )); - } - - // Ex) set(set(...)) - if outer == "set" && inner == "set" { - return Some(Check::new( - CheckKind::UnnecessaryDoubleCastOrProcess( - inner.to_string(), - outer.to_string(), - ), - Range::from_located(expr), - )); - } - } - } - } - } - } - None -} - -pub fn unnecessary_subscript_reversal(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { - if let Some(first_arg) = args.first() { - if let ExprKind::Name { id, .. } = &func.node { - if id == "set" || id == "sorted" || id == "reversed" { - if let ExprKind::Subscript { slice, .. } = &first_arg.node { - if let ExprKind::Slice { lower, upper, step } = &slice.node { - if lower.is_none() && upper.is_none() { - if let Some(step) = step { - if let ExprKind::UnaryOp { - op: Unaryop::USub, - operand, - } = &step.node - { - if let ExprKind::Constant { - value: Constant::Int(val), - .. - } = &operand.node - { - if *val == BigInt::from(1) { - return Some(Check::new( - CheckKind::UnnecessarySubscriptReversal( - id.to_string(), - ), - Range::from_located(expr), - )); - } - } - } - } - } - } - } - } - } - } - None -} - -pub fn unnecessary_comprehension( - expr: &Expr, - elt: &Expr, - generators: &[Comprehension], -) -> Option { - if generators.len() == 1 { - let generator = &generators[0]; - if generator.ifs.is_empty() && generator.is_async == 0 { - if let ExprKind::Name { id: elt_id, .. } = &elt.node { - if let ExprKind::Name { id: target_id, .. } = &generator.target.node { - if elt_id == target_id { - match &expr.node { - ExprKind::ListComp { .. } => { - return Some(Check::new( - CheckKind::UnnecessaryComprehension("list".to_string()), - Range::from_located(expr), - )) - } - ExprKind::SetComp { .. } => { - return Some(Check::new( - CheckKind::UnnecessaryComprehension("set".to_string()), - Range::from_located(expr), - )) - } - _ => {} - }; - } - } - } - } - } - - None -} - -pub fn unnecessary_map(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { - if let ExprKind::Name { id, .. } = &func.node { - if id == "map" { - if args.len() == 2 { - if let ExprKind::Lambda { .. } = &args[0].node { - return Some(Check::new( - CheckKind::UnnecessaryMap("generator".to_string()), - Range::from_located(expr), - )); - } - } - } else if id == "list" || id == "set" { - if let Some(arg) = args.first() { - if let ExprKind::Call { func, args, .. } = &arg.node { - if let ExprKind::Name { id: f, .. } = &func.node { - if f == "map" { - if let Some(arg) = args.first() { - if let ExprKind::Lambda { .. } = &arg.node { - return Some(Check::new( - CheckKind::UnnecessaryMap(id.to_string()), - Range::from_located(expr), - )); - } - } - } - } - } - } - } else if id == "dict" { - if args.len() == 1 { - if let ExprKind::Call { func, args, .. } = &args[0].node { - if let ExprKind::Name { id: f, .. } = &func.node { - if f == "map" { - if let Some(arg) = args.first() { - if let ExprKind::Lambda { body, .. } = &arg.node { - match &body.node { - ExprKind::Tuple { elts, .. } - | ExprKind::List { elts, .. } - if elts.len() == 2 => - { - return Some(Check::new( - CheckKind::UnnecessaryMap(id.to_string()), - Range::from_located(expr), - )) - } - _ => {} - } - } - } - } - } - } - } - } - } - None -} - -// flake8-super -/// Check that `super()` has no args -pub fn super_args( - scope: &Scope, - parents: &[&Stmt], - expr: &Expr, - func: &Expr, - args: &[Expr], -) -> Option { - if !helpers::is_super_call_with_arguments(func, args) { - return None; - } - - // Check: are we in a Function scope? - if !matches!(scope.kind, ScopeKind::Function { .. }) { - return None; - } - - let mut parents = parents.iter().rev(); - - // For a `super` invocation to be unnecessary, the first argument needs to match the enclosing - // class, and the second argument needs to match the first argument to the enclosing function. - if let [first_arg, second_arg] = args { - // Find the enclosing function definition (if any). - if let Some(StmtKind::FunctionDef { - args: parent_args, .. - }) = parents - .find(|stmt| matches!(stmt.node, StmtKind::FunctionDef { .. })) - .map(|stmt| &stmt.node) - { - // Extract the name of the first argument to the enclosing function. - if let Some(ArgData { - arg: parent_arg, .. - }) = parent_args.args.first().map(|expr| &expr.node) - { - // Find the enclosing class definition (if any). - if let Some(StmtKind::ClassDef { - name: parent_name, .. - }) = parents - .find(|stmt| matches!(stmt.node, StmtKind::ClassDef { .. })) - .map(|stmt| &stmt.node) - { - if let ( - ExprKind::Name { - id: first_arg_id, .. - }, - ExprKind::Name { - id: second_arg_id, .. - }, - ) = (&first_arg.node, &second_arg.node) - { - if first_arg_id == parent_name && second_arg_id == parent_arg { - return Some(Check::new( - CheckKind::SuperCallWithParameters, - Range::from_located(expr), - )); - } - } - } - } - } - } - - None -} - -// flake8-print -/// Check whether a function call is a `print` or `pprint` invocation -pub fn print_call( - expr: &Expr, - func: &Expr, - check_print: bool, - check_pprint: bool, -) -> Option { - if let ExprKind::Name { id, .. } = &func.node { - if check_print && id == "print" { - return Some(Check::new(CheckKind::PrintFound, Range::from_located(expr))); - } else if check_pprint && id == "pprint" { - return Some(Check::new( - CheckKind::PPrintFound, - Range::from_located(expr), - )); - } - } - - if let ExprKind::Attribute { value, attr, .. } = &func.node { - if let ExprKind::Name { id, .. } = &value.node { - if check_pprint && id == "pprint" && attr == "pprint" { - return Some(Check::new( - CheckKind::PPrintFound, - Range::from_located(expr), - )); - } - } - } - - None -} - -// pep8-naming -pub fn invalid_class_name(class_def: &Stmt, name: &str) -> Option { - let stripped = name.strip_prefix('_').unwrap_or(name); - if !stripped - .chars() - .next() - .map(|c| c.is_uppercase()) - .unwrap_or(false) - || stripped.contains('_') - { - return Some(Check::new( - CheckKind::InvalidClassName(name.to_string()), - Range::from_located(class_def), - )); - } - None -} - -pub fn invalid_function_name(func_def: &Stmt, name: &str) -> Option { - if name.chars().any(|c| c.is_uppercase()) { - return Some(Check::new( - CheckKind::InvalidFunctionName(name.to_string()), - Range::from_located(func_def), - )); - } - None -} - -pub fn invalid_argument_name(location: Range, name: &str) -> Option { - if name.chars().any(|c| c.is_uppercase()) { - return Some(Check::new( - CheckKind::InvalidArgumentName(name.to_string()), - location, - )); - } - None -} - -pub fn invalid_first_argument_name_for_class_method( - scope: &Scope, - decorator_list: &[Expr], - args: &Arguments, -) -> Option { - if !matches!(scope.kind, ScopeKind::Class) { - return None; - } - - if decorator_list.iter().any(|decorator| { - if let ExprKind::Name { id, .. } = &decorator.node { - id == "classmethod" - } else { - false - } - }) { - if let Some(arg) = args.args.first() { - if arg.node.arg != "cls" { - return Some(Check::new( - CheckKind::InvalidFirstArgumentNameForClassMethod, - Range::from_located(arg), - )); - } - } - } - None -} - -pub fn invalid_first_argument_name_for_method( - scope: &Scope, - decorator_list: &[Expr], - args: &Arguments, -) -> Option { - if !matches!(scope.kind, ScopeKind::Class) { - return None; - } - - if decorator_list.iter().any(|decorator| { - if let ExprKind::Name { id, .. } = &decorator.node { - id == "classmethod" || id == "staticmethod" - } else { - false - } - }) { - return None; - } - - if let Some(arg) = args.args.first() { - if arg.node.arg != "self" { - return Some(Check::new( - CheckKind::InvalidFirstArgumentNameForMethod, - Range::from_located(arg), - )); - } - } - None -} diff --git a/src/autofix.rs b/src/autofix/mod.rs similarity index 100% rename from src/autofix.rs rename to src/autofix/mod.rs diff --git a/src/check_ast.rs b/src/check_ast.rs index bf47c66213..5a436472b6 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -17,7 +17,7 @@ use crate::ast::types::{ ScopeKind, }; use crate::ast::visitor::{walk_excepthandler, Visitor}; -use crate::ast::{checkers, helpers, operations, visitor}; +use crate::ast::{helpers, operations, visitor}; use crate::autofix::{fixer, fixes}; use crate::checks::{Check, CheckCode, CheckKind}; use crate::docstrings::definition::{Definition, DefinitionKind, Documentable}; @@ -25,7 +25,10 @@ use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS}; use crate::python::future::ALL_FEATURE_NAMES; use crate::settings::{PythonVersion, Settings}; use crate::visibility::{module_visibility, transition_scope, Modifier, Visibility, VisibleScope}; -use crate::{docstrings, plugins}; +use crate::{ + docstrings, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_print, pep8_naming, + pycodestyle, pydocstyle, pyflakes, pyupgrade, +}; pub const GLOBAL_SCOPE_INDEX: usize = 0; @@ -162,25 +165,26 @@ where if self.settings.enabled.contains(&CheckCode::E741) { let location = self.locate_check(Range::from_located(stmt)); - self.checks.extend( - names - .iter() - .filter_map(|name| checkers::ambiguous_variable_name(name, location)), - ); + self.checks.extend(names.iter().filter_map(|name| { + pycodestyle::checks::ambiguous_variable_name(name, location) + })); } } StmtKind::Break => { if self.settings.enabled.contains(&CheckCode::F701) { - if let Some(check) = - checkers::break_outside_loop(stmt, &self.parents, &self.parent_stack, self) - { + if let Some(check) = pyflakes::checks::break_outside_loop( + stmt, + &self.parents, + &self.parent_stack, + self, + ) { self.checks.push(check); } } } StmtKind::Continue => { if self.settings.enabled.contains(&CheckCode::F702) { - if let Some(check) = checkers::continue_outside_loop( + if let Some(check) = pyflakes::checks::continue_outside_loop( stmt, &self.parents, &self.parent_stack, @@ -205,7 +209,7 @@ where .. } => { if self.settings.enabled.contains(&CheckCode::E743) { - if let Some(check) = checkers::ambiguous_function_name( + if let Some(check) = pycodestyle::checks::ambiguous_function_name( name, self.locate_check(Range::from_located(stmt)), ) { @@ -214,23 +218,25 @@ where } if self.settings.enabled.contains(&CheckCode::N802) { - if let Some(check) = checkers::invalid_function_name(stmt, name) { + if let Some(check) = pep8_naming::checks::invalid_function_name(stmt, name) { self.checks.push(check); } } if self.settings.enabled.contains(&CheckCode::N804) { - if let Some(check) = checkers::invalid_first_argument_name_for_class_method( - self.current_scope(), - decorator_list, - args, - ) { + if let Some(check) = + pep8_naming::checks::invalid_first_argument_name_for_class_method( + self.current_scope(), + decorator_list, + args, + ) + { self.checks.push(check); } } if self.settings.enabled.contains(&CheckCode::N805) { - if let Some(check) = checkers::invalid_first_argument_name_for_method( + if let Some(check) = pep8_naming::checks::invalid_first_argument_name_for_method( self.current_scope(), decorator_list, args, @@ -311,11 +317,13 @@ where .. } => { if self.settings.enabled.contains(&CheckCode::U004) { - plugins::useless_object_inheritance(self, stmt, name, bases, keywords); + pyupgrade::plugins::useless_object_inheritance( + self, stmt, name, bases, keywords, + ); } if self.settings.enabled.contains(&CheckCode::E742) { - if let Some(check) = checkers::ambiguous_class_name( + if let Some(check) = pycodestyle::checks::ambiguous_class_name( name, self.locate_check(Range::from_located(stmt)), ) { @@ -324,7 +332,7 @@ where } if self.settings.enabled.contains(&CheckCode::N801) { - if let Some(check) = checkers::invalid_class_name(stmt, name) { + if let Some(check) = pep8_naming::checks::invalid_class_name(stmt, name) { self.checks.push(check); } } @@ -522,7 +530,7 @@ where StmtKind::Raise { exc, .. } => { if self.settings.enabled.contains(&CheckCode::F901) { if let Some(expr) = exc { - if let Some(check) = checkers::raise_not_implemented(expr) { + if let Some(check) = pyflakes::checks::raise_not_implemented(expr) { self.checks.push(check); } } @@ -533,32 +541,32 @@ where } StmtKind::If { test, .. } => { if self.settings.enabled.contains(&CheckCode::F634) { - plugins::if_tuple(self, stmt, test); + pyflakes::plugins::if_tuple(self, stmt, test); } } StmtKind::Assert { test, msg } => { if self.settings.enabled.contains(&CheckCode::F631) { - plugins::assert_tuple(self, stmt, test); + pyflakes::plugins::assert_tuple(self, stmt, test); } if self.settings.enabled.contains(&CheckCode::B011) { - plugins::assert_false(self, stmt, test, msg); + flake8_bugbear::plugins::assert_false(self, stmt, test, msg); } } StmtKind::Try { handlers, .. } => { if self.settings.enabled.contains(&CheckCode::F707) { - if let Some(check) = checkers::default_except_not_last(handlers) { + if let Some(check) = pyflakes::checks::default_except_not_last(handlers) { self.checks.push(check); } } if self.settings.enabled.contains(&CheckCode::B014) || self.settings.enabled.contains(&CheckCode::B025) { - plugins::duplicate_exceptions(self, stmt, handlers); + flake8_bugbear::plugins::duplicate_exceptions(self, stmt, handlers); } } StmtKind::Assign { targets, value, .. } => { if self.settings.enabled.contains(&CheckCode::E731) { - if let Some(check) = checkers::do_not_assign_lambda( + if let Some(check) = pycodestyle::checks::do_not_assign_lambda( value, self.locate_check(Range::from_located(stmt)), ) { @@ -566,13 +574,13 @@ where } } if self.settings.enabled.contains(&CheckCode::U001) { - plugins::useless_metaclass_type(self, stmt, value, targets); + pyupgrade::plugins::useless_metaclass_type(self, stmt, value, targets); } } StmtKind::AnnAssign { value, .. } => { if self.settings.enabled.contains(&CheckCode::E731) { if let Some(value) = value { - if let Some(check) = checkers::do_not_assign_lambda( + if let Some(check) = pycodestyle::checks::do_not_assign_lambda( value, self.locate_check(Range::from_located(stmt)), ) { @@ -699,7 +707,7 @@ where if self.settings.enabled.contains(&CheckCode::U007) && self.settings.target_version >= PythonVersion::Py39 { - plugins::use_pep604_annotation(self, expr, value, slice); + pyupgrade::plugins::use_pep604_annotation(self, expr, value, slice); } if match_name_or_attr(value, "Literal") { @@ -712,7 +720,7 @@ where self.settings.enabled.contains(&CheckCode::F621); let check_two_starred_expressions = self.settings.enabled.contains(&CheckCode::F622); - if let Some(check) = checkers::starred_expressions( + if let Some(check) = pyflakes::checks::starred_expressions( elts, check_too_many_expressions, check_two_starred_expressions, @@ -728,14 +736,14 @@ where if self.settings.enabled.contains(&CheckCode::U006) && self.settings.target_version >= PythonVersion::Py39 { - plugins::use_pep585_annotation(self, expr, id); + pyupgrade::plugins::use_pep585_annotation(self, expr, id); } self.handle_node_load(expr); } ExprContext::Store => { if self.settings.enabled.contains(&CheckCode::E741) { - if let Some(check) = checkers::ambiguous_variable_name( + if let Some(check) = pycodestyle::checks::ambiguous_variable_name( id, self.locate_check(Range::from_located(expr)), ) { @@ -756,7 +764,7 @@ where { if let ExprKind::Name { id, .. } = &value.node { if id == "typing" { - plugins::use_pep585_annotation(self, expr, attr); + pyupgrade::plugins::use_pep585_annotation(self, expr, attr); } } } @@ -768,43 +776,51 @@ where .. } => { if self.settings.enabled.contains(&CheckCode::U005) { - plugins::deprecated_unittest_alias(self, func); + pyupgrade::plugins::deprecated_unittest_alias(self, func); } // flake8-super if self.settings.enabled.contains(&CheckCode::U008) { - plugins::super_call_with_parameters(self, expr, func, args); + pyupgrade::plugins::super_call_with_parameters(self, expr, func, args); } // flake8-print if self.settings.enabled.contains(&CheckCode::T201) || self.settings.enabled.contains(&CheckCode::T203) { - plugins::print_call(self, expr, func); + flake8_print::plugins::print_call(self, expr, func); } // flake8-comprehensions if self.settings.enabled.contains(&CheckCode::C400) { - if let Some(check) = checkers::unnecessary_generator_list(expr, func, args) { + if let Some(check) = + flake8_comprehensions::checks::unnecessary_generator_list(expr, func, args) + { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C401) { - if let Some(check) = checkers::unnecessary_generator_set(expr, func, args) { + if let Some(check) = + flake8_comprehensions::checks::unnecessary_generator_set(expr, func, args) + { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C402) { - if let Some(check) = checkers::unnecessary_generator_dict(expr, func, args) { + if let Some(check) = + flake8_comprehensions::checks::unnecessary_generator_dict(expr, func, args) + { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C403) { if let Some(check) = - checkers::unnecessary_list_comprehension_set(expr, func, args) + flake8_comprehensions::checks::unnecessary_list_comprehension_set( + expr, func, args, + ) { self.checks.push(check); }; @@ -812,35 +828,43 @@ where if self.settings.enabled.contains(&CheckCode::C404) { if let Some(check) = - checkers::unnecessary_list_comprehension_dict(expr, func, args) + flake8_comprehensions::checks::unnecessary_list_comprehension_dict( + expr, func, args, + ) { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C405) { - if let Some(check) = checkers::unnecessary_literal_set(expr, func, args) { + if let Some(check) = + flake8_comprehensions::checks::unnecessary_literal_set(expr, func, args) + { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C406) { - if let Some(check) = checkers::unnecessary_literal_dict(expr, func, args) { + if let Some(check) = + flake8_comprehensions::checks::unnecessary_literal_dict(expr, func, args) + { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C408) { - if let Some(check) = - checkers::unnecessary_collection_call(expr, func, args, keywords) - { + if let Some(check) = flake8_comprehensions::checks::unnecessary_collection_call( + expr, func, args, keywords, + ) { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C409) { if let Some(check) = - checkers::unnecessary_literal_within_tuple_call(expr, func, args) + flake8_comprehensions::checks::unnecessary_literal_within_tuple_call( + expr, func, args, + ) { self.checks.push(check); }; @@ -848,20 +872,27 @@ where if self.settings.enabled.contains(&CheckCode::C410) { if let Some(check) = - checkers::unnecessary_literal_within_list_call(expr, func, args) + flake8_comprehensions::checks::unnecessary_literal_within_list_call( + expr, func, args, + ) { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C411) { - if let Some(check) = checkers::unnecessary_list_call(expr, func, args) { + if let Some(check) = + flake8_comprehensions::checks::unnecessary_list_call(expr, func, args) + { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C413) { - if let Some(check) = checkers::unnecessary_call_around_sorted(expr, func, args) + if let Some(check) = + flake8_comprehensions::checks::unnecessary_call_around_sorted( + expr, func, args, + ) { self.checks.push(check); }; @@ -869,21 +900,28 @@ where if self.settings.enabled.contains(&CheckCode::C414) { if let Some(check) = - checkers::unnecessary_double_cast_or_process(expr, func, args) + flake8_comprehensions::checks::unnecessary_double_cast_or_process( + expr, func, args, + ) { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C415) { - if let Some(check) = checkers::unnecessary_subscript_reversal(expr, func, args) + if let Some(check) = + flake8_comprehensions::checks::unnecessary_subscript_reversal( + expr, func, args, + ) { self.checks.push(check); }; } if self.settings.enabled.contains(&CheckCode::C417) { - if let Some(check) = checkers::unnecessary_map(expr, func, args) { + if let Some(check) = + flake8_comprehensions::checks::unnecessary_map(expr, func, args) + { self.checks.push(check); }; } @@ -892,11 +930,11 @@ where if self.settings.enabled.contains(&CheckCode::U002) && self.settings.target_version >= PythonVersion::Py310 { - plugins::unnecessary_abspath(self, expr, func, args); + pyupgrade::plugins::unnecessary_abspath(self, expr, func, args); } if self.settings.enabled.contains(&CheckCode::U003) { - plugins::type_of_primitive(self, expr, func, args); + pyupgrade::plugins::type_of_primitive(self, expr, func, args); } if let ExprKind::Name { id, ctx } = &func.node { @@ -918,7 +956,7 @@ where let check_repeated_literals = self.settings.enabled.contains(&CheckCode::F601); let check_repeated_variables = self.settings.enabled.contains(&CheckCode::F602); if check_repeated_literals || check_repeated_variables { - self.checks.extend(checkers::repeated_keys( + self.checks.extend(pyflakes::checks::repeated_keys( keys, check_repeated_literals, check_repeated_variables, @@ -958,14 +996,14 @@ where .. } => { if self.settings.enabled.contains(&CheckCode::F633) { - plugins::invalid_print_syntax(self, left); + pyflakes::plugins::invalid_print_syntax(self, left); } } ExprKind::UnaryOp { op, operand } => { let check_not_in = self.settings.enabled.contains(&CheckCode::E713); let check_not_is = self.settings.enabled.contains(&CheckCode::E714); if check_not_in || check_not_is { - self.checks.extend(checkers::not_tests( + self.checks.extend(pycodestyle::checks::not_tests( op, operand, check_not_in, @@ -982,7 +1020,7 @@ where let check_none_comparisons = self.settings.enabled.contains(&CheckCode::E711); let check_true_false_comparisons = self.settings.enabled.contains(&CheckCode::E712); if check_none_comparisons || check_true_false_comparisons { - self.checks.extend(checkers::literal_comparisons( + self.checks.extend(pycodestyle::checks::literal_comparisons( left, ops, comparators, @@ -993,7 +1031,7 @@ where } if self.settings.enabled.contains(&CheckCode::F632) { - self.checks.extend(checkers::is_literal( + self.checks.extend(pyflakes::checks::is_literal( left, ops, comparators, @@ -1002,7 +1040,7 @@ where } if self.settings.enabled.contains(&CheckCode::E721) { - self.checks.extend(checkers::type_comparison( + self.checks.extend(pycodestyle::checks::type_comparison( ops, comparators, self.locate_check(Range::from_located(expr)), @@ -1055,8 +1093,9 @@ where ExprKind::ListComp { elt, generators } | ExprKind::SetComp { elt, generators } => { if self.settings.enabled.contains(&CheckCode::C416) { - if let Some(check) = checkers::unnecessary_comprehension(expr, elt, generators) - { + if let Some(check) = flake8_comprehensions::checks::unnecessary_comprehension( + expr, elt, generators, + ) { self.checks.push(check); }; } @@ -1239,7 +1278,7 @@ where match name { Some(name) => { if self.settings.enabled.contains(&CheckCode::E741) { - if let Some(check) = checkers::ambiguous_variable_name( + if let Some(check) = pycodestyle::checks::ambiguous_variable_name( name, self.locate_check(Range::from_located(excepthandler)), ) { @@ -1307,7 +1346,8 @@ where fn visit_arguments(&mut self, arguments: &'b Arguments) { if self.settings.enabled.contains(&CheckCode::F831) { - self.checks.extend(checkers::duplicate_arguments(arguments)); + self.checks + .extend(pyflakes::checks::duplicate_arguments(arguments)); } // Bind, but intentionally avoid walking default expressions, as we handle them upstream. @@ -1340,7 +1380,7 @@ where ); if self.settings.enabled.contains(&CheckCode::E741) { - if let Some(check) = checkers::ambiguous_variable_name( + if let Some(check) = pycodestyle::checks::ambiguous_variable_name( &arg.node.arg, self.locate_check(Range::from_located(arg)), ) { @@ -1350,7 +1390,7 @@ where if self.settings.enabled.contains(&CheckCode::N803) { if let Some(check) = - checkers::invalid_argument_name(Range::from_located(arg), &arg.node.arg) + pep8_naming::checks::invalid_argument_name(Range::from_located(arg), &arg.node.arg) { self.checks.push(check); } @@ -1796,7 +1836,7 @@ impl<'a> Checker<'a> { fn check_deferred_assignments(&mut self) { if self.settings.enabled.contains(&CheckCode::F841) { while let Some(index) = self.deferred_assignments.pop() { - self.checks.extend(checkers::unused_variables( + self.checks.extend(pyflakes::checks::unused_variables( &self.scopes[index], self, &self.settings.dummy_variable_rgx, @@ -1946,66 +1986,66 @@ impl<'a> Checker<'a> { fn check_docstrings(&mut self) { while let Some((docstring, visibility)) = self.docstrings.pop() { - if !docstrings::plugins::not_empty(self, &docstring) { + if !pydocstyle::plugins::not_empty(self, &docstring) { continue; } - if !docstrings::plugins::not_missing(self, &docstring, &visibility) { + if !pydocstyle::plugins::not_missing(self, &docstring, &visibility) { continue; } if self.settings.enabled.contains(&CheckCode::D200) { - docstrings::plugins::one_liner(self, &docstring); + pydocstyle::plugins::one_liner(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D201) || self.settings.enabled.contains(&CheckCode::D202) { - docstrings::plugins::blank_before_after_function(self, &docstring); + pydocstyle::plugins::blank_before_after_function(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D203) || self.settings.enabled.contains(&CheckCode::D204) || self.settings.enabled.contains(&CheckCode::D211) { - docstrings::plugins::blank_before_after_class(self, &docstring); + pydocstyle::plugins::blank_before_after_class(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D205) { - docstrings::plugins::blank_after_summary(self, &docstring); + pydocstyle::plugins::blank_after_summary(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D206) || self.settings.enabled.contains(&CheckCode::D207) || self.settings.enabled.contains(&CheckCode::D208) { - docstrings::plugins::indent(self, &docstring); + pydocstyle::plugins::indent(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D209) { - docstrings::plugins::newline_after_last_paragraph(self, &docstring); + pydocstyle::plugins::newline_after_last_paragraph(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D210) { - docstrings::plugins::no_surrounding_whitespace(self, &docstring); + pydocstyle::plugins::no_surrounding_whitespace(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D212) || self.settings.enabled.contains(&CheckCode::D213) { - docstrings::plugins::multi_line_summary_start(self, &docstring); + pydocstyle::plugins::multi_line_summary_start(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D300) { - docstrings::plugins::triple_quotes(self, &docstring); + pydocstyle::plugins::triple_quotes(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D400) { - docstrings::plugins::ends_with_period(self, &docstring); + pydocstyle::plugins::ends_with_period(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D402) { - docstrings::plugins::no_signature(self, &docstring); + pydocstyle::plugins::no_signature(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D403) { - docstrings::plugins::capitalized(self, &docstring); + pydocstyle::plugins::capitalized(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D404) { - docstrings::plugins::starts_with_this(self, &docstring); + pydocstyle::plugins::starts_with_this(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D415) { - docstrings::plugins::ends_with_punctuation(self, &docstring); + pydocstyle::plugins::ends_with_punctuation(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D418) { - docstrings::plugins::if_needed(self, &docstring); + pydocstyle::plugins::if_needed(self, &docstring); } if self.settings.enabled.contains(&CheckCode::D212) || self.settings.enabled.contains(&CheckCode::D214) @@ -2023,29 +2063,28 @@ impl<'a> Checker<'a> { || self.settings.enabled.contains(&CheckCode::D416) || self.settings.enabled.contains(&CheckCode::D417) { - docstrings::plugins::sections(self, &docstring); + pydocstyle::plugins::sections(self, &docstring); } } } fn check_builtin_shadowing(&mut self, name: &str, location: Range, is_attribute: bool) { - // flake8-builtins if is_attribute && matches!(self.current_scope().kind, ScopeKind::Class) { if self.settings.enabled.contains(&CheckCode::A003) { - if let Some(check) = checkers::builtin_shadowing( + if let Some(check) = flake8_builtins::checks::builtin_shadowing( name, self.locate_check(location), - checkers::ShadowingType::Attribute, + flake8_builtins::types::ShadowingType::Attribute, ) { self.checks.push(check); } } } else { if self.settings.enabled.contains(&CheckCode::A001) { - if let Some(check) = checkers::builtin_shadowing( + if let Some(check) = flake8_builtins::checks::builtin_shadowing( name, self.locate_check(location), - checkers::ShadowingType::Variable, + flake8_builtins::types::ShadowingType::Variable, ) { self.checks.push(check); } @@ -2054,12 +2093,11 @@ impl<'a> Checker<'a> { } fn check_builtin_arg_shadowing(&mut self, name: &str, location: Range) { - // flake8-builtins if self.settings.enabled.contains(&CheckCode::A002) { - if let Some(check) = checkers::builtin_shadowing( + if let Some(check) = flake8_builtins::checks::builtin_shadowing( name, self.locate_check(location), - checkers::ShadowingType::Argument, + flake8_builtins::types::ShadowingType::Argument, ) { self.checks.push(check); } diff --git a/src/checks.rs b/src/checks.rs index ee9717e0d3..8d1d441416 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -5,8 +5,8 @@ use rustpython_parser::ast::Location; use serde::{Deserialize, Serialize}; use strum_macros::{AsRefStr, EnumIter, EnumString}; -use crate::ast::checkers::Primitive; use crate::ast::types::Range; +use crate::pyupgrade::types::Primitive; #[derive( AsRefStr, @@ -211,18 +211,29 @@ pub enum CheckKind { AmbiguousClassName(String), AmbiguousFunctionName(String), AmbiguousVariableName(String), + DoNotAssignLambda, + DoNotUseBareExcept, + IOError(String), + LineTooLong(usize, usize), + ModuleImportNotAtTopOfFile, + NoneComparison(RejectedCmpop), + NotInTest, + NotIsTest, + SyntaxError(String), + TrueFalseComparison(bool, RejectedCmpop), + TypeComparison, + // pycodestyle warnings + NoNewLineAtEndOfFile, + // pyflakes AssertTuple, BreakOutsideLoop, ContinueOutsideLoop, DefaultExceptNotLast, - DoNotAssignLambda, - DoNotUseBareExcept, DuplicateArgumentName, ExpressionsInStarAssignment, FStringMissingPlaceholders, ForwardAnnotationSyntaxError(String), FutureFeatureNotDefined(String), - IOError(String), IfTuple, ImportShadowedByLoopVar(String, usize), ImportStarNotPermitted(String), @@ -231,28 +242,18 @@ pub enum CheckKind { InvalidPrintSyntax, IsLiteral, LateFutureImport, - LineTooLong(usize, usize), - ModuleImportNotAtTopOfFile, MultiValueRepeatedKeyLiteral, MultiValueRepeatedKeyVariable(String), - NoneComparison(RejectedCmpop), - NotInTest, - NotIsTest, RaiseNotImplemented, ReturnOutsideFunction, - SyntaxError(String), - TrueFalseComparison(bool, RejectedCmpop), TwoStarredExpressions, - TypeComparison, UndefinedExport(String), UndefinedLocal(String), UndefinedName(String), UnusedImport(Vec), UnusedVariable(String), YieldOutsideFunction, - // pycodestyle warnings - NoNewLineAtEndOfFile, - // flake8-builtin + // flake8-builtins BuiltinVariableShadowing(String), BuiltinArgumentShadowing(String), BuiltinAttributeShadowing(String), diff --git a/src/docstrings.rs b/src/docstrings.rs deleted file mode 100644 index d8ad156e78..0000000000 --- a/src/docstrings.rs +++ /dev/null @@ -1,8 +0,0 @@ -pub mod definition; -pub mod extraction; -mod google; -mod helpers; -mod numpy; -pub mod plugins; -pub mod sections; -mod styles; diff --git a/src/docstrings/google.rs b/src/docstrings/google.rs index 8982c88dc1..22169a8c5d 100644 --- a/src/docstrings/google.rs +++ b/src/docstrings/google.rs @@ -2,16 +2,7 @@ use std::collections::BTreeSet; -use crate::ast::types::Range; use once_cell::sync::Lazy; -use regex::Regex; - -use crate::check_ast::Checker; -use crate::checks::{Check, CheckCode, CheckKind}; -use crate::docstrings::definition::Definition; -use crate::docstrings::sections; -use crate::docstrings::sections::SectionContext; -use crate::docstrings::styles::SectionStyle; pub(crate) static GOOGLE_SECTION_NAMES: Lazy> = Lazy::new(|| { BTreeSet::from([ @@ -78,73 +69,3 @@ pub(crate) static LOWERCASE_GOOGLE_SECTION_NAMES: Lazy> = "yields", ]) }); - -// See: `GOOGLE_ARGS_REGEX` in `pydocstyle/checker.py`. -static GOOGLE_ARGS_REGEX: Lazy = - Lazy::new(|| Regex::new(r"^\s*(\w+)\s*(\(.*?\))?\s*:\n?\s*.+").expect("Invalid regex")); - -fn check_args_section(checker: &mut Checker, definition: &Definition, context: &SectionContext) { - let mut args_sections: Vec = vec![]; - for line in textwrap::dedent(&context.following_lines.join("\n")).lines() { - if line - .chars() - .next() - .map(|char| char.is_whitespace()) - .unwrap_or(true) - { - // This is a continuation of documentation for the last - // parameter because it does start with whitespace. - if let Some(current) = args_sections.last_mut() { - current.push_str(line); - } - } else { - // This line is the start of documentation for the next - // parameter because it doesn't start with any whitespace. - args_sections.push(line.to_string()); - } - } - - sections::check_missing_args( - checker, - definition, - // Collect the list of arguments documented in the docstring. - &BTreeSet::from_iter(args_sections.iter().filter_map(|section| { - match GOOGLE_ARGS_REGEX.captures(section.as_str()) { - Some(caps) => caps.get(1).map(|arg_name| arg_name.as_str()), - None => None, - } - })), - ) -} - -pub(crate) fn check_google_section( - checker: &mut Checker, - definition: &Definition, - context: &SectionContext, -) { - sections::check_common_section(checker, definition, context, &SectionStyle::Google); - - if checker.settings.enabled.contains(&CheckCode::D416) { - let suffix = context - .line - .trim() - .strip_prefix(&context.section_name) - .unwrap(); - if suffix != ":" { - let docstring = definition - .docstring - .expect("Sections are only available for docstrings."); - checker.add_check(Check::new( - CheckKind::SectionNameEndsInColon(context.section_name.to_string()), - Range::from_located(docstring), - )) - } - } - - if checker.settings.enabled.contains(&CheckCode::D417) { - let capitalized_section_name = titlecase::titlecase(&context.section_name); - if capitalized_section_name == "Args" || capitalized_section_name == "Arguments" { - check_args_section(checker, definition, context); - } - } -} diff --git a/src/docstrings/mod.rs b/src/docstrings/mod.rs new file mode 100644 index 0000000000..1c19b1f217 --- /dev/null +++ b/src/docstrings/mod.rs @@ -0,0 +1,7 @@ +pub mod definition; +pub mod extraction; +pub mod google; +pub mod helpers; +pub mod numpy; +pub mod sections; +pub mod styles; diff --git a/src/docstrings/numpy.rs b/src/docstrings/numpy.rs index 4472ba049d..27cf8ac919 100644 --- a/src/docstrings/numpy.rs +++ b/src/docstrings/numpy.rs @@ -2,16 +2,8 @@ use std::collections::BTreeSet; -use crate::ast::types::Range; use once_cell::sync::Lazy; -use crate::check_ast::Checker; -use crate::checks::{Check, CheckCode, CheckKind}; -use crate::docstrings::definition::Definition; -use crate::docstrings::sections::SectionContext; -use crate::docstrings::styles::SectionStyle; -use crate::docstrings::{helpers, sections}; - pub(crate) static LOWERCASE_NUMPY_SECTION_NAMES: Lazy> = Lazy::new(|| { BTreeSet::from([ "short summary", @@ -47,68 +39,3 @@ pub(crate) static NUMPY_SECTION_NAMES: Lazy> = Lazy::new( "Methods", ]) }); - -fn check_parameters_section( - checker: &mut Checker, - definition: &Definition, - context: &SectionContext, -) { - // Collect the list of arguments documented in the docstring. - let mut docstring_args: BTreeSet<&str> = Default::default(); - let section_level_indent = helpers::leading_space(context.line); - for i in 1..context.following_lines.len() { - let current_line = context.following_lines[i - 1]; - let current_leading_space = helpers::leading_space(current_line); - let next_line = context.following_lines[i]; - if current_leading_space == section_level_indent - && (helpers::leading_space(next_line).len() > current_leading_space.len()) - && !next_line.trim().is_empty() - { - let parameters = if let Some(semi_index) = current_line.find(':') { - // If the parameter has a type annotation, exclude it. - ¤t_line[..semi_index] - } else { - // Otherwise, it's just a list of parameters on the current line. - current_line.trim() - }; - // Notably, NumPy lets you put multiple parameters of the same type on the same line. - for parameter in parameters.split(',') { - docstring_args.insert(parameter.trim()); - } - } - } - // Validate that all arguments were documented. - sections::check_missing_args(checker, definition, &docstring_args); -} - -pub(crate) fn check_numpy_section( - checker: &mut Checker, - definition: &Definition, - context: &SectionContext, -) { - sections::check_common_section(checker, definition, context, &SectionStyle::NumPy); - - if checker.settings.enabled.contains(&CheckCode::D406) { - let suffix = context - .line - .trim() - .strip_prefix(&context.section_name) - .unwrap(); - if !suffix.is_empty() { - let docstring = definition - .docstring - .expect("Sections are only available for docstrings."); - checker.add_check(Check::new( - CheckKind::NewLineAfterSectionName(context.section_name.to_string()), - Range::from_located(docstring), - )) - } - } - - if checker.settings.enabled.contains(&CheckCode::D417) { - let capitalized_section_name = titlecase::titlecase(&context.section_name); - if capitalized_section_name == "Parameters" { - check_parameters_section(checker, definition, context); - } - } -} diff --git a/src/docstrings/sections.rs b/src/docstrings/sections.rs index 63e33b4129..d87fa8162c 100644 --- a/src/docstrings/sections.rs +++ b/src/docstrings/sections.rs @@ -1,17 +1,5 @@ -use std::collections::BTreeSet; - -use itertools::Itertools; -use rustpython_ast::{Arg, Location, StmtKind}; -use titlecase::titlecase; - -use crate::ast::types::Range; -use crate::autofix::fixer; -use crate::check_ast::Checker; -use crate::checks::{Check, CheckCode, CheckKind, Fix}; -use crate::docstrings::definition::{Definition, DefinitionKind}; use crate::docstrings::helpers; use crate::docstrings::styles::SectionStyle; -use crate::visibility::is_static; #[derive(Debug)] pub(crate) struct SectionContext<'a> { @@ -20,7 +8,7 @@ pub(crate) struct SectionContext<'a> { pub(crate) line: &'a str, pub(crate) following_lines: &'a [&'a str], pub(crate) is_last_section: bool, - original_index: usize, + pub(crate) original_index: usize, } fn suspected_as_section(line: &str, style: &SectionStyle) -> bool { @@ -109,295 +97,3 @@ pub(crate) fn section_contexts<'a>( truncated_contexts.reverse(); truncated_contexts } - -fn check_blanks_and_section_underline( - checker: &mut Checker, - definition: &Definition, - context: &SectionContext, -) { - let docstring = definition - .docstring - .expect("Sections are only available for docstrings."); - - let mut blank_lines_after_header = 0; - for line in context.following_lines { - if !line.trim().is_empty() { - break; - } - blank_lines_after_header += 1; - } - - // Nothing but blank lines after the section header. - if blank_lines_after_header == context.following_lines.len() { - if checker.settings.enabled.contains(&CheckCode::D407) { - checker.add_check(Check::new( - CheckKind::DashedUnderlineAfterSection(context.section_name.to_string()), - Range::from_located(docstring), - )); - } - if checker.settings.enabled.contains(&CheckCode::D414) { - checker.add_check(Check::new( - CheckKind::NonEmptySection(context.section_name.to_string()), - Range::from_located(docstring), - )); - } - return; - } - - let non_empty_line = context.following_lines[blank_lines_after_header]; - let dash_line_found = non_empty_line - .chars() - .all(|char| char.is_whitespace() || char == '-'); - - if !dash_line_found { - if checker.settings.enabled.contains(&CheckCode::D407) { - checker.add_check(Check::new( - CheckKind::DashedUnderlineAfterSection(context.section_name.to_string()), - Range::from_located(docstring), - )); - } - if blank_lines_after_header > 0 { - if checker.settings.enabled.contains(&CheckCode::D212) { - checker.add_check(Check::new( - CheckKind::NoBlankLinesBetweenHeaderAndContent( - context.section_name.to_string(), - ), - Range::from_located(docstring), - )); - } - } - } else { - if blank_lines_after_header > 0 { - if checker.settings.enabled.contains(&CheckCode::D408) { - checker.add_check(Check::new( - CheckKind::SectionUnderlineAfterName(context.section_name.to_string()), - Range::from_located(docstring), - )); - } - } - - if non_empty_line - .trim() - .chars() - .filter(|char| *char == '-') - .count() - != context.section_name.len() - { - if checker.settings.enabled.contains(&CheckCode::D409) { - checker.add_check(Check::new( - CheckKind::SectionUnderlineMatchesSectionLength( - context.section_name.to_string(), - ), - Range::from_located(docstring), - )); - } - } - - if checker.settings.enabled.contains(&CheckCode::D215) { - if helpers::leading_space(non_empty_line).len() - > helpers::indentation(checker, docstring).len() - { - checker.add_check(Check::new( - CheckKind::SectionUnderlineNotOverIndented(context.section_name.to_string()), - Range::from_located(docstring), - )); - } - } - - let line_after_dashes_index = blank_lines_after_header + 1; - - if line_after_dashes_index < context.following_lines.len() { - let line_after_dashes = context.following_lines[line_after_dashes_index]; - if line_after_dashes.trim().is_empty() { - let rest_of_lines = &context.following_lines[line_after_dashes_index..]; - if rest_of_lines.iter().all(|line| line.trim().is_empty()) { - if checker.settings.enabled.contains(&CheckCode::D414) { - checker.add_check(Check::new( - CheckKind::NonEmptySection(context.section_name.to_string()), - Range::from_located(docstring), - )); - } - } else { - if checker.settings.enabled.contains(&CheckCode::D412) { - checker.add_check(Check::new( - CheckKind::NoBlankLinesBetweenHeaderAndContent( - context.section_name.to_string(), - ), - Range::from_located(docstring), - )); - } - } - } - } else { - if checker.settings.enabled.contains(&CheckCode::D414) { - checker.add_check(Check::new( - CheckKind::NonEmptySection(context.section_name.to_string()), - Range::from_located(docstring), - )); - } - } - } -} - -pub(crate) fn check_common_section( - checker: &mut Checker, - definition: &Definition, - context: &SectionContext, - style: &SectionStyle, -) { - let docstring = definition - .docstring - .expect("Sections are only available for docstrings."); - - if checker.settings.enabled.contains(&CheckCode::D405) { - if !style - .section_names() - .contains(&context.section_name.as_str()) - && style - .section_names() - .contains(titlecase(&context.section_name).as_str()) - { - checker.add_check(Check::new( - CheckKind::CapitalizeSectionName(context.section_name.to_string()), - Range::from_located(docstring), - )) - } - } - - if checker.settings.enabled.contains(&CheckCode::D214) { - if helpers::leading_space(context.line).len() - > helpers::indentation(checker, docstring).len() - { - checker.add_check(Check::new( - CheckKind::SectionNotOverIndented(context.section_name.to_string()), - Range::from_located(docstring), - )) - } - } - - if context - .following_lines - .last() - .map(|line| !line.trim().is_empty()) - .unwrap_or(true) - { - if context.is_last_section { - if checker.settings.enabled.contains(&CheckCode::D413) { - let mut check = Check::new( - CheckKind::BlankLineAfterLastSection(context.section_name.to_string()), - Range::from_located(docstring), - ); - if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) { - check.amend(Fix::insertion( - "\n".to_string(), - Location::new( - docstring.location.row() - + context.original_index - + 1 - + context.following_lines.len(), - 1, - ), - )) - } - checker.add_check(check); - } - } else { - if checker.settings.enabled.contains(&CheckCode::D410) { - let mut check = Check::new( - CheckKind::BlankLineAfterSection(context.section_name.to_string()), - Range::from_located(docstring), - ); - if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) { - check.amend(Fix::insertion( - "\n".to_string(), - Location::new( - docstring.location.row() - + context.original_index - + 1 - + context.following_lines.len(), - 1, - ), - )) - } - checker.add_check(check); - } - } - } - - if checker.settings.enabled.contains(&CheckCode::D411) { - if !context.previous_line.is_empty() { - checker.add_check(Check::new( - CheckKind::BlankLineBeforeSection(context.section_name.to_string()), - Range::from_located(docstring), - )) - } - } - - check_blanks_and_section_underline(checker, definition, context); -} - -pub(crate) fn check_missing_args( - checker: &mut Checker, - definition: &Definition, - docstrings_args: &BTreeSet<&str>, -) { - if let DefinitionKind::Function(parent) - | DefinitionKind::NestedFunction(parent) - | DefinitionKind::Method(parent) = definition.kind - { - if let StmtKind::FunctionDef { - args: arguments, .. - } - | StmtKind::AsyncFunctionDef { - args: arguments, .. - } = &parent.node - { - // Collect all the arguments into a single vector. - let mut all_arguments: Vec<&Arg> = arguments - .args - .iter() - .chain(arguments.posonlyargs.iter()) - .chain(arguments.kwonlyargs.iter()) - .skip( - // If this is a non-static method, skip `cls` or `self`. - if matches!(definition.kind, DefinitionKind::Method(_)) && !is_static(parent) { - 1 - } else { - 0 - }, - ) - .collect(); - if let Some(arg) = &arguments.vararg { - all_arguments.push(arg); - } - if let Some(arg) = &arguments.kwarg { - all_arguments.push(arg); - } - - // Look for arguments that weren't included in the docstring. - let mut missing_args: BTreeSet<&str> = Default::default(); - for arg in all_arguments { - let arg_name = arg.node.arg.as_str(); - if arg_name.starts_with('_') { - continue; - } - if docstrings_args.contains(&arg_name) { - continue; - } - missing_args.insert(arg_name); - } - - if !missing_args.is_empty() { - let names = missing_args - .into_iter() - .map(String::from) - .sorted() - .collect(); - checker.add_check(Check::new( - CheckKind::DocumentAllArguments(names), - Range::from_located(parent), - )); - } - } - } -} diff --git a/src/flake8_bugbear/mod.rs b/src/flake8_bugbear/mod.rs new file mode 100644 index 0000000000..25a06164e5 --- /dev/null +++ b/src/flake8_bugbear/mod.rs @@ -0,0 +1 @@ +pub mod plugins; diff --git a/src/plugins/assert_false.rs b/src/flake8_bugbear/plugins/assert_false.rs similarity index 100% rename from src/plugins/assert_false.rs rename to src/flake8_bugbear/plugins/assert_false.rs diff --git a/src/plugins/duplicate_exceptions.rs b/src/flake8_bugbear/plugins/duplicate_exceptions.rs similarity index 100% rename from src/plugins/duplicate_exceptions.rs rename to src/flake8_bugbear/plugins/duplicate_exceptions.rs diff --git a/src/flake8_bugbear/plugins/mod.rs b/src/flake8_bugbear/plugins/mod.rs new file mode 100644 index 0000000000..fc09f2ab5f --- /dev/null +++ b/src/flake8_bugbear/plugins/mod.rs @@ -0,0 +1,6 @@ +pub use assert_false::assert_false; +pub use duplicate_exceptions::duplicate_exceptions; +pub use duplicate_exceptions::duplicate_handler_exceptions; + +mod assert_false; +mod duplicate_exceptions; diff --git a/src/flake8_builtins/checks.rs b/src/flake8_builtins/checks.rs new file mode 100644 index 0000000000..ffb57580fb --- /dev/null +++ b/src/flake8_builtins/checks.rs @@ -0,0 +1,20 @@ +use crate::ast::types::Range; +use crate::checks::{Check, CheckKind}; +use crate::flake8_builtins::types::ShadowingType; +use crate::python::builtins::BUILTINS; + +/// Check builtin name shadowing. +pub fn builtin_shadowing(name: &str, location: Range, node_type: ShadowingType) -> Option { + if BUILTINS.contains(&name) { + Some(Check::new( + match node_type { + ShadowingType::Variable => CheckKind::BuiltinVariableShadowing(name.to_string()), + ShadowingType::Argument => CheckKind::BuiltinArgumentShadowing(name.to_string()), + ShadowingType::Attribute => CheckKind::BuiltinAttributeShadowing(name.to_string()), + }, + location, + )) + } else { + None + } +} diff --git a/src/flake8_builtins/mod.rs b/src/flake8_builtins/mod.rs new file mode 100644 index 0000000000..62a8881c0e --- /dev/null +++ b/src/flake8_builtins/mod.rs @@ -0,0 +1,2 @@ +pub mod checks; +pub mod types; diff --git a/src/flake8_builtins/types.rs b/src/flake8_builtins/types.rs new file mode 100644 index 0000000000..48b4c70b21 --- /dev/null +++ b/src/flake8_builtins/types.rs @@ -0,0 +1,5 @@ +pub enum ShadowingType { + Variable, + Argument, + Attribute, +} diff --git a/src/flake8_comprehensions/checks.rs b/src/flake8_comprehensions/checks.rs new file mode 100644 index 0000000000..5106fbba7b --- /dev/null +++ b/src/flake8_comprehensions/checks.rs @@ -0,0 +1,505 @@ +use num_bigint::BigInt; +use rustpython_ast::{Comprehension, Constant, Expr, ExprKind, KeywordData, Located, Unaryop}; + +use crate::ast::types::Range; +use crate::checks::{Check, CheckKind}; + +/// Check `list(generator)` compliance. +pub fn unnecessary_generator_list(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { + if args.len() == 1 { + if let ExprKind::Name { id, .. } = &func.node { + if id == "list" { + if let ExprKind::GeneratorExp { .. } = &args[0].node { + return Some(Check::new( + CheckKind::UnnecessaryGeneratorList, + Range::from_located(expr), + )); + } + } + } + } + None +} + +/// Check `set(generator)` compliance. +pub fn unnecessary_generator_set(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { + if args.len() == 1 { + if let ExprKind::Name { id, .. } = &func.node { + if id == "set" { + if let ExprKind::GeneratorExp { .. } = &args[0].node { + return Some(Check::new( + CheckKind::UnnecessaryGeneratorSet, + Range::from_located(expr), + )); + } + } + } + } + None +} + +/// Check `dict((x, y) for x, y in iterable)` compliance. +pub fn unnecessary_generator_dict(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { + if args.len() == 1 { + if let ExprKind::Name { id, .. } = &func.node { + if id == "dict" { + if let ExprKind::GeneratorExp { elt, .. } = &args[0].node { + match &elt.node { + ExprKind::Tuple { elts, .. } if elts.len() == 2 => { + return Some(Check::new( + CheckKind::UnnecessaryGeneratorDict, + Range::from_located(expr), + )); + } + _ => {} + } + } + } + } + } + None +} + +/// Check `set([...])` compliance. +pub fn unnecessary_list_comprehension_set( + expr: &Expr, + func: &Expr, + args: &[Expr], +) -> Option { + if args.len() == 1 { + if let ExprKind::Name { id, .. } = &func.node { + if id == "set" { + if let ExprKind::ListComp { .. } = &args[0].node { + return Some(Check::new( + CheckKind::UnnecessaryListComprehensionSet, + Range::from_located(expr), + )); + } + } + } + } + None +} + +/// Check `dict([...])` compliance. +pub fn unnecessary_list_comprehension_dict( + expr: &Expr, + func: &Expr, + args: &[Expr], +) -> Option { + if args.len() == 1 { + if let ExprKind::Name { id, .. } = &func.node { + if id == "dict" { + if let ExprKind::ListComp { elt, .. } = &args[0].node { + match &elt.node { + ExprKind::Tuple { elts, .. } if elts.len() == 2 => { + return Some(Check::new( + CheckKind::UnnecessaryListComprehensionDict, + Range::from_located(expr), + )); + } + _ => {} + } + } + } + } + } + None +} + +/// Check `set([1, 2])` compliance. +pub fn unnecessary_literal_set(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { + if args.len() == 1 { + if let ExprKind::Name { id, .. } = &func.node { + if id == "set" { + match &args[0].node { + ExprKind::List { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralSet("list".to_string()), + Range::from_located(expr), + )); + } + ExprKind::Tuple { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralSet("tuple".to_string()), + Range::from_located(expr), + )); + } + _ => {} + } + } + } + } + None +} + +/// Check `dict([(1, 2)])` compliance. +pub fn unnecessary_literal_dict(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { + if args.len() == 1 { + if let ExprKind::Name { id, .. } = &func.node { + if id == "dict" { + match &args[0].node { + ExprKind::Tuple { elts, .. } => { + if let Some(elt) = elts.first() { + match &elt.node { + // dict((1, 2), ...)) + ExprKind::Tuple { elts, .. } if elts.len() == 2 => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralDict("tuple".to_string()), + Range::from_located(expr), + )); + } + _ => {} + } + } else { + // dict(()) + return Some(Check::new( + CheckKind::UnnecessaryLiteralDict("tuple".to_string()), + Range::from_located(expr), + )); + } + } + ExprKind::List { elts, .. } => { + if let Some(elt) = elts.first() { + match &elt.node { + // dict([(1, 2), ...]) + ExprKind::Tuple { elts, .. } if elts.len() == 2 => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralDict("list".to_string()), + Range::from_located(expr), + )); + } + _ => {} + } + } else { + // dict([]) + return Some(Check::new( + CheckKind::UnnecessaryLiteralDict("list".to_string()), + Range::from_located(expr), + )); + } + } + _ => {} + } + } + } + } + None +} + +pub fn unnecessary_collection_call( + expr: &Expr, + func: &Expr, + args: &[Expr], + keywords: &[Located], +) -> Option { + if args.is_empty() { + if let ExprKind::Name { id, .. } = &func.node { + if id == "list" || id == "tuple" { + // list() or tuple() + return Some(Check::new( + CheckKind::UnnecessaryCollectionCall(id.to_string()), + Range::from_located(expr), + )); + } else if id == "dict" { + // dict() or dict(a=1) + if keywords.is_empty() || keywords.iter().all(|kw| kw.node.arg.is_some()) { + return Some(Check::new( + CheckKind::UnnecessaryCollectionCall(id.to_string()), + Range::from_located(expr), + )); + } + } + } + } + None +} + +pub fn unnecessary_literal_within_tuple_call( + expr: &Expr, + func: &Expr, + args: &[Expr], +) -> Option { + if let ExprKind::Name { id, .. } = &func.node { + if id == "tuple" { + if let Some(arg) = args.first() { + match &arg.node { + ExprKind::Tuple { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralWithinTupleCall("tuple".to_string()), + Range::from_located(expr), + )); + } + ExprKind::List { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralWithinTupleCall("list".to_string()), + Range::from_located(expr), + )); + } + _ => {} + } + } + } + } + + None +} + +pub fn unnecessary_literal_within_list_call( + expr: &Expr, + func: &Expr, + args: &[Expr], +) -> Option { + if let ExprKind::Name { id, .. } = &func.node { + if id == "list" { + if let Some(arg) = args.first() { + match &arg.node { + ExprKind::Tuple { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralWithinListCall("tuple".to_string()), + Range::from_located(expr), + )); + } + ExprKind::List { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralWithinListCall("list".to_string()), + Range::from_located(expr), + )); + } + _ => {} + } + } + } + } + + None +} + +pub fn unnecessary_list_call(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { + if let ExprKind::Name { id, .. } = &func.node { + if id == "list" { + if let Some(arg) = args.first() { + if let ExprKind::ListComp { .. } = &arg.node { + return Some(Check::new( + CheckKind::UnnecessaryListCall, + Range::from_located(expr), + )); + } + } + } + } + None +} + +pub fn unnecessary_call_around_sorted(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { + if let ExprKind::Name { id: outer, .. } = &func.node { + if outer == "list" || outer == "reversed" { + if let Some(arg) = args.first() { + if let ExprKind::Call { func, .. } = &arg.node { + if let ExprKind::Name { id: inner, .. } = &func.node { + if inner == "sorted" { + return Some(Check::new( + CheckKind::UnnecessaryCallAroundSorted(outer.to_string()), + Range::from_located(expr), + )); + } + } + } + } + } + } + None +} + +pub fn unnecessary_double_cast_or_process( + expr: &Expr, + func: &Expr, + args: &[Expr], +) -> Option { + if let ExprKind::Name { id: outer, .. } = &func.node { + if outer == "list" + || outer == "tuple" + || outer == "set" + || outer == "reversed" + || outer == "sorted" + { + if let Some(arg) = args.first() { + if let ExprKind::Call { func, .. } = &arg.node { + if let ExprKind::Name { id: inner, .. } = &func.node { + // Ex) set(tuple(...)) + if (outer == "set" || outer == "sorted") + && (inner == "list" + || inner == "tuple" + || inner == "reversed" + || inner == "sorted") + { + return Some(Check::new( + CheckKind::UnnecessaryDoubleCastOrProcess( + inner.to_string(), + outer.to_string(), + ), + Range::from_located(expr), + )); + } + + // Ex) list(tuple(...)) + if (outer == "list" || outer == "tuple") + && (inner == "list" || inner == "tuple") + { + return Some(Check::new( + CheckKind::UnnecessaryDoubleCastOrProcess( + inner.to_string(), + outer.to_string(), + ), + Range::from_located(expr), + )); + } + + // Ex) set(set(...)) + if outer == "set" && inner == "set" { + return Some(Check::new( + CheckKind::UnnecessaryDoubleCastOrProcess( + inner.to_string(), + outer.to_string(), + ), + Range::from_located(expr), + )); + } + } + } + } + } + } + None +} + +pub fn unnecessary_subscript_reversal(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { + if let Some(first_arg) = args.first() { + if let ExprKind::Name { id, .. } = &func.node { + if id == "set" || id == "sorted" || id == "reversed" { + if let ExprKind::Subscript { slice, .. } = &first_arg.node { + if let ExprKind::Slice { lower, upper, step } = &slice.node { + if lower.is_none() && upper.is_none() { + if let Some(step) = step { + if let ExprKind::UnaryOp { + op: Unaryop::USub, + operand, + } = &step.node + { + if let ExprKind::Constant { + value: Constant::Int(val), + .. + } = &operand.node + { + if *val == BigInt::from(1) { + return Some(Check::new( + CheckKind::UnnecessarySubscriptReversal( + id.to_string(), + ), + Range::from_located(expr), + )); + } + } + } + } + } + } + } + } + } + } + None +} + +pub fn unnecessary_comprehension( + expr: &Expr, + elt: &Expr, + generators: &[Comprehension], +) -> Option { + if generators.len() == 1 { + let generator = &generators[0]; + if generator.ifs.is_empty() && generator.is_async == 0 { + if let ExprKind::Name { id: elt_id, .. } = &elt.node { + if let ExprKind::Name { id: target_id, .. } = &generator.target.node { + if elt_id == target_id { + match &expr.node { + ExprKind::ListComp { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryComprehension("list".to_string()), + Range::from_located(expr), + )) + } + ExprKind::SetComp { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryComprehension("set".to_string()), + Range::from_located(expr), + )) + } + _ => {} + }; + } + } + } + } + } + + None +} + +pub fn unnecessary_map(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { + if let ExprKind::Name { id, .. } = &func.node { + if id == "map" { + if args.len() == 2 { + if let ExprKind::Lambda { .. } = &args[0].node { + return Some(Check::new( + CheckKind::UnnecessaryMap("generator".to_string()), + Range::from_located(expr), + )); + } + } + } else if id == "list" || id == "set" { + if let Some(arg) = args.first() { + if let ExprKind::Call { func, args, .. } = &arg.node { + if let ExprKind::Name { id: f, .. } = &func.node { + if f == "map" { + if let Some(arg) = args.first() { + if let ExprKind::Lambda { .. } = &arg.node { + return Some(Check::new( + CheckKind::UnnecessaryMap(id.to_string()), + Range::from_located(expr), + )); + } + } + } + } + } + } + } else if id == "dict" { + if args.len() == 1 { + if let ExprKind::Call { func, args, .. } = &args[0].node { + if let ExprKind::Name { id: f, .. } = &func.node { + if f == "map" { + if let Some(arg) = args.first() { + if let ExprKind::Lambda { body, .. } = &arg.node { + match &body.node { + ExprKind::Tuple { elts, .. } + | ExprKind::List { elts, .. } + if elts.len() == 2 => + { + return Some(Check::new( + CheckKind::UnnecessaryMap(id.to_string()), + Range::from_located(expr), + )) + } + _ => {} + } + } + } + } + } + } + } + } + } + None +} diff --git a/src/flake8_comprehensions/mod.rs b/src/flake8_comprehensions/mod.rs new file mode 100644 index 0000000000..f6b5329a02 --- /dev/null +++ b/src/flake8_comprehensions/mod.rs @@ -0,0 +1 @@ +pub mod checks; diff --git a/src/flake8_print/checks.rs b/src/flake8_print/checks.rs new file mode 100644 index 0000000000..241e8ca677 --- /dev/null +++ b/src/flake8_print/checks.rs @@ -0,0 +1,36 @@ +use rustpython_ast::{Expr, ExprKind}; + +use crate::ast::types::Range; +use crate::checks::{Check, CheckKind}; + +/// Check whether a function call is a `print` or `pprint` invocation +pub fn print_call( + expr: &Expr, + func: &Expr, + check_print: bool, + check_pprint: bool, +) -> Option { + if let ExprKind::Name { id, .. } = &func.node { + if check_print && id == "print" { + return Some(Check::new(CheckKind::PrintFound, Range::from_located(expr))); + } else if check_pprint && id == "pprint" { + return Some(Check::new( + CheckKind::PPrintFound, + Range::from_located(expr), + )); + } + } + + if let ExprKind::Attribute { value, attr, .. } = &func.node { + if let ExprKind::Name { id, .. } = &value.node { + if check_pprint && id == "pprint" && attr == "pprint" { + return Some(Check::new( + CheckKind::PPrintFound, + Range::from_located(expr), + )); + } + } + } + + None +} diff --git a/src/flake8_print/mod.rs b/src/flake8_print/mod.rs new file mode 100644 index 0000000000..a6dbe125cb --- /dev/null +++ b/src/flake8_print/mod.rs @@ -0,0 +1,2 @@ +mod checks; +pub mod plugins; diff --git a/src/flake8_print/plugins/mod.rs b/src/flake8_print/plugins/mod.rs new file mode 100644 index 0000000000..feda2e53f4 --- /dev/null +++ b/src/flake8_print/plugins/mod.rs @@ -0,0 +1,3 @@ +pub use print_call::print_call; + +mod print_call; diff --git a/src/plugins/print_call.rs b/src/flake8_print/plugins/print_call.rs similarity index 94% rename from src/plugins/print_call.rs rename to src/flake8_print/plugins/print_call.rs index 8f8604a1a3..a9730be214 100644 --- a/src/plugins/print_call.rs +++ b/src/flake8_print/plugins/print_call.rs @@ -1,13 +1,13 @@ use log::error; use rustpython_ast::{Expr, Stmt, StmtKind}; -use crate::ast::checkers; use crate::autofix::{fixer, fixes}; use crate::check_ast::Checker; use crate::checks::CheckCode; +use crate::flake8_print::checks; pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) { - if let Some(mut check) = checkers::print_call( + if let Some(mut check) = checks::print_call( expr, func, checker.settings.enabled.contains(&CheckCode::T201), diff --git a/src/lib.rs b/src/lib.rs index 305f240a01..1793b0301c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,15 +20,23 @@ pub mod checks; pub mod cli; pub mod code_gen; mod docstrings; +mod flake8_bugbear; +mod flake8_builtins; +mod flake8_comprehensions; +mod flake8_print; pub mod fs; pub mod linter; pub mod logging; pub mod message; mod noqa; -mod plugins; +mod pep8_naming; pub mod printer; +mod pycodestyle; +mod pydocstyle; +mod pyflakes; pub mod pyproject; mod python; +mod pyupgrade; pub mod settings; pub mod visibility; diff --git a/src/pep8_naming/checks.rs b/src/pep8_naming/checks.rs new file mode 100644 index 0000000000..4c5781c2db --- /dev/null +++ b/src/pep8_naming/checks.rs @@ -0,0 +1,99 @@ +use rustpython_ast::{Arguments, Expr, ExprKind, Stmt}; + +use crate::ast::types::{Range, Scope, ScopeKind}; +use crate::checks::{Check, CheckKind}; + +pub fn invalid_class_name(class_def: &Stmt, name: &str) -> Option { + let stripped = name.strip_prefix('_').unwrap_or(name); + if !stripped + .chars() + .next() + .map(|c| c.is_uppercase()) + .unwrap_or(false) + || stripped.contains('_') + { + return Some(Check::new( + CheckKind::InvalidClassName(name.to_string()), + Range::from_located(class_def), + )); + } + None +} + +pub fn invalid_function_name(func_def: &Stmt, name: &str) -> Option { + if name.chars().any(|c| c.is_uppercase()) { + return Some(Check::new( + CheckKind::InvalidFunctionName(name.to_string()), + Range::from_located(func_def), + )); + } + None +} + +pub fn invalid_argument_name(location: Range, name: &str) -> Option { + if name.chars().any(|c| c.is_uppercase()) { + return Some(Check::new( + CheckKind::InvalidArgumentName(name.to_string()), + location, + )); + } + None +} + +pub fn invalid_first_argument_name_for_class_method( + scope: &Scope, + decorator_list: &[Expr], + args: &Arguments, +) -> Option { + if !matches!(scope.kind, ScopeKind::Class) { + return None; + } + + if decorator_list.iter().any(|decorator| { + if let ExprKind::Name { id, .. } = &decorator.node { + id == "classmethod" + } else { + false + } + }) { + if let Some(arg) = args.args.first() { + if arg.node.arg != "cls" { + return Some(Check::new( + CheckKind::InvalidFirstArgumentNameForClassMethod, + Range::from_located(arg), + )); + } + } + } + None +} + +pub fn invalid_first_argument_name_for_method( + scope: &Scope, + decorator_list: &[Expr], + args: &Arguments, +) -> Option { + if !matches!(scope.kind, ScopeKind::Class) { + return None; + } + + if decorator_list.iter().any(|decorator| { + if let ExprKind::Name { id, .. } = &decorator.node { + id == "classmethod" || id == "staticmethod" + } else { + false + } + }) { + return None; + } + + if let Some(arg) = args.args.first() { + if arg.node.arg != "self" { + return Some(Check::new( + CheckKind::InvalidFirstArgumentNameForMethod, + Range::from_located(arg), + )); + } + } + None +} diff --git a/src/pep8_naming/mod.rs b/src/pep8_naming/mod.rs new file mode 100644 index 0000000000..f6b5329a02 --- /dev/null +++ b/src/pep8_naming/mod.rs @@ -0,0 +1 @@ +pub mod checks; diff --git a/src/pycodestyle/checks.rs b/src/pycodestyle/checks.rs new file mode 100644 index 0000000000..f02965ca65 --- /dev/null +++ b/src/pycodestyle/checks.rs @@ -0,0 +1,238 @@ +use itertools::izip; +use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprKind, Unaryop}; + +use crate::ast::types::{CheckLocator, Range}; +use crate::checks::{Check, CheckKind, RejectedCmpop}; + +fn is_ambiguous_name(name: &str) -> bool { + name == "l" || name == "I" || name == "O" +} + +/// Check AmbiguousVariableName compliance. +pub fn ambiguous_variable_name(name: &str, location: Range) -> Option { + if is_ambiguous_name(name) { + Some(Check::new( + CheckKind::AmbiguousVariableName(name.to_string()), + location, + )) + } else { + None + } +} + +/// Check AmbiguousClassName compliance. +pub fn ambiguous_class_name(name: &str, location: Range) -> Option { + if is_ambiguous_name(name) { + Some(Check::new( + CheckKind::AmbiguousClassName(name.to_string()), + location, + )) + } else { + None + } +} + +/// Check AmbiguousFunctionName compliance. +pub fn ambiguous_function_name(name: &str, location: Range) -> Option { + if is_ambiguous_name(name) { + Some(Check::new( + CheckKind::AmbiguousFunctionName(name.to_string()), + location, + )) + } else { + None + } +} + +/// Check DoNotAssignLambda compliance. +pub fn do_not_assign_lambda(value: &Expr, location: Range) -> Option { + if let ExprKind::Lambda { .. } = &value.node { + Some(Check::new(CheckKind::DoNotAssignLambda, location)) + } else { + None + } +} + +/// Check NotInTest and NotIsTest compliance. +pub fn not_tests( + op: &Unaryop, + operand: &Expr, + check_not_in: bool, + check_not_is: bool, + locator: &dyn CheckLocator, +) -> Vec { + let mut checks: Vec = vec![]; + + if matches!(op, Unaryop::Not) { + if let ExprKind::Compare { ops, .. } = &operand.node { + for op in ops { + match op { + Cmpop::In => { + if check_not_in { + checks.push(Check::new( + CheckKind::NotInTest, + locator.locate_check(Range::from_located(operand)), + )); + } + } + Cmpop::Is => { + if check_not_is { + checks.push(Check::new( + CheckKind::NotIsTest, + locator.locate_check(Range::from_located(operand)), + )); + } + } + _ => {} + } + } + } + } + + checks +} + +/// Check TrueFalseComparison and NoneComparison compliance. +pub fn literal_comparisons( + left: &Expr, + ops: &[Cmpop], + comparators: &[Expr], + check_none_comparisons: bool, + check_true_false_comparisons: bool, + locator: &dyn CheckLocator, +) -> Vec { + let mut checks: Vec = vec![]; + + let op = ops.first().unwrap(); + let comparator = left; + + // Check `left`. + if check_none_comparisons + && matches!( + comparator.node, + ExprKind::Constant { + value: Constant::None, + kind: None + } + ) + { + if matches!(op, Cmpop::Eq) { + checks.push(Check::new( + CheckKind::NoneComparison(RejectedCmpop::Eq), + locator.locate_check(Range::from_located(comparator)), + )); + } + if matches!(op, Cmpop::NotEq) { + checks.push(Check::new( + CheckKind::NoneComparison(RejectedCmpop::NotEq), + locator.locate_check(Range::from_located(comparator)), + )); + } + } + + if check_true_false_comparisons { + if let ExprKind::Constant { + value: Constant::Bool(value), + kind: None, + } = comparator.node + { + if matches!(op, Cmpop::Eq) { + checks.push(Check::new( + CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq), + locator.locate_check(Range::from_located(comparator)), + )); + } + if matches!(op, Cmpop::NotEq) { + checks.push(Check::new( + CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq), + locator.locate_check(Range::from_located(comparator)), + )); + } + } + } + + // Check each comparator in order. + for (op, comparator) in izip!(ops, comparators) { + if check_none_comparisons + && matches!( + comparator.node, + ExprKind::Constant { + value: Constant::None, + kind: None + } + ) + { + if matches!(op, Cmpop::Eq) { + checks.push(Check::new( + CheckKind::NoneComparison(RejectedCmpop::Eq), + locator.locate_check(Range::from_located(comparator)), + )); + } + if matches!(op, Cmpop::NotEq) { + checks.push(Check::new( + CheckKind::NoneComparison(RejectedCmpop::NotEq), + locator.locate_check(Range::from_located(comparator)), + )); + } + } + + if check_true_false_comparisons { + if let ExprKind::Constant { + value: Constant::Bool(value), + kind: None, + } = comparator.node + { + if matches!(op, Cmpop::Eq) { + checks.push(Check::new( + CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq), + locator.locate_check(Range::from_located(comparator)), + )); + } + if matches!(op, Cmpop::NotEq) { + checks.push(Check::new( + CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq), + locator.locate_check(Range::from_located(comparator)), + )); + } + } + } + } + + checks +} + +/// Check TypeComparison compliance. +pub fn type_comparison(ops: &[Cmpop], comparators: &[Expr], location: Range) -> Vec { + let mut checks: Vec = vec![]; + + for (op, right) in izip!(ops, comparators) { + if matches!(op, Cmpop::Is | Cmpop::IsNot | Cmpop::Eq | Cmpop::NotEq) { + match &right.node { + ExprKind::Call { func, args, .. } => { + if let ExprKind::Name { id, .. } = &func.node { + // Ex) type(False) + if id == "type" { + if let Some(arg) = args.first() { + // Allow comparison for types which are not obvious. + if !matches!(arg.node, ExprKind::Name { .. }) { + checks.push(Check::new(CheckKind::TypeComparison, location)); + } + } + } + } + } + ExprKind::Attribute { value, .. } => { + if let ExprKind::Name { id, .. } = &value.node { + // Ex) types.IntType + if id == "types" { + checks.push(Check::new(CheckKind::TypeComparison, location)); + } + } + } + _ => {} + } + } + } + + checks +} diff --git a/src/pycodestyle/mod.rs b/src/pycodestyle/mod.rs new file mode 100644 index 0000000000..f6b5329a02 --- /dev/null +++ b/src/pycodestyle/mod.rs @@ -0,0 +1 @@ +pub mod checks; diff --git a/src/pydocstyle/mod.rs b/src/pydocstyle/mod.rs new file mode 100644 index 0000000000..25a06164e5 --- /dev/null +++ b/src/pydocstyle/mod.rs @@ -0,0 +1 @@ +pub mod plugins; diff --git a/src/docstrings/plugins.rs b/src/pydocstyle/plugins.rs similarity index 65% rename from src/docstrings/plugins.rs rename to src/pydocstyle/plugins.rs index 36275d9c08..d5e5360abd 100644 --- a/src/docstrings/plugins.rs +++ b/src/pydocstyle/plugins.rs @@ -1,22 +1,20 @@ -//! Abstractions for tracking and validating docstrings in Python code. +use std::collections::BTreeSet; +use itertools::Itertools; use once_cell::sync::Lazy; use regex::Regex; -use rustpython_ast::{Constant, ExprKind, Location, StmtKind}; +use rustpython_ast::{Arg, Constant, ExprKind, Location, StmtKind}; +use titlecase::titlecase; use crate::ast::types::Range; use crate::autofix::fixer; use crate::check_ast::Checker; use crate::checks::{Check, CheckCode, CheckKind, Fix}; use crate::docstrings::definition::{Definition, DefinitionKind}; -use crate::docstrings::google::check_google_section; -use crate::docstrings::helpers::{ - indentation, leading_space, SINGLE_QUOTE_PREFIXES, TRIPLE_QUOTE_PREFIXES, -}; -use crate::docstrings::numpy::check_numpy_section; -use crate::docstrings::sections::section_contexts; +use crate::docstrings::helpers; +use crate::docstrings::sections::{section_contexts, SectionContext}; use crate::docstrings::styles::SectionStyle; -use crate::visibility::{is_init, is_magic, is_overload, Visibility}; +use crate::visibility::{is_init, is_magic, is_overload, is_static, Visibility}; /// D100, D101, D102, D103, D104, D105, D106, D107 pub fn not_missing( @@ -385,7 +383,7 @@ pub fn indent(checker: &mut Checker, definition: &Definition) { let mut has_seen_over_indent = false; let mut has_seen_under_indent = false; - let docstring_indent = indentation(checker, docstring).to_string(); + let docstring_indent = helpers::indentation(checker, docstring).to_string(); if !has_seen_tab { if docstring_indent.contains('\t') { if checker.settings.enabled.contains(&CheckCode::D206) { @@ -410,7 +408,7 @@ pub fn indent(checker: &mut Checker, definition: &Definition) { continue; } - let line_indent = leading_space(lines[i]); + let line_indent = helpers::leading_space(lines[i]); if !has_seen_tab { if line_indent.contains('\t') { if checker.settings.enabled.contains(&CheckCode::D206) { @@ -478,7 +476,7 @@ pub fn newline_after_last_paragraph(checker: &mut Checker, definition: &Definiti { // Insert a newline just before the end-quote(s). let mut content = "\n".to_string(); - content.push_str(indentation(checker, docstring)); + content.push_str(helpers::indentation(checker, docstring)); check.amend(Fix::insertion( content, Location::new( @@ -524,7 +522,9 @@ pub fn no_surrounding_whitespace(checker: &mut Checker, definition: &Definition) .next() .map(|line| line.to_lowercase()) { - for pattern in TRIPLE_QUOTE_PREFIXES.iter().chain(SINGLE_QUOTE_PREFIXES) + for pattern in helpers::TRIPLE_QUOTE_PREFIXES + .iter() + .chain(helpers::SINGLE_QUOTE_PREFIXES) { if first_line.starts_with(pattern) { check.amend(Fix::replacement( @@ -568,7 +568,7 @@ pub fn multi_line_summary_start(checker: &mut Checker, definition: &Definition) .next() .map(|line| line.to_lowercase()) { - if TRIPLE_QUOTE_PREFIXES.contains(&first_line.as_str()) { + if helpers::TRIPLE_QUOTE_PREFIXES.contains(&first_line.as_str()) { if checker.settings.enabled.contains(&CheckCode::D212) { checker.add_check(Check::new( CheckKind::MultiLineSummaryFirstLine, @@ -812,15 +812,426 @@ pub fn sections(checker: &mut Checker, definition: &Definition) { let mut found_numpy_section = false; for context in §ion_contexts(&lines, &SectionStyle::NumPy) { found_numpy_section = true; - check_numpy_section(checker, definition, context); + numpy_section(checker, definition, context); } // If no such sections were identified, interpret as Google-style sections. if !found_numpy_section { for context in §ion_contexts(&lines, &SectionStyle::Google) { - check_google_section(checker, definition, context); + google_section(checker, definition, context); } } } } } + +fn blanks_and_section_underline( + checker: &mut Checker, + definition: &Definition, + context: &SectionContext, +) { + let docstring = definition + .docstring + .expect("Sections are only available for docstrings."); + + let mut blank_lines_after_header = 0; + for line in context.following_lines { + if !line.trim().is_empty() { + break; + } + blank_lines_after_header += 1; + } + + // Nothing but blank lines after the section header. + if blank_lines_after_header == context.following_lines.len() { + if checker.settings.enabled.contains(&CheckCode::D407) { + checker.add_check(Check::new( + CheckKind::DashedUnderlineAfterSection(context.section_name.to_string()), + Range::from_located(docstring), + )); + } + if checker.settings.enabled.contains(&CheckCode::D414) { + checker.add_check(Check::new( + CheckKind::NonEmptySection(context.section_name.to_string()), + Range::from_located(docstring), + )); + } + return; + } + + let non_empty_line = context.following_lines[blank_lines_after_header]; + let dash_line_found = non_empty_line + .chars() + .all(|char| char.is_whitespace() || char == '-'); + + if !dash_line_found { + if checker.settings.enabled.contains(&CheckCode::D407) { + checker.add_check(Check::new( + CheckKind::DashedUnderlineAfterSection(context.section_name.to_string()), + Range::from_located(docstring), + )); + } + if blank_lines_after_header > 0 { + if checker.settings.enabled.contains(&CheckCode::D212) { + checker.add_check(Check::new( + CheckKind::NoBlankLinesBetweenHeaderAndContent( + context.section_name.to_string(), + ), + Range::from_located(docstring), + )); + } + } + } else { + if blank_lines_after_header > 0 { + if checker.settings.enabled.contains(&CheckCode::D408) { + checker.add_check(Check::new( + CheckKind::SectionUnderlineAfterName(context.section_name.to_string()), + Range::from_located(docstring), + )); + } + } + + if non_empty_line + .trim() + .chars() + .filter(|char| *char == '-') + .count() + != context.section_name.len() + { + if checker.settings.enabled.contains(&CheckCode::D409) { + checker.add_check(Check::new( + CheckKind::SectionUnderlineMatchesSectionLength( + context.section_name.to_string(), + ), + Range::from_located(docstring), + )); + } + } + + if checker.settings.enabled.contains(&CheckCode::D215) { + if helpers::leading_space(non_empty_line).len() + > helpers::indentation(checker, docstring).len() + { + checker.add_check(Check::new( + CheckKind::SectionUnderlineNotOverIndented(context.section_name.to_string()), + Range::from_located(docstring), + )); + } + } + + let line_after_dashes_index = blank_lines_after_header + 1; + + if line_after_dashes_index < context.following_lines.len() { + let line_after_dashes = context.following_lines[line_after_dashes_index]; + if line_after_dashes.trim().is_empty() { + let rest_of_lines = &context.following_lines[line_after_dashes_index..]; + if rest_of_lines.iter().all(|line| line.trim().is_empty()) { + if checker.settings.enabled.contains(&CheckCode::D414) { + checker.add_check(Check::new( + CheckKind::NonEmptySection(context.section_name.to_string()), + Range::from_located(docstring), + )); + } + } else { + if checker.settings.enabled.contains(&CheckCode::D412) { + checker.add_check(Check::new( + CheckKind::NoBlankLinesBetweenHeaderAndContent( + context.section_name.to_string(), + ), + Range::from_located(docstring), + )); + } + } + } + } else { + if checker.settings.enabled.contains(&CheckCode::D414) { + checker.add_check(Check::new( + CheckKind::NonEmptySection(context.section_name.to_string()), + Range::from_located(docstring), + )); + } + } + } +} + +fn common_section( + checker: &mut Checker, + definition: &Definition, + context: &SectionContext, + style: &SectionStyle, +) { + let docstring = definition + .docstring + .expect("Sections are only available for docstrings."); + + if checker.settings.enabled.contains(&CheckCode::D405) { + if !style + .section_names() + .contains(&context.section_name.as_str()) + && style + .section_names() + .contains(titlecase(&context.section_name).as_str()) + { + checker.add_check(Check::new( + CheckKind::CapitalizeSectionName(context.section_name.to_string()), + Range::from_located(docstring), + )) + } + } + + if checker.settings.enabled.contains(&CheckCode::D214) { + if helpers::leading_space(context.line).len() + > helpers::indentation(checker, docstring).len() + { + checker.add_check(Check::new( + CheckKind::SectionNotOverIndented(context.section_name.to_string()), + Range::from_located(docstring), + )) + } + } + + if context + .following_lines + .last() + .map(|line| !line.trim().is_empty()) + .unwrap_or(true) + { + if context.is_last_section { + if checker.settings.enabled.contains(&CheckCode::D413) { + let mut check = Check::new( + CheckKind::BlankLineAfterLastSection(context.section_name.to_string()), + Range::from_located(docstring), + ); + if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) { + check.amend(Fix::insertion( + "\n".to_string(), + Location::new( + docstring.location.row() + + context.original_index + + 1 + + context.following_lines.len(), + 1, + ), + )) + } + checker.add_check(check); + } + } else { + if checker.settings.enabled.contains(&CheckCode::D410) { + let mut check = Check::new( + CheckKind::BlankLineAfterSection(context.section_name.to_string()), + Range::from_located(docstring), + ); + if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) { + check.amend(Fix::insertion( + "\n".to_string(), + Location::new( + docstring.location.row() + + context.original_index + + 1 + + context.following_lines.len(), + 1, + ), + )) + } + checker.add_check(check); + } + } + } + + if checker.settings.enabled.contains(&CheckCode::D411) { + if !context.previous_line.is_empty() { + checker.add_check(Check::new( + CheckKind::BlankLineBeforeSection(context.section_name.to_string()), + Range::from_located(docstring), + )) + } + } + + blanks_and_section_underline(checker, definition, context); +} + +fn missing_args(checker: &mut Checker, definition: &Definition, docstrings_args: &BTreeSet<&str>) { + if let DefinitionKind::Function(parent) + | DefinitionKind::NestedFunction(parent) + | DefinitionKind::Method(parent) = definition.kind + { + if let StmtKind::FunctionDef { + args: arguments, .. + } + | StmtKind::AsyncFunctionDef { + args: arguments, .. + } = &parent.node + { + // Collect all the arguments into a single vector. + let mut all_arguments: Vec<&Arg> = arguments + .args + .iter() + .chain(arguments.posonlyargs.iter()) + .chain(arguments.kwonlyargs.iter()) + .skip( + // If this is a non-static method, skip `cls` or `self`. + if matches!(definition.kind, DefinitionKind::Method(_)) && !is_static(parent) { + 1 + } else { + 0 + }, + ) + .collect(); + if let Some(arg) = &arguments.vararg { + all_arguments.push(arg); + } + if let Some(arg) = &arguments.kwarg { + all_arguments.push(arg); + } + + // Look for arguments that weren't included in the docstring. + let mut missing_args: BTreeSet<&str> = Default::default(); + for arg in all_arguments { + let arg_name = arg.node.arg.as_str(); + if arg_name.starts_with('_') { + continue; + } + if docstrings_args.contains(&arg_name) { + continue; + } + missing_args.insert(arg_name); + } + + if !missing_args.is_empty() { + let names = missing_args + .into_iter() + .map(String::from) + .sorted() + .collect(); + checker.add_check(Check::new( + CheckKind::DocumentAllArguments(names), + Range::from_located(parent), + )); + } + } + } +} + +// See: `GOOGLE_ARGS_REGEX` in `pydocstyle/checker.py`. +static GOOGLE_ARGS_REGEX: Lazy = + Lazy::new(|| Regex::new(r"^\s*(\w+)\s*(\(.*?\))?\s*:\n?\s*.+").expect("Invalid regex")); + +fn args_section(checker: &mut Checker, definition: &Definition, context: &SectionContext) { + let mut args_sections: Vec = vec![]; + for line in textwrap::dedent(&context.following_lines.join("\n")).lines() { + if line + .chars() + .next() + .map(|char| char.is_whitespace()) + .unwrap_or(true) + { + // This is a continuation of documentation for the last + // parameter because it does start with whitespace. + if let Some(current) = args_sections.last_mut() { + current.push_str(line); + } + } else { + // This line is the start of documentation for the next + // parameter because it doesn't start with any whitespace. + args_sections.push(line.to_string()); + } + } + + missing_args( + checker, + definition, + // Collect the list of arguments documented in the docstring. + &BTreeSet::from_iter(args_sections.iter().filter_map(|section| { + match GOOGLE_ARGS_REGEX.captures(section.as_str()) { + Some(caps) => caps.get(1).map(|arg_name| arg_name.as_str()), + None => None, + } + })), + ) +} + +fn parameters_section(checker: &mut Checker, definition: &Definition, context: &SectionContext) { + // Collect the list of arguments documented in the docstring. + let mut docstring_args: BTreeSet<&str> = Default::default(); + let section_level_indent = helpers::leading_space(context.line); + for i in 1..context.following_lines.len() { + let current_line = context.following_lines[i - 1]; + let current_leading_space = helpers::leading_space(current_line); + let next_line = context.following_lines[i]; + if current_leading_space == section_level_indent + && (helpers::leading_space(next_line).len() > current_leading_space.len()) + && !next_line.trim().is_empty() + { + let parameters = if let Some(semi_index) = current_line.find(':') { + // If the parameter has a type annotation, exclude it. + ¤t_line[..semi_index] + } else { + // Otherwise, it's just a list of parameters on the current line. + current_line.trim() + }; + // Notably, NumPy lets you put multiple parameters of the same type on the same line. + for parameter in parameters.split(',') { + docstring_args.insert(parameter.trim()); + } + } + } + // Validate that all arguments were documented. + missing_args(checker, definition, &docstring_args); +} + +fn numpy_section(checker: &mut Checker, definition: &Definition, context: &SectionContext) { + common_section(checker, definition, context, &SectionStyle::NumPy); + + if checker.settings.enabled.contains(&CheckCode::D406) { + let suffix = context + .line + .trim() + .strip_prefix(&context.section_name) + .unwrap(); + if !suffix.is_empty() { + let docstring = definition + .docstring + .expect("Sections are only available for docstrings."); + checker.add_check(Check::new( + CheckKind::NewLineAfterSectionName(context.section_name.to_string()), + Range::from_located(docstring), + )) + } + } + + if checker.settings.enabled.contains(&CheckCode::D417) { + let capitalized_section_name = titlecase::titlecase(&context.section_name); + if capitalized_section_name == "Parameters" { + parameters_section(checker, definition, context); + } + } +} + +fn google_section(checker: &mut Checker, definition: &Definition, context: &SectionContext) { + common_section(checker, definition, context, &SectionStyle::Google); + + if checker.settings.enabled.contains(&CheckCode::D416) { + let suffix = context + .line + .trim() + .strip_prefix(&context.section_name) + .unwrap(); + if suffix != ":" { + let docstring = definition + .docstring + .expect("Sections are only available for docstrings."); + checker.add_check(Check::new( + CheckKind::SectionNameEndsInColon(context.section_name.to_string()), + Range::from_located(docstring), + )) + } + } + + if checker.settings.enabled.contains(&CheckCode::D417) { + let capitalized_section_name = titlecase::titlecase(&context.section_name); + if capitalized_section_name == "Args" || capitalized_section_name == "Arguments" { + args_section(checker, definition, context); + } + } +} diff --git a/src/pyflakes/checks.rs b/src/pyflakes/checks.rs new file mode 100644 index 0000000000..da4cb61360 --- /dev/null +++ b/src/pyflakes/checks.rs @@ -0,0 +1,345 @@ +use std::collections::BTreeSet; + +use itertools::izip; +use regex::Regex; +use rustpython_parser::ast::{ + Arg, Arguments, Cmpop, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, + StmtKind, +}; + +use crate::ast::types::{BindingKind, CheckLocator, FunctionScope, Range, Scope, ScopeKind}; +use crate::checks::{Check, CheckKind}; + +/// Check IfTuple compliance. +pub fn if_tuple(test: &Expr, location: Range) -> Option { + if let ExprKind::Tuple { elts, .. } = &test.node { + if !elts.is_empty() { + return Some(Check::new(CheckKind::IfTuple, location)); + } + } + None +} + +/// Check AssertTuple compliance. +pub fn assert_tuple(test: &Expr, location: Range) -> Option { + if let ExprKind::Tuple { elts, .. } = &test.node { + if !elts.is_empty() { + return Some(Check::new(CheckKind::AssertTuple, location)); + } + } + None +} + +/// Check UnusedVariable compliance. +pub fn unused_variables( + scope: &Scope, + locator: &dyn CheckLocator, + dummy_variable_rgx: &Regex, +) -> Vec { + let mut checks: Vec = vec![]; + + if matches!( + scope.kind, + ScopeKind::Function(FunctionScope { uses_locals: true }) + ) { + return checks; + } + + for (name, binding) in scope.values.iter() { + if binding.used.is_none() + && matches!(binding.kind, BindingKind::Assignment) + && !dummy_variable_rgx.is_match(name) + && name != "__tracebackhide__" + && name != "__traceback_info__" + && name != "__traceback_supplement__" + { + checks.push(Check::new( + CheckKind::UnusedVariable(name.to_string()), + locator.locate_check(binding.range), + )); + } + } + + checks +} + +/// Check DefaultExceptNotLast compliance. +pub fn default_except_not_last(handlers: &[Excepthandler]) -> Option { + for (idx, handler) in handlers.iter().enumerate() { + let ExcepthandlerKind::ExceptHandler { type_, .. } = &handler.node; + if type_.is_none() && idx < handlers.len() - 1 { + return Some(Check::new( + CheckKind::DefaultExceptNotLast, + Range::from_located(handler), + )); + } + } + + None +} + +/// Check RaiseNotImplemented compliance. +pub fn raise_not_implemented(expr: &Expr) -> Option { + match &expr.node { + ExprKind::Call { func, .. } => { + if let ExprKind::Name { id, .. } = &func.node { + if id == "NotImplemented" { + return Some(Check::new( + CheckKind::RaiseNotImplemented, + Range::from_located(expr), + )); + } + } + } + ExprKind::Name { id, .. } => { + if id == "NotImplemented" { + return Some(Check::new( + CheckKind::RaiseNotImplemented, + Range::from_located(expr), + )); + } + } + _ => {} + } + + None +} + +/// Check DuplicateArgumentName compliance. +pub fn duplicate_arguments(arguments: &Arguments) -> Vec { + let mut checks: Vec = vec![]; + + // Collect all the arguments into a single vector. + let mut all_arguments: Vec<&Arg> = arguments + .args + .iter() + .chain(arguments.posonlyargs.iter()) + .chain(arguments.kwonlyargs.iter()) + .collect(); + if let Some(arg) = &arguments.vararg { + all_arguments.push(arg); + } + if let Some(arg) = &arguments.kwarg { + all_arguments.push(arg); + } + + // Search for duplicates. + let mut idents: BTreeSet<&str> = BTreeSet::new(); + for arg in all_arguments { + let ident = &arg.node.arg; + if idents.contains(ident.as_str()) { + checks.push(Check::new( + CheckKind::DuplicateArgumentName, + Range::from_located(arg), + )); + } + idents.insert(ident); + } + + checks +} + +#[derive(Debug, PartialEq)] +enum DictionaryKey<'a> { + Constant(&'a Constant), + Variable(&'a String), +} + +fn convert_to_value(expr: &Expr) -> Option { + match &expr.node { + ExprKind::Constant { value, .. } => Some(DictionaryKey::Constant(value)), + ExprKind::Name { id, .. } => Some(DictionaryKey::Variable(id)), + _ => None, + } +} + +/// Check MultiValueRepeatedKeyLiteral and MultiValueRepeatedKeyVariable compliance. +pub fn repeated_keys( + keys: &[Expr], + check_repeated_literals: bool, + check_repeated_variables: bool, + locator: &dyn CheckLocator, +) -> Vec { + let mut checks: Vec = vec![]; + + let num_keys = keys.len(); + for i in 0..num_keys { + let k1 = &keys[i]; + let v1 = convert_to_value(k1); + for k2 in keys.iter().take(num_keys).skip(i + 1) { + let v2 = convert_to_value(k2); + match (&v1, &v2) { + (Some(DictionaryKey::Constant(v1)), Some(DictionaryKey::Constant(v2))) => { + if check_repeated_literals && v1 == v2 { + checks.push(Check::new( + CheckKind::MultiValueRepeatedKeyLiteral, + locator.locate_check(Range::from_located(k2)), + )) + } + } + (Some(DictionaryKey::Variable(v1)), Some(DictionaryKey::Variable(v2))) => { + if check_repeated_variables && v1 == v2 { + checks.push(Check::new( + CheckKind::MultiValueRepeatedKeyVariable((*v2).to_string()), + locator.locate_check(Range::from_located(k2)), + )) + } + } + _ => {} + } + } + } + + checks +} + +fn is_constant(expr: &Expr) -> bool { + match &expr.node { + ExprKind::Constant { .. } => true, + ExprKind::Tuple { elts, .. } => elts.iter().all(is_constant), + _ => false, + } +} + +fn is_singleton(expr: &Expr) -> bool { + matches!( + expr.node, + ExprKind::Constant { + value: Constant::None | Constant::Bool(_) | Constant::Ellipsis, + .. + } + ) +} + +fn is_constant_non_singleton(expr: &Expr) -> bool { + is_constant(expr) && !is_singleton(expr) +} + +/// Check IsLiteral compliance. +pub fn is_literal(left: &Expr, ops: &[Cmpop], comparators: &[Expr], location: Range) -> Vec { + let mut checks: Vec = vec![]; + + let mut left = left; + for (op, right) in izip!(ops, comparators) { + if matches!(op, Cmpop::Is | Cmpop::IsNot) + && (is_constant_non_singleton(left) || is_constant_non_singleton(right)) + { + checks.push(Check::new(CheckKind::IsLiteral, location)); + } + left = right; + } + + checks +} + +/// Check TwoStarredExpressions and TooManyExpressionsInStarredAssignment compliance. +pub fn starred_expressions( + elts: &[Expr], + check_too_many_expressions: bool, + check_two_starred_expressions: bool, + location: Range, +) -> Option { + let mut has_starred: bool = false; + let mut starred_index: Option = None; + for (index, elt) in elts.iter().enumerate() { + if matches!(elt.node, ExprKind::Starred { .. }) { + if has_starred && check_two_starred_expressions { + return Some(Check::new(CheckKind::TwoStarredExpressions, location)); + } + has_starred = true; + starred_index = Some(index); + } + } + + if check_too_many_expressions { + if let Some(starred_index) = starred_index { + if starred_index >= 1 << 8 || elts.len() - starred_index > 1 << 24 { + return Some(Check::new(CheckKind::ExpressionsInStarAssignment, location)); + } + } + } + + None +} + +/// Check BreakOutsideLoop compliance. +pub fn break_outside_loop( + stmt: &Stmt, + parents: &[&Stmt], + parent_stack: &[usize], + locator: &dyn CheckLocator, +) -> Option { + let mut allowed: bool = false; + let mut parent = stmt; + for index in parent_stack.iter().rev() { + let child = parent; + parent = parents[*index]; + match &parent.node { + StmtKind::For { orelse, .. } + | StmtKind::AsyncFor { orelse, .. } + | StmtKind::While { orelse, .. } => { + if !orelse.contains(child) { + allowed = true; + break; + } + } + + StmtKind::FunctionDef { .. } + | StmtKind::AsyncFunctionDef { .. } + | StmtKind::ClassDef { .. } => { + break; + } + _ => {} + } + } + + if !allowed { + Some(Check::new( + CheckKind::BreakOutsideLoop, + locator.locate_check(Range::from_located(stmt)), + )) + } else { + None + } +} + +/// Check ContinueOutsideLoop compliance. +pub fn continue_outside_loop( + stmt: &Stmt, + parents: &[&Stmt], + parent_stack: &[usize], + locator: &dyn CheckLocator, +) -> Option { + let mut allowed: bool = false; + let mut parent = stmt; + for index in parent_stack.iter().rev() { + let child = parent; + parent = parents[*index]; + match &parent.node { + StmtKind::For { orelse, .. } + | StmtKind::AsyncFor { orelse, .. } + | StmtKind::While { orelse, .. } => { + if !orelse.contains(child) { + allowed = true; + break; + } + } + + StmtKind::FunctionDef { .. } + | StmtKind::AsyncFunctionDef { .. } + | StmtKind::ClassDef { .. } => { + break; + } + _ => {} + } + } + + if !allowed { + Some(Check::new( + CheckKind::ContinueOutsideLoop, + locator.locate_check(Range::from_located(stmt)), + )) + } else { + None + } +} diff --git a/src/pyflakes/mod.rs b/src/pyflakes/mod.rs new file mode 100644 index 0000000000..6cab51a666 --- /dev/null +++ b/src/pyflakes/mod.rs @@ -0,0 +1,2 @@ +pub mod checks; +pub mod plugins; diff --git a/src/plugins/assert_tuple.rs b/src/pyflakes/plugins/assert_tuple.rs similarity index 62% rename from src/plugins/assert_tuple.rs rename to src/pyflakes/plugins/assert_tuple.rs index 247b6483ec..875238cd4b 100644 --- a/src/plugins/assert_tuple.rs +++ b/src/pyflakes/plugins/assert_tuple.rs @@ -1,12 +1,11 @@ use rustpython_ast::{Expr, Stmt}; -use crate::ast::checkers; use crate::ast::types::{CheckLocator, Range}; use crate::check_ast::Checker; +use crate::pyflakes::checks; pub fn assert_tuple(checker: &mut Checker, stmt: &Stmt, test: &Expr) { - if let Some(check) = - checkers::assert_tuple(test, checker.locate_check(Range::from_located(stmt))) + if let Some(check) = checks::assert_tuple(test, checker.locate_check(Range::from_located(stmt))) { checker.add_check(check); } diff --git a/src/plugins/if_tuple.rs b/src/pyflakes/plugins/if_tuple.rs similarity index 63% rename from src/plugins/if_tuple.rs rename to src/pyflakes/plugins/if_tuple.rs index 3a7a31bb70..4e0d63eeb3 100644 --- a/src/plugins/if_tuple.rs +++ b/src/pyflakes/plugins/if_tuple.rs @@ -1,11 +1,11 @@ use rustpython_ast::{Expr, Stmt}; -use crate::ast::checkers; use crate::ast::types::{CheckLocator, Range}; use crate::check_ast::Checker; +use crate::pyflakes::checks; pub fn if_tuple(checker: &mut Checker, stmt: &Stmt, test: &Expr) { - if let Some(check) = checkers::if_tuple(test, checker.locate_check(Range::from_located(stmt))) { + if let Some(check) = checks::if_tuple(test, checker.locate_check(Range::from_located(stmt))) { checker.add_check(check); } } diff --git a/src/plugins/invalid_print_syntax.rs b/src/pyflakes/plugins/invalid_print_syntax.rs similarity index 100% rename from src/plugins/invalid_print_syntax.rs rename to src/pyflakes/plugins/invalid_print_syntax.rs diff --git a/src/pyflakes/plugins/mod.rs b/src/pyflakes/plugins/mod.rs new file mode 100644 index 0000000000..d76f432b60 --- /dev/null +++ b/src/pyflakes/plugins/mod.rs @@ -0,0 +1,7 @@ +pub use assert_tuple::assert_tuple; +pub use if_tuple::if_tuple; +pub use invalid_print_syntax::invalid_print_syntax; + +mod assert_tuple; +mod if_tuple; +mod invalid_print_syntax; diff --git a/src/python.rs b/src/python/mod.rs similarity index 100% rename from src/python.rs rename to src/python/mod.rs diff --git a/src/pyupgrade/checks.rs b/src/pyupgrade/checks.rs new file mode 100644 index 0000000000..edf8dbd41c --- /dev/null +++ b/src/pyupgrade/checks.rs @@ -0,0 +1,156 @@ +use rustpython_parser::ast::{ArgData, Expr, ExprKind, Stmt, StmtKind}; + +use crate::ast::helpers; +use crate::ast::types::{Binding, BindingKind, Range, Scope, ScopeKind}; +use crate::checks::{Check, CheckKind}; +use crate::pyupgrade::types::Primitive; + +/// Check that `super()` has no args +pub fn super_args( + scope: &Scope, + parents: &[&Stmt], + expr: &Expr, + func: &Expr, + args: &[Expr], +) -> Option { + if !helpers::is_super_call_with_arguments(func, args) { + return None; + } + + // Check: are we in a Function scope? + if !matches!(scope.kind, ScopeKind::Function { .. }) { + return None; + } + + let mut parents = parents.iter().rev(); + + // For a `super` invocation to be unnecessary, the first argument needs to match the enclosing + // class, and the second argument needs to match the first argument to the enclosing function. + if let [first_arg, second_arg] = args { + // Find the enclosing function definition (if any). + if let Some(StmtKind::FunctionDef { + args: parent_args, .. + }) = parents + .find(|stmt| matches!(stmt.node, StmtKind::FunctionDef { .. })) + .map(|stmt| &stmt.node) + { + // Extract the name of the first argument to the enclosing function. + if let Some(ArgData { + arg: parent_arg, .. + }) = parent_args.args.first().map(|expr| &expr.node) + { + // Find the enclosing class definition (if any). + if let Some(StmtKind::ClassDef { + name: parent_name, .. + }) = parents + .find(|stmt| matches!(stmt.node, StmtKind::ClassDef { .. })) + .map(|stmt| &stmt.node) + { + if let ( + ExprKind::Name { + id: first_arg_id, .. + }, + ExprKind::Name { + id: second_arg_id, .. + }, + ) = (&first_arg.node, &second_arg.node) + { + if first_arg_id == parent_name && second_arg_id == parent_arg { + return Some(Check::new( + CheckKind::SuperCallWithParameters, + Range::from_located(expr), + )); + } + } + } + } + } + } + + None +} + +/// Check UselessMetaclassType compliance. +pub fn useless_metaclass_type(targets: &[Expr], value: &Expr, location: Range) -> Option { + if targets.len() == 1 { + if let ExprKind::Name { id, .. } = targets.first().map(|expr| &expr.node).unwrap() { + if id == "__metaclass__" { + if let ExprKind::Name { id, .. } = &value.node { + if id == "type" { + return Some(Check::new(CheckKind::UselessMetaclassType, location)); + } + } + } + } + } + None +} + +/// Check UnnecessaryAbspath compliance. +pub fn unnecessary_abspath(func: &Expr, args: &[Expr], location: Range) -> Option { + // Validate the arguments. + if args.len() == 1 { + if let ExprKind::Name { id, .. } = &args[0].node { + if id == "__file__" { + match &func.node { + ExprKind::Attribute { attr: id, .. } | ExprKind::Name { id, .. } => { + if id == "abspath" { + return Some(Check::new(CheckKind::UnnecessaryAbspath, location)); + } + } + _ => {} + } + } + } + } + None +} + +/// Check UselessObjectInheritance compliance. +pub fn useless_object_inheritance(name: &str, bases: &[Expr], scope: &Scope) -> Option { + for expr in bases { + if let ExprKind::Name { id, .. } = &expr.node { + if id == "object" { + match scope.values.get(id) { + None + | Some(Binding { + kind: BindingKind::Builtin, + .. + }) => { + return Some(Check::new( + CheckKind::UselessObjectInheritance(name.to_string()), + Range::from_located(expr), + )); + } + _ => {} + } + } + } + } + + None +} + +/// Check TypeOfPrimitive compliance. +pub fn type_of_primitive(func: &Expr, args: &[Expr], location: Range) -> Option { + // Validate the arguments. + if args.len() == 1 { + match &func.node { + ExprKind::Attribute { attr: id, .. } | ExprKind::Name { id, .. } => { + if id == "type" { + if let ExprKind::Constant { value, .. } = &args[0].node { + if let Some(primitive) = Primitive::from_constant(value) { + return Some(Check::new( + CheckKind::TypeOfPrimitive(primitive), + location, + )); + } + } + } + } + _ => {} + } + } + + None +} diff --git a/src/pyupgrade/mod.rs b/src/pyupgrade/mod.rs new file mode 100644 index 0000000000..320be4a3ab --- /dev/null +++ b/src/pyupgrade/mod.rs @@ -0,0 +1,3 @@ +mod checks; +pub mod plugins; +pub mod types; diff --git a/src/plugins/deprecated_unittest_alias.rs b/src/pyupgrade/plugins/deprecated_unittest_alias.rs similarity index 100% rename from src/plugins/deprecated_unittest_alias.rs rename to src/pyupgrade/plugins/deprecated_unittest_alias.rs diff --git a/src/plugins.rs b/src/pyupgrade/plugins/mod.rs similarity index 65% rename from src/plugins.rs rename to src/pyupgrade/plugins/mod.rs index 9fcbff5121..84ba6a2116 100644 --- a/src/plugins.rs +++ b/src/pyupgrade/plugins/mod.rs @@ -1,10 +1,4 @@ -pub use assert_false::assert_false; -pub use assert_tuple::assert_tuple; pub use deprecated_unittest_alias::deprecated_unittest_alias; -pub use duplicate_exceptions::duplicate_exceptions; -pub use if_tuple::if_tuple; -pub use invalid_print_syntax::invalid_print_syntax; -pub use print_call::print_call; pub use super_call_with_parameters::super_call_with_parameters; pub use type_of_primitive::type_of_primitive; pub use unnecessary_abspath::unnecessary_abspath; @@ -13,13 +7,7 @@ pub use use_pep604_annotation::use_pep604_annotation; pub use useless_metaclass_type::useless_metaclass_type; pub use useless_object_inheritance::useless_object_inheritance; -mod assert_false; -mod assert_tuple; mod deprecated_unittest_alias; -mod duplicate_exceptions; -mod if_tuple; -mod invalid_print_syntax; -mod print_call; mod super_call_with_parameters; mod type_of_primitive; mod unnecessary_abspath; diff --git a/src/plugins/super_call_with_parameters.rs b/src/pyupgrade/plugins/super_call_with_parameters.rs similarity index 86% rename from src/plugins/super_call_with_parameters.rs rename to src/pyupgrade/plugins/super_call_with_parameters.rs index 0018e51f05..29d584384c 100644 --- a/src/plugins/super_call_with_parameters.rs +++ b/src/pyupgrade/plugins/super_call_with_parameters.rs @@ -1,8 +1,9 @@ use rustpython_ast::{Expr, Stmt}; -use crate::ast::{checkers, helpers}; +use crate::ast::helpers; use crate::autofix::{fixer, fixes}; use crate::check_ast::Checker; +use crate::pyupgrade::checks; pub fn super_call_with_parameters(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { // Only bother going through the super check at all if we're in a `super` call. @@ -14,7 +15,7 @@ pub fn super_call_with_parameters(checker: &mut Checker, expr: &Expr, func: &Exp .iter() .map(|index| checker.parents[*index]) .collect(); - if let Some(mut check) = checkers::super_args(scope, &parents, expr, func, args) { + if let Some(mut check) = checks::super_args(scope, &parents, expr, func, args) { if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) { if let Some(fix) = fixes::remove_super_arguments(&mut checker.locator, expr) { check.amend(fix); diff --git a/src/plugins/type_of_primitive.rs b/src/pyupgrade/plugins/type_of_primitive.rs similarity index 85% rename from src/plugins/type_of_primitive.rs rename to src/pyupgrade/plugins/type_of_primitive.rs index e358d91364..1ab099ccf5 100644 --- a/src/plugins/type_of_primitive.rs +++ b/src/pyupgrade/plugins/type_of_primitive.rs @@ -1,14 +1,14 @@ use rustpython_ast::Expr; -use crate::ast::checkers; use crate::ast::types::{CheckLocator, Range}; use crate::autofix::fixer; use crate::check_ast::Checker; use crate::checks::{CheckKind, Fix}; +use crate::pyupgrade::checks; pub fn type_of_primitive(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { if let Some(mut check) = - checkers::type_of_primitive(func, args, checker.locate_check(Range::from_located(expr))) + checks::type_of_primitive(func, args, checker.locate_check(Range::from_located(expr))) { if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) { if let CheckKind::TypeOfPrimitive(primitive) = &check.kind { diff --git a/src/plugins/unnecessary_abspath.rs b/src/pyupgrade/plugins/unnecessary_abspath.rs similarity index 83% rename from src/plugins/unnecessary_abspath.rs rename to src/pyupgrade/plugins/unnecessary_abspath.rs index 2256d2aabe..b5215559a0 100644 --- a/src/plugins/unnecessary_abspath.rs +++ b/src/pyupgrade/plugins/unnecessary_abspath.rs @@ -1,14 +1,14 @@ use rustpython_ast::Expr; -use crate::ast::checkers; use crate::ast::types::{CheckLocator, Range}; use crate::autofix::fixer; use crate::check_ast::Checker; use crate::checks::Fix; +use crate::pyupgrade::checks; pub fn unnecessary_abspath(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { if let Some(mut check) = - checkers::unnecessary_abspath(func, args, checker.locate_check(Range::from_located(expr))) + checks::unnecessary_abspath(func, args, checker.locate_check(Range::from_located(expr))) { if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) { check.amend(Fix { diff --git a/src/plugins/use_pep585_annotation.rs b/src/pyupgrade/plugins/use_pep585_annotation.rs similarity index 100% rename from src/plugins/use_pep585_annotation.rs rename to src/pyupgrade/plugins/use_pep585_annotation.rs diff --git a/src/plugins/use_pep604_annotation.rs b/src/pyupgrade/plugins/use_pep604_annotation.rs similarity index 100% rename from src/plugins/use_pep604_annotation.rs rename to src/pyupgrade/plugins/use_pep604_annotation.rs diff --git a/src/plugins/useless_metaclass_type.rs b/src/pyupgrade/plugins/useless_metaclass_type.rs similarity index 93% rename from src/plugins/useless_metaclass_type.rs rename to src/pyupgrade/plugins/useless_metaclass_type.rs index caaa9bd6e4..04b3d1ff8f 100644 --- a/src/plugins/useless_metaclass_type.rs +++ b/src/pyupgrade/plugins/useless_metaclass_type.rs @@ -1,13 +1,13 @@ use log::error; use rustpython_ast::{Expr, Stmt}; -use crate::ast::checkers; use crate::ast::types::{CheckLocator, Range}; use crate::autofix::{fixer, fixes}; use crate::check_ast::Checker; +use crate::pyupgrade::checks; pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr, targets: &[Expr]) { - if let Some(mut check) = checkers::useless_metaclass_type( + if let Some(mut check) = checks::useless_metaclass_type( targets, value, checker.locate_check(Range::from_located(stmt)), diff --git a/src/plugins/useless_object_inheritance.rs b/src/pyupgrade/plugins/useless_object_inheritance.rs similarity index 85% rename from src/plugins/useless_object_inheritance.rs rename to src/pyupgrade/plugins/useless_object_inheritance.rs index ad6dc2a0d7..c8e928f2a0 100644 --- a/src/plugins/useless_object_inheritance.rs +++ b/src/pyupgrade/plugins/useless_object_inheritance.rs @@ -1,8 +1,8 @@ use rustpython_ast::{Expr, Keyword, Stmt}; -use crate::ast::checkers; use crate::autofix::{fixer, fixes}; use crate::check_ast::Checker; +use crate::pyupgrade::checks; pub fn useless_object_inheritance( checker: &mut Checker, @@ -12,7 +12,7 @@ pub fn useless_object_inheritance( keywords: &[Keyword], ) { let scope = checker.current_scope(); - if let Some(mut check) = checkers::useless_object_inheritance(name, bases, scope) { + if let Some(mut check) = checks::useless_object_inheritance(name, bases, scope) { if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) { if let Some(fix) = fixes::remove_class_def_base( &mut checker.locator, diff --git a/src/pyupgrade/types.rs b/src/pyupgrade/types.rs new file mode 100644 index 0000000000..18e3f0836d --- /dev/null +++ b/src/pyupgrade/types.rs @@ -0,0 +1,37 @@ +use rustpython_ast::Constant; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] +pub enum Primitive { + Bool, + Str, + Bytes, + Int, + Float, + Complex, +} + +impl Primitive { + pub fn from_constant(constant: &Constant) -> Option { + match constant { + Constant::Bool(_) => Some(Primitive::Bool), + Constant::Str(_) => Some(Primitive::Str), + Constant::Bytes(_) => Some(Primitive::Bytes), + Constant::Int(_) => Some(Primitive::Int), + Constant::Float(_) => Some(Primitive::Float), + Constant::Complex { .. } => Some(Primitive::Complex), + _ => None, + } + } + + pub fn builtin(&self) -> String { + match self { + Primitive::Bool => "bool".to_string(), + Primitive::Str => "str".to_string(), + Primitive::Bytes => "bytes".to_string(), + Primitive::Int => "int".to_string(), + Primitive::Float => "float".to_string(), + Primitive::Complex => "complex".to_string(), + } + } +}