Skip to content
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

Parse each format-string component separately #3390

Merged

Conversation

lincolnq
Copy link
Contributor

@lincolnq lincolnq commented May 21, 2017

Fixes #3385.

The old code evaluated each FormattedValue as an expression potentially returning the type of the value, rather than a string. But that's wrong, because a FormattedValue can exist on its own when it's not being joined to any other format strings + those FormattedValues need type str. The new code evaluates each FormattedValue by synthesizing '{}'.format(expr) and then, if necessary, joins them in JoinedStr using ''.join(items).

I had to create a new unittest fixture for this with list, str, and int.

[case testNewSyntaxFStringSingleField]
# flags: --python-version 3.6
v = 1
f'{v}' + ''
Copy link
Member

Choose a reason for hiding this comment

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

I'd write reveal_type(f'{v}') # E: Revealed type is 'builtins.str', which tests more explicitly what you want to achieve.


T = TypeVar('T')

@builtinclass
Copy link
Member

Choose a reason for hiding this comment

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

Curious, what does this do? I haven't seen it before and grep shows no uses outside of test fixtures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, I think nothing -- maybe it's a relic? I just copied that code from fixtures/list.pyi. I've removed it.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Just nits, this is basically right.

format_args,
format_arg_kinds)
"""Each of n.values is a str or FormattedValue; we just concatenate
them all using ''.join."""
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be a comment, not a docstring. Ditto below. (Docstrings are more for the users of a function, comments are for readers of the function body.)

def __mul__(self, x: int) -> list[T]: pass
def __getitem__(self, x: int) -> T: pass
def append(self, x: T) -> None: pass
def extend(self, x: Iterable[T]) -> None: pass
Copy link
Member

Choose a reason for hiding this comment

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

You probably don't need all these methods.

Fixes python#3385.

The old code evaluated each FormattedValue as an expression potentially
returning the type of the value, rather than a string. But that's wrong,
because a FormattedValue can exist on its own when it's not being joined
to any other format strings. The new code evaluates each FormattedValue
by synthesizing '{}'.format(expr) and then, if necessary, joins them in
JoinedStr using ''.join(items).
@lincolnq lincolnq force-pushed the lincoln/fix-fstring-single-formatting-field branch from d4fba25 to a564e4b Compare May 27, 2017 08:46
@lincolnq lincolnq force-pushed the lincoln/fix-fstring-single-formatting-field branch from a564e4b to f586e2d Compare May 27, 2017 08:51
@lincolnq
Copy link
Contributor Author

Ready for re-review.

@gvanrossum gvanrossum merged commit d4864a9 into python:master May 30, 2017
@gvanrossum
Copy link
Member

Thanks!

carljm added a commit to carljm/mypy that referenced this pull request May 30, 2017
* master: (23 commits)
  Make return type of open() more precise (python#3477)
  Add test cases that delete a file during incremental checking (python#3461)
  Parse each format-string component separately (python#3390)
  Don't warn about returning Any if it is a proper subtype of the return type (python#3473)
  Add __setattr__ support (python#3451)
  Remove bundled lib-typing (python#3337)
  Move version of extensions to post-release (python#3348)
  Fix None slice bounds with strict-optional (python#3445)
  Allow NewType subclassing NewType. (python#3465)
  Add console scripts (python#3074)
  Fix 'variance' label.
  Change label for variance section to just 'variance' (python#3429)
  Better error message for invalid package names passed to mypy (python#3447)
  Fix last character cut in html-report if file does not end with newline (python#3466)
  Print pytest output as it happens (python#3463)
  Add mypy roadmap (python#3460)
  Add flag to avoid interpreting arguments with a default of None as Optional (python#3248)
  Add type checking plugin support for functions (python#3299)
  Mismatch of inferred type and return type note (python#3428)
  Sync typeshed (python#3449)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect errors for fstrings + generators
3 participants