Skip to content

Fixes #2589: type redeclarations cause spurious warnings #2591

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 1 commit into from
Dec 22, 2016
Merged

Fixes #2589: type redeclarations cause spurious warnings #2591

merged 1 commit into from
Dec 22, 2016

Conversation

ambv
Copy link
Contributor

@ambv ambv commented Dec 19, 2016

This addresses the problem by refusing to re-bind the explicit type for a name once one type was already present. This means it changes the logic from "last type declaration wins" to "first type declaration wins". The error about redeclaring the type is still being reported but the type does not change, which changes the errors given, producing less surprising results.

Test plan: added a new test case to unit/semanal. Without the change, there's spurious errors about names not conforming to the type defined last. With the change, only the "Name already defined" errors remain.

This addresses the problem by refusing to re-bind the explit type for a name
once one type was already present.  This means it changes the logic from "last
type declaration wins" to "first type declaration wins".  The error about
redeclaring the type is still being reported but the type does not change,
which changes the errors given, producing less surprising results.

Test plan: added a new test case to unit/semanal.  Without the change, there's
spurious errors about names not conforming to the type defined last.  With the
change, only the "Name already defined" errors remain.
@JukkaL
Copy link
Collaborator

JukkaL commented Dec 22, 2016

Looks good -- thanks for the fix!

@JukkaL JukkaL merged commit ca2d2c6 into python:master Dec 22, 2016
@ddfisher
Copy link
Collaborator

This change prevents an "incompatible types in assignment" error from being generated on lines with a "Name already defined" errors. E.g.:

x = 0 # type: int
x = "" # type: str

now gives only the error:

test.py:2: error: Name 'x' already defined

whereas previously it would also give:

test.py:2:error: Incompatible types in assignment (expression has type "str", variable has type "int")

I think we want that error, so this should perhaps be fixed in a slightly different place.

@ambv
Copy link
Contributor Author

ambv commented Dec 22, 2016

In the previous case, it was the first line that would emit the error:

/tmp/qwe.py:1: error: Incompatible types in assignment (expression has type "int", variable has type "str")
/tmp/qwe.py:2: error: Name 'x' already defined

This is confusing and the point of this pull request. With the fix, there is an error being emitted only on line 2:

/tmp/qwe.py:2: error: Name 'x' already defined

Removing the second type comment will emit your original error:

/tmp/qwe.py:2: error: Incompatible types in assignment (expression has type "str", variable has type "int")

In other words, no information is lost but there's no confusing error being emitted on a line that preceeds the problem.

@ddfisher
Copy link
Collaborator

Oh, you're right! Sorry, I didn't read the output carefully enough.

This is strictly an improvement, then. Thanks! 👍

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