Skip to content

Commit

Permalink
Explicit as_str (no deref), add no allocation methods
Browse files Browse the repository at this point in the history
  • Loading branch information
dhruvmanila committed Nov 23, 2023
1 parent 1e72af2 commit 8b261bc
Show file tree
Hide file tree
Showing 28 changed files with 95 additions and 77 deletions.
18 changes: 13 additions & 5 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -1219,7 +1219,7 @@ where
{
self.deferred.string_type_definitions.push((
expr.range(),
value,
value.as_str(),
self.semantic.snapshot(),
));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,14 +505,10 @@ fn check_dynamically_typed<F>(
) 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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, .. }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pylint/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl TryFrom<char> 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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
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`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,13 +185,13 @@ fn create_fields_from_fields_arg(fields: &Expr) -> Option<Vec<Stmt>> {
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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,13 @@ fn fields_from_dict_literal(keys: &[Option<Expr>], values: &[Expr]) -> Option<Ve
.zip(values.iter())
.map(|(key, value)| match key {
Some(Expr::StringLiteral(ast::ExprStringLiteral { value: field, .. })) => {
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,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 2 additions & 6 deletions crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 8b261bc

Please sign in to comment.