Fix some &String, &Option, and &Vec usages (#1670)

This commit is contained in:
Charlie Marsh 2023-01-05 18:56:03 -05:00 committed by GitHub
parent d34e6c02a1
commit 2464cf6fe9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 65 additions and 42 deletions

View file

@ -316,7 +316,7 @@ pub fn is_super_call_with_arguments(func: &Expr, args: &[Expr]) -> bool {
} }
/// Format the module name for a relative import. /// Format the module name for a relative import.
pub fn format_import_from(level: Option<&usize>, module: Option<&String>) -> String { pub fn format_import_from(level: Option<&usize>, module: Option<&str>) -> String {
let mut module_name = String::with_capacity(16); let mut module_name = String::with_capacity(16);
if let Some(level) = level { if let Some(level) = level {
for _ in 0..*level { for _ in 0..*level {

View file

@ -926,9 +926,11 @@ where
} }
if self.settings.enabled.contains(&CheckCode::PT013) { if self.settings.enabled.contains(&CheckCode::PT013) {
if let Some(check) = if let Some(check) = flake8_pytest_style::plugins::import_from(
flake8_pytest_style::plugins::import_from(stmt, module, level) stmt,
{ module.as_ref().map(String::as_str),
level.as_ref(),
) {
self.add_check(check); self.add_check(check);
} }
} }
@ -992,7 +994,7 @@ where
self.add_check(Check::new( self.add_check(Check::new(
CheckKind::ImportStarNotPermitted(helpers::format_import_from( CheckKind::ImportStarNotPermitted(helpers::format_import_from(
level.as_ref(), level.as_ref(),
module.as_ref(), module.as_ref().map(String::as_str),
)), )),
Range::from_located(stmt), Range::from_located(stmt),
)); ));
@ -1003,7 +1005,7 @@ where
self.add_check(Check::new( self.add_check(Check::new(
CheckKind::ImportStarUsed(helpers::format_import_from( CheckKind::ImportStarUsed(helpers::format_import_from(
level.as_ref(), level.as_ref(),
module.as_ref(), module.as_ref().map(String::as_str),
)), )),
Range::from_located(stmt), Range::from_located(stmt),
)); ));
@ -2577,7 +2579,11 @@ where
} }
} }
if self.settings.enabled.contains(&CheckCode::UP025) { if self.settings.enabled.contains(&CheckCode::UP025) {
pyupgrade::plugins::rewrite_unicode_literal(self, expr, kind); pyupgrade::plugins::rewrite_unicode_literal(
self,
expr,
kind.as_ref().map(String::as_str),
);
} }
} }
ExprKind::Lambda { args, body, .. } => { ExprKind::Lambda { args, body, .. } => {
@ -3376,7 +3382,7 @@ impl<'a> Checker<'a> {
if let BindingKind::StarImportation(level, module) = &binding.kind { if let BindingKind::StarImportation(level, module) = &binding.kind {
from_list.push(helpers::format_import_from( from_list.push(helpers::format_import_from(
level.as_ref(), level.as_ref(),
module.as_ref(), module.as_ref().map(String::as_str),
)); ));
} }
} }
@ -3857,7 +3863,7 @@ impl<'a> Checker<'a> {
if let BindingKind::StarImportation(level, module) = &binding.kind { if let BindingKind::StarImportation(level, module) = &binding.kind {
from_list.push(helpers::format_import_from( from_list.push(helpers::format_import_from(
level.as_ref(), level.as_ref(),
module.as_ref(), module.as_ref().map(String::as_str),
)); ));
} }
} }
@ -3882,7 +3888,7 @@ impl<'a> Checker<'a> {
if self.settings.enabled.contains(&CheckCode::F401) { if self.settings.enabled.contains(&CheckCode::F401) {
// Collect all unused imports by location. (Multiple unused imports at the same // Collect all unused imports by location. (Multiple unused imports at the same
// location indicates an `import from`.) // location indicates an `import from`.)
type UnusedImport<'a> = (&'a String, &'a Range); type UnusedImport<'a> = (&'a str, &'a Range);
type BindingContext<'a, 'b> = type BindingContext<'a, 'b> =
(&'a RefEquality<'b, Stmt>, Option<&'a RefEquality<'b, Stmt>>); (&'a RefEquality<'b, Stmt>, Option<&'a RefEquality<'b, Stmt>>);
@ -3954,9 +3960,7 @@ impl<'a> Checker<'a> {
let deleted: Vec<&Stmt> = let deleted: Vec<&Stmt> =
self.deletions.iter().map(|node| node.0).collect(); self.deletions.iter().map(|node| node.0).collect();
match autofix::helpers::remove_unused_imports( match autofix::helpers::remove_unused_imports(
unused_imports unused_imports.iter().map(|(full_name, _)| *full_name),
.iter()
.map(|(full_name, _)| full_name.as_str()),
child, child,
parent, parent,
&deleted, &deleted,
@ -4002,7 +4006,7 @@ impl<'a> Checker<'a> {
let multiple = unused_imports.len() > 1; let multiple = unused_imports.len() > 1;
for (full_name, range) in unused_imports { for (full_name, range) in unused_imports {
let mut check = Check::new( let mut check = Check::new(
CheckKind::UnusedImport(full_name.clone(), ignore_init, multiple), CheckKind::UnusedImport(full_name.to_string(), ignore_init, multiple),
*range, *range,
); );
if matches!(child.node, StmtKind::ImportFrom { .. }) if matches!(child.node, StmtKind::ImportFrom { .. })

View file

@ -42,7 +42,7 @@ pub fn compare_to_hardcoded_password_string(left: &Expr, comparators: &[Expr]) -
} }
/// S105 /// S105
pub fn assign_hardcoded_password_string(value: &Expr, targets: &Vec<Expr>) -> Option<Check> { pub fn assign_hardcoded_password_string(value: &Expr, targets: &[Expr]) -> Option<Check> {
if let Some(string) = string_literal(value) { if let Some(string) = string_literal(value) {
for target in targets { for target in targets {
if is_password_target(target) { if is_password_target(target) {

View file

@ -25,8 +25,8 @@ pub fn import(import_from: &Stmt, name: &str, asname: Option<&str>) -> Option<Ch
/// PT013 /// PT013
pub fn import_from( pub fn import_from(
import_from: &Stmt, import_from: &Stmt,
module: &Option<String>, module: Option<&str>,
level: &Option<usize>, level: Option<&usize>,
) -> Option<Check> { ) -> Option<Check> {
// If level is not zero or module is none, return // If level is not zero or module is none, return
if let Some(level) = level { if let Some(level) = level {

View file

@ -18,7 +18,7 @@ fn is_pytest_raises(
match_module_member(func, "pytest", "raises", from_imports, import_aliases) match_module_member(func, "pytest", "raises", from_imports, import_aliases)
} }
fn is_non_trivial_with_body(body: &Vec<Stmt>) -> bool { fn is_non_trivial_with_body(body: &[Stmt]) -> bool {
if body.len() > 1 { if body.len() > 1 {
true true
} else if let Some(first_body_stmt) = body.first() { } else if let Some(first_body_stmt) = body.first() {
@ -28,7 +28,7 @@ fn is_non_trivial_with_body(body: &Vec<Stmt>) -> bool {
} }
} }
pub fn raises_call(checker: &mut Checker, func: &Expr, args: &Vec<Expr>, keywords: &Vec<Keyword>) { pub fn raises_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: &[Keyword]) {
if is_pytest_raises(func, &checker.from_imports, &checker.import_aliases) { if is_pytest_raises(func, &checker.from_imports, &checker.import_aliases) {
if checker.settings.enabled.contains(&CheckCode::PT010) { if checker.settings.enabled.contains(&CheckCode::PT010) {
if args.is_empty() && keywords.is_empty() { if args.is_empty() && keywords.is_empty() {
@ -57,7 +57,7 @@ pub fn raises_call(checker: &mut Checker, func: &Expr, args: &Vec<Expr>, keyword
} }
} }
pub fn complex_raises(checker: &mut Checker, stmt: &Stmt, items: &[Withitem], body: &Vec<Stmt>) { pub fn complex_raises(checker: &mut Checker, stmt: &Stmt, items: &[Withitem], body: &[Stmt]) {
let mut is_too_complex = false; let mut is_too_complex = false;
let raises_called = items.iter().any(|item| match &item.context_expr.node { let raises_called = items.iter().any(|item| match &item.context_expr.node {

View file

@ -62,7 +62,7 @@ impl<'a> Visitor<'a> for ReturnVisitor<'a> {
StmtKind::Global { names } | StmtKind::Nonlocal { names } => { StmtKind::Global { names } | StmtKind::Nonlocal { names } => {
self.stack self.stack
.non_locals .non_locals
.extend(names.iter().map(std::string::String::as_str)); .extend(names.iter().map(String::as_str));
} }
StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => { StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => {
// Don't recurse. // Don't recurse.

View file

@ -33,7 +33,7 @@ pub mod types;
#[derive(Debug)] #[derive(Debug)]
pub struct AnnotatedAliasData<'a> { pub struct AnnotatedAliasData<'a> {
pub name: &'a str, pub name: &'a str,
pub asname: Option<&'a String>, pub asname: Option<&'a str>,
pub atop: Vec<Comment<'a>>, pub atop: Vec<Comment<'a>>,
pub inline: Vec<Comment<'a>>, pub inline: Vec<Comment<'a>>,
} }
@ -87,7 +87,7 @@ fn annotate_imports<'a>(
.iter() .iter()
.map(|alias| AliasData { .map(|alias| AliasData {
name: &alias.node.name, name: &alias.node.name,
asname: alias.node.asname.as_ref(), asname: alias.node.asname.as_deref(),
}) })
.collect(), .collect(),
atop, atop,
@ -145,7 +145,7 @@ fn annotate_imports<'a>(
aliases.push(AnnotatedAliasData { aliases.push(AnnotatedAliasData {
name: &alias.node.name, name: &alias.node.name,
asname: alias.node.asname.as_ref(), asname: alias.node.asname.as_deref(),
atop: alias_atop, atop: alias_atop,
inline: alias_inline, inline: alias_inline,
}); });
@ -219,19 +219,28 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
let entry = if alias.name == "*" { let entry = if alias.name == "*" {
block block
.import_from_star .import_from_star
.entry(ImportFromData { module, level }) .entry(ImportFromData {
module: module.map(String::as_str),
level,
})
.or_default() .or_default()
} else if alias.asname.is_none() || combine_as_imports { } else if alias.asname.is_none() || combine_as_imports {
&mut block &mut block
.import_from .import_from
.entry(ImportFromData { module, level }) .entry(ImportFromData {
module: module.map(String::as_str),
level,
})
.or_default() .or_default()
.0 .0
} else { } else {
block block
.import_from_as .import_from_as
.entry(( .entry((
ImportFromData { module, level }, ImportFromData {
module: module.map(String::as_str),
level,
},
AliasData { AliasData {
name: alias.name, name: alias.name,
asname: alias.asname, asname: alias.asname,
@ -254,12 +263,18 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
let entry = if alias.name == "*" { let entry = if alias.name == "*" {
block block
.import_from_star .import_from_star
.entry(ImportFromData { module, level }) .entry(ImportFromData {
module: module.map(String::as_str),
level,
})
.or_default() .or_default()
} else if alias.asname.is_none() || combine_as_imports { } else if alias.asname.is_none() || combine_as_imports {
block block
.import_from .import_from
.entry(ImportFromData { module, level }) .entry(ImportFromData {
module: module.map(String::as_str),
level,
})
.or_default() .or_default()
.1 .1
.entry(AliasData { .entry(AliasData {
@ -271,7 +286,10 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
block block
.import_from_as .import_from_as
.entry(( .entry((
ImportFromData { module, level }, ImportFromData {
module: module.map(String::as_str),
level,
},
AliasData { AliasData {
name: alias.name, name: alias.name,
asname: alias.asname, asname: alias.asname,
@ -290,9 +308,10 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
// Propagate trailing commas. // Propagate trailing commas.
if matches!(trailing_comma, TrailingComma::Present) { if matches!(trailing_comma, TrailingComma::Present) {
if let Some(entry) = if let Some(entry) = block.import_from.get_mut(&ImportFromData {
block.import_from.get_mut(&ImportFromData { module, level }) module: module.map(String::as_str),
{ level,
}) {
entry.2 = trailing_comma; entry.2 = trailing_comma;
} }
} }

View file

@ -18,14 +18,14 @@ impl Default for TrailingComma {
#[derive(Debug, Hash, Ord, PartialOrd, Eq, PartialEq, Clone)] #[derive(Debug, Hash, Ord, PartialOrd, Eq, PartialEq, Clone)]
pub struct ImportFromData<'a> { pub struct ImportFromData<'a> {
pub module: Option<&'a String>, pub module: Option<&'a str>,
pub level: Option<&'a usize>, pub level: Option<&'a usize>,
} }
#[derive(Debug, Hash, Ord, PartialOrd, Eq, PartialEq)] #[derive(Debug, Hash, Ord, PartialOrd, Eq, PartialEq)]
pub struct AliasData<'a> { pub struct AliasData<'a> {
pub name: &'a str, pub name: &'a str,
pub asname: Option<&'a String>, pub asname: Option<&'a str>,
} }
#[derive(Debug, Default, Clone)] #[derive(Debug, Default, Clone)]

View file

@ -303,7 +303,7 @@ impl<'a> Printer<'a> {
} }
} }
fn group_messages_by_filename(messages: &Vec<Message>) -> BTreeMap<&String, Vec<&Message>> { fn group_messages_by_filename(messages: &[Message]) -> BTreeMap<&String, Vec<&Message>> {
let mut grouped_messages = BTreeMap::default(); let mut grouped_messages = BTreeMap::default();
for message in messages { for message in messages {
grouped_messages grouped_messages

View file

@ -132,7 +132,7 @@ pub fn default_except_not_last(
#[derive(Debug, PartialEq)] #[derive(Debug, PartialEq)]
enum DictionaryKey<'a> { enum DictionaryKey<'a> {
Constant(&'a Constant), Constant(&'a Constant),
Variable(&'a String), Variable(&'a str),
} }
fn convert_to_value(expr: &Expr) -> Option<DictionaryKey> { fn convert_to_value(expr: &Expr) -> Option<DictionaryKey> {

View file

@ -6,7 +6,7 @@ use crate::checkers::ast::Checker;
use crate::registry::{Check, CheckKind}; use crate::registry::{Check, CheckKind};
/// UP025 /// UP025
pub fn rewrite_unicode_literal(checker: &mut Checker, expr: &Expr, kind: &Option<String>) { pub fn rewrite_unicode_literal(checker: &mut Checker, expr: &Expr, kind: Option<&str>) {
if let Some(const_kind) = kind { if let Some(const_kind) = kind {
if const_kind.to_lowercase() == "u" { if const_kind.to_lowercase() == "u" {
let mut check = Check::new(CheckKind::RewriteUnicodeLiteral, Range::from_located(expr)); let mut check = Check::new(CheckKind::RewriteUnicodeLiteral, Range::from_located(expr));

View file

@ -34,7 +34,7 @@ fn is_utf8_encoding_arg(arg: &Expr) -> bool {
} }
} }
fn is_default_encode(args: &Vec<Expr>, kwargs: &Vec<Keyword>) -> bool { fn is_default_encode(args: &[Expr], kwargs: &[Keyword]) -> bool {
match (args.len(), kwargs.len()) { match (args.len(), kwargs.len()) {
// .encode() // .encode()
(0, 0) => true, (0, 0) => true,
@ -106,8 +106,8 @@ pub fn unnecessary_encode_utf8(
checker: &mut Checker, checker: &mut Checker,
expr: &Expr, expr: &Expr,
func: &Expr, func: &Expr,
args: &Vec<Expr>, args: &[Expr],
kwargs: &Vec<Keyword>, kwargs: &[Keyword],
) { ) {
let Some(variable) = match_encoded_variable(func) else { let Some(variable) = match_encoded_variable(func) else {
return; return;

View file

@ -73,7 +73,7 @@ pub fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, names: &[Lo
.map(|alias| format!("__future__.{}", alias.node.name)) .map(|alias| format!("__future__.{}", alias.node.name))
.collect(); .collect();
match autofix::helpers::remove_unused_imports( match autofix::helpers::remove_unused_imports(
unused_imports.iter().map(std::string::String::as_str), unused_imports.iter().map(String::as_str),
defined_by.0, defined_by.0,
defined_in.map(|node| node.0), defined_in.map(|node| node.0),
&deleted, &deleted,