Skip to content
This repository has been archived by the owner on Jul 5, 2023. It is now read-only.

SyntaxError when parsing per-argument type comments with comma on the next line #58

Closed
mbdevpl opened this issue Sep 26, 2017 · 4 comments

Comments

@mbdevpl
Copy link

mbdevpl commented Sep 26, 2017

In the PR adding per-argument type comment support #5 there is an unhandled case when comma for an argument is after the type comment, therefore the current version of typed_ast errors out on the following.

In [1]: import inspect

In [2]: import typed_ast.ast3

In [3]: def lovely(spam  # type: bool
   ...:         , eggs  # type: int
   ...:         , ham  # type: str
   ...:         ):
   ...:     # type: (...) -> bool
   ...:     return spam
   ...: 

In [4]: typed_ast.ast3.parse(inspect.getsource(lovely))
  File "<unknown>", line 2
    , eggs  # type: int
    ^
SyntaxError: invalid syntax

Are there any plans to support this case?

In [5]: import ast

In [6]: ast.parse(inspect.getsource(lovely))
Out[6]: <_ast.Module at 0x7fb9584b84e0>

The built-in ast succeeds, which seems to be in conflict with the development philosophy of typed_ast.

This project is a drop-in replacement for the builtin ast module. It is intended to be bug-for-bug compatible and behave identically, except for the presence of a few additional fields on the returned classes and a few additional optional arguments to the parse call.

@ethanhs
Copy link
Contributor

ethanhs commented Sep 26, 2017

This syntax is incompatible with PEP 484. See the paragraph here: https://github.com/python/peps/blob/master/pep-0484.txt#L2097

Relevant quote:

you may list the arguments one per line and add a # type: comment
per line after an argument's associated comma

(Emphasis mine)

You may also want to see the discussion in the PR you referenced here: #5 (comment)

@ddfisher
Copy link
Collaborator

Great question! The short answer is: no plans currently. Here's why:

First, the major point of departure of typed_ast from ast is that it considers type comments part of the language (which in some senses they are, as specified by PEP 484). As part of this, it considers misplaced type comments to be syntax errors. This is partially due to technical limitations, and I'd likely accept a patch which changed them into warnings, provided it didn't require a massive overhaul.

Second, as @ethanhs mentioned above, PEP 484 is specific about the location of the type comments relative to the comma, so per the spec those type comments are not in a valid position. (IIRC that's partly due to beginning-of-the-line commas being considered unpythonic). It could be valuable for typed_ast to be more flexible here, but I'm not sure it's worth the additional complexity.

The combination of those two factors results in the behavior that you see. Neither of these things has planned development. I've looked at the referenced issue, but could you talk a bit more about why you want this? Maybe a workaround can be found instead.

@gvanrossum
Copy link
Member

Also, I've always despised the style where the comma comes at the start of the next line. It violates esthetics and good sense.

@mbdevpl
Copy link
Author

mbdevpl commented Sep 27, 2017

The code sample above is an example of what my unparser generates. Implementing unparsing in that way was... very simple. And I was surprised that typed_ast errors out on it. But indeed, per PEP 484, a fix should be done not on the parser side, but on the unparser side of things. Thanks for taking time to explain everything!

tbbharaj pushed a commit to tbbharaj/typed_ast that referenced this issue Dec 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants