-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable checking for f strings #17732
base: master
Are you sure you want to change the base?
Changes from all commits
ee8a4ed
ba0a46d
c594440
1fa153c
bda3dc1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,20 @@ | |
from __future__ import annotations | ||
|
||
import re | ||
from typing import TYPE_CHECKING, Callable, Dict, Final, Match, Pattern, Tuple, Union, cast | ||
from itertools import chain | ||
from typing import ( | ||
TYPE_CHECKING, | ||
Callable, | ||
Dict, | ||
Final, | ||
Iterator, | ||
Match, | ||
Pattern, | ||
Sequence, | ||
Tuple, | ||
Union, | ||
cast, | ||
) | ||
from typing_extensions import TypeAlias as _TypeAlias | ||
|
||
import mypy.errorcodes as codes | ||
|
@@ -343,6 +356,54 @@ def check_str_format_call(self, call: CallExpr, format_value: str) -> None: | |
return | ||
self.check_specs_in_format_call(call, conv_specs, format_value) | ||
|
||
def _spec_expression_with_peek( | ||
self, specs: list[ConversionSpecifier], expressions: list[Expression] | ||
) -> Iterator[tuple[ConversionSpecifier, Expression, Expression | None]]: | ||
""" | ||
Basically zip specs with expressions and the next expression | ||
""" | ||
optional_expression: Sequence[Expression | None] = expressions | ||
expression_it = chain(optional_expression, [None]) | ||
next(expression_it) | ||
return zip(specs, expressions, expression_it) | ||
|
||
def inline_semi_dynamic_specs( | ||
self, call: CallExpr, specs: list[ConversionSpecifier], expressions: list[Expression] | ||
) -> tuple[list[ConversionSpecifier], list[Expression]]: | ||
""" | ||
Try to inline literal expressions into "dynamic" format specifiers | ||
|
||
e.g. "{:{}}".format(123, "foo") becomes "{:foo}.format(123)" | ||
|
||
This works by checking if a spec if a simple dynamic specifier and if the next expression is a literal string. | ||
If so, the literal string is inlined into the spec and the next spec-expression pair is dropped | ||
|
||
This is useful for f-strings | ||
""" | ||
inlined_specs = [] | ||
inlined_expressions = [] | ||
|
||
spec_with_pairwise_expression = self._spec_expression_with_peek(specs, expressions) | ||
for spec, expression, next_expression in spec_with_pairwise_expression: | ||
if spec.format_spec == ":{}": # most simple dynamic case | ||
assert ( | ||
expression is not None | ||
) # dynamic spec cannot be last, this should have been detected earlier | ||
|
||
if isinstance(next_expression, StrExpr): # now inline the literal | ||
new_format_string = f"{{{spec.conversion or ''}:{next_expression.value}}}" | ||
parsed = parse_format_value(new_format_string, call, self.msg) | ||
if parsed is None or len(parsed) != 1: | ||
continue | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume this happens if the resulting format string isn't a valid format string. Is that caught elsewhere, or does that deserve a |
||
spec = parsed[0] | ||
next(spec_with_pairwise_expression) | ||
|
||
inlined_specs.append(spec) | ||
inlined_expressions.append(expression) | ||
|
||
self.auto_generate_keys(inlined_specs, call) | ||
return inlined_specs, inlined_expressions | ||
|
||
def check_specs_in_format_call( | ||
self, call: CallExpr, specs: list[ConversionSpecifier], format_value: str | ||
) -> None: | ||
|
@@ -353,6 +414,7 @@ def check_specs_in_format_call( | |
assert all(s.key for s in specs), "Keys must be auto-generated first!" | ||
replacements = self.find_replacements_in_call(call, [cast(str, s.key) for s in specs]) | ||
assert len(replacements) == len(specs) | ||
specs, replacements = self.inline_semi_dynamic_specs(call, specs, replacements) | ||
for spec, repl in zip(specs, replacements): | ||
repl = self.apply_field_accessors(spec, repl, ctx=call) | ||
actual_type = repl.type if isinstance(repl, TempNode) else self.chk.lookup_type(repl) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO this should go to |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
[case acceptFstringWithoutSpecs] | ||
[builtins fixtures/f_string.pyi] | ||
reveal_type(f"{123} {True} {1 + 2} {'foo'}") # N: Revealed type is "builtins.str" | ||
|
||
[case acceptFstringWithValidSpecs] | ||
[builtins fixtures/f_string.pyi] | ||
f"{123:+04} {True!r} {1 + 2:+} {'foo':3<}" | ||
|
||
[case denyFstringsWithInvalidSpecs] | ||
[builtins fixtures/f_string.pyi] | ||
f"{'hi':+}" # E: Numeric flags are only allowed for numeric types | ||
f"{123:foo}" # E: Unrecognized format specification "foo" | ||
|
||
class MyClass: | ||
pass | ||
|
||
f"{MyClass:abc}" # E: Unrecognized format specification "abc" | ||
|
||
|
||
f"{1} {2} {3:x} {4:y} {5:z}" # E: Unsupported format character "y" # E: Unsupported format character "z" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind adding a test case? I imagine just
"{:{}}".format("aaaa")
would work... (I'm not sure what could possibly trigger this though)