Disallow implicit concatenation of t-strings and other string types

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S104.py

@@ -25,5 +25,5 @@ def my_func():
 
 # t-strings - all ok
 t"0.0.0.0"
-"0.0.0.0" t"0.0.0.0{expr}0.0.0.0"
-"0.0.0.0" f"0.0.0.0{expr}0.0.0.0" t"0.0.0.0{expr}0.0.0.0"
+t"0.0.0.0" t"0.0.0.0{expr}0.0.0.0"
+t"0.0.0.0" t"0.0.0.0{expr}0.0.0.0" t"0.0.0.0{expr}0.0.0.0"

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);
-                }
-                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);
-                            }
-                        }
-                    }
+        for element in value.elements() {
+            match element {
+                ast::InterpolatedStringElement::Literal(literal) => {
+                    collector.push_literal(&literal.value);
                 }
-                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);
         }
     }
 }