Skip to content

Commit b6b1947

Browse files
authored
Improve API exposed on ExprStringLiteral nodes (#16192)
## Summary This PR makes the following changes: - It adjusts various callsites to use the new `ast::StringLiteral::contents_range()` method that was introduced in #16183. This is less verbose and more type-safe than using the `ast::str::raw_contents()` helper function. - It adds a new `ast::ExprStringLiteral::as_unconcatenated_literal()` helper method, and adjusts various callsites to use it. This addresses @MichaReiser's review comment at #16183 (comment). There is no functional change here, but it helps readability to make it clearer that we're differentiating between implicitly concatenated strings and unconcatenated strings at various points. - It renames the `StringLiteralValue::flags()` method to `StringLiteralFlags::first_literal_flags()`. If you're dealing with an implicitly concatenated string `string_node`, `string_node.value.flags().closer_len()` could give an incorrect result; this renaming makes it clearer that the `StringLiteralFlags` instance returned by the method is only guaranteed to give accurate information for the first `StringLiteral` contained in the `ExprStringLiteral` node. - It deletes the unused `BytesLiteralValue::flags()` method. This seems prone to misuse in the same way as `StringLiteralValue::flags()`: if it's an implicitly concatenated bytestring, the `BytesLiteralFlags` instance returned by the method would only give accurate information for the first `BytesLiteral` in the bytestring. ## Test Plan `cargo test`
1 parent 21999b3 commit b6b1947

File tree

8 files changed

+37
-43
lines changed

8 files changed

+37
-43
lines changed

crates/red_knot_python_semantic/src/types/string_annotation.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use ruff_db::source::source_text;
2-
use ruff_python_ast::str::raw_contents;
32
use ruff_python_ast::{self as ast, ModExpression};
43
use ruff_python_parser::Parsed;
54
use ruff_text_size::Ranged;
@@ -138,9 +137,8 @@ pub(crate) fn parse_string_annotation(
138137
let _span = tracing::trace_span!("parse_string_annotation", string=?string_expr.range(), file=%file.path(db)).entered();
139138

140139
let source = source_text(db.upcast(), file);
141-
let node_text = &source[string_expr.range()];
142140

143-
if let [string_literal] = string_expr.value.as_slice() {
141+
if let Some(string_literal) = string_expr.as_unconcatenated_literal() {
144142
let prefix = string_literal.flags.prefix();
145143
if prefix.is_raw() {
146144
context.report_lint(
@@ -150,9 +148,7 @@ pub(crate) fn parse_string_annotation(
150148
);
151149
// Compare the raw contents (without quotes) of the expression with the parsed contents
152150
// contained in the string literal.
153-
} else if raw_contents(node_text)
154-
.is_some_and(|raw_contents| raw_contents == string_literal.as_str())
155-
{
151+
} else if &source[string_literal.content_range()] == string_literal.as_str() {
156152
match ruff_python_parser::parse_string_annotation(source.as_str(), string_literal) {
157153
Ok(parsed) => return Some(parsed),
158154
Err(parse_error) => context.report_lint(

crates/ruff_linter/src/checkers/ast/analyze/definitions.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,8 @@ pub(crate) fn definitions(checker: &mut Checker) {
182182
continue;
183183
};
184184

185-
// If the `ExprStringLiteral` has multiple parts, it is implicitly concatenated.
186-
// We don't support recognising such strings as docstrings in our model currently.
187-
let [sole_string_part] = string_literal.value.as_slice() else {
185+
// We don't recognise implicitly concatenated strings as valid docstrings in our model currently.
186+
let Some(sole_string_part) = string_literal.as_unconcatenated_literal() else {
188187
#[allow(deprecated)]
189188
let location = checker
190189
.locator

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1537,7 +1537,7 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
15371537
}
15381538
}
15391539
if checker.enabled(Rule::MissingFStringSyntax) {
1540-
for string_literal in value.as_slice() {
1540+
for string_literal in value {
15411541
ruff::rules::missing_fstring_syntax(checker, string_literal);
15421542
}
15431543
}

crates/ruff_linter/src/rules/flake8_simplify/rules/split_static_string.rs

+9-3
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,17 @@ fn split_default(str_value: &StringLiteralValue, max_split: i32) -> Option<Expr>
159159
}
160160
Ordering::Equal => {
161161
let list_items: Vec<&str> = vec![str_value.to_str()];
162-
Some(construct_replacement(&list_items, str_value.flags()))
162+
Some(construct_replacement(
163+
&list_items,
164+
str_value.first_literal_flags(),
165+
))
163166
}
164167
Ordering::Less => {
165168
let list_items: Vec<&str> = str_value.to_str().split_whitespace().collect();
166-
Some(construct_replacement(&list_items, str_value.flags()))
169+
Some(construct_replacement(
170+
&list_items,
171+
str_value.first_literal_flags(),
172+
))
167173
}
168174
}
169175
}
@@ -187,7 +193,7 @@ fn split_sep(
187193
}
188194
};
189195

190-
construct_replacement(&list_items, str_value.flags())
196+
construct_replacement(&list_items, str_value.first_literal_flags())
191197
}
192198

193199
/// Returns the value of the `maxsplit` argument as an `i32`, if it is a numeric value.

crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ fn build_fstring(joiner: &str, joinees: &[Expr], flags: FStringFlags) -> Option<
7272
if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr {
7373
if flags.is_none() {
7474
// take the flags from the first Expr
75-
flags = Some(value.flags());
75+
flags = Some(value.first_literal_flags());
7676
}
7777
Some(value.to_str())
7878
} else {

crates/ruff_python_ast/src/nodes.rs

+19-19
Original file line numberDiff line numberDiff line change
@@ -1287,6 +1287,17 @@ pub struct ExprStringLiteral {
12871287
pub value: StringLiteralValue,
12881288
}
12891289

1290+
impl ExprStringLiteral {
1291+
/// Return `Some(literal)` if the string only consists of a single `StringLiteral` part
1292+
/// (indicating that it is not implicitly concatenated). Otherwise, return `None`.
1293+
pub fn as_unconcatenated_literal(&self) -> Option<&StringLiteral> {
1294+
match &self.value.inner {
1295+
StringLiteralValueInner::Single(value) => Some(value),
1296+
StringLiteralValueInner::Concatenated(_) => None,
1297+
}
1298+
}
1299+
}
1300+
12901301
/// The value representing a [`ExprStringLiteral`].
12911302
#[derive(Clone, Debug, PartialEq)]
12921303
pub struct StringLiteralValue {
@@ -1304,7 +1315,7 @@ impl StringLiteralValue {
13041315
/// Returns the [`StringLiteralFlags`] associated with this string literal.
13051316
///
13061317
/// For an implicitly concatenated string, it returns the flags for the first literal.
1307-
pub fn flags(&self) -> StringLiteralFlags {
1318+
pub fn first_literal_flags(&self) -> StringLiteralFlags {
13081319
self.iter()
13091320
.next()
13101321
.expect(
@@ -1485,8 +1496,8 @@ bitflags! {
14851496
///
14861497
/// If you're using a `Generator` from the `ruff_python_codegen` crate to generate a lint-rule fix
14871498
/// from an existing string literal, consider passing along the [`StringLiteral::flags`] field or
1488-
/// the result of the [`StringLiteralValue::flags`] method. If you don't have an existing string but
1489-
/// have a `Checker` from the `ruff_linter` crate available, consider using
1499+
/// the result of the [`StringLiteralValue::first_literal_flags`] method. If you don't have an
1500+
/// existing string but have a `Checker` from the `ruff_linter` crate available, consider using
14901501
/// `Checker::default_string_flags` to create instances of this struct; this method will properly
14911502
/// handle surrounding f-strings. For usage that doesn't fit into one of these categories, the
14921503
/// public constructor [`StringLiteralFlags::empty`] can be used.
@@ -1791,16 +1802,6 @@ impl BytesLiteralValue {
17911802
pub fn bytes(&self) -> impl Iterator<Item = u8> + '_ {
17921803
self.iter().flat_map(|part| part.as_slice().iter().copied())
17931804
}
1794-
1795-
/// Returns the [`BytesLiteralFlags`] associated with this literal.
1796-
///
1797-
/// For an implicitly concatenated literal, it returns the flags for the first literal.
1798-
pub fn flags(&self) -> BytesLiteralFlags {
1799-
self.iter()
1800-
.next()
1801-
.expect("There should always be at least one literal in an `ExprBytesLiteral` node")
1802-
.flags
1803-
}
18041805
}
18051806

18061807
impl<'a> IntoIterator for &'a BytesLiteralValue {
@@ -1890,12 +1891,11 @@ bitflags! {
18901891
/// ## Notes on usage
18911892
///
18921893
/// If you're using a `Generator` from the `ruff_python_codegen` crate to generate a lint-rule fix
1893-
/// from an existing bytes literal, consider passing along the [`BytesLiteral::flags`] field or the
1894-
/// result of the [`BytesLiteralValue::flags`] method. If you don't have an existing literal but
1895-
/// have a `Checker` from the `ruff_linter` crate available, consider using
1896-
/// `Checker::default_bytes_flags` to create instances of this struct; this method will properly
1897-
/// handle surrounding f-strings. For usage that doesn't fit into one of these categories, the
1898-
/// public constructor [`BytesLiteralFlags::empty`] can be used.
1894+
/// from an existing bytes literal, consider passing along the [`BytesLiteral::flags`] field. If
1895+
/// you don't have an existing literal but have a `Checker` from the `ruff_linter` crate available,
1896+
/// consider using `Checker::default_bytes_flags` to create instances of this struct; this method
1897+
/// will properly handle surrounding f-strings. For usage that doesn't fit into one of these
1898+
/// categories, the public constructor [`BytesLiteralFlags::empty`] can be used.
18991899
#[derive(Copy, Clone, Eq, PartialEq, Hash)]
19001900
pub struct BytesLiteralFlags(BytesLiteralFlagsInner);
19011901

crates/ruff_python_formatter/src/expression/expr_string_literal.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ impl FormatRuleWithOptions<ExprStringLiteral, PyFormatContext<'_>> for FormatExp
2828

2929
impl FormatNodeRule<ExprStringLiteral> for FormatExprStringLiteral {
3030
fn fmt_fields(&self, item: &ExprStringLiteral, f: &mut PyFormatter) -> FormatResult<()> {
31-
let ExprStringLiteral { value, .. } = item;
32-
33-
if let [string_literal] = value.as_slice() {
31+
if let Some(string_literal) = item.as_unconcatenated_literal() {
3432
string_literal.format().with_options(self.kind).fmt(f)
3533
} else {
3634
// Always join strings that aren't parenthesized and thus, always on a single line.

crates/ruff_python_parser/src/typing.rs

+2-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! This module takes care of parsing a type annotation.
22
33
use ruff_python_ast::relocate::relocate_expr;
4-
use ruff_python_ast::str::raw_contents;
54
use ruff_python_ast::{Expr, ExprStringLiteral, ModExpression, StringLiteral};
65
use ruff_text_size::Ranged;
76

@@ -57,14 +56,10 @@ pub fn parse_type_annotation(
5756
string_expr: &ExprStringLiteral,
5857
source: &str,
5958
) -> AnnotationParseResult {
60-
let expr_text = &source[string_expr.range()];
61-
62-
if let [string_literal] = string_expr.value.as_slice() {
59+
if let Some(string_literal) = string_expr.as_unconcatenated_literal() {
6360
// Compare the raw contents (without quotes) of the expression with the parsed contents
6461
// contained in the string literal.
65-
if raw_contents(expr_text)
66-
.is_some_and(|raw_contents| raw_contents == string_literal.as_str())
67-
{
62+
if &source[string_literal.content_range()] == string_literal.as_str() {
6863
parse_simple_type_annotation(string_literal, source)
6964
} else {
7065
// The raw contents of the string doesn't match the parsed content. This could be the

0 commit comments

Comments
 (0)