Skip to content

Conversation

@nielsbuwen
Copy link

@nielsbuwen nielsbuwen commented Sep 3, 2024

Resolves #17714 , fixes #11723

Support for f-string checking was already partially implemented. Consider this code:

f"{123:abc}"
"{:abc}".format(123)

Mypy ran these lines through the format checker. The call expressions look like this:

# f-string
CallExpr:1(
  MemberExpr:1(
    StrExpr({:{}})
    format)
  Args(
    IntExpr(123)
    StrExpr(abc)))

# format
CallExpr:2(
  MemberExpr:2(
    StrExpr({:abc})
    format)
  Args(
    IntExpr(123)))

The difference is, that the actually static "abc" is converted to a "dynamic" spec.

This MR attempts to enable type checking of f-strings by inlining these "semi dynamic" specs.

Open Questions:

  • is this a good idea? Or should the actual parsing of f-strings be changed?
  • the test i added always passes on my machine: pytest mypy/test/testcheck.py::TypeCheckSuite::check-string-format.test. It never finds any errors even when it should clearly fail

Checklist

  • Read the Contributing Guidelines
  • Add tests for all changed behavior
  • Make sure CI passes
  • Do not force push to PR once reviewed

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Collaborator

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

According to mypy-issues, this PR also fixes:

(I just realized after I ran it that that issue is mentioned in 17714 but whatever.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO this should go to test-data/unit/check-formatting.test

if spec.format_spec == ":{}": # most simple dynamic case
assert (
expression is not None
) # dynamic spec cannot be last, this should have been detected earlier
Copy link
Collaborator

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)

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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 self.msg.fail here?

@@ -0,0 +1,20 @@
[case acceptFstringWithoutSpecs]
[builtins fixtures/f_string.pyi]
Copy link
Collaborator

Choose a reason for hiding this comment

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

the test i added always passes on my machine

Yep, [builtins ...] should go after the case body. Now you defined three empty tests with some text in between. (Ough, the README is really a bit confusing regarding this - I'll open a documentation PR to improve the language there).

[case acceptFstringWithoutSpecs]
reveal_type(f"{123} {True} {1 + 2} {'foo'}")  # N: Revealed type is "builtins.str"
[builtins fixtures/f_string.pyi]

@hauntsaninja hauntsaninja force-pushed the enable-checking-for-f-strings branch from 4c38381 to 834be34 Compare November 29, 2025 01:35
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/connector.py:1344:36: error: Incompatible types in string interpolation (expression has type "None", placeholder has type "int")  [str-format]
+ aiohttp/connector.py:1344:36: note: See https://mypy.rtfd.io/en/stable/_refs.html#code-str-format for more info

colour (https://github.com/colour-science/colour)
+ colour/plotting/phenomena.py:328: error: Incompatible types in string interpolation (expression has type "ndarray[tuple[Any, ...], dtype[floating[_16Bit] | floating[_32Bit] | float64]]", placeholder has type "int | float")  [str-format]
+ colour/plotting/phenomena.py:656: error: Incompatible types in string interpolation (expression has type "Buffer", placeholder has type "int | float")  [str-format]
+ colour/plotting/phenomena.py:656: error: Incompatible types in string interpolation (expression has type "_SupportsArray[dtype[Any]] | _NestedSequence[_SupportsArray[dtype[Any]]] | complex | bytes | str | _NestedSequence[complex | bytes | str]", placeholder has type "int | float")  [str-format]

async-utils (https://github.com/mikeshardmind/async-utils)
+ src/async_utils/lockout.py:78: error: Unrecognized format specification "!r"  [str-format]
+ src/async_utils/lockout.py:151: error: Unrecognized format specification "!r"  [str-format]

core (https://github.com/home-assistant/core)
+ homeassistant/components/stiebel_eltron/climate.py:146: error: Incompatible types in string interpolation (expression has type "None", placeholder has type "int | float")  [str-format]

pandas (https://github.com/pandas-dev/pandas)
+ pandas/tests/scalar/test_na_scalar.py:38: error: Unrecognized format specification "xxx"  [str-format]

dulwich (https://github.com/dulwich/dulwich)
+ dulwich/patch.py:479: error: Incompatible types in string interpolation (expression has type "None", placeholder has type "int")  [str-format]

websockets (https://github.com/aaugustin/websockets)
+ src/websockets/sync/messages.py:89: error: Incompatible types in string interpolation (expression has type "None", placeholder has type "int | float")  [str-format]

materialize (https://github.com/MaterializeInc/materialize)
+ misc/python/materialize/feature_benchmark/benchmark_result.py:93: error: Incompatible types in string interpolation (expression has type "T", placeholder has type "int | float")  [str-format]
+ misc/python/materialize/feature_benchmark/benchmark_result.py:102: error: Incompatible types in string interpolation (expression has type "T", placeholder has type "int | float")  [str-format]

freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/persistence/trade_model.py:878: error: Incompatible types in string interpolation (expression has type "None", placeholder has type "int | float")  [str-format]
+ freqtrade/rpc/telegram.py:438: error: Incompatible types in string interpolation (expression has type "None", placeholder has type "int | float")  [str-format]
+ freqtrade/rpc/telegram.py:460: error: Incompatible types in string interpolation (expression has type "None", placeholder has type "int | float")  [str-format]
+ freqtrade/strategy/interface.py:1638: error: Incompatible types in string interpolation (expression has type "None", placeholder has type "int | float")  [str-format]

ignite (https://github.com/pytorch/ignite)
+ ignite/utils.py:134: error: Incompatible types in string interpolation (expression has type "Number", placeholder has type "int | float")  [str-format]
+ ignite/utils.py:165: error: Incompatible types in string interpolation (expression has type "Number", placeholder has type "int | float")  [str-format]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Basic checking of format string compatibility in f-strings mypy doesn't check f-strings conversions

4 participants