diff --git a/resources/test/fixtures/B024.py b/resources/test/fixtures/B024.py index e385874e9a..86e5eb4c2a 100644 --- a/resources/test/fixtures/B024.py +++ b/resources/test/fixtures/B024.py @@ -1,6 +1,6 @@ """ Should emit: -B024 - on lines 17, 34, 52, 58, 69, 74, 84, 89 +B024 - on lines 17, 34, 52, 58, 69, 74, 79, 84, 89 """ import abc @@ -76,7 +76,7 @@ class abc_Base_2(metaclass=abc.ABCMeta): # error foo() -class notabc_Base_1(notabc.ABC): # safe +class notabc_Base_1(notabc.ABC): # error def method(self): foo() diff --git a/resources/test/fixtures/B027.py b/resources/test/fixtures/B027.py index 98c6b635f5..6a5472a616 100644 --- a/resources/test/fixtures/B027.py +++ b/resources/test/fixtures/B027.py @@ -2,7 +2,6 @@ Should emit: B027 - on lines 12, 15, 18, 22, 30 """ - import abc from abc import ABC from abc import abstractmethod @@ -28,7 +27,7 @@ class AbstractClass(ABC): pass @notabstract - def empty_5(self): # error + def abstract_0(self): ... @abstractmethod diff --git a/resources/test/fixtures/F821_4.py b/resources/test/fixtures/F821_4.py new file mode 100644 index 0000000000..081bbb1f4b --- /dev/null +++ b/resources/test/fixtures/F821_4.py @@ -0,0 +1,24 @@ +"""Test: import alias tracking.""" +from typing import List + +_ = List["Model"] + + +from typing import List as IList + +_ = IList["Model"] + + +from collections.abc import ItemsView + +_ = ItemsView["Model"] + + +import collections.abc + +_ = collections.abc.ItemsView["Model"] + + +from collections import abc + +_ = abc.ItemsView["Model"] diff --git a/resources/test/fixtures/U006.py b/resources/test/fixtures/U006.py index db0d4332cb..b316cb55e0 100644 --- a/resources/test/fixtures/U006.py +++ b/resources/test/fixtures/U006.py @@ -1,3 +1,10 @@ +import typing + + +def f(x: typing.List[str]) -> None: + ... + + from typing import List @@ -5,8 +12,15 @@ def f(x: List[str]) -> None: ... -import typing +import typing as t -def f(x: typing.List[str]) -> None: +def f(x: t.List[str]) -> None: + ... + + +from typing import List as IList + + +def f(x: IList[str]) -> None: ... diff --git a/resources/test/fixtures/YTT101.py b/resources/test/fixtures/YTT101.py index d21870b9bb..d35ce54ac9 100644 --- a/resources/test/fixtures/YTT101.py +++ b/resources/test/fixtures/YTT101.py @@ -5,9 +5,8 @@ print(sys.version) print(sys.version[:3]) print(version[:3]) - -# ignore from imports with aliases, patches welcome print(v[:3]) + # the tool is timid and only flags certain numeric slices i = 3 print(sys.version[:i]) diff --git a/src/ast/helpers.rs b/src/ast/helpers.rs index df203e5c37..a8f4c49029 100644 --- a/src/ast/helpers.rs +++ b/src/ast/helpers.rs @@ -39,6 +39,26 @@ pub fn collect_call_paths(expr: &Expr) -> Vec<&str> { segments } +/// Rewrite any import aliases on a call path. +pub fn dealias_call_path<'a>( + call_path: Vec<&'a str>, + import_aliases: &FnvHashMap<&str, &'a str>, +) -> Vec<&'a str> { + if let Some(head) = call_path.first() { + if let Some(origin) = import_aliases.get(head) { + let tail = &call_path[1..]; + let mut call_path: Vec<&str> = vec![]; + call_path.extend(origin.split('.')); + call_path.extend(tail); + call_path + } else { + call_path + } + } else { + call_path + } +} + /// Return `true` if the `Expr` is a name or attribute reference to `${target}`. pub fn match_name_or_attr(expr: &Expr, target: &str) -> bool { match &expr.node { @@ -48,6 +68,94 @@ pub fn match_name_or_attr(expr: &Expr, target: &str) -> bool { } } +/// Return `true` if the `Expr` is a reference to `${module}.${target}`. +/// +/// Useful for, e.g., ensuring that a `Union` reference represents +/// `typing.Union`. +pub fn match_module_member( + expr: &Expr, + module: &str, + member: &str, + from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, + import_aliases: &FnvHashMap<&str, &str>, +) -> bool { + match_call_path( + &dealias_call_path(collect_call_paths(expr), import_aliases), + module, + member, + from_imports, + ) +} + +/// Return `true` if the `call_path` is a reference to `${module}.${target}`. +/// +/// Optimized version of `match_module_member` for pre-computed call paths. +pub fn match_call_path( + call_path: &[&str], + module: &str, + member: &str, + from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, +) -> bool { + // If we have no segments, we can't ever match. + let num_segments = call_path.len(); + if num_segments == 0 { + return false; + } + + // If the last segment doesn't match the member, we can't ever match. + if call_path[num_segments - 1] != member { + return false; + } + + // We now only need the module path, so throw out the member name. + let call_path = &call_path[..num_segments - 1]; + let num_segments = call_path.len(); + + // Case (1): It's a builtin (like `list`). + // Case (2a): We imported from the parent (`from typing.re import Match`, + // `Match`). + // Case (2b): We imported star from the parent (`from typing.re import *`, + // `Match`). + if num_segments == 0 { + module.is_empty() + || from_imports + .get(module) + .map(|imports| imports.contains(member) || imports.contains("*")) + .unwrap_or(false) + } else { + let components: Vec<&str> = module.split('.').collect(); + + // Case (3a): it's a fully qualified call path (`import typing`, + // `typing.re.Match`). Case (3b): it's a fully qualified call path (`import + // typing.re`, `typing.re.Match`). + if components == call_path { + return true; + } + + // Case (4): We imported from the grandparent (`from typing import re`, + // `re.Match`) + let num_matches = (0..components.len()) + .take(num_segments) + .take_while(|i| components[components.len() - 1 - i] == call_path[num_segments - 1 - i]) + .count(); + if num_matches > 0 { + let cut = components.len() - num_matches; + // TODO(charlie): Rewrite to avoid this allocation. + let module = components[..cut].join("."); + let member = components[cut]; + if from_imports + .get(&module.as_str()) + .map(|imports| imports.contains(member)) + .unwrap_or(false) + { + return true; + } + } + + false + } +} + static DUNDER_REGEX: Lazy = Lazy::new(|| Regex::new(r"__[^\s]+__").unwrap()); pub fn is_assignment_to_a_dunder(node: &StmtKind) -> bool { @@ -130,100 +238,6 @@ pub fn to_absolute(relative: &Location, base: &Location) -> Location { } } -/// Return `true` if the `Expr` is a reference to `${module}.${target}`. -/// -/// Useful for, e.g., ensuring that a `Union` reference represents -/// `typing.Union`. -pub fn match_module_member( - expr: &Expr, - module: &str, - member: &str, - from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, -) -> bool { - match_call_path(&collect_call_paths(expr), module, member, from_imports) -} - -/// Return `true` if the `call_path` is a reference to `${module}.${target}`. -/// -/// Optimized version of `match_module_member` for pre-computed call paths. -pub fn match_call_path( - call_path: &[&str], - module: &str, - member: &str, - from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, -) -> bool { - // If we have no segments, we can't ever match. - let num_segments = call_path.len(); - if num_segments == 0 { - return false; - } - - // If the last segment doesn't match the member, we can't ever match. - if call_path[num_segments - 1] != member { - return false; - } - - // We now only need the module path, so throw out the member name. - let call_path = &call_path[..num_segments - 1]; - let num_segments = call_path.len(); - - // Case (1a): We imported star from the parent (`from typing.re import *`, - // `Match`). - // Case (1b): We imported from the parent (`from typing.re import Match`, - // `Match`). - // Case (2): It's a builtin (like `list`). - if num_segments == 0 - && (module.is_empty() - || from_imports - .get(module) - .map(|imports| imports.contains(member) || imports.contains("*")) - .unwrap_or(false)) - { - return true; - } - - // Case (3a): it's a fully qualified call path (`import typing`, - // `typing.re.Match`). Case (3b): it's a fully qualified call path (`import - // typing.re`, `typing.re.Match`). - if num_segments > 0 - && module - .split('.') - .enumerate() - .all(|(index, segment)| index < num_segments && call_path[index] == segment) - { - return true; - } - - // Case (4): We imported from the grandparent (`from typing import re`, - // `re.Match`) - if num_segments > 0 { - // Find the number of common segments. - // TODO(charlie): Rewrite to avoid this allocation. - let parts: Vec<&str> = module.split('.').collect(); - let num_matches = (0..parts.len()) - .take(num_segments) - .take_while(|i| parts[parts.len() - 1 - i] == call_path[num_segments - 1 - i]) - .count(); - if num_matches > 0 { - // Verify that we have an import of the final common segment, from the remaining - // parent. - let cut = parts.len() - num_matches; - // TODO(charlie): Rewrite to avoid this allocation. - let module = parts[..cut].join("."); - let member = parts[cut]; - if from_imports - .get(&module.as_str()) - .map(|imports| imports.contains(member)) - .unwrap_or(false) - { - return true; - } - } - } - - false -} - #[cfg(test)] mod tests { use anyhow::Result; @@ -239,7 +253,8 @@ mod tests { &expr, "", "list", - &FnvHashMap::default() + &FnvHashMap::default(), + &FnvHashMap::default(), )); Ok(()) } @@ -251,7 +266,8 @@ mod tests { &expr, "typing.re", "Match", - &FnvHashMap::default() + &FnvHashMap::default(), + &FnvHashMap::default(), )); Ok(()) } @@ -264,6 +280,7 @@ mod tests { "typing.re", "Match", &FnvHashMap::default(), + &FnvHashMap::default(), )); let expr = parser::parse_expression("re.Match", "")?; assert!(!match_module_member( @@ -271,6 +288,7 @@ mod tests { "typing.re", "Match", &FnvHashMap::default(), + &FnvHashMap::default(), )); Ok(()) } @@ -282,7 +300,8 @@ mod tests { &expr, "typing.re", "Match", - &FnvHashMap::from_iter([("typing.re", FnvHashSet::from_iter(["*"]))]) + &FnvHashMap::from_iter([("typing.re", FnvHashSet::from_iter(["*"]))]), + &FnvHashMap::default() )); Ok(()) } @@ -294,7 +313,8 @@ mod tests { &expr, "typing.re", "Match", - &FnvHashMap::from_iter([("typing.re", FnvHashSet::from_iter(["Match"]))]) + &FnvHashMap::from_iter([("typing.re", FnvHashSet::from_iter(["Match"]))]), + &FnvHashMap::default() )); Ok(()) } @@ -306,7 +326,8 @@ mod tests { &expr, "typing.re", "Match", - &FnvHashMap::from_iter([("typing", FnvHashSet::from_iter(["re"]))]) + &FnvHashMap::from_iter([("typing", FnvHashSet::from_iter(["re"]))]), + &FnvHashMap::default() )); let expr = parser::parse_expression("match.Match", "")?; @@ -314,7 +335,8 @@ mod tests { &expr, "typing.re.match", "Match", - &FnvHashMap::from_iter([("typing.re", FnvHashSet::from_iter(["match"]))]) + &FnvHashMap::from_iter([("typing.re", FnvHashSet::from_iter(["match"]))]), + &FnvHashMap::default() )); let expr = parser::parse_expression("re.match.Match", "")?; @@ -322,7 +344,47 @@ mod tests { &expr, "typing.re.match", "Match", - &FnvHashMap::from_iter([("typing", FnvHashSet::from_iter(["re"]))]) + &FnvHashMap::from_iter([("typing", FnvHashSet::from_iter(["re"]))]), + &FnvHashMap::default() + )); + Ok(()) + } + + #[test] + fn from_alias() -> Result<()> { + let expr = parser::parse_expression("IMatch", "")?; + assert!(match_module_member( + &expr, + "typing.re", + "Match", + &FnvHashMap::from_iter([("typing.re", FnvHashSet::from_iter(["Match"]))]), + &FnvHashMap::from_iter([("IMatch", "Match")]), + )); + Ok(()) + } + + #[test] + fn from_aliased_parent() -> Result<()> { + let expr = parser::parse_expression("t.Match", "")?; + assert!(match_module_member( + &expr, + "typing.re", + "Match", + &FnvHashMap::default(), + &FnvHashMap::from_iter([("t", "typing.re")]), + )); + Ok(()) + } + + #[test] + fn from_aliased_grandparent() -> Result<()> { + let expr = parser::parse_expression("t.re.Match", "")?; + assert!(match_module_member( + &expr, + "typing.re", + "Match", + &FnvHashMap::default(), + &FnvHashMap::from_iter([("t", "typing")]), )); Ok(()) } diff --git a/src/check_ast.rs b/src/check_ast.rs index e1cda22722..164309bad9 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -12,7 +12,9 @@ use rustpython_parser::ast::{ }; use rustpython_parser::parser; -use crate::ast::helpers::{collect_call_paths, extract_handler_names, match_call_path}; +use crate::ast::helpers::{ + collect_call_paths, dealias_call_path, extract_handler_names, match_call_path, +}; use crate::ast::operations::extract_all_names; use crate::ast::relocate::relocate_expr; use crate::ast::types::{ @@ -54,6 +56,7 @@ pub struct Checker<'a> { pub(crate) deletions: FnvHashSet, // Import tracking. pub(crate) from_imports: FnvHashMap<&'a str, FnvHashSet<&'a str>>, + pub(crate) import_aliases: FnvHashMap<&'a str, &'a str>, // Retain all scopes and parent nodes, along with a stack of indexes to track which are active // at various points in time. pub(crate) parents: Vec<&'a Stmt>, @@ -94,6 +97,7 @@ impl<'a> Checker<'a> { definitions: Default::default(), deletions: Default::default(), from_imports: Default::default(), + import_aliases: Default::default(), parents: Default::default(), parent_stack: Default::default(), scopes: Default::default(), @@ -546,6 +550,12 @@ where } if let Some(asname) = &alias.node.asname { + for alias in names { + if let Some(asname) = &alias.node.asname { + self.import_aliases.insert(asname, &alias.node.name); + } + } + let name = alias.node.name.split('.').last().unwrap(); if self.settings.enabled.contains(&CheckCode::N811) { if let Some(check) = @@ -614,6 +624,11 @@ where .map(|alias| alias.node.name.as_str()), ) } + for alias in names { + if let Some(asname) = &alias.node.asname { + self.import_aliases.insert(asname, &alias.node.name); + } + } } if self.settings.enabled.contains(&CheckCode::E402) { @@ -1076,7 +1091,11 @@ where // Ex) List[...] if self.settings.enabled.contains(&CheckCode::U006) && self.settings.target_version >= PythonVersion::Py39 - && typing::is_pep585_builtin(expr, &self.from_imports) + && typing::is_pep585_builtin( + expr, + &self.from_imports, + &self.import_aliases, + ) { pyupgrade::plugins::use_pep585_annotation(self, expr, id); } @@ -1108,7 +1127,7 @@ where // Ex) typing.List[...] if self.settings.enabled.contains(&CheckCode::U006) && self.settings.target_version >= PythonVersion::Py39 - && typing::is_pep585_builtin(expr, &self.from_imports) + && typing::is_pep585_builtin(expr, &self.from_imports, &self.import_aliases) { pyupgrade::plugins::use_pep585_annotation(self, expr, attr); } @@ -1625,7 +1644,7 @@ where args, keywords, } => { - let call_path = collect_call_paths(func); + let call_path = dealias_call_path(collect_call_paths(func), &self.import_aliases); if self.match_typing_module(&call_path, "ForwardRef") { self.visit_expr(func); for expr in args { @@ -1732,7 +1751,11 @@ where visitor::walk_expr(self, expr); } else { self.in_subscript = true; - match typing::match_annotated_subscript(value, &self.from_imports) { + match typing::match_annotated_subscript( + value, + &self.from_imports, + &self.import_aliases, + ) { Some(subscript) => { match subscript { // Ex) Optional[int] @@ -2732,8 +2755,5 @@ pub fn check_ast( // Check docstrings. checker.check_definitions(); - // Check import blocks. - // checker.check_import_blocks(); - checker.checks } diff --git a/src/flake8_2020/plugins.rs b/src/flake8_2020/plugins.rs index 05feb813ff..38b14a2ec8 100644 --- a/src/flake8_2020/plugins.rs +++ b/src/flake8_2020/plugins.rs @@ -7,7 +7,13 @@ use crate::check_ast::Checker; use crate::checks::{Check, CheckCode, CheckKind}; fn is_sys(checker: &Checker, expr: &Expr, target: &str) -> bool { - match_module_member(expr, "sys", target, &checker.from_imports) + match_module_member( + expr, + "sys", + target, + &checker.from_imports, + &checker.import_aliases, + ) } /// YTT101, YTT102, YTT301, YTT303 @@ -181,7 +187,13 @@ pub fn compare(checker: &mut Checker, left: &Expr, ops: &[Cmpop], comparators: & /// YTT202 pub fn name_or_attribute(checker: &mut Checker, expr: &Expr) { - if match_module_member(expr, "six", "PY3", &checker.from_imports) { + if match_module_member( + expr, + "six", + "PY3", + &checker.from_imports, + &checker.import_aliases, + ) { checker.add_check(Check::new( CheckKind::SixPY3Referenced, Range::from_located(expr), diff --git a/src/flake8_annotations/plugins.rs b/src/flake8_annotations/plugins.rs index dc75ec8416..0955bd59d5 100644 --- a/src/flake8_annotations/plugins.rs +++ b/src/flake8_annotations/plugins.rs @@ -50,10 +50,13 @@ fn is_none_returning(body: &[Stmt]) -> bool { } /// ANN401 -fn check_dynamically_typed(checker: &mut Checker, annotation: &Expr, name: &str) { +fn check_dynamically_typed(checker: &mut Checker, annotation: &Expr, func: F) +where + F: FnOnce() -> String, +{ if checker.match_typing_module(&collect_call_paths(annotation), "Any") { checker.add_check(Check::new( - CheckKind::DynamicallyTypedExpression(name.to_string()), + CheckKind::DynamicallyTypedExpression(func()), Range::from_located(annotation), )); }; @@ -101,7 +104,7 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V { if let Some(expr) = &arg.node.annotation { if checker.settings.enabled.contains(&CheckCode::ANN401) { - check_dynamically_typed(checker, expr, &arg.node.arg); + check_dynamically_typed(checker, expr, || arg.node.arg.to_string()); }; } else { if !(checker.settings.flake8_annotations.suppress_dummy_args @@ -123,7 +126,7 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if !checker.settings.flake8_annotations.allow_star_arg_any { if checker.settings.enabled.contains(&CheckCode::ANN401) { let name = arg.node.arg.to_string(); - check_dynamically_typed(checker, expr, &format!("*{name}")); + check_dynamically_typed(checker, expr, || format!("*{name}")); } } } else { @@ -146,7 +149,7 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if !checker.settings.flake8_annotations.allow_star_arg_any { if checker.settings.enabled.contains(&CheckCode::ANN401) { let name = arg.node.arg.to_string(); - check_dynamically_typed(checker, expr, &format!("**{name}")); + check_dynamically_typed(checker, expr, || format!("**{name}")); } } } else { @@ -166,7 +169,7 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V // ANN201, ANN202, ANN401 if let Some(expr) = &returns { if checker.settings.enabled.contains(&CheckCode::ANN401) { - check_dynamically_typed(checker, expr, name); + check_dynamically_typed(checker, expr, || name.to_string()); }; } else { // Allow omission of return annotation in `__init__` functions, if the function @@ -216,7 +219,7 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if let Some(annotation) = &arg.node.annotation { has_any_typed_arg = true; if checker.settings.enabled.contains(&CheckCode::ANN401) { - check_dynamically_typed(checker, annotation, &arg.node.arg); + check_dynamically_typed(checker, annotation, || arg.node.arg.to_string()); } } else { if !(checker.settings.flake8_annotations.suppress_dummy_args @@ -239,7 +242,7 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if !checker.settings.flake8_annotations.allow_star_arg_any { if checker.settings.enabled.contains(&CheckCode::ANN401) { let name = arg.node.arg.to_string(); - check_dynamically_typed(checker, expr, &format!("*{name}")); + check_dynamically_typed(checker, expr, || format!("*{name}")); } } } else { @@ -263,7 +266,7 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if !checker.settings.flake8_annotations.allow_star_arg_any { if checker.settings.enabled.contains(&CheckCode::ANN401) { let name = arg.node.arg.to_string(); - check_dynamically_typed(checker, expr, &format!("**{name}")); + check_dynamically_typed(checker, expr, || format!("**{name}")); } } } else { @@ -306,7 +309,7 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V // ANN201, ANN202 if let Some(expr) = &returns { if checker.settings.enabled.contains(&CheckCode::ANN401) { - check_dynamically_typed(checker, expr, name); + check_dynamically_typed(checker, expr, || name.to_string()); } } else { // Allow omission of return annotation in `__init__` functions, if the function diff --git a/src/flake8_bugbear/plugins/abstract_base_class.rs b/src/flake8_bugbear/plugins/abstract_base_class.rs index 667b59cf38..b61ce8a62f 100644 --- a/src/flake8_bugbear/plugins/abstract_base_class.rs +++ b/src/flake8_bugbear/plugins/abstract_base_class.rs @@ -1,15 +1,16 @@ use fnv::{FnvHashMap, FnvHashSet}; use rustpython_ast::{Constant, Expr, ExprKind, Keyword, Stmt, StmtKind}; -use crate::ast::helpers::{collect_call_paths, match_call_path}; +use crate::ast::helpers::match_module_member; use crate::ast::types::Range; use crate::check_ast::Checker; -use crate::checks::{Check, CheckKind}; +use crate::checks::{Check, CheckCode, CheckKind}; fn is_abc_class( bases: &[Expr], keywords: &[Keyword], from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, + import_aliases: &FnvHashMap<&str, &str>, ) -> bool { keywords.iter().any(|keyword| { keyword @@ -18,15 +19,16 @@ fn is_abc_class( .as_ref() .map(|a| a == "metaclass") .unwrap_or(false) - && match_call_path( - &collect_call_paths(&keyword.node.value), + && match_module_member( + &keyword.node.value, "abc", "ABCMeta", from_imports, + import_aliases, ) }) || bases .iter() - .any(|base| match_call_path(&collect_call_paths(base), "abc", "ABC", from_imports)) + .any(|base| match_module_member(base, "abc", "ABC", from_imports, import_aliases)) } fn is_empty_body(body: &[Stmt]) -> bool { @@ -42,22 +44,20 @@ fn is_empty_body(body: &[Stmt]) -> bool { }) } -fn is_abstractmethod(expr: &Expr, from_imports: &FnvHashMap<&str, FnvHashSet<&str>>) -> bool { - match_call_path( - &collect_call_paths(expr), - "abc", - "abstractmethod", - from_imports, - ) +fn is_abstractmethod( + expr: &Expr, + from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, + import_aliases: &FnvHashMap<&str, &str>, +) -> bool { + match_module_member(expr, "abc", "abstractmethod", from_imports, import_aliases) } -fn is_overload(expr: &Expr, from_imports: &FnvHashMap<&str, FnvHashSet<&str>>) -> bool { - match_call_path( - &collect_call_paths(expr), - "typing", - "overload", - from_imports, - ) +fn is_overload( + expr: &Expr, + from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, + import_aliases: &FnvHashMap<&str, &str>, +) -> bool { + match_module_member(expr, "typing", "overload", from_imports, import_aliases) } pub fn abstract_base_class( @@ -68,7 +68,14 @@ pub fn abstract_base_class( keywords: &[Keyword], body: &[Stmt], ) { - if bases.len() + keywords.len() == 1 && is_abc_class(bases, keywords, &checker.from_imports) { + if bases.len() + keywords.len() == 1 + && is_abc_class( + bases, + keywords, + &checker.from_imports, + &checker.import_aliases, + ) + { let mut has_abstract_method = false; for stmt in body { // https://github.com/PyCQA/flake8-bugbear/issues/293 @@ -91,28 +98,32 @@ pub fn abstract_base_class( { let has_abstract_decorator = decorator_list .iter() - .any(|d| is_abstractmethod(d, &checker.from_imports)); + .any(|d| is_abstractmethod(d, &checker.from_imports, &checker.import_aliases)); has_abstract_method |= has_abstract_decorator; - if !has_abstract_decorator - && is_empty_body(body) - && !decorator_list - .iter() - .any(|d| is_overload(d, &checker.from_imports)) - { - checker.add_check(Check::new( - CheckKind::EmptyMethodWithoutAbstractDecorator(name.to_string()), - Range::from_located(stmt), - )); + if checker.settings.enabled.contains(&CheckCode::B027) { + if !has_abstract_decorator + && is_empty_body(body) + && !decorator_list + .iter() + .any(|d| is_overload(d, &checker.from_imports, &checker.import_aliases)) + { + checker.add_check(Check::new( + CheckKind::EmptyMethodWithoutAbstractDecorator(name.to_string()), + Range::from_located(stmt), + )); + } } } } - if !has_abstract_method { - checker.add_check(Check::new( - CheckKind::AbstractBaseClassWithoutAbstractMethod(name.to_string()), - Range::from_located(stmt), - )); + if checker.settings.enabled.contains(&CheckCode::B024) { + if !has_abstract_method { + checker.add_check(Check::new( + CheckKind::AbstractBaseClassWithoutAbstractMethod(name.to_string()), + Range::from_located(stmt), + )); + } } } } diff --git a/src/flake8_bugbear/plugins/cached_instance_method.rs b/src/flake8_bugbear/plugins/cached_instance_method.rs index 298dd7f1bc..b78bb51718 100644 --- a/src/flake8_bugbear/plugins/cached_instance_method.rs +++ b/src/flake8_bugbear/plugins/cached_instance_method.rs @@ -1,12 +1,12 @@ use rustpython_ast::{Expr, ExprKind}; -use crate::ast::helpers::{collect_call_paths, match_call_path}; +use crate::ast::helpers::{collect_call_paths, dealias_call_path, match_call_path}; use crate::ast::types::{Range, ScopeKind}; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; fn is_cache_func(checker: &Checker, expr: &Expr) -> bool { - let call_path = collect_call_paths(expr); + let call_path = dealias_call_path(collect_call_paths(expr), &checker.import_aliases); match_call_path(&call_path, "functools", "lru_cache", &checker.from_imports) || match_call_path(&call_path, "functools", "cache", &checker.from_imports) } diff --git a/src/flake8_bugbear/plugins/duplicate_exceptions.rs b/src/flake8_bugbear/plugins/duplicate_exceptions.rs index 2d1acfeafe..3d7e14e816 100644 --- a/src/flake8_bugbear/plugins/duplicate_exceptions.rs +++ b/src/flake8_bugbear/plugins/duplicate_exceptions.rs @@ -21,20 +21,21 @@ fn type_pattern(elts: Vec<&Expr>) -> Expr { ) } -pub fn duplicate_handler_exceptions( +fn duplicate_handler_exceptions<'a>( checker: &mut Checker, - expr: &Expr, - elts: &[Expr], -) -> BTreeSet { - let mut seen: BTreeSet = Default::default(); - let mut duplicates: BTreeSet = Default::default(); + expr: &'a Expr, + elts: &'a [Expr], +) -> BTreeSet> { + let mut seen: BTreeSet> = Default::default(); + let mut duplicates: BTreeSet> = Default::default(); let mut unique_elts: Vec<&Expr> = Default::default(); for type_ in elts { - if let Some(name) = helpers::compose_call_path(type_) { - if seen.contains(&name) { - duplicates.insert(name); + let call_path = helpers::collect_call_paths(type_); + if !call_path.is_empty() { + if seen.contains(&call_path) { + duplicates.insert(call_path); } else { - seen.insert(name); + seen.insert(call_path); unique_elts.push(type_); } } @@ -45,7 +46,11 @@ pub fn duplicate_handler_exceptions( if !duplicates.is_empty() { let mut check = Check::new( CheckKind::DuplicateHandlerException( - duplicates.into_iter().sorted().collect::>(), + duplicates + .into_iter() + .map(|call_path| call_path.join(".")) + .sorted() + .collect::>(), ), Range::from_located(expr), ); @@ -70,19 +75,20 @@ pub fn duplicate_handler_exceptions( } pub fn duplicate_exceptions(checker: &mut Checker, stmt: &Stmt, handlers: &[Excepthandler]) { - let mut seen: BTreeSet = Default::default(); - let mut duplicates: BTreeSet = Default::default(); + let mut seen: BTreeSet> = Default::default(); + let mut duplicates: BTreeSet> = Default::default(); for handler in handlers { match &handler.node { ExcepthandlerKind::ExceptHandler { type_, .. } => { if let Some(type_) = type_ { match &type_.node { ExprKind::Attribute { .. } | ExprKind::Name { .. } => { - if let Some(name) = helpers::compose_call_path(type_) { - if seen.contains(&name) { - duplicates.insert(name); + let call_path = helpers::collect_call_paths(type_); + if !call_path.is_empty() { + if seen.contains(&call_path) { + duplicates.insert(call_path); } else { - seen.insert(name); + seen.insert(call_path); } } } @@ -105,7 +111,7 @@ pub fn duplicate_exceptions(checker: &mut Checker, stmt: &Stmt, handlers: &[Exce if checker.settings.enabled.contains(&CheckCode::B025) { for duplicate in duplicates.into_iter().sorted() { checker.add_check(Check::new( - CheckKind::DuplicateTryBlockException(duplicate), + CheckKind::DuplicateTryBlockException(duplicate.join(".")), Range::from_located(stmt), )); } diff --git a/src/flake8_bugbear/plugins/function_call_argument_default.rs b/src/flake8_bugbear/plugins/function_call_argument_default.rs index 3ae687badc..ab27275450 100644 --- a/src/flake8_bugbear/plugins/function_call_argument_default.rs +++ b/src/flake8_bugbear/plugins/function_call_argument_default.rs @@ -1,7 +1,9 @@ use fnv::{FnvHashMap, FnvHashSet}; use rustpython_ast::{Arguments, Constant, Expr, ExprKind}; -use crate::ast::helpers::{collect_call_paths, compose_call_path, match_call_path}; +use crate::ast::helpers::{ + collect_call_paths, compose_call_path, dealias_call_path, match_call_path, +}; use crate::ast::types::Range; use crate::ast::visitor; use crate::ast::visitor::Visitor; @@ -23,8 +25,9 @@ fn is_immutable_func( expr: &Expr, extend_immutable_calls: &[(&str, &str)], from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, + import_aliases: &FnvHashMap<&str, &str>, ) -> bool { - let call_path = collect_call_paths(expr); + let call_path = dealias_call_path(collect_call_paths(expr), import_aliases); IMMUTABLE_FUNCS .iter() .chain(extend_immutable_calls) @@ -35,6 +38,7 @@ struct ArgumentDefaultVisitor<'a> { checks: Vec<(CheckKind, Range)>, extend_immutable_calls: &'a [(&'a str, &'a str)], from_imports: &'a FnvHashMap<&'a str, FnvHashSet<&'a str>>, + import_aliases: &'a FnvHashMap<&'a str, &'a str>, } impl<'a, 'b> Visitor<'b> for ArgumentDefaultVisitor<'b> @@ -44,8 +48,13 @@ where fn visit_expr(&mut self, expr: &'b Expr) { match &expr.node { ExprKind::Call { func, args, .. } => { - if !is_mutable_func(func, self.from_imports) - && !is_immutable_func(func, self.extend_immutable_calls, self.from_imports) + if !is_mutable_func(func, self.from_imports, self.import_aliases) + && !is_immutable_func( + func, + self.extend_immutable_calls, + self.from_imports, + self.import_aliases, + ) && !is_nan_or_infinity(func, args) { self.checks.push(( @@ -108,6 +117,7 @@ pub fn function_call_argument_default(checker: &mut Checker, arguments: &Argumen checks: vec![], extend_immutable_calls: &extend_immutable_cells, from_imports: &checker.from_imports, + import_aliases: &checker.import_aliases, }; for expr in arguments .defaults diff --git a/src/flake8_bugbear/plugins/mod.rs b/src/flake8_bugbear/plugins/mod.rs index 7cfc44b8a6..a92a0501bb 100644 --- a/src/flake8_bugbear/plugins/mod.rs +++ b/src/flake8_bugbear/plugins/mod.rs @@ -4,7 +4,7 @@ pub use assert_raises_exception::assert_raises_exception; pub use assignment_to_os_environ::assignment_to_os_environ; pub use cached_instance_method::cached_instance_method; pub use cannot_raise_literal::cannot_raise_literal; -pub use duplicate_exceptions::{duplicate_exceptions, duplicate_handler_exceptions}; +pub use duplicate_exceptions::duplicate_exceptions; pub use f_string_docstring::f_string_docstring; pub use function_call_argument_default::function_call_argument_default; pub use getattr_with_constant::getattr_with_constant; diff --git a/src/flake8_bugbear/plugins/mutable_argument_default.rs b/src/flake8_bugbear/plugins/mutable_argument_default.rs index 1ca00e4ba0..be8b878363 100644 --- a/src/flake8_bugbear/plugins/mutable_argument_default.rs +++ b/src/flake8_bugbear/plugins/mutable_argument_default.rs @@ -1,7 +1,7 @@ use fnv::{FnvHashMap, FnvHashSet}; use rustpython_ast::{Arguments, Expr, ExprKind}; -use crate::ast::helpers::{collect_call_paths, match_call_path}; +use crate::ast::helpers::{collect_call_paths, dealias_call_path, match_call_path}; use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -16,8 +16,12 @@ const MUTABLE_FUNCS: [(&str, &str); 7] = [ ("collections", "deque"), ]; -pub fn is_mutable_func(expr: &Expr, from_imports: &FnvHashMap<&str, FnvHashSet<&str>>) -> bool { - let call_path = collect_call_paths(expr); +pub fn is_mutable_func( + expr: &Expr, + from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, + import_aliases: &FnvHashMap<&str, &str>, +) -> bool { + let call_path = dealias_call_path(collect_call_paths(expr), import_aliases); MUTABLE_FUNCS .iter() .any(|(module, member)| match_call_path(&call_path, module, member, from_imports)) @@ -43,7 +47,7 @@ pub fn mutable_argument_default(checker: &mut Checker, arguments: &Arguments) { )); } ExprKind::Call { func, .. } => { - if is_mutable_func(func, &checker.from_imports) { + if is_mutable_func(func, &checker.from_imports, &checker.import_aliases) { checker.add_check(Check::new( CheckKind::MutableArgumentDefault, Range::from_located(expr), diff --git a/src/linter.rs b/src/linter.rs index 8d8b397b4c..cb6c3691ee 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -455,6 +455,7 @@ mod tests { #[test_case(CheckCode::F821, Path::new("F821_1.py"); "F821_1")] #[test_case(CheckCode::F821, Path::new("F821_2.py"); "F821_2")] #[test_case(CheckCode::F821, Path::new("F821_3.py"); "F821_3")] + #[test_case(CheckCode::F821, Path::new("F821_4.py"); "F821_4")] #[test_case(CheckCode::F822, Path::new("F822.py"); "F822")] #[test_case(CheckCode::F823, Path::new("F823.py"); "F823")] #[test_case(CheckCode::F831, Path::new("F831.py"); "F831")] diff --git a/src/python/typing.rs b/src/python/typing.rs index 7cd5f22c2a..2a9a9353e6 100644 --- a/src/python/typing.rs +++ b/src/python/typing.rs @@ -1,6 +1,8 @@ use fnv::{FnvHashMap, FnvHashSet}; use once_cell::sync::Lazy; -use rustpython_ast::{Expr, ExprKind}; +use rustpython_ast::Expr; + +use crate::ast::helpers::{collect_call_paths, dealias_call_path, match_call_path}; // See: https://pypi.org/project/typing-extensions/ static TYPING_EXTENSIONS: Lazy> = Lazy::new(|| { @@ -63,144 +65,144 @@ pub fn in_extensions(name: &str) -> bool { } // See: https://docs.python.org/3/library/typing.html -static IMPORTED_SUBSCRIPTS: Lazy>> = - Lazy::new(|| { - let mut import_map = FnvHashMap::default(); - for (name, module) in [ - // `collections` - ("ChainMap", "collections"), - ("Counter", "collections"), - ("OrderedDict", "collections"), - ("defaultdict", "collections"), - ("deque", "collections"), - // `collections.abc` - ("AsyncGenerator", "collections.abc"), - ("AsyncIterable", "collections.abc"), - ("AsyncIterator", "collections.abc"), - ("Awaitable", "collections.abc"), - ("ByteString", "collections.abc"), - ("Callable", "collections.abc"), - ("Collection", "collections.abc"), - ("Container", "collections.abc"), - ("Coroutine", "collections.abc"), - ("Generator", "collections.abc"), - ("ItemsView", "collections.abc"), - ("Iterable", "collections.abc"), - ("Iterator", "collections.abc"), - ("KeysView", "collections.abc"), - ("Mapping", "collections.abc"), - ("MappingView", "collections.abc"), - ("MutableMapping", "collections.abc"), - ("MutableSequence", "collections.abc"), - ("MutableSet", "collections.abc"), - ("Reversible", "collections.abc"), - ("Sequence", "collections.abc"), - ("Set", "collections.abc"), - ("ValuesView", "collections.abc"), - // `contextlib` - ("AbstractAsyncContextManager", "contextlib"), - ("AbstractContextManager", "contextlib"), - // `re` - ("Match", "re"), - ("Pattern", "re"), - // `typing` - ("AbstractSet", "typing"), - ("Annotated", "typing"), - ("AsyncContextManager", "typing"), - ("AsyncGenerator", "typing"), - ("AsyncIterator", "typing"), - ("Awaitable", "typing"), - ("BinaryIO", "typing"), - ("ByteString", "typing"), - ("Callable", "typing"), - ("ChainMap", "typing"), - ("ClassVar", "typing"), - ("Collection", "typing"), - ("Concatenate", "typing"), - ("Container", "typing"), - ("ContextManager", "typing"), - ("Coroutine", "typing"), - ("Counter", "typing"), - ("DefaultDict", "typing"), - ("Deque", "typing"), - ("Dict", "typing"), - ("Final", "typing"), - ("FrozenSet", "typing"), - ("Generator", "typing"), - ("Generic", "typing"), - ("IO", "typing"), - ("ItemsView", "typing"), - ("Iterable", "typing"), - ("Iterator", "typing"), - ("KeysView", "typing"), - ("List", "typing"), - ("Mapping", "typing"), - ("Match", "typing"), - ("MutableMapping", "typing"), - ("MutableSequence", "typing"), - ("MutableSet", "typing"), - ("Optional", "typing"), - ("OrderedDict", "typing"), - ("Pattern", "typing"), - ("Reversible", "typing"), - ("Sequence", "typing"), - ("Set", "typing"), - ("TextIO", "typing"), - ("Tuple", "typing"), - ("Type", "typing"), - ("TypeGuard", "typing"), - ("Union", "typing"), - ("Unpack", "typing"), - ("ValuesView", "typing"), - // `typing.io` - ("BinaryIO", "typing.io"), - ("IO", "typing.io"), - ("TextIO", "typing.io"), - // `typing.re` - ("Match", "typing.re"), - ("Pattern", "typing.re"), - // `typing_extensions` - ("Annotated", "typing_extensions"), - ("AsyncContextManager", "typing_extensions"), - ("AsyncGenerator", "typing_extensions"), - ("AsyncIterable", "typing_extensions"), - ("AsyncIterator", "typing_extensions"), - ("Awaitable", "typing_extensions"), - ("ChainMap", "typing_extensions"), - ("ClassVar", "typing_extensions"), - ("Concatenate", "typing_extensions"), - ("ContextManager", "typing_extensions"), - ("Coroutine", "typing_extensions"), - ("Counter", "typing_extensions"), - ("DefaultDict", "typing_extensions"), - ("Deque", "typing_extensions"), - ("Type", "typing_extensions"), - // `weakref` - ("WeakKeyDictionary", "weakref"), - ("WeakSet", "weakref"), - ("WeakValueDictionary", "weakref"), - ] { - import_map - .entry(name) - .or_insert_with(FnvHashSet::default) - .insert(module); - } - import_map - }); +const SUBSCRIPTS: &[(&str, &str)] = &[ + // builtins + ("", "dict"), + ("", "frozenset"), + ("", "list"), + ("", "set"), + ("", "tuple"), + ("", "type"), + // `collections` + ("collections", "ChainMap"), + ("collections", "Counter"), + ("collections", "OrderedDict"), + ("collections", "defaultdict"), + ("collections", "deque"), + // `collections.abc` + ("collections.abc", "AsyncGenerator"), + ("collections.abc", "AsyncIterable"), + ("collections.abc", "AsyncIterator"), + ("collections.abc", "Awaitable"), + ("collections.abc", "ByteString"), + ("collections.abc", "Callable"), + ("collections.abc", "Collection"), + ("collections.abc", "Container"), + ("collections.abc", "Coroutine"), + ("collections.abc", "Generator"), + ("collections.abc", "ItemsView"), + ("collections.abc", "Iterable"), + ("collections.abc", "Iterator"), + ("collections.abc", "KeysView"), + ("collections.abc", "Mapping"), + ("collections.abc", "MappingView"), + ("collections.abc", "MutableMapping"), + ("collections.abc", "MutableSequence"), + ("collections.abc", "MutableSet"), + ("collections.abc", "Reversible"), + ("collections.abc", "Sequence"), + ("collections.abc", "Set"), + ("collections.abc", "ValuesView"), + // `contextlib` + ("contextlib", "AbstractAsyncContextManager"), + ("contextlib", "AbstractContextManager"), + // `re` + ("re", "Match"), + ("re", "Pattern"), + // `typing` + ("typing", "AbstractSet"), + ("typing", "AsyncContextManager"), + ("typing", "AsyncGenerator"), + ("typing", "AsyncIterator"), + ("typing", "Awaitable"), + ("typing", "BinaryIO"), + ("typing", "ByteString"), + ("typing", "Callable"), + ("typing", "ChainMap"), + ("typing", "ClassVar"), + ("typing", "Collection"), + ("typing", "Concatenate"), + ("typing", "Container"), + ("typing", "ContextManager"), + ("typing", "Coroutine"), + ("typing", "Counter"), + ("typing", "DefaultDict"), + ("typing", "Deque"), + ("typing", "Dict"), + ("typing", "Final"), + ("typing", "FrozenSet"), + ("typing", "Generator"), + ("typing", "Generic"), + ("typing", "IO"), + ("typing", "ItemsView"), + ("typing", "Iterable"), + ("typing", "Iterator"), + ("typing", "KeysView"), + ("typing", "List"), + ("typing", "Mapping"), + ("typing", "Match"), + ("typing", "MutableMapping"), + ("typing", "MutableSequence"), + ("typing", "MutableSet"), + ("typing", "Optional"), + ("typing", "OrderedDict"), + ("typing", "Pattern"), + ("typing", "Reversible"), + ("typing", "Sequence"), + ("typing", "Set"), + ("typing", "TextIO"), + ("typing", "Tuple"), + ("typing", "Type"), + ("typing", "TypeGuard"), + ("typing", "Union"), + ("typing", "Unpack"), + ("typing", "ValuesView"), + // `typing.io` + ("typing.io", "BinaryIO"), + ("typing.io", "IO"), + ("typing.io", "TextIO"), + // `typing.re` + ("typing.re", "Match"), + ("typing.re", "Pattern"), + // `typing_extensions` + ("typing_extensions", "AsyncContextManager"), + ("typing_extensions", "AsyncGenerator"), + ("typing_extensions", "AsyncIterable"), + ("typing_extensions", "AsyncIterator"), + ("typing_extensions", "Awaitable"), + ("typing_extensions", "ChainMap"), + ("typing_extensions", "ClassVar"), + ("typing_extensions", "Concatenate"), + ("typing_extensions", "ContextManager"), + ("typing_extensions", "Coroutine"), + ("typing_extensions", "Counter"), + ("typing_extensions", "DefaultDict"), + ("typing_extensions", "Deque"), + ("typing_extensions", "Type"), + // `weakref` + ("weakref", "WeakKeyDictionary"), + ("weakref", "WeakSet"), + ("weakref", "WeakValueDictionary"), +]; + +// See: https://docs.python.org/3/library/typing.html +const PEP_583_SUBSCRIPTS: &[(&str, &str)] = &[ + // `typing` + ("typing", "Annotated"), + // `typing_extensions` + ("typing_extensions", "Annotated"), +]; -// These are all assumed to come from the `typing` module. // See: https://peps.python.org/pep-0585/ -static PEP_585_BUILTINS_ELIGIBLE: Lazy> = - Lazy::new(|| FnvHashSet::from_iter(["Dict", "FrozenSet", "List", "Set", "Tuple", "Type"])); - -// These are all assumed to come from the `typing` module. -// See: https://peps.python.org/pep-0585/ -static PEP_585_BUILTINS: Lazy> = - Lazy::new(|| FnvHashSet::from_iter(["dict", "frozenset", "list", "set", "tuple", "type"])); - -fn is_pep593_annotated_subscript(name: &str) -> bool { - name == "Annotated" -} +const PEP_585_BUILTINS_ELIGIBLE: &[(&str, &str)] = &[ + ("typing", "Dict"), + ("typing", "FrozenSet"), + ("typing", "List"), + ("typing", "Set"), + ("typing", "Tuple"), + ("typing", "Type"), + ("typing_extensions", "Type"), +]; pub enum SubscriptKind { AnnotatedSubscript, @@ -210,72 +212,38 @@ pub enum SubscriptKind { pub fn match_annotated_subscript( expr: &Expr, from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, + import_aliases: &FnvHashMap<&str, &str>, ) -> Option { - match &expr.node { - ExprKind::Attribute { attr, value, .. } => { - if let ExprKind::Name { id, .. } = &value.node { - // If `id` is `typing` and `attr` is `Union`, verify that `typing.Union` is an - // annotated subscript. - if IMPORTED_SUBSCRIPTS - .get(&attr.as_str()) - .map(|imports| imports.contains(&id.as_str())) - .unwrap_or_default() - { - return if is_pep593_annotated_subscript(attr) { - Some(SubscriptKind::PEP593AnnotatedSubscript) - } else { - Some(SubscriptKind::AnnotatedSubscript) - }; - } - } - } - ExprKind::Name { id, .. } => { - // Built-ins (no import necessary). - if PEP_585_BUILTINS.contains(&id.as_str()) { + let call_path = dealias_call_path(collect_call_paths(expr), import_aliases); + if !call_path.is_empty() { + for (module, member) in SUBSCRIPTS { + if match_call_path(&call_path, module, member, from_imports) { return Some(SubscriptKind::AnnotatedSubscript); } - - // Verify that, e.g., `Union` is a reference to `typing.Union`. - if let Some(modules) = IMPORTED_SUBSCRIPTS.get(&id.as_str()) { - for module in modules { - if from_imports - .get(module) - .map(|imports| imports.contains(&id.as_str()) || imports.contains("*")) - .unwrap_or_default() - { - return if is_pep593_annotated_subscript(id) { - Some(SubscriptKind::PEP593AnnotatedSubscript) - } else { - Some(SubscriptKind::AnnotatedSubscript) - }; - } - } + } + for (module, member) in PEP_583_SUBSCRIPTS { + if match_call_path(&call_path, module, member, from_imports) { + return Some(SubscriptKind::PEP593AnnotatedSubscript); } } - _ => {} } None } /// Returns `true` if `Expr` represents a reference to a typing object with a -/// PEP 585 built-in. Note that none of the PEP 585 built-ins are in -/// `typing_extensions`. -pub fn is_pep585_builtin(expr: &Expr, from_imports: &FnvHashMap<&str, FnvHashSet<&str>>) -> bool { - match &expr.node { - ExprKind::Attribute { attr, value, .. } => { - if let ExprKind::Name { id, .. } = &value.node { - id == "typing" && PEP_585_BUILTINS_ELIGIBLE.contains(&attr.as_str()) - } else { - false +/// PEP 585 built-in. +pub fn is_pep585_builtin( + expr: &Expr, + from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, + import_aliases: &FnvHashMap<&str, &str>, +) -> bool { + let call_path = dealias_call_path(collect_call_paths(expr), import_aliases); + if !call_path.is_empty() { + for (module, member) in PEP_585_BUILTINS_ELIGIBLE { + if match_call_path(&call_path, module, member, from_imports) { + return true; } } - ExprKind::Name { id, .. } => { - from_imports - .get("typing") - .map(|imports| imports.contains(&id.as_str()) || imports.contains("*")) - .unwrap_or_default() - && PEP_585_BUILTINS_ELIGIBLE.contains(&id.as_str()) - } - _ => false, } + false } diff --git a/src/pyupgrade/checks.rs b/src/pyupgrade/checks.rs index 9b1ee4d702..56a9df0071 100644 --- a/src/pyupgrade/checks.rs +++ b/src/pyupgrade/checks.rs @@ -163,7 +163,8 @@ pub fn type_of_primitive(func: &Expr, args: &[Expr], location: Range) -> Option< pub fn unnecessary_lru_cache_params( decorator_list: &[Expr], target_version: PythonVersion, - imports: &FnvHashMap<&str, FnvHashSet<&str>>, + from_imports: &FnvHashMap<&str, FnvHashSet<&str>>, + import_aliases: &FnvHashMap<&str, &str>, ) -> Option { for expr in decorator_list.iter() { if let ExprKind::Call { @@ -173,7 +174,13 @@ pub fn unnecessary_lru_cache_params( } = &expr.node { if args.is_empty() - && helpers::match_module_member(func, "functools", "lru_cache", imports) + && helpers::match_module_member( + func, + "functools", + "lru_cache", + from_imports, + import_aliases, + ) { // Ex) `functools.lru_cache()` if keywords.is_empty() { diff --git a/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs b/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs index 95e9420850..79e1533860 100644 --- a/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs +++ b/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs @@ -9,6 +9,7 @@ pub fn unnecessary_lru_cache_params(checker: &mut Checker, decorator_list: &[Exp decorator_list, checker.settings.target_version, &checker.from_imports, + &checker.import_aliases, ) { if checker.patch() { if let Some(fix) = diff --git a/src/pyupgrade/plugins/use_pep585_annotation.rs b/src/pyupgrade/plugins/use_pep585_annotation.rs index fb5a018ce4..1654a09a35 100644 --- a/src/pyupgrade/plugins/use_pep585_annotation.rs +++ b/src/pyupgrade/plugins/use_pep585_annotation.rs @@ -7,13 +7,14 @@ use crate::checks::{Check, CheckKind}; /// U006 pub fn use_pep585_annotation(checker: &mut Checker, expr: &Expr, id: &str) { + let replacement = checker.import_aliases.get(id).unwrap_or(&id); let mut check = Check::new( - CheckKind::UsePEP585Annotation(id.to_string()), + CheckKind::UsePEP585Annotation(replacement.to_string()), Range::from_located(expr), ); if checker.patch() { check.amend(Fix::replacement( - id.to_lowercase(), + replacement.to_lowercase(), expr.location, expr.end_location.unwrap(), )); diff --git a/src/snapshots/ruff__linter__tests__B024_B024.py.snap b/src/snapshots/ruff__linter__tests__B024_B024.py.snap index 87c04075be..bca0dc11f1 100644 --- a/src/snapshots/ruff__linter__tests__B024_B024.py.snap +++ b/src/snapshots/ruff__linter__tests__B024_B024.py.snap @@ -11,42 +11,6 @@ expression: checks row: 22 column: 0 fix: ~ -- kind: - AbstractBaseClassWithoutAbstractMethod: Base_4 - location: - row: 34 - column: 0 - end_location: - row: 40 - column: 0 - fix: ~ -- kind: - AbstractBaseClassWithoutAbstractMethod: Base_5 - location: - row: 40 - column: 0 - end_location: - row: 46 - column: 0 - fix: ~ -- kind: - AbstractBaseClassWithoutAbstractMethod: Base_6 - location: - row: 46 - column: 0 - end_location: - row: 52 - column: 0 - fix: ~ -- kind: - AbstractBaseClassWithoutAbstractMethod: Base_7 - location: - row: 52 - column: 0 - end_location: - row: 58 - column: 0 - fix: ~ - kind: AbstractBaseClassWithoutAbstractMethod: MetaBase_1 location: @@ -74,6 +38,15 @@ expression: checks row: 79 column: 0 fix: ~ +- kind: + AbstractBaseClassWithoutAbstractMethod: notabc_Base_1 + location: + row: 79 + column: 0 + end_location: + row: 84 + column: 0 + fix: ~ - kind: AbstractBaseClassWithoutAbstractMethod: abc_set_class_variable_4 location: diff --git a/src/snapshots/ruff__linter__tests__B027_B027.py.snap b/src/snapshots/ruff__linter__tests__B027_B027.py.snap index 52b565f26c..a00e9be5f1 100644 --- a/src/snapshots/ruff__linter__tests__B027_B027.py.snap +++ b/src/snapshots/ruff__linter__tests__B027_B027.py.snap @@ -5,64 +5,37 @@ expression: checks - kind: EmptyMethodWithoutAbstractDecorator: AbstractClass location: - row: 13 + row: 12 column: 4 end_location: - row: 16 + row: 15 column: 4 fix: ~ - kind: EmptyMethodWithoutAbstractDecorator: AbstractClass location: - row: 16 + row: 15 column: 4 end_location: - row: 19 + row: 18 column: 4 fix: ~ - kind: EmptyMethodWithoutAbstractDecorator: AbstractClass location: - row: 19 + row: 18 column: 4 end_location: - row: 23 + row: 22 column: 4 fix: ~ - kind: EmptyMethodWithoutAbstractDecorator: AbstractClass location: - row: 23 + row: 22 column: 4 end_location: - row: 30 - column: 4 - fix: ~ -- kind: - EmptyMethodWithoutAbstractDecorator: AbstractClass - location: - row: 31 - column: 4 - end_location: - row: 34 - column: 4 - fix: ~ -- kind: - EmptyMethodWithoutAbstractDecorator: AbstractClass - location: - row: 80 - column: 4 - end_location: - row: 83 - column: 4 - fix: ~ -- kind: - EmptyMethodWithoutAbstractDecorator: AbstractClass - location: - row: 84 - column: 4 - end_location: - row: 87 + row: 29 column: 4 fix: ~ diff --git a/src/snapshots/ruff__linter__tests__F821_F821_4.py.snap b/src/snapshots/ruff__linter__tests__F821_F821_4.py.snap new file mode 100644 index 0000000000..f086a070a4 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__F821_F821_4.py.snap @@ -0,0 +1,50 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + UndefinedName: Model + location: + row: 4 + column: 9 + end_location: + row: 4 + column: 16 + fix: ~ +- kind: + UndefinedName: Model + location: + row: 9 + column: 10 + end_location: + row: 9 + column: 17 + fix: ~ +- kind: + UndefinedName: Model + location: + row: 14 + column: 14 + end_location: + row: 14 + column: 21 + fix: ~ +- kind: + UndefinedName: Model + location: + row: 19 + column: 30 + end_location: + row: 19 + column: 37 + fix: ~ +- kind: + UndefinedName: Model + location: + row: 24 + column: 18 + end_location: + row: 24 + column: 25 + fix: ~ + diff --git a/src/snapshots/ruff__linter__tests__U006_U006.py.snap b/src/snapshots/ruff__linter__tests__U006_U006.py.snap index 28fbe0d4f3..8e05bc151e 100644 --- a/src/snapshots/ruff__linter__tests__U006_U006.py.snap +++ b/src/snapshots/ruff__linter__tests__U006_U006.py.snap @@ -9,7 +9,7 @@ expression: checks column: 9 end_location: row: 4 - column: 13 + column: 20 fix: patch: content: list @@ -18,7 +18,7 @@ expression: checks column: 9 end_location: row: 4 - column: 13 + column: 20 applied: false - kind: UsePEP585Annotation: List @@ -27,7 +27,7 @@ expression: checks column: 9 end_location: row: 11 - column: 20 + column: 13 fix: patch: content: list @@ -36,6 +36,42 @@ expression: checks column: 9 end_location: row: 11 - column: 20 + column: 13 + applied: false +- kind: + UsePEP585Annotation: List + location: + row: 18 + column: 9 + end_location: + row: 18 + column: 15 + fix: + patch: + content: list + location: + row: 18 + column: 9 + end_location: + row: 18 + column: 15 + applied: false +- kind: + UsePEP585Annotation: List + location: + row: 25 + column: 9 + end_location: + row: 25 + column: 14 + fix: + patch: + content: list + location: + row: 25 + column: 9 + end_location: + row: 25 + column: 14 applied: false diff --git a/src/snapshots/ruff__linter__tests__YTT101_YTT101.py.snap b/src/snapshots/ruff__linter__tests__YTT101_YTT101.py.snap index 023a3bf7c3..c6a6b7760c 100644 --- a/src/snapshots/ruff__linter__tests__YTT101_YTT101.py.snap +++ b/src/snapshots/ruff__linter__tests__YTT101_YTT101.py.snap @@ -18,4 +18,12 @@ expression: checks row: 7 column: 13 fix: ~ +- kind: SysVersionSlice3Referenced + location: + row: 8 + column: 6 + end_location: + row: 8 + column: 7 + fix: ~