Disallow implicit concatenation of t-strings and other string types (#19485)

As of [this cpython PR](https://github.com/python/cpython/pull/135996),
it is not allowed to concatenate t-strings with non-t-strings,
implicitly or explicitly. Expressions such as `"foo" t"{bar}"` are now
syntax errors.

This PR updates some AST nodes and parsing to reflect this change.

The structural change is that `TStringPart` is no longer needed, since,
as in the case of `BytesStringLiteral`, the only possibilities are that
we have a single `TString` or a vector of such (representing an implicit
concatenation of t-strings). This removes a level of nesting from many
AST expressions (which is what all the snapshot changes reflect), and
simplifies some logic in the implementation of visitors, for example.

The other change of note is in the parser. When we meet an implicit
concatenation of string-like literals, we now count the number of
t-string literals. If these do not exhaust the total number of
implicitly concatenated pieces, then we emit a syntax error. To recover
from this syntax error, we encode any t-string pieces as _invalid_
string literals (which means we flag them as invalid, record their
range, and record the value as `""`). Note that if at least one of the
pieces is an f-string we prefer to parse the entire string as an
f-string; otherwise we parse it as a string.

This logic is exactly the same as how we currently treat
`BytesStringLiteral` parsing and error recovery - and carries with it
the same pros and cons.

Finally, note that I have not implemented any changes in the
implementation of the formatter. As far as I can tell, none are needed.
I did change a few of the fixtures so that we are always concatenating
t-strings with t-strings.

crates/ruff_python_ast/src/comparable.rs

@@ -708,23 +708,10 @@ pub struct ComparableTString<'a> {
 }
 
 impl<'a> From<&'a ast::TStringValue> for ComparableTString<'a> {
-    // The approach taken below necessarily deviates from the
-    // corresponding implementation for [`ast::FStringValue`].
-    // The reason is that a t-string value is composed of _three_
-    // non-comparable parts: literals, f-string expressions, and
-    // t-string interpolations. Since we have merged the AST nodes
-    // that capture f-string expressions and t-string interpolations
-    // into the shared [`ast::InterpolatedElement`], we must
-    // be careful to distinguish between them here.
+    // We model a [`ComparableTString`] on the actual
+    // [CPython implementation] of a `string.templatelib.Template` object.
     //
-    // Consequently, we model a [`ComparableTString`] on the actual
-    // [CPython implementation] of a `string.templatelib.Template` object:
-    // it is composed of `strings` and `interpolations`. In CPython,
-    // the `strings` field is a tuple of honest strings (since f-strings
-    // are evaluated). Our `strings` field will house both f-string
-    // expressions and string literals.
-    //
-    // Finally, as in CPython, we must be careful to ensure that the length
+    // As in CPython, we must be careful to ensure that the length
     // of `strings` is always one more than the length of `interpolations` -
     // that way we can recover the original reading order by interleaving
     // starting with `strings`. This is how we can tell the
@@ -768,19 +755,6 @@ impl<'a> From<&'a ast::TStringValue> for ComparableTString<'a> {
                     .push(ComparableInterpolatedStringElement::Literal("".into()));
             }
 
