Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use ruff_db::source::source_text;
use ruff_python_ast::str::raw_contents;
use ruff_python_ast::{self as ast, ModExpression};
use ruff_python_parser::Parsed;
use ruff_text_size::Ranged;
Expand Down Expand Up @@ -138,9 +137,8 @@ pub(crate) fn parse_string_annotation(
let _span = tracing::trace_span!("parse_string_annotation", string=?string_expr.range(), file=%file.path(db)).entered();

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

if let [string_literal] = string_expr.value.as_slice() {
if let Some(string_literal) = string_expr.as_unconcatenated_literal() {
let prefix = string_literal.flags.prefix();
if prefix.is_raw() {
context.report_lint(
Expand All @@ -150,9 +148,7 @@ pub(crate) fn parse_string_annotation(
);
// Compare the raw contents (without quotes) of the expression with the parsed contents
// contained in the string literal.
} else if raw_contents(node_text)
.is_some_and(|raw_contents| raw_contents == string_literal.as_str())
{
} else if &source[string_literal.content_range()] == string_literal.as_str() {
match ruff_python_parser::parse_string_annotation(source.as_str(), string_literal) {
Ok(parsed) => return Some(parsed),
Err(parse_error) => context.report_lint(
Expand Down
5 changes: 2 additions & 3 deletions crates/ruff_linter/src/checkers/ast/analyze/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,8 @@ pub(crate) fn definitions(checker: &mut Checker) {
continue;
};

// If the `ExprStringLiteral` has multiple parts, it is implicitly concatenated.
// We don't support recognising such strings as docstrings in our model currently.
let [sole_string_part] = string_literal.value.as_slice() else {
// We don't recognise implicitly concatenated strings as valid docstrings in our model currently.
let Some(sole_string_part) = string_literal.as_unconcatenated_literal() else {
#[allow(deprecated)]
let location = checker
.locator
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1537,7 +1537,7 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
}
}
if checker.enabled(Rule::MissingFStringSyntax) {
for string_literal in value.as_slice() {
for string_literal in value {
ruff::rules::missing_fstring_syntax(checker, string_literal);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,17 @@ fn split_default(str_value: &StringLiteralValue, max_split: i32) -> Option<Expr>
}
Ordering::Equal => {
let list_items: Vec<&str> = vec![str_value.to_str()];
Some(construct_replacement(&list_items, str_value.flags()))
Some(construct_replacement(
&list_items,
str_value.first_literal_flags(),
))
}
Ordering::Less => {
let list_items: Vec<&str> = str_value.to_str().split_whitespace().collect();
Some(construct_replacement(&list_items, str_value.flags()))
Some(construct_replacement(
&list_items,
str_value.first_literal_flags(),
))
}
}
}
Expand All @@ -187,7 +193,7 @@ fn split_sep(
}
};

construct_replacement(&list_items, str_value.flags())
construct_replacement(&list_items, str_value.first_literal_flags())
}

/// Returns the value of the `maxsplit` argument as an `i32`, if it is a numeric value.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ fn build_fstring(joiner: &str, joinees: &[Expr], flags: FStringFlags) -> Option<
if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr {
if flags.is_none() {
// take the flags from the first Expr
flags = Some(value.flags());
flags = Some(value.first_literal_flags());
}
Some(value.to_str())
} else {
Expand Down
38 changes: 19 additions & 19 deletions crates/ruff_python_ast/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1287,6 +1287,17 @@ pub struct ExprStringLiteral {
pub value: StringLiteralValue,
}

impl ExprStringLiteral {
/// Return `Some(literal)` if the string only consists of a single `StringLiteral` part
/// (indicating that it is not implicitly concatenated). Otherwise, return `None`.
pub fn as_unconcatenated_literal(&self) -> Option<&StringLiteral> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I wonder if it should be as_only_literal to align with first_literal but I'm fine with either.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to use as_single_literal / as_only_literal instead as reading unconcatenated and matching against Single confused me at the first glance

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I weakly prefer the current name as I think it makes it clearer that the method is used to differentiate between strings that are implicitly concatenated and ones that aren't

Copy link
Copy Markdown
Member Author

@AlexWaygood AlexWaygood Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh shoot, I didn't see Dhruv's message until after I merged — sorry. I'll rename it as a followup, since you both dislike the current name!

match &self.value.inner {
StringLiteralValueInner::Single(value) => Some(value),
StringLiteralValueInner::Concatenated(_) => None,
}
}
}

/// The value representing a [`ExprStringLiteral`].
#[derive(Clone, Debug, PartialEq)]
pub struct StringLiteralValue {
Expand All @@ -1304,7 +1315,7 @@ impl StringLiteralValue {
/// Returns the [`StringLiteralFlags`] associated with this string literal.
///
/// For an implicitly concatenated string, it returns the flags for the first literal.
pub fn flags(&self) -> StringLiteralFlags {
pub fn first_literal_flags(&self) -> StringLiteralFlags {
self.iter()
.next()
.expect(
Expand Down Expand Up @@ -1485,8 +1496,8 @@ bitflags! {
///
/// If you're using a `Generator` from the `ruff_python_codegen` crate to generate a lint-rule fix
/// from an existing string literal, consider passing along the [`StringLiteral::flags`] field or
/// the result of the [`StringLiteralValue::flags`] method. If you don't have an existing string but
/// have a `Checker` from the `ruff_linter` crate available, consider using
/// the result of the [`StringLiteralValue::first_literal_flags`] method. If you don't have an
/// existing string but have a `Checker` from the `ruff_linter` crate available, consider using
/// `Checker::default_string_flags` to create instances of this struct; this method will properly
/// handle surrounding f-strings. For usage that doesn't fit into one of these categories, the
/// public constructor [`StringLiteralFlags::empty`] can be used.
Expand Down Expand Up @@ -1791,16 +1802,6 @@ impl BytesLiteralValue {
pub fn bytes(&self) -> impl Iterator<Item = u8> + '_ {
self.iter().flat_map(|part| part.as_slice().iter().copied())
}

/// Returns the [`BytesLiteralFlags`] associated with this literal.
///
/// For an implicitly concatenated literal, it returns the flags for the first literal.
pub fn flags(&self) -> BytesLiteralFlags {
self.iter()
.next()
.expect("There should always be at least one literal in an `ExprBytesLiteral` node")
.flags
}
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ impl FormatRuleWithOptions<ExprStringLiteral, PyFormatContext<'_>> for FormatExp

impl FormatNodeRule<ExprStringLiteral> for FormatExprStringLiteral {
fn fmt_fields(&self, item: &ExprStringLiteral, f: &mut PyFormatter) -> FormatResult<()> {
let ExprStringLiteral { value, .. } = item;

if let [string_literal] = value.as_slice() {
if let Some(string_literal) = item.as_unconcatenated_literal() {
string_literal.format().with_options(self.kind).fmt(f)
} else {
// Always join strings that aren't parenthesized and thus, always on a single line.
Expand Down
9 changes: 2 additions & 7 deletions crates/ruff_python_parser/src/typing.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! This module takes care of parsing a type annotation.

use ruff_python_ast::relocate::relocate_expr;
use ruff_python_ast::str::raw_contents;
use ruff_python_ast::{Expr, ExprStringLiteral, ModExpression, StringLiteral};
use ruff_text_size::Ranged;

Expand Down Expand Up @@ -57,14 +56,10 @@ pub fn parse_type_annotation(
string_expr: &ExprStringLiteral,
source: &str,
) -> AnnotationParseResult {
let expr_text = &source[string_expr.range()];

if let [string_literal] = string_expr.value.as_slice() {
if let Some(string_literal) = string_expr.as_unconcatenated_literal() {
// Compare the raw contents (without quotes) of the expression with the parsed contents
// contained in the string literal.
if raw_contents(expr_text)
.is_some_and(|raw_contents| raw_contents == string_literal.as_str())
{
if &source[string_literal.content_range()] == string_literal.as_str() {
parse_simple_type_annotation(string_literal, source)
} else {
// The raw contents of the string doesn't match the parsed content. This could be the
Expand Down