From c5b58187da1809c778d886595ec50f7db68fd3b9 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 16 Jun 2025 11:44:42 -0500 Subject: [PATCH] Add syntax error when conversion flag does not immediately follow exclamation mark (#18706) Closes #18671 Note that while this has, I believe, always been invalid syntax, it was reported as a different syntax error until Python 3.12: Python 3.11: ```pycon >>> x = 1 >>> f"{x! s}" File "", line 1 f"{x! s}" ^ SyntaxError: f-string: invalid conversion character: expected 's', 'r', or 'a' ``` Python 3.12: ```pycon >>> x = 1 >>> f"{x! s}" File "", line 1 f"{x! s}" ^^^ SyntaxError: f-string: conversion type must come right after the exclamanation mark ``` --- .../test/fixtures/ruff/expression/fstring.py | 8 +- .../test/fixtures/ruff/expression/tstring.py | 8 +- .../format@expression__fstring.py.snap | 16 +- .../format@expression__tstring.py.snap | 12 +- ...f_string_conversion_follows_exclamation.py | 3 + crates/ruff_python_parser/src/error.rs | 11 +- .../src/parser/expression.rs | 13 ++ ...ing_conversion_follows_exclamation.py.snap | 186 ++++++++++++++++++ 8 files changed, 219 insertions(+), 38 deletions(-) create mode 100644 crates/ruff_python_parser/resources/inline/err/f_string_conversion_follows_exclamation.py create mode 100644 crates/ruff_python_parser/tests/snapshots/invalid_syntax@f_string_conversion_follows_exclamation.py.snap diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py index d3a19d24e8..f70290c01d 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py @@ -267,17 +267,13 @@ aaaaaaaaaaa = f"""asaaaaaaaaaaaaaaaa { # Conversion flags # -# This is not a valid Python code because of the additional whitespace between the `!` -# and conversion type. But, our parser isn't strict about this. This should probably be -# removed once we have a strict parser. -x = f"aaaaaaaaa { x ! r }" # Even in the case of debug expressions, we only need to preserve the whitespace within # the expression part of the replacement field. -x = f"aaaaaaaaa { x = ! r }" +x = f"aaaaaaaaa { x = !r }" # Combine conversion flags with format specifiers -x = f"{x = ! s +x = f"{x = !s :>0 }" diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/tstring.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/tstring.py index 31087f1610..f430faab9b 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/tstring.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/tstring.py @@ -265,17 +265,13 @@ aaaaaaaaaaa = t"""asaaaaaaaaaaaaaaaa { # Conversion flags # -# This is not a valid Python code because of the additional whitespace between the `!` -# and conversion type. But, our parser isn't strict about this. This should probably be -# removed once we have a strict parser. -x = t"aaaaaaaaa { x ! r }" # Even in the case of debug expressions, we only need to preserve the whitespace within # the expression part of the replacement field. -x = t"aaaaaaaaa { x = ! r }" +x = t"aaaaaaaaa { x = !r }" # Combine conversion flags with format specifiers -x = t"{x = ! s +x = t"{x = !s :>0 }" diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap index b357a01b9f..913dcaf547 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap @@ -273,17 +273,13 @@ aaaaaaaaaaa = f"""asaaaaaaaaaaaaaaaa { # Conversion flags # -# This is not a valid Python code because of the additional whitespace between the `!` -# and conversion type. But, our parser isn't strict about this. This should probably be -# removed once we have a strict parser. -x = f"aaaaaaaaa { x ! r }" # Even in the case of debug expressions, we only need to preserve the whitespace within # the expression part of the replacement field. -x = f"aaaaaaaaa { x = ! r }" +x = f"aaaaaaaaa { x = !r }" # Combine conversion flags with format specifiers -x = f"{x = ! s +x = f"{x = !s :>0 }" @@ -1036,10 +1032,6 @@ aaaaaaaaaaa = f"""asaaaaaaaaaaaaaaaa {aaaaaaaaaaaa + bbbbbbbbbbbb + cccccccccccc # Conversion flags # -# This is not a valid Python code because of the additional whitespace between the `!` -# and conversion type. But, our parser isn't strict about this. This should probably be -# removed once we have a strict parser. -x = f"aaaaaaaaa {x!r}" # Even in the case of debug expressions, we only need to preserve the whitespace within # the expression part of the replacement field. @@ -1836,10 +1828,6 @@ aaaaaaaaaaa = f"""asaaaaaaaaaaaaaaaa {aaaaaaaaaaaa + bbbbbbbbbbbb + cccccccccccc # Conversion flags # -# This is not a valid Python code because of the additional whitespace between the `!` -# and conversion type. But, our parser isn't strict about this. This should probably be -# removed once we have a strict parser. -x = f"aaaaaaaaa {x!r}" # Even in the case of debug expressions, we only need to preserve the whitespace within # the expression part of the replacement field. diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__tstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__tstring.py.snap index 5c08140e3e..e916c1ee27 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__tstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__tstring.py.snap @@ -271,17 +271,13 @@ aaaaaaaaaaa = t"""asaaaaaaaaaaaaaaaa { # Conversion flags # -# This is not a valid Python code because of the additional whitespace between the `!` -# and conversion type. But, our parser isn't strict about this. This should probably be -# removed once we have a strict parser. -x = t"aaaaaaaaa { x ! r }" # Even in the case of debug expressions, we only need to preserve the whitespace within # the expression part of the replacement field. -x = t"aaaaaaaaa { x = ! r }" +x = t"aaaaaaaaa { x = !r }" # Combine conversion flags with format specifiers -x = t"{x = ! s +x = t"{x = !s :>0 }" @@ -1032,10 +1028,6 @@ aaaaaaaaaaa = t"""asaaaaaaaaaaaaaaaa {aaaaaaaaaaaa + bbbbbbbbbbbb + cccccccccccc # Conversion flags # -# This is not a valid Python code because of the additional whitespace between the `!` -# and conversion type. But, our parser isn't strict about this. This should probably be -# removed once we have a strict parser. -x = t"aaaaaaaaa {x!r}" # Even in the case of debug expressions, we only need to preserve the whitespace within # the expression part of the replacement field. diff --git a/crates/ruff_python_parser/resources/inline/err/f_string_conversion_follows_exclamation.py b/crates/ruff_python_parser/resources/inline/err/f_string_conversion_follows_exclamation.py new file mode 100644 index 0000000000..17d523150f --- /dev/null +++ b/crates/ruff_python_parser/resources/inline/err/f_string_conversion_follows_exclamation.py @@ -0,0 +1,3 @@ +f"{x! s}" +t"{x! s}" +f"{x! z}" diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index b7bacffb57..ad00df0f76 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/crates/ruff_python_parser/src/error.rs @@ -63,13 +63,16 @@ pub enum InterpolatedStringErrorType { UnterminatedTripleQuotedString, /// A lambda expression without parentheses was encountered. LambdaWithoutParentheses, + /// Conversion flag does not immediately follow exclamation. + ConversionFlagNotImmediatelyAfterExclamation, } impl std::fmt::Display for InterpolatedStringErrorType { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { use InterpolatedStringErrorType::{ - InvalidConversionFlag, LambdaWithoutParentheses, SingleRbrace, UnclosedLbrace, - UnterminatedString, UnterminatedTripleQuotedString, + ConversionFlagNotImmediatelyAfterExclamation, InvalidConversionFlag, + LambdaWithoutParentheses, SingleRbrace, UnclosedLbrace, UnterminatedString, + UnterminatedTripleQuotedString, }; match self { UnclosedLbrace => write!(f, "expecting '}}'"), @@ -80,6 +83,10 @@ impl std::fmt::Display for InterpolatedStringErrorType { LambdaWithoutParentheses => { write!(f, "lambda expressions are not allowed without parentheses") } + ConversionFlagNotImmediatelyAfterExclamation => write!( + f, + "conversion type must come right after the exclamation mark" + ), } } } diff --git a/crates/ruff_python_parser/src/parser/expression.rs b/crates/ruff_python_parser/src/parser/expression.rs index 9eff91aadb..e1f5f8c124 100644 --- a/crates/ruff_python_parser/src/parser/expression.rs +++ b/crates/ruff_python_parser/src/parser/expression.rs @@ -1771,6 +1771,19 @@ impl<'src> Parser<'src> { let conversion = if self.eat(TokenKind::Exclamation) { let conversion_flag_range = self.current_token_range(); if self.at(TokenKind::Name) { + // test_err f_string_conversion_follows_exclamation + // f"{x! s}" + // t"{x! s}" + // f"{x! z}" + if self.prev_token_end != conversion_flag_range.start() { + self.add_error( + ParseErrorType::from_interpolated_string_error( + InterpolatedStringErrorType::ConversionFlagNotImmediatelyAfterExclamation, + string_kind, + ), + TextRange::new(self.prev_token_end, conversion_flag_range.start()), + ); + } let TokenValue::Name(name) = self.bump_value(TokenKind::Name) else { unreachable!(); }; diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@f_string_conversion_follows_exclamation.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@f_string_conversion_follows_exclamation.py.snap new file mode 100644 index 0000000000..f3725bcbab --- /dev/null +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@f_string_conversion_follows_exclamation.py.snap @@ -0,0 +1,186 @@ +--- +source: crates/ruff_python_parser/tests/fixtures.rs +input_file: crates/ruff_python_parser/resources/inline/err/f_string_conversion_follows_exclamation.py +--- +## AST + +``` +Module( + ModModule { + node_index: AtomicNodeIndex(..), + range: 0..30, + body: [ + Expr( + StmtExpr { + node_index: AtomicNodeIndex(..), + range: 0..9, + value: FString( + ExprFString { + node_index: AtomicNodeIndex(..), + range: 0..9, + value: FStringValue { + inner: Single( + FString( + FString { + range: 0..9, + node_index: AtomicNodeIndex(..), + elements: [ + Interpolation( + InterpolatedElement { + range: 2..8, + node_index: AtomicNodeIndex(..), + expression: Name( + ExprName { + node_index: AtomicNodeIndex(..), + range: 3..4, + id: Name("x"), + ctx: Load, + }, + ), + debug_text: None, + conversion: Str, + format_spec: None, + }, + ), + ], + flags: FStringFlags { + quote_style: Double, + prefix: Regular, + triple_quoted: false, + }, + }, + ), + ), + }, + }, + ), + }, + ), + Expr( + StmtExpr { + node_index: AtomicNodeIndex(..), + range: 10..19, + value: TString( + ExprTString { + node_index: AtomicNodeIndex(..), + range: 10..19, + value: TStringValue { + inner: Single( + TString( + TString { + range: 10..19, + node_index: AtomicNodeIndex(..), + elements: [ + Interpolation( + InterpolatedElement { + range: 12..18, + node_index: AtomicNodeIndex(..), + expression: Name( + ExprName { + node_index: AtomicNodeIndex(..), + range: 13..14, + id: Name("x"), + ctx: Load, + }, + ), + debug_text: None, + conversion: Str, + format_spec: None, + }, + ), + ], + flags: TStringFlags { + quote_style: Double, + prefix: Regular, + triple_quoted: false, + }, + }, + ), + ), + }, + }, + ), + }, + ), + Expr( + StmtExpr { + node_index: AtomicNodeIndex(..), + range: 20..29, + value: FString( + ExprFString { + node_index: AtomicNodeIndex(..), + range: 20..29, + value: FStringValue { + inner: Single( + FString( + FString { + range: 20..29, + node_index: AtomicNodeIndex(..), + elements: [ + Interpolation( + InterpolatedElement { + range: 22..28, + node_index: AtomicNodeIndex(..), + expression: Name( + ExprName { + node_index: AtomicNodeIndex(..), + range: 23..24, + id: Name("x"), + ctx: Load, + }, + ), + debug_text: None, + conversion: None, + format_spec: None, + }, + ), + ], + flags: FStringFlags { + quote_style: Double, + prefix: Regular, + triple_quoted: false, + }, + }, + ), + ), + }, + }, + ), + }, + ), + ], + }, +) +``` +## Errors + + | +1 | f"{x! s}" + | ^ Syntax Error: f-string: conversion type must come right after the exclamation mark +2 | t"{x! s}" +3 | f"{x! z}" + | + + + | +1 | f"{x! s}" +2 | t"{x! s}" + | ^ Syntax Error: t-string: conversion type must come right after the exclamation mark +3 | f"{x! z}" + | + + + | +1 | f"{x! s}" +2 | t"{x! s}" +3 | f"{x! z}" + | ^ Syntax Error: f-string: conversion type must come right after the exclamation mark + | + + + | +1 | f"{x! s}" +2 | t"{x! s}" +3 | f"{x! z}" + | ^ Syntax Error: f-string: invalid conversion character + |