Skip to content

Commit

Permalink
Fix some &String, &Option, and &Vec usages (#1670)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 5, 2023
1 parent d34e6c0 commit 2464cf6
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 42 deletions.
2 changes: 1 addition & 1 deletion src/ast/helpers.rs
Expand Up @@ -316,7 +316,7 @@ pub fn is_super_call_with_arguments(func: &Expr, args: &[Expr]) -> bool {
}

/// 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);
if let Some(level) = level {
for _ in 0..*level {
Expand Down
30 changes: 17 additions & 13 deletions src/checkers/ast.rs
Expand Up @@ -926,9 +926,11 @@ where
}

if self.settings.enabled.contains(&CheckCode::PT013) {
if let Some(check) =
flake8_pytest_style::plugins::import_from(stmt, module, level)
{
if let Some(check) = flake8_pytest_style::plugins::import_from(
stmt,
module.as_ref().map(String::as_str),
level.as_ref(),
) {
self.add_check(check);
}
}
Expand Down Expand Up @@ -992,7 +994,7 @@ where
self.add_check(Check::new(
CheckKind::ImportStarNotPermitted(helpers::format_import_from(
level.as_ref(),
module.as_ref(),
module.as_ref().map(String::as_str),
)),
Range::from_located(stmt),
));
Expand All @@ -1003,7 +1005,7 @@ where
self.add_check(Check::new(
CheckKind::ImportStarUsed(helpers::format_import_from(
level.as_ref(),
module.as_ref(),
module.as_ref().map(String::as_str),
)),
Range::from_located(stmt),
));
Expand Down Expand Up @@ -2577,7 +2579,11 @@ where
}
}
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, .. } => {
Expand Down Expand Up @@ -3376,7 +3382,7 @@ impl<'a> Checker<'a> {
if let BindingKind::StarImportation(level, module) = &binding.kind {
from_list.push(helpers::format_import_from(
level.as_ref(),
module.as_ref(),
module.as_ref().map(String::as_str),
));
}
}
Expand Down Expand Up @@ -3857,7 +3863,7 @@ impl<'a> Checker<'a> {
if let BindingKind::StarImportation(level, module) = &binding.kind {
from_list.push(helpers::format_import_from(
level.as_ref(),
module.as_ref(),
module.as_ref().map(String::as_str),
));
}
}
Expand All @@ -3882,7 +3888,7 @@ impl<'a> Checker<'a> {
if self.settings.enabled.contains(&CheckCode::F401) {
// Collect all unused imports by location. (Multiple unused imports at the same
// location indicates an `import from`.)
type UnusedImport<'a> = (&'a String, &'a Range);
type UnusedImport<'a> = (&'a str, &'a Range);
type BindingContext<'a, 'b> =
(&'a RefEquality<'b, Stmt>, Option<&'a RefEquality<'b, Stmt>>);

Expand Down Expand Up @@ -3954,9 +3960,7 @@ impl<'a> Checker<'a> {
let deleted: Vec<&Stmt> =
self.deletions.iter().map(|node| node.0).collect();
match autofix::helpers::remove_unused_imports(
unused_imports
.iter()
.map(|(full_name, _)| full_name.as_str()),
unused_imports.iter().map(|(full_name, _)| *full_name),
child,
parent,
&deleted,
Expand Down Expand Up @@ -4002,7 +4006,7 @@ impl<'a> Checker<'a> {
let multiple = unused_imports.len() > 1;
for (full_name, range) in unused_imports {
let mut check = Check::new(
CheckKind::UnusedImport(full_name.clone(), ignore_init, multiple),
CheckKind::UnusedImport(full_name.to_string(), ignore_init, multiple),
*range,
);
if matches!(child.node, StmtKind::ImportFrom { .. })
Expand Down
2 changes: 1 addition & 1 deletion src/flake8_bandit/checks/hardcoded_password_string.rs
Expand Up @@ -42,7 +42,7 @@ pub fn compare_to_hardcoded_password_string(left: &Expr, comparators: &[Expr]) -
}

/// 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) {
for target in targets {
if is_password_target(target) {
Expand Down
4 changes: 2 additions & 2 deletions src/flake8_pytest_style/plugins/imports.rs
Expand Up @@ -25,8 +25,8 @@ pub fn import(import_from: &Stmt, name: &str, asname: Option<&str>) -> Option<Ch
/// PT013
pub fn import_from(
import_from: &Stmt,
module: &Option<String>,
level: &Option<usize>,
module: Option<&str>,
level: Option<&usize>,
) -> Option<Check> {
// If level is not zero or module is none, return
if let Some(level) = level {
Expand Down
6 changes: 3 additions & 3 deletions src/flake8_pytest_style/plugins/raises.rs
Expand Up @@ -18,7 +18,7 @@ fn is_pytest_raises(
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 {
true
} else if let Some(first_body_stmt) = body.first() {
Expand All @@ -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 checker.settings.enabled.contains(&CheckCode::PT010) {
if args.is_empty() && keywords.is_empty() {
Expand Down Expand Up @@ -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 raises_called = items.iter().any(|item| match &item.context_expr.node {
Expand Down
2 changes: 1 addition & 1 deletion src/flake8_return/visitor.rs
Expand Up @@ -62,7 +62,7 @@ impl<'a> Visitor<'a> for ReturnVisitor<'a> {
StmtKind::Global { names } | StmtKind::Nonlocal { names } => {
self.stack
.non_locals
.extend(names.iter().map(std::string::String::as_str));
.extend(names.iter().map(String::as_str));
}
StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => {
// Don't recurse.
Expand Down
43 changes: 31 additions & 12 deletions src/isort/mod.rs
Expand Up @@ -33,7 +33,7 @@ pub mod types;
#[derive(Debug)]
pub struct AnnotatedAliasData<'a> {
pub name: &'a str,
pub asname: Option<&'a String>,
pub asname: Option<&'a str>,
pub atop: Vec<Comment<'a>>,
pub inline: Vec<Comment<'a>>,
}
Expand Down Expand Up @@ -87,7 +87,7 @@ fn annotate_imports<'a>(
.iter()
.map(|alias| AliasData {
name: &alias.node.name,
asname: alias.node.asname.as_ref(),
asname: alias.node.asname.as_deref(),
})
.collect(),
atop,
Expand Down Expand Up @@ -145,7 +145,7 @@ fn annotate_imports<'a>(

aliases.push(AnnotatedAliasData {
name: &alias.node.name,
asname: alias.node.asname.as_ref(),
asname: alias.node.asname.as_deref(),
atop: alias_atop,
inline: alias_inline,
});
Expand Down Expand Up @@ -219,19 +219,28 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
let entry = if alias.name == "*" {
block
.import_from_star
.entry(ImportFromData { module, level })
.entry(ImportFromData {
module: module.map(String::as_str),
level,
})
.or_default()
} else if alias.asname.is_none() || combine_as_imports {
&mut block
.import_from
.entry(ImportFromData { module, level })
.entry(ImportFromData {
module: module.map(String::as_str),
level,
})
.or_default()
.0
} else {
block
.import_from_as
.entry((
ImportFromData { module, level },
ImportFromData {
module: module.map(String::as_str),
level,
},
AliasData {
name: alias.name,
asname: alias.asname,
Expand All @@ -254,12 +263,18 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
let entry = if alias.name == "*" {
block
.import_from_star
.entry(ImportFromData { module, level })
.entry(ImportFromData {
module: module.map(String::as_str),
level,
})
.or_default()
} else if alias.asname.is_none() || combine_as_imports {
block
.import_from
.entry(ImportFromData { module, level })
.entry(ImportFromData {
module: module.map(String::as_str),
level,
})
.or_default()
.1
.entry(AliasData {
Expand All @@ -271,7 +286,10 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->
block
.import_from_as
.entry((
ImportFromData { module, level },
ImportFromData {
module: module.map(String::as_str),
level,
},
AliasData {
name: alias.name,
asname: alias.asname,
Expand All @@ -290,9 +308,10 @@ fn normalize_imports(imports: Vec<AnnotatedImport>, combine_as_imports: bool) ->

// Propagate trailing commas.
if matches!(trailing_comma, TrailingComma::Present) {
if let Some(entry) =
block.import_from.get_mut(&ImportFromData { module, level })
{
if let Some(entry) = block.import_from.get_mut(&ImportFromData {
module: module.map(String::as_str),
level,
}) {
entry.2 = trailing_comma;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/isort/types.rs
Expand Up @@ -18,14 +18,14 @@ impl Default for TrailingComma {

#[derive(Debug, Hash, Ord, PartialOrd, Eq, PartialEq, Clone)]
pub struct ImportFromData<'a> {
pub module: Option<&'a String>,
pub module: Option<&'a str>,
pub level: Option<&'a usize>,
}

#[derive(Debug, Hash, Ord, PartialOrd, Eq, PartialEq)]
pub struct AliasData<'a> {
pub name: &'a str,
pub asname: Option<&'a String>,
pub asname: Option<&'a str>,
}

#[derive(Debug, Default, Clone)]
Expand Down
2 changes: 1 addition & 1 deletion src/printer.rs
Expand Up @@ -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();
for message in messages {
grouped_messages
Expand Down
2 changes: 1 addition & 1 deletion src/pyflakes/checks.rs
Expand Up @@ -132,7 +132,7 @@ pub fn default_except_not_last(
#[derive(Debug, PartialEq)]
enum DictionaryKey<'a> {
Constant(&'a Constant),
Variable(&'a String),
Variable(&'a str),
}

fn convert_to_value(expr: &Expr) -> Option<DictionaryKey> {
Expand Down
2 changes: 1 addition & 1 deletion src/pyupgrade/plugins/rewrite_unicode_literal.rs
Expand Up @@ -6,7 +6,7 @@ use crate::checkers::ast::Checker;
use crate::registry::{Check, CheckKind};

/// 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 const_kind.to_lowercase() == "u" {
let mut check = Check::new(CheckKind::RewriteUnicodeLiteral, Range::from_located(expr));
Expand Down
6 changes: 3 additions & 3 deletions src/pyupgrade/plugins/unnecessary_encode_utf8.rs
Expand Up @@ -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()) {
// .encode()
(0, 0) => true,
Expand Down Expand Up @@ -106,8 +106,8 @@ pub fn unnecessary_encode_utf8(
checker: &mut Checker,
expr: &Expr,
func: &Expr,
args: &Vec<Expr>,
kwargs: &Vec<Keyword>,
args: &[Expr],
kwargs: &[Keyword],
) {
let Some(variable) = match_encoded_variable(func) else {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/pyupgrade/plugins/unnecessary_future_import.rs
Expand Up @@ -73,7 +73,7 @@ pub fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, names: &[Lo
.map(|alias| format!("__future__.{}", alias.node.name))
.collect();
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_in.map(|node| node.0),
&deleted,
Expand Down

0 comments on commit 2464cf6

Please sign in to comment.