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.

Sign up for GitHub

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

Jump to bottom

Add line # of previous definition (resubmit #3396) #3424

Merged
merged 7 commits into from
Jun 16, 2017
Merged

Add line # of previous definition (resubmit #3396) #3424

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
82c5313
Add line # of previous definition
pkch May 22, 2017
5b077ae
CR fix
pkch May 22, 2017
39c3058
Fix tests
pkch May 22, 2017
8449ed5
Add failiing test
pkch May 23, 2017
295c203
Fix the case where original context is None
pkch May 23, 2017
e1bfb11
CR fixes
pkch May 23, 2017
3d77de6
CR
pkch Jun 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions mypy/semanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3315,7 +3315,7 @@ def check_no_global(self, n: str, ctx: Context,
elif prev_is_overloaded:
self.fail("Definition of '{}' missing 'overload'".format(n), ctx)
else:
self.name_already_defined(n, ctx)
self.name_already_defined(n, ctx, self.globals[n])

def name_not_defined(self, name: str, ctx: Context) -> None:
message = "Name '{}' is not defined".format(name)
Expand All @@ -3324,8 +3324,16 @@ def name_not_defined(self, name: str, ctx: Context) -> None:
message += ' {}'.format(extra)
self.fail(message, ctx)

def name_already_defined(self, name: str, ctx: Context) -> None:
self.fail("Name '{}' already defined".format(name), ctx)
def name_already_defined(self, name: str, ctx: Context,
original_ctx: Optional[SymbolTableNode] = None) -> None:
if original_ctx:
if original_ctx.node and original_ctx.node.get_line() != -1:
extra_msg = ' on line {}'.format(original_ctx.node.get_line())
else:
extra_msg = ' (line unavailable; possibly an import)'
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could simplify the message a bit. What about just (possibly by an import). The 'line unavailable' part doesn't provide useful information really.

else:
extra_msg = ''
self.fail("Name '{}' already defined{}".format(name, extra_msg), ctx)

def fail(self, msg: str, ctx: Context, serious: bool = False, *,
blocker: bool = False) -> None:
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-classes.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ A()
class A: pass
class A: pass
[out]
main:4: error: Name 'A' already defined
main:4: error: Name 'A' already defined on line 3

[case testDocstringInClass]
import typing
Expand Down
4 changes: 2 additions & 2 deletions test-data/unit/check-functions.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1212,7 +1212,7 @@ from typing import Any
x = None # type: Any
if x:
def f(): pass
def f(): pass # E: Name 'f' already defined
def f(): pass # E: Name 'f' already defined on line 4

[case testIncompatibleConditionalFunctionDefinition]
from typing import Any
Expand Down Expand Up @@ -1835,7 +1835,7 @@ f = g # E: Incompatible types in assignment (expression has type Callable[[Any,

[case testRedefineFunction2]
def f() -> None: pass
def f() -> None: pass # E: Name 'f' already defined
def f() -> None: pass # E: Name 'f' already defined on line 1


-- Special cases
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-overloading.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,7 @@ def f(a: int) -> None: pass
def f(a: str) -> None: pass
[out]
tmp/foo.pyi:2: error: Single overload definition, multiple required
tmp/foo.pyi:5: error: Name 'f' already defined
tmp/foo.pyi:5: error: Name 'f' already defined on line 2

[case testOverloadTuple]
from foo import *
Expand Down
20 changes: 15 additions & 5 deletions test-data/unit/semanal-errors.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,16 @@ import typing
class A: pass
class A: pass
[out]
main:3: error: Name 'A' already defined
main:3: error: Name 'A' already defined on line 2

[case testMultipleMixedDefinitions]
import typing
x = 1
def x(): pass
class x: pass
[out]
main:3: error: Name 'x' already defined
main:4: error: Name 'x' already defined
main:3: error: Name 'x' already defined on line 2
main:4: error: Name 'x' already defined on line 2

[case testNameNotImported]
import typing
Expand Down Expand Up @@ -1037,13 +1037,13 @@ t = 1 # E: Invalid assignment target
[case testRedefineTypevar2]
from typing import TypeVar
t = TypeVar('t')
def t(): pass # E: Name 't' already defined
def t(): pass # E: Name 't' already defined on line 2
[out]

[case testRedefineTypevar3]
from typing import TypeVar
t = TypeVar('t')
class t: pass # E: Name 't' already defined
class t: pass # E: Name 't' already defined on line 2
[out]

[case testRedefineTypevar4]
Expand Down Expand Up @@ -1330,3 +1330,13 @@ a = ('spam', 'spam', 'eggs', 'spam') # type: Tuple[str]
[out]
main:3: error: Name 'a' already defined
main:4: error: Name 'a' already defined

[case testUnknownContextDuplicateDef]
from m import A
class A:
pass
[file m.py]
class A:
pass
[out]
main:2: error: Name 'A' already defined (line unavailable; possibly an import)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here are further ideas for test cases:

  • Original definition is a decorated function.
  • Original definition is an overloaded function.
  • Original definition is a named tuple definition.
  • Original definition is a typed dict definition.
  • Original definition is a module (import m).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a module is simply imported twice, there's no error message. Did you mean some other test?

Copy link
Member

Choose a reason for hiding this comment

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

@pkch Maybe Jukka means something like this:

import re
re = 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but that will cause a completely different error message (types are not compatible). It won't get to the point of complaining about duplicate definition.

Copy link
Member

Choose a reason for hiding this comment

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

And what about

import re
import math
re = math

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, there's no error message at all (as expected?).