From 626b0577cdc458bbeaeb00f8b2af5268bb917a39 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 24 Nov 2023 18:03:59 -0600 Subject: [PATCH] Explicit `as_str` (no deref), add no allocation methods (#8826) ## Summary This PR is a follow-up to the AST refactor which does the following: - Remove `Deref` implementation on `StringLiteralValue` and use explicit `as_str` calls instead. The `Deref` implementation would implicitly perform allocations in case of implicitly concatenated strings. This is to make sure the allocation is explicit. - Now, certain methods can be implemented to do zero allocations which have been implemented in this PR. They are: - `is_empty` - `len` - `chars` - Custom `PartialEq` implementation to compare each character ## Test Plan Run the linter test suite and make sure all tests pass. --- .../src/checkers/ast/analyze/expression.rs | 18 +++++++--- crates/ruff_linter/src/checkers/ast/mod.rs | 4 +-- .../flake8_annotations/rules/definition.rs | 8 ++--- .../rules/hardcoded_password_string.rs | 2 +- .../rules/hardcoded_sql_expression.rs | 4 +-- .../rules/hardcoded_tmp_directory.rs | 2 +- .../rules/suspicious_function_call.rs | 2 +- .../rules/getattr_with_constant.rs | 4 +-- .../rules/setattr_with_constant.rs | 6 ++-- .../call_datetime_strptime_without_zone.rs | 2 +- .../rules/logging_call.rs | 2 +- .../rules/unnecessary_dict_kwargs.rs | 2 +- .../flake8_pytest_style/rules/parametrize.rs | 15 ++++---- .../rules/flake8_simplify/rules/ast_expr.rs | 8 ++--- .../ruff_linter/src/rules/pylint/helpers.rs | 2 +- .../src/rules/pylint/rules/bad_open_mode.rs | 2 +- .../rules/pylint/rules/bad_str_strip_call.rs | 2 +- .../pylint/rules/repeated_keyword_argument.rs | 2 +- .../pylint/rules/unspecified_encoding.rs | 7 +++- ...convert_named_tuple_functional_to_class.rs | 6 ++-- .../convert_typed_dict_functional_to_class.rs | 6 ++-- .../rules/printf_string_formatting.rs | 4 +-- .../pyupgrade/rules/redundant_open_modes.rs | 4 +-- .../rules/unnecessary_encode_utf8.rs | 4 +-- .../src/rules/ruff/rules/implicit_optional.rs | 8 ++--- crates/ruff_linter/src/rules/ruff/typing.rs | 9 +++-- crates/ruff_python_ast/src/all.rs | 2 +- crates/ruff_python_ast/src/nodes.rs | 35 +++++++++++++------ 28 files changed, 95 insertions(+), 77 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 76b7359449..e5ca3bbee3 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -369,18 +369,24 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { ]) { if let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() { let attr = attr.as_str(); - if let Expr::StringLiteral(ast::ExprStringLiteral { value: string, .. }) = - value.as_ref() + if let Expr::StringLiteral(ast::ExprStringLiteral { + value: string_value, + .. + }) = value.as_ref() { if attr == "join" { // "...".join(...) call if checker.enabled(Rule::StaticJoinToFString) { - flynt::rules::static_join_to_fstring(checker, expr, string); + flynt::rules::static_join_to_fstring( + checker, + expr, + string_value.as_str(), + ); } } else if attr == "format" { // "...".format(...) call let location = expr.range(); - match pyflakes::format::FormatSummary::try_from(string.as_ref()) { + match pyflakes::format::FormatSummary::try_from(string_value.as_str()) { Err(e) => { if checker.enabled(Rule::StringDotFormatInvalidFormat) { checker.diagnostics.push(Diagnostic::new( @@ -425,7 +431,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::BadStringFormatCharacter) { pylint::rules::bad_string_format_character::call( - checker, string, location, + checker, + string_value.as_str(), + location, ); } } diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 7357937c1a..bf0eba8e4f 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -799,7 +799,7 @@ where if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr { self.deferred.string_type_definitions.push(( expr.range(), - value, + value.as_str(), self.semantic.snapshot(), )); } else { @@ -1219,7 +1219,7 @@ where { self.deferred.string_type_definitions.push(( expr.range(), - value, + value.as_str(), self.semantic.snapshot(), )); } diff --git a/crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs b/crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs index c73f8da87c..b1bfc9df67 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs +++ b/crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs @@ -505,14 +505,10 @@ fn check_dynamically_typed( ) where F: FnOnce() -> String, { - if let Expr::StringLiteral(ast::ExprStringLiteral { - range, - value: string, - }) = annotation - { + if let Expr::StringLiteral(ast::ExprStringLiteral { range, value }) = annotation { // Quoted annotations if let Ok((parsed_annotation, _)) = - parse_type_annotation(string, *range, checker.locator().contents()) + parse_type_annotation(value.as_str(), *range, checker.locator().contents()) { if type_hint_resolves_to_any( &parsed_annotation, diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_password_string.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_password_string.rs index 9839cd58ae..0509986804 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_password_string.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_password_string.rs @@ -55,7 +55,7 @@ fn password_target(target: &Expr) -> Option<&str> { Expr::Name(ast::ExprName { id, .. }) => id.as_str(), // d["password"] = "s3cr3t" Expr::Subscript(ast::ExprSubscript { slice, .. }) => match slice.as_ref() { - Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => value, + Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.as_str(), _ => return None, }, // obj.password = "s3cr3t" diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs index d4d3df9bad..e132e806d8 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs @@ -93,7 +93,7 @@ pub(crate) fn hardcoded_sql_expression(checker: &mut Checker, expr: &Expr) { let Some(string) = left.as_string_literal_expr() else { return; }; - string.value.escape_default().to_string() + string.value.as_str().escape_default().to_string() } Expr::Call(ast::ExprCall { func, .. }) => { let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() else { @@ -106,7 +106,7 @@ pub(crate) fn hardcoded_sql_expression(checker: &mut Checker, expr: &Expr) { let Some(string) = value.as_string_literal_expr() else { return; }; - string.value.escape_default().to_string() + string.value.as_str().escape_default().to_string() } // f"select * from table where val = {val}" Expr::FString(f_string) => concatenated_f_string(f_string, checker.locator()), diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs index 592f4e7621..9b5a913dd7 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs @@ -57,7 +57,7 @@ pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, string: &ast::ExprS .flake8_bandit .hardcoded_tmp_directory .iter() - .any(|prefix| string.value.starts_with(prefix)) + .any(|prefix| string.value.as_str().starts_with(prefix)) { return; } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs index 0c6d9b9300..46fbec7612 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs @@ -855,7 +855,7 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) { // If the `url` argument is a string literal, allow `http` and `https` schemes. if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) { if let Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) = &call.arguments.find_argument("url", 0) { - let url = value.trim_start(); + let url = value.as_str().trim_start(); if url.starts_with("http://") || url.starts_with("https://") { return None; } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs index 4c6133d779..66f563a234 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs @@ -69,10 +69,10 @@ pub(crate) fn getattr_with_constant( let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = arg else { return; }; - if !is_identifier(value) { + if !is_identifier(value.as_str()) { return; } - if is_mangled_private(value) { + if is_mangled_private(value.as_str()) { return; } if !checker.semantic().is_builtin("getattr") { diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs index 92599a5807..bc17ed3ffd 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs @@ -83,10 +83,10 @@ pub(crate) fn setattr_with_constant( let Expr::StringLiteral(ast::ExprStringLiteral { value: name, .. }) = name else { return; }; - if !is_identifier(name) { + if !is_identifier(name.as_str()) { return; } - if is_mangled_private(name) { + if is_mangled_private(name.as_str()) { return; } if !checker.semantic().is_builtin("setattr") { @@ -104,7 +104,7 @@ pub(crate) fn setattr_with_constant( if expr == child.as_ref() { let mut diagnostic = Diagnostic::new(SetAttrWithConstant, expr.range()); diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - assignment(obj, name, value, checker.generator()), + assignment(obj, name.as_str(), value, checker.generator()), expr.range(), ))); checker.diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_strptime_without_zone.rs b/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_strptime_without_zone.rs index 6efde68d0d..0a9fe6c0e7 100644 --- a/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_strptime_without_zone.rs +++ b/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_strptime_without_zone.rs @@ -78,7 +78,7 @@ pub(crate) fn call_datetime_strptime_without_zone(checker: &mut Checker, call: & if let Some(Expr::StringLiteral(ast::ExprStringLiteral { value: format, .. })) = call.arguments.args.get(1).as_ref() { - if format.contains("%z") { + if format.as_str().contains("%z") { return; } }; diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs index 4b214fffec..db56255ff4 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs +++ b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs @@ -93,7 +93,7 @@ fn check_log_record_attr_clash(checker: &mut Checker, extra: &Keyword) { for key in keys { if let Some(key) = &key { if let Expr::StringLiteral(ast::ExprStringLiteral { value: attr, .. }) = key { - if is_reserved_attr(attr) { + if is_reserved_attr(attr.as_str()) { checker.diagnostics.push(Diagnostic::new( LoggingExtraAttrClash(attr.to_string()), key.range(), diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs index a37da79ade..fcf5707fe7 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs @@ -110,7 +110,7 @@ pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, expr: &Expr, kwargs /// Return `Some` if a key is a valid keyword argument name, or `None` otherwise. fn as_kwarg(key: &Expr) -> Option<&str> { if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = key { - if is_identifier(value) { + if is_identifier(value.as_str()) { return Some(value.as_str()); } } diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs index e9b5b7a895..cead79a028 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs @@ -300,8 +300,8 @@ fn check_names(checker: &mut Checker, decorator: &Decorator, expr: &Expr) { let names_type = checker.settings.flake8_pytest_style.parametrize_names_type; match expr { - Expr::StringLiteral(ast::ExprStringLiteral { value: string, .. }) => { - let names = split_names(string); + Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => { + let names = split_names(value.as_str()); if names.len() > 1 { match names_type { types::ParametrizeNameType::Tuple => { @@ -475,12 +475,11 @@ fn check_values(checker: &mut Checker, names: &Expr, values: &Expr) { .flake8_pytest_style .parametrize_values_row_type; - let is_multi_named = - if let Expr::StringLiteral(ast::ExprStringLiteral { value: string, .. }) = &names { - split_names(string).len() > 1 - } else { - true - }; + let is_multi_named = if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &names { + split_names(value.as_str()).len() > 1 + } else { + true + }; match values { Expr::List(ast::ExprList { elts, .. }) => { diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/ast_expr.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/ast_expr.rs index deee6d9b24..392b2507ce 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/ast_expr.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/ast_expr.rs @@ -161,11 +161,11 @@ pub(crate) fn use_capital_environment_variables(checker: &mut Checker, expr: &Ex return; } - if is_lowercase_allowed(env_var) { + if is_lowercase_allowed(env_var.as_str()) { return; } - let capital_env_var = env_var.to_ascii_uppercase(); + let capital_env_var = env_var.as_str().to_ascii_uppercase(); if capital_env_var == env_var.as_str() { return; } @@ -201,11 +201,11 @@ fn check_os_environ_subscript(checker: &mut Checker, expr: &Expr) { return; }; - if is_lowercase_allowed(env_var) { + if is_lowercase_allowed(env_var.as_str()) { return; } - let capital_env_var = env_var.to_ascii_uppercase(); + let capital_env_var = env_var.as_str().to_ascii_uppercase(); if capital_env_var == env_var.as_str() { return; } diff --git a/crates/ruff_linter/src/rules/pylint/helpers.rs b/crates/ruff_linter/src/rules/pylint/helpers.rs index 8c040f2ed2..562b5e2304 100644 --- a/crates/ruff_linter/src/rules/pylint/helpers.rs +++ b/crates/ruff_linter/src/rules/pylint/helpers.rs @@ -12,7 +12,7 @@ pub(super) fn type_param_name(arguments: &Arguments) -> Option<&str> { // Handle both `TypeVar("T")` and `TypeVar(name="T")`. let name_param = arguments.find_argument("name", 0)?; if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &name_param { - Some(value) + Some(value.as_str()) } else { None } diff --git a/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs b/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs index 0750af43b5..a5e7f5f325 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs @@ -154,7 +154,7 @@ impl TryFrom for OpenMode { } /// Returns `true` if the open mode is valid. -fn is_valid_mode(mode: &str) -> bool { +fn is_valid_mode(mode: &ast::StringLiteralValue) -> bool { // Flag duplicates and invalid characters. let mut flags = OpenMode::empty(); for char in mode.chars() { diff --git a/crates/ruff_linter/src/rules/pylint/rules/bad_str_strip_call.rs b/crates/ruff_linter/src/rules/pylint/rules/bad_str_strip_call.rs index 36fc831d59..dafa3b0c4f 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/bad_str_strip_call.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/bad_str_strip_call.rs @@ -122,7 +122,7 @@ impl fmt::Display for RemovalKind { /// Return `true` if a string contains duplicate characters, taking into account /// escapes. -fn has_duplicates(s: &str) -> bool { +fn has_duplicates(s: &ast::StringLiteralValue) -> bool { let mut escaped = false; let mut seen = FxHashSet::default(); for ch in s.chars() { diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_keyword_argument.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_keyword_argument.rs index 42e8960540..f3a45e5694 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/repeated_keyword_argument.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/repeated_keyword_argument.rs @@ -60,7 +60,7 @@ pub(crate) fn repeated_keyword_argument(checker: &mut Checker, call: &ExprCall) // Ex) `func(**{"a": 1, "a": 2})` for key in keys.iter().flatten() { if let Expr::StringLiteral(ExprStringLiteral { value, .. }) = key { - if !seen.insert(value) { + if !seen.insert(value.as_str()) { checker.diagnostics.push(Diagnostic::new( RepeatedKeywordArgument { duplicate_keyword: value.to_string(), diff --git a/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs b/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs index 49c5575199..0559919d8e 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs @@ -81,7 +81,12 @@ pub(crate) fn unspecified_encoding(checker: &mut Checker, call: &ast::ExprCall) /// Returns `true` if the given expression is a string literal containing a `b` character. fn is_binary_mode(expr: &ast::Expr) -> Option { - Some(expr.as_string_literal_expr()?.value.contains('b')) + Some( + expr.as_string_literal_expr()? + .value + .chars() + .any(|c| c == 'b'), + ) } /// Returns `true` if the given call lacks an explicit `encoding`. diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs index 161b63bfa2..70e50d0c22 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs @@ -185,13 +185,13 @@ fn create_fields_from_fields_arg(fields: &Expr) -> Option> { return None; } let ast::ExprStringLiteral { value: field, .. } = field.as_string_literal_expr()?; - if !is_identifier(field) { + if !is_identifier(field.as_str()) { return None; } - if is_dunder(field) { + if is_dunder(field.as_str()) { return None; } - Some(create_field_assignment_stmt(field, annotation)) + Some(create_field_assignment_stmt(field.as_str(), annotation)) }) .collect() } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs index dd5503ce22..a61ac82245 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs @@ -174,13 +174,13 @@ fn fields_from_dict_literal(keys: &[Option], values: &[Expr]) -> Option { - if !is_identifier(field) { + if !is_identifier(field.as_str()) { return None; } - if is_dunder(field) { + if is_dunder(field.as_str()) { return None; } - Some(create_field_assignment_stmt(field, value)) + Some(create_field_assignment_stmt(field.as_str(), value)) } _ => None, }) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/printf_string_formatting.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/printf_string_formatting.rs index 3c757820d9..ec9cae1295 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/printf_string_formatting.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/printf_string_formatting.rs @@ -228,14 +228,14 @@ fn clean_params_dictionary(right: &Expr, locator: &Locator, stylist: &Stylist) - }) = key { // If the dictionary key is not a valid variable name, abort. - if !is_identifier(key_string) { + if !is_identifier(key_string.as_str()) { return None; } // If there are multiple entries of the same key, abort. if seen.contains(&key_string.as_str()) { return None; } - seen.push(key_string); + seen.push(key_string.as_str()); if is_multi_line { if indent.is_none() { indent = indentation(locator, key); diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/redundant_open_modes.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/redundant_open_modes.rs index 549b340a76..7a87d03948 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/redundant_open_modes.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/redundant_open_modes.rs @@ -76,7 +76,7 @@ pub(crate) fn redundant_open_modes(checker: &mut Checker, call: &ast::ExprCall) .. }) = &keyword.value { - if let Ok(mode) = OpenMode::from_str(mode_param_value) { + if let Ok(mode) = OpenMode::from_str(mode_param_value.as_str()) { checker.diagnostics.push(create_check( call, &keyword.value, @@ -91,7 +91,7 @@ pub(crate) fn redundant_open_modes(checker: &mut Checker, call: &ast::ExprCall) } Some(mode_param) => { if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &mode_param { - if let Ok(mode) = OpenMode::from_str(value) { + if let Ok(mode) = OpenMode::from_str(value.as_str()) { checker.diagnostics.push(create_check( call, mode_param, diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_encode_utf8.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_encode_utf8.rs index 5966624d9e..1f9e5ff7eb 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_encode_utf8.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_encode_utf8.rs @@ -74,7 +74,7 @@ fn match_encoded_variable(func: &Expr) -> Option<&Expr> { fn is_utf8_encoding_arg(arg: &Expr) -> bool { if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &arg { - UTF8_LITERALS.contains(&value.to_lowercase().as_str()) + UTF8_LITERALS.contains(&value.as_str().to_lowercase().as_str()) } else { false } @@ -161,7 +161,7 @@ pub(crate) fn unnecessary_encode_utf8(checker: &mut Checker, call: &ast::ExprCal Expr::StringLiteral(ast::ExprStringLiteral { value: literal, .. }) => { // Ex) `"str".encode()`, `"str".encode("utf-8")` if let Some(encoding_arg) = match_encoding_arg(&call.arguments) { - if literal.is_ascii() { + if literal.as_str().is_ascii() { // Ex) Convert `"foo".encode()` to `b"foo"`. let mut diagnostic = Diagnostic::new( UnnecessaryEncodeUTF8 { diff --git a/crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs b/crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs index 065fa94ec1..36259cdc86 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs @@ -181,14 +181,10 @@ pub(crate) fn implicit_optional(checker: &mut Checker, parameters: &Parameters) continue; }; - if let Expr::StringLiteral(ast::ExprStringLiteral { - range, - value: string, - }) = annotation.as_ref() - { + if let Expr::StringLiteral(ast::ExprStringLiteral { range, value }) = annotation.as_ref() { // Quoted annotation. if let Ok((annotation, kind)) = - parse_type_annotation(string, *range, checker.locator().contents()) + parse_type_annotation(value.as_str(), *range, checker.locator().contents()) { let Some(expr) = type_hint_explicitly_allows_none( &annotation, diff --git a/crates/ruff_linter/src/rules/ruff/typing.rs b/crates/ruff_linter/src/rules/ruff/typing.rs index f02cc09ad5..de898ff285 100644 --- a/crates/ruff_linter/src/rules/ruff/typing.rs +++ b/crates/ruff_linter/src/rules/ruff/typing.rs @@ -108,11 +108,10 @@ impl<'a> TypingTarget<'a> { .. }) => Some(TypingTarget::PEP604Union(left, right)), Expr::NoneLiteral(_) => Some(TypingTarget::None), - Expr::StringLiteral(ast::ExprStringLiteral { - value: string, - range, - }) => parse_type_annotation(string, *range, locator.contents()) - .map_or(None, |(expr, _)| Some(TypingTarget::ForwardReference(expr))), + Expr::StringLiteral(ast::ExprStringLiteral { value, range }) => { + parse_type_annotation(value.as_str(), *range, locator.contents()) + .map_or(None, |(expr, _)| Some(TypingTarget::ForwardReference(expr))) + } _ => semantic.resolve_call_path(expr).map_or( // If we can't resolve the call path, it must be defined in the // same file, so we assume it's `Any` as it could be a type alias. diff --git a/crates/ruff_python_ast/src/all.rs b/crates/ruff_python_ast/src/all.rs index 23472dafb6..5ac523cf1b 100644 --- a/crates/ruff_python_ast/src/all.rs +++ b/crates/ruff_python_ast/src/all.rs @@ -24,7 +24,7 @@ where fn add_to_names<'a>(elts: &'a [Expr], names: &mut Vec<&'a str>, flags: &mut DunderAllFlags) { for elt in elts { if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = elt { - names.push(value); + names.push(value.as_str()); } else { *flags |= DunderAllFlags::INVALID_OBJECT; } diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index e0bc5b4730..23faaa9419 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -1213,7 +1213,26 @@ impl StringLiteralValue { } } + /// Returns `true` if the string literal value is empty. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Returns the total length of the string literal value, in bytes, not + /// [`char`]s or graphemes. + pub fn len(&self) -> usize { + self.parts().fold(0, |acc, part| acc + part.value.len()) + } + + /// Returns an iterator over the [`char`]s of each string literal part. + pub fn chars(&self) -> impl Iterator + '_ { + self.parts().flat_map(|part| part.value.chars()) + } + /// Returns the concatenated string value as a [`str`]. + /// + /// Note that this will perform an allocation on the first invocation if the + /// string value is implicitly concatenated. pub fn as_str(&self) -> &str { match &self.inner { StringLiteralValueInner::Single(value) => value.as_str(), @@ -1224,21 +1243,17 @@ impl StringLiteralValue { impl PartialEq for StringLiteralValue { fn eq(&self, other: &str) -> bool { - self.as_str() == other + if self.len() != other.len() { + return false; + } + // The `zip` here is safe because we have checked the length of both parts. + self.chars().zip(other.chars()).all(|(c1, c2)| c1 == c2) } } impl PartialEq for StringLiteralValue { fn eq(&self, other: &String) -> bool { - self.as_str() == other - } -} - -impl Deref for StringLiteralValue { - type Target = str; - - fn deref(&self) -> &Self::Target { - self.as_str() + self == other.as_str() } }