Introduce StringLike enum (#9016)

## Summary

This PR introduces a new `StringLike` enum which is a narrow type to
indicate string-like nodes. These includes the string literals, bytes
literals, and the literal parts of f-strings.

The main motivation behind this is to avoid repetition of rule calling
in the AST checker. We add a new `analyze::string_like` function which
takes in the enum and calls all the respective rule functions which
expects atleast 2 of the variants of this enum.

I'm open to discarding this if others think it's not that useful at this
stage as currently only 3 rules require these nodes.

As suggested
[here](https://github.com/astral-sh/ruff/pull/8835#discussion_r1414746934)
and
[here](https://github.com/astral-sh/ruff/pull/8835#discussion_r1414750204).

## Test Plan

`cargo test`
This commit is contained in:
Dhruv Manilawala 2023-12-07 10:39:13 -06:00 committed by GitHub
parent cdac90ef68
commit 96ae9fe685
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 108 additions and 72 deletions

View file

@ -4,7 +4,6 @@ use ruff_python_literal::cformat::{CFormatError, CFormatErrorType};
use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Diagnostic;
use ruff_python_ast::types::Node; use ruff_python_ast::types::Node;
use ruff_python_ast::AstNode;
use ruff_python_semantic::analyze::typing; use ruff_python_semantic::analyze::typing;
use ruff_python_semantic::ScopeKind; use ruff_python_semantic::ScopeKind;
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
@ -1007,30 +1006,6 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
pyupgrade::rules::unicode_kind_prefix(checker, string_literal); pyupgrade::rules::unicode_kind_prefix(checker, string_literal);
} }
} }
for literal in value.elements().filter_map(|element| element.as_literal()) {
if checker.enabled(Rule::HardcodedBindAllInterfaces) {
flake8_bandit::rules::hardcoded_bind_all_interfaces(
checker,
&literal.value,
literal.range,
);
}
if checker.enabled(Rule::HardcodedTempFile) {
flake8_bandit::rules::hardcoded_tmp_directory(
checker,
&literal.value,
literal.range,
);
}
if checker.source_type.is_stub() {
if checker.enabled(Rule::StringOrBytesTooLong) {
flake8_pyi::rules::string_or_bytes_too_long(
checker,
literal.as_any_node_ref(),
);
}
}
}
} }
Expr::BinOp(ast::ExprBinOp { Expr::BinOp(ast::ExprBinOp {
left, left,
@ -1295,38 +1270,12 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
refurb::rules::math_constant(checker, number_literal); refurb::rules::math_constant(checker, number_literal);
} }
} }
Expr::BytesLiteral(bytes_literal) => { Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => {
if checker.source_type.is_stub() && checker.enabled(Rule::StringOrBytesTooLong) {
flake8_pyi::rules::string_or_bytes_too_long(
checker,
bytes_literal.as_any_node_ref(),
);
}
}
Expr::StringLiteral(string_literal @ ast::ExprStringLiteral { value, range }) => {
if checker.enabled(Rule::HardcodedBindAllInterfaces) {
flake8_bandit::rules::hardcoded_bind_all_interfaces(
checker,
value.to_str(),
*range,
);
}
if checker.enabled(Rule::HardcodedTempFile) {
flake8_bandit::rules::hardcoded_tmp_directory(checker, value.to_str(), *range);
}
if checker.enabled(Rule::UnicodeKindPrefix) { if checker.enabled(Rule::UnicodeKindPrefix) {
for string_part in value.parts() { for string_part in value.parts() {
pyupgrade::rules::unicode_kind_prefix(checker, string_part); pyupgrade::rules::unicode_kind_prefix(checker, string_part);
} }
} }
if checker.source_type.is_stub() {
if checker.enabled(Rule::StringOrBytesTooLong) {
flake8_pyi::rules::string_or_bytes_too_long(
checker,
string_literal.as_any_node_ref(),
);
}
}
} }
Expr::IfExp( Expr::IfExp(
if_exp @ ast::ExprIfExp { if_exp @ ast::ExprIfExp {

View file

@ -10,6 +10,7 @@ pub(super) use module::module;
pub(super) use parameter::parameter; pub(super) use parameter::parameter;
pub(super) use parameters::parameters; pub(super) use parameters::parameters;
pub(super) use statement::statement; pub(super) use statement::statement;
pub(super) use string_like::string_like;
pub(super) use suite::suite; pub(super) use suite::suite;
pub(super) use unresolved_references::unresolved_references; pub(super) use unresolved_references::unresolved_references;
@ -25,5 +26,6 @@ mod module;
mod parameter; mod parameter;
mod parameters; mod parameters;
mod statement; mod statement;
mod string_like;
mod suite; mod suite;
mod unresolved_references; mod unresolved_references;

View file

@ -0,0 +1,20 @@
use ruff_python_ast::StringLike;
use crate::checkers::ast::Checker;
use crate::codes::Rule;
use crate::rules::{flake8_bandit, flake8_pyi};
/// Run lint rules over a [`StringLike`] syntax nodes.
pub(crate) fn string_like(string_like: StringLike, checker: &mut Checker) {
if checker.enabled(Rule::HardcodedBindAllInterfaces) {
flake8_bandit::rules::hardcoded_bind_all_interfaces(checker, string_like);
}
if checker.enabled(Rule::HardcodedTempFile) {
flake8_bandit::rules::hardcoded_tmp_directory(checker, string_like);
}
if checker.source_type.is_stub() {
if checker.enabled(Rule::StringOrBytesTooLong) {
flake8_pyi::rules::string_or_bytes_too_long(checker, string_like);
}
}
}

View file

@ -44,7 +44,7 @@ use ruff_python_ast::helpers::{
}; };
use ruff_python_ast::identifier::Identifier; use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::str::trailing_quote; use ruff_python_ast::str::trailing_quote;
use ruff_python_ast::visitor::{walk_except_handler, walk_pattern, Visitor}; use ruff_python_ast::visitor::{walk_except_handler, walk_f_string_element, walk_pattern, Visitor};
use ruff_python_ast::{helpers, str, visitor, PySourceType}; use ruff_python_ast::{helpers, str, visitor, PySourceType};
use ruff_python_codegen::{Generator, Quote, Stylist}; use ruff_python_codegen::{Generator, Quote, Stylist};
use ruff_python_index::Indexer; use ruff_python_index::Indexer;
@ -1267,6 +1267,13 @@ where
// Step 4: Analysis // Step 4: Analysis
analyze::expression(expr, self); analyze::expression(expr, self);
match expr {
Expr::StringLiteral(string_literal) => {
analyze::string_like(string_literal.into(), self);
}
Expr::BytesLiteral(bytes_literal) => analyze::string_like(bytes_literal.into(), self),
_ => {}
}
self.semantic.flags = flags_snapshot; self.semantic.flags = flags_snapshot;
self.semantic.pop_node(); self.semantic.pop_node();
@ -1431,6 +1438,16 @@ where
.push((bound, self.semantic.snapshot())); .push((bound, self.semantic.snapshot()));
} }
} }
fn visit_f_string_element(&mut self, f_string_element: &'b ast::FStringElement) {
// Step 2: Traversal
walk_f_string_element(self, f_string_element);
// Step 4: Analysis
if let Some(literal) = f_string_element.as_literal() {
analyze::string_like(literal.into(), self);
}
}
} }
impl<'a> Checker<'a> { impl<'a> Checker<'a> {

View file

@ -1,6 +1,7 @@
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_text_size::TextRange; use ruff_python_ast::{self as ast, StringLike};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -36,10 +37,16 @@ impl Violation for HardcodedBindAllInterfaces {
} }
/// S104 /// S104
pub(crate) fn hardcoded_bind_all_interfaces(checker: &mut Checker, value: &str, range: TextRange) { pub(crate) fn hardcoded_bind_all_interfaces(checker: &mut Checker, string: StringLike) {
if value == "0.0.0.0" { let is_bind_all_interface = match string {
StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => value == "0.0.0.0",
StringLike::FStringLiteral(ast::FStringLiteralElement { value, .. }) => value == "0.0.0.0",
StringLike::BytesLiteral(_) => return,
};
if is_bind_all_interface {
checker checker
.diagnostics .diagnostics
.push(Diagnostic::new(HardcodedBindAllInterfaces, range)); .push(Diagnostic::new(HardcodedBindAllInterfaces, string.range()));
} }
} }

View file

@ -1,5 +1,5 @@
use ruff_python_ast::{self as ast, Expr}; use ruff_python_ast::{self as ast, Expr, StringLike};
use ruff_text_size::TextRange; use ruff_text_size::Ranged;
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
@ -52,7 +52,13 @@ impl Violation for HardcodedTempFile {
} }
/// S108 /// S108
pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, value: &str, range: TextRange) { pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, string: StringLike) {
let value = match string {
StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.to_str(),
StringLike::FStringLiteral(ast::FStringLiteralElement { value, .. }) => value,
StringLike::BytesLiteral(_) => return,
};
if !checker if !checker
.settings .settings
.flake8_bandit .flake8_bandit
@ -79,6 +85,6 @@ pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, value: &str, range:
HardcodedTempFile { HardcodedTempFile {
string: value.to_string(), string: value.to_string(),
}, },
range, string.range(),
)); ));
} }