-            fn push_fstring_expression(&mut self, expression: &'a ast::InterpolatedElement) {
-                if let Some(ComparableInterpolatedStringElement::Literal(last_literal)) =
-                    self.strings.last()
-                {
-                    // Recall that we insert empty strings after
-                    // each interpolation. If we encounter an f-string
-                    // expression, we replace the empty string with it.
-                    if last_literal.is_empty() {
-                        self.strings.pop();
-                    }
-                }
-                self.strings.push(expression.into());
-            }
             fn push_tstring_interpolation(&mut self, expression: &'a ast::InterpolatedElement) {
                 self.interpolations.push(expression.into());
                 self.start_new_literal();
@@ -789,34 +763,13 @@ impl<'a> From<&'a ast::TStringValue> for ComparableTString<'a> {
 
         let mut collector = Collector::default();
 
-        for part in value {
-            match part {
-                ast::TStringPart::Literal(string_literal) => {
-                    collector.push_literal(&string_literal.value);
+        for element in value.elements() {
+            match element {
+                ast::InterpolatedStringElement::Literal(literal) => {
+                    collector.push_literal(&literal.value);
                 }
-                ast::TStringPart::TString(fstring) => {
-                    for element in &fstring.elements {
-                        match element {
-                            ast::InterpolatedStringElement::Literal(literal) => {
-                                collector.push_literal(&literal.value);
-                            }
-                            ast::InterpolatedStringElement::Interpolation(interpolation) => {
-                                collector.push_tstring_interpolation(interpolation);
-                            }
-                        }
-                    }
-                }
-                ast::TStringPart::FString(fstring) => {
-                    for element in &fstring.elements {
-                        match element {
-                            ast::InterpolatedStringElement::Literal(literal) => {
-                                collector.push_literal(&literal.value);
-                            }
-                            ast::InterpolatedStringElement::Interpolation(expression) => {
-                                collector.push_fstring_expression(expression);
-                            }
-                        }
-                    }
+                ast::InterpolatedStringElement::Interpolation(interpolation) => {
+                    collector.push_tstring_interpolation(interpolation);
                 }
             }
         }

crates/ruff_python_ast/src/expression.rs

@@ -320,7 +320,7 @@ pub enum StringLikePartIter<'a> {
     String(std::slice::Iter<'a, ast::StringLiteral>),
     Bytes(std::slice::Iter<'a, ast::BytesLiteral>),
     FString(std::slice::Iter<'a, ast::FStringPart>),
-    TString(std::slice::Iter<'a, ast::TStringPart>),
+    TString(std::slice::Iter<'a, ast::TString>),
 }
 
 impl<'a> Iterator for StringLikePartIter<'a> {
@@ -339,16 +339,7 @@ impl<'a> Iterator for StringLikePartIter<'a> {
                     ast::FStringPart::FString(f_string) => StringLikePart::FString(f_string),
                 }
             }
-            StringLikePartIter::TString(inner) => {
-                let part = inner.next()?;
-                match part {
-                    ast::TStringPart::Literal(string_literal) => {
-                        StringLikePart::String(string_literal)
-                    }
-                    ast::TStringPart::TString(t_string) => StringLikePart::TString(t_string),
-                    ast::TStringPart::FString(f_string) => StringLikePart::FString(f_string),
-                }
-            }
+            StringLikePartIter::TString(inner) => StringLikePart::TString(inner.next()?),
         };
 
         Some(part)
@@ -378,16 +369,7 @@ impl DoubleEndedIterator for StringLikePartIter<'_> {
                     ast::FStringPart::FString(f_string) => StringLikePart::FString(f_string),
                 }
             }
-            StringLikePartIter::TString(inner) => {
-                let part = inner.next_back()?;
-                match part {
-                    ast::TStringPart::Literal(string_literal) => {
-                        StringLikePart::String(string_literal)
-                    }
-                    ast::TStringPart::TString(t_string) => StringLikePart::TString(t_string),
-                    ast::TStringPart::FString(f_string) => StringLikePart::FString(f_string),
-                }
-            }
+            StringLikePartIter::TString(inner) => StringLikePart::TString(inner.next_back()?),
         };
 
         Some(part)

crates/ruff_python_ast/src/helpers.rs

@@ -1274,6 +1274,7 @@ impl Truthiness {
                     Self::Unknown
                 }
             }
+            Expr::TString(_) => Self::Truthy,
             Expr::List(ast::ExprList { elts, .. })
             | Expr::Set(ast::ExprSet { elts, .. })
             | Expr::Tuple(ast::ExprTuple { elts, .. }) => {
@@ -1362,6 +1363,7 @@ fn is_non_empty_f_string(expr: &ast::ExprFString) -> bool {
             Expr::EllipsisLiteral(_) => true,
             Expr::List(_) => true,
             Expr::Tuple(_) => true,
+            Expr::TString(_) => true,
 
             // These expressions must resolve to the inner expression.
             Expr::If(ast::ExprIf { body, orelse, .. }) => inner(body) && inner(orelse),
@@ -1386,7 +1388,6 @@ fn is_non_empty_f_string(expr: &ast::ExprFString) -> bool {
             // These literals may or may not be empty.
             Expr::FString(f_string) => is_non_empty_f_string(f_string),
             // These literals may or may not be empty.
-            Expr::TString(f_string) => is_non_empty_t_string(f_string),
             Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => !value.is_empty(),
             Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => !value.is_empty(),
         }
@@ -1403,76 +1404,6 @@ fn is_non_empty_f_string(expr: &ast::ExprFString) -> bool {
     })
 }
 
-/// Returns `true` if the expression definitely resolves to a non-empty string, when used as an
-/// f-string expression, or `false` if the expression may resolve to an empty string.
-fn is_non_empty_t_string(expr: &ast::ExprTString) -> bool {
-    fn inner(expr: &Expr) -> bool {
-        match expr {
-            // When stringified, these expressions are always non-empty.
-            Expr::Lambda(_) => true,
-            Expr::Dict(_) => true,
-            Expr::Set(_) => true,
-            Expr::ListComp(_) => true,
-            Expr::SetComp(_) => true,
-            Expr::DictComp(_) => true,
-            Expr::Compare(_) => true,
-            Expr::NumberLiteral(_) => true,
-            Expr::BooleanLiteral(_) => true,
-            Expr::NoneLiteral(_) => true,
-            Expr::EllipsisLiteral(_) => true,
-            Expr::List(_) => true,
-            Expr::Tuple(_) => true,
-
-            // These expressions must resolve to the inner expression.
-            Expr::If(ast::ExprIf { body, orelse, .. }) => inner(body) && inner(orelse),
-            Expr::Named(ast::ExprNamed { value, .. }) => inner(value),
-
-            // These expressions are complex. We can't determine whether they're empty or not.
-            Expr::BoolOp(ast::ExprBoolOp { .. }) => false,
-            Expr::BinOp(ast::ExprBinOp { .. }) => false,
-            Expr::UnaryOp(ast::ExprUnaryOp { .. }) => false,
-            Expr::Generator(_) => false,
-            Expr::Await(_) => false,
-            Expr::Yield(_) => false,
-            Expr::YieldFrom(_) => false,
-            Expr::Call(_) => false,
-            Expr::Attribute(_) => false,
-            Expr::Subscript(_) => false,
-            Expr::Starred(_) => false,
-            Expr::Name(_) => false,
-            Expr::Slice(_) => false,
-            Expr::IpyEscapeCommand(_) => false,
-
-            // These literals may or may not be empty.
-            Expr::FString(f_string) => is_non_empty_f_string(f_string),
-            // These literals may or may not be empty.
-            Expr::TString(t_string) => is_non_empty_t_string(t_string),
-            Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => !value.is_empty(),
-            Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => !value.is_empty(),
-        }
-    }
-
-    expr.value.iter().any(|part| match part {
-        ast::TStringPart::Literal(string_literal) => !string_literal.is_empty(),
-        ast::TStringPart::TString(t_string) => {
-            t_string.elements.iter().all(|element| match element {
-                ast::InterpolatedStringElement::Literal(string_literal) => {
-                    !string_literal.is_empty()
-                }
-                ast::InterpolatedStringElement::Interpolation(t_string) => {
-                    inner(&t_string.expression)
-                }
-            })
-        }
-        ast::TStringPart::FString(f_string) => {
-            f_string.elements.iter().all(|element| match element {
-                InterpolatedStringElement::Literal(string_literal) => !string_literal.is_empty(),
-                InterpolatedStringElement::Interpolation(f_string) => inner(&f_string.expression),
-            })
-        }
-    })
-}
-
 /// Returns `true` if the expression definitely resolves to the empty string, when used as an f-string
 /// expression.
 fn is_empty_f_string(expr: &ast::ExprFString) -> bool {

crates/ruff_python_ast/src/node.rs

@@ -171,18 +171,8 @@ impl ast::ExprTString {
             node_index: _,
         } = self;
 
-        for t_string_part in value {
-            match t_string_part {
-                ast::TStringPart::Literal(string_literal) => {
-                    visitor.visit_string_literal(string_literal);
-                }
-                ast::TStringPart::FString(f_string) => {
-                    visitor.visit_f_string(f_string);
-                }
-                ast::TStringPart::TString(t_string) => {
-                    visitor.visit_t_string(t_string);
-                }
-            }
+        for t_string in value {
+            visitor.visit_t_string(t_string);
         }
     }
 }

crates/ruff_python_ast/src/nodes.rs

@@ -597,8 +597,8 @@ impl ExprTString {
     /// otherwise.
     pub const fn as_single_part_tstring(&self) -> Option<&TString> {
         match &self.value.inner {
-            TStringValueInner::Single(TStringPart::TString(tstring)) => Some(tstring),
-            _ => None,
+            TStringValueInner::Single(tstring) => Some(tstring),
+            TStringValueInner::Concatenated(_) => None,
         }
     }
 }
@@ -614,7 +614,7 @@ impl TStringValue {
     /// Creates a new t-string literal with a single [`TString`] part.
     pub fn single(value: TString) -> Self {
         Self {
-            inner: TStringValueInner::Single(TStringPart::TString(value)),
+            inner: TStringValueInner::Single(value),
         }
     }
 
@@ -625,7 +625,7 @@ impl TStringValue {
     ///
     /// Panics if `values` has less than 2 elements.
     /// Use [`TStringValue::single`] instead.
-    pub fn concatenated(values: Vec<TStringPart>) -> Self {
+    pub fn concatenated(values: Vec<TString>) -> Self {
         assert!(
             values.len() > 1,
             "Use `TStringValue::single` to create single-part t-strings"
@@ -640,78 +640,52 @@ impl TStringValue {
         matches!(self.inner, TStringValueInner::Concatenated(_))
     }
 
-    /// Returns a slice of all the [`TStringPart`]s contained in this value.
-    pub fn as_slice(&self) -> &[TStringPart] {
+    /// Returns a slice of all the [`TString`]s contained in this value.
+    pub fn as_slice(&self) -> &[TString] {
         match &self.inner {
             TStringValueInner::Single(part) => std::slice::from_ref(part),
             TStringValueInner::Concatenated(parts) => parts,
         }
     }
 
-    /// Returns a mutable slice of all the [`TStringPart`]s contained in this value.
-    fn as_mut_slice(&mut self) -> &mut [TStringPart] {
+    /// Returns a mutable slice of all the [`TString`]s contained in this value.
+    fn as_mut_slice(&mut self) -> &mut [TString] {
         match &mut self.inner {
             TStringValueInner::Single(part) => std::slice::from_mut(part),
             TStringValueInner::Concatenated(parts) => parts,
         }
     }
 
-    /// Returns an iterator over all the [`TStringPart`]s contained in this value.
-    pub fn iter(&self) -> Iter<TStringPart> {
+    /// Returns an iterator over all the [`TString`]s contained in this value.
+    pub fn iter(&self) -> Iter<TString> {
         self.as_slice().iter()
     }
 
-    /// Returns an iterator over all the [`TStringPart`]s contained in this value
+    /// Returns an iterator over all the [`TString`]s contained in this value
     /// that allows modification.
-    pub fn iter_mut(&mut self) -> IterMut<TStringPart> {
+    pub fn iter_mut(&mut self) -> IterMut<TString> {
         self.as_mut_slice().iter_mut()
     }
 
-    /// Returns an iterator over the [`StringLiteral`] parts contained in this value.
-    ///
-    /// Note that this doesn't recurse into the t-string parts. For example,
-    ///
-    /// ```python
-    /// "foo" t"bar {x}" "baz" t"qux"
-    /// ```
-    ///
-    /// Here, the string literal parts returned would be `"foo"` and `"baz"`.
-    pub fn literals(&self) -> impl Iterator<Item = &StringLiteral> {
-        self.iter().filter_map(|part| part.as_literal())
-    }
-
-    /// Returns an iterator over the [`TString`] parts contained in this value.
-    ///
-    /// Note that this doesn't recurse into the t-string parts. For example,
-    ///
-    /// ```python
-    /// "foo" t"bar {x}" "baz" t"qux"
-    /// ```
-    ///
-    /// Here, the t-string parts returned would be `f"bar {x}"` and `f"qux"`.
-    pub fn t_strings(&self) -> impl Iterator<Item = &TString> {
-        self.iter().filter_map(|part| part.as_t_string())
-    }
-
     /// Returns an iterator over all the [`InterpolatedStringElement`] contained in this value.
     ///
-    /// An t-string element is what makes up an [`TString`] i.e., it is either a
+    /// An interpolated string element is what makes up an [`TString`] i.e., it is either a
     /// string literal or an interpolation. In the following example,
     ///
     /// ```python
-    /// "foo" t"bar {x}" "baz" t"qux"
+    /// t"foo" t"bar {x}" t"baz" t"qux"
     /// ```
     ///
-    /// The t-string elements returned would be string literal (`"bar "`),
+    /// The interpolated string elements returned would be string literal (`"bar "`),
     /// interpolation (`x`) and string literal (`"qux"`).
     pub fn elements(&self) -> impl Iterator<Item = &InterpolatedStringElement> {
-        self.t_strings().flat_map(|fstring| fstring.elements.iter())
+        self.iter().flat_map(|tstring| tstring.elements.iter())
     }
 }
 
 impl<'a> IntoIterator for &'a TStringValue {
-    type Item = &'a TStringPart;
-    type IntoIter = Iter<'a, TStringPart>;
+    type Item = &'a TString;
+    type IntoIter = Iter<'a, TString>;
 
     fn into_iter(self) -> Self::IntoIter {
         self.iter()
@@ -719,8 +693,8 @@ impl<'a> IntoIterator for &'a TStringValue {
 }
 
 impl<'a> IntoIterator for &'a mut TStringValue {
-    type Item = &'a mut TStringPart;
-    type IntoIter = IterMut<'a, TStringPart>;
+    type Item = &'a mut TString;
+    type IntoIter = IterMut<'a, TString>;
     fn into_iter(self) -> Self::IntoIter {
         self.iter_mut()
     }
@@ -731,43 +705,10 @@ impl<'a> IntoIterator for &'a mut TStringValue {
 #[cfg_attr(feature = "get-size", derive(get_size2::GetSize))]
 enum TStringValueInner {
     /// A single t-string i.e., `t"foo"`.
-    ///
-    /// This is always going to be `TStringPart::TString` variant which is
-    /// maintained by the `TStringValue::single` constructor.
-    Single(TStringPart),
+    Single(TString),
 
-    /// An implicitly concatenated t-string i.e., `"foo" t"bar {x}"`.
-    Concatenated(Vec<TStringPart>),
-}
-
-/// An t-string part which is either a string literal, an f-string,
-/// or a t-string.
-#[derive(Clone, Debug, PartialEq, is_macro::Is)]
-#[cfg_attr(feature = "get-size", derive(get_size2::GetSize))]
-pub enum TStringPart {
-    Literal(StringLiteral),
-    FString(FString),
-    TString(TString),
-}
-
-impl TStringPart {
-    pub fn quote_style(&self) -> Quote {
-        match self {
-            Self::Literal(string_literal) => string_literal.flags.quote_style(),
-            Self::FString(f_string) => f_string.flags.quote_style(),
-            Self::TString(t_string) => t_string.flags.quote_style(),
-        }
-    }
-}
-
-impl Ranged for TStringPart {
-    fn range(&self) -> TextRange {
-        match self {
-            TStringPart::Literal(string_literal) => string_literal.range(),
-            TStringPart::FString(f_string) => f_string.range(),
-            TStringPart::TString(t_string) => t_string.range(),
-        }
-    }
+    /// An implicitly concatenated t-string i.e., `t"foo" t"bar {x}"`.
+    Concatenated(Vec<TString>),
 }
 
 pub trait StringFlags: Copy {
@@ -1237,6 +1178,12 @@ pub struct TString {
     pub flags: TStringFlags,
 }
 
+impl TString {
+    pub fn quote_style(&self) -> Quote {
+        self.flags.quote_style()
+    }
+}
+
 impl From<TString> for Expr {
     fn from(payload: TString) -> Self {
         ExprTString {

crates/ruff_python_ast/src/visitor.rs

@@ -7,8 +7,8 @@ use crate::{
     self as ast, Alias, AnyParameterRef, Arguments, BoolOp, BytesLiteral, CmpOp, Comprehension,
     Decorator, ElifElseClause, ExceptHandler, Expr, ExprContext, FString, FStringPart,
     InterpolatedStringElement, Keyword, MatchCase, Operator, Parameter, Parameters, Pattern,
-    PatternArguments, PatternKeyword, Stmt, StringLiteral, TString, TStringPart, TypeParam,
-    TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, TypeParams, UnaryOp, WithItem,
+    PatternArguments, PatternKeyword, Stmt, StringLiteral, TString, TypeParam, TypeParamParamSpec,
+    TypeParamTypeVar, TypeParamTypeVarTuple, TypeParams, UnaryOp, WithItem,
 };
 
 /// A trait for AST visitors. Visits all nodes in the AST recursively in evaluation-order.
@@ -547,14 +547,8 @@ pub fn walk_expr<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, expr: &'a Expr) {
             }
         }
         Expr::TString(ast::ExprTString { value, .. }) => {
-            for part in value {
-                match part {
-                    TStringPart::Literal(string_literal) => {
-                        visitor.visit_string_literal(string_literal);
-                    }
-                    TStringPart::FString(f_string) => visitor.visit_f_string(f_string),
-                    TStringPart::TString(t_string) => visitor.visit_t_string(t_string),
-                }
+            for t_string in value {
+                visitor.visit_t_string(t_string);
             }
         }
         Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => {

crates/ruff_python_ast/src/visitor/transformer.rs

@@ -533,18 +533,8 @@ pub fn walk_expr<V: Transformer + ?Sized>(visitor: &V, expr: &mut Expr) {
             }
         }
         Expr::TString(ast::ExprTString { value, .. }) => {
-            for t_string_part in value.iter_mut() {
-                match t_string_part {
-                    ast::TStringPart::Literal(string_literal) => {
-                        visitor.visit_string_literal(string_literal);
-                    }
-                    ast::TStringPart::FString(f_string) => {
-                        visitor.visit_f_string(f_string);
-                    }
-                    ast::TStringPart::TString(t_string) => {
-                        visitor.visit_t_string(t_string);
-                    }
-                }
+            for t_string in value.iter_mut() {
+                visitor.visit_t_string(t_string);
             }
         }
         Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => {