Skip to content

Gracefully handle invalid type comments in the fast parser #1600

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

Merged
merged 4 commits into from
Jun 3, 2016

Conversation

ddfisher
Copy link
Collaborator

@ddfisher ddfisher commented Jun 1, 2016

Gives errors with the appropriate line number (instead of unhelpful stack traces) on syntax errors or invalid AST nodes in type comments. E.g.:

syntax error: x = None # type: a : b
invalid AST node: x = None # type: a + b

@gvanrossum
Copy link
Member

gvanrossum commented Jun 1, 2016 via email

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 2, 2016

Fixed lint failures (and missing annotation).

except SyntaxError as e:
if errors:
errors.set_file('<input>' if fnam is None else fnam)
errors.report(e.lineno, e.msg) # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

Remind me what we're ignoring here again?

Copy link
Collaborator Author

@ddfisher ddfisher Jun 2, 2016

Choose a reason for hiding this comment

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

I'm not sure what we were ignoring here before, because it works without it... except when I combine the except clauses. It looks like then it's taking the join of the two exceptions as the type of e (instead of the Union) and gives me:

mypy/fastparse.py:73: error: "Exception" has no attribute "lineno"
mypy/fastparse.py:73: error: "Exception" has no attribute "msg"

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps related to #1610 (which hasn't been merged yet)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep! That PR (after it's fixed up a bit more) should fix this.

@gvanrossum
Copy link
Member

Looks okay, but I'll leave it up to @JukkaL to decide whether to merge this before or after the 0.4.2 release.

@ddfisher ddfisher force-pushed the PR/type-comment-errors branch from 5a5f8b0 to 57e2b02 Compare June 3, 2016 01:00
@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 3, 2016

Rebase on master and remove now-unnecessary # type: ignore

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.

3 participants