View file

@ -1,7 +1,7 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_docstring_stmt; use ruff_python_ast::helpers::is_docstring_stmt;
use ruff_python_ast::{self as ast, AnyNodeRef}; use ruff_python_ast::{self as ast, StringLike};
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -43,30 +43,27 @@ impl AlwaysFixableViolation for StringOrBytesTooLong {
} }
/// PYI053 /// PYI053
pub(crate) fn string_or_bytes_too_long(checker: &mut Checker, node: AnyNodeRef) { pub(crate) fn string_or_bytes_too_long(checker: &mut Checker, string: StringLike) {
// Ignore docstrings. // Ignore docstrings.
if is_docstring_stmt(checker.semantic().current_statement()) { if is_docstring_stmt(checker.semantic().current_statement()) {
return; return;
} }
let length = match node { let length = match string {
AnyNodeRef::ExprStringLiteral(ast::ExprStringLiteral { value, .. }) => { StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.chars().count(),
StringLike::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => value.len(),
StringLike::FStringLiteral(ast::FStringLiteralElement { value, .. }) => {
value.chars().count() value.chars().count()
} }
AnyNodeRef::ExprBytesLiteral(ast::ExprBytesLiteral { value, .. }) => value.len(),
AnyNodeRef::FStringLiteralElement(ast::FStringLiteralElement { value, .. }) => {
value.chars().count()
}
_ => return,
}; };
if length <= 50 { if length <= 50 {
return; return;
} }
let mut diagnostic = Diagnostic::new(StringOrBytesTooLong, node.range()); let mut diagnostic = Diagnostic::new(StringOrBytesTooLong, string.range());
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
"...".to_string(), "...".to_string(),
node.range(), string.range(),
))); )));
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
} }

View file

@ -393,3 +393,41 @@ impl LiteralExpressionRef<'_> {
} }
} }
} }
/// An enum that holds a reference to a string-like literal from the AST.
/// This includes string literals, bytes literals, and the literal parts of
/// f-strings.
#[derive(Copy, Clone, Debug, PartialEq)]
pub enum StringLike<'a> {
StringLiteral(&'a ast::ExprStringLiteral),
BytesLiteral(&'a ast::ExprBytesLiteral),
FStringLiteral(&'a ast::FStringLiteralElement),
}
impl<'a> From<&'a ast::ExprStringLiteral> for StringLike<'a> {
fn from(value: &'a ast::ExprStringLiteral) -> Self {
StringLike::StringLiteral(value)
}
}
impl<'a> From<&'a ast::ExprBytesLiteral> for StringLike<'a> {
fn from(value: &'a ast::ExprBytesLiteral) -> Self {
StringLike::BytesLiteral(value)
}
}
impl<'a> From<&'a ast::FStringLiteralElement> for StringLike<'a> {
fn from(value: &'a ast::FStringLiteralElement) -> Self {
StringLike::FStringLiteral(value)
}
}
impl Ranged for StringLike<'_> {
fn range(&self) -> TextRange {
match self {
StringLike::StringLiteral(literal) => literal.range(),
StringLike::BytesLiteral(literal) => literal.range(),
StringLike::FStringLiteral(literal) => literal.range(),
}
}
}