Skip to content

--warn-return-any is too specialized #2901

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

Closed
JukkaL opened this issue Feb 24, 2017 · 15 comments
Closed

--warn-return-any is too specialized #2901

JukkaL opened this issue Feb 24, 2017 · 15 comments

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 24, 2017

Recently the --warn-return-any option was added which causes mypy to warn if returning a value with an Any type (#2854). @ddfisher pointed out the this is not very general:

Thinking out loud here: this helps catch accidental Anys in one particular circumstance, but it might be nice to have something a bit more comprehensive as well. Perhaps we should consider adding a flag which disallows use of Any-typed values except as the right hand side of an assignment with a type comment (i.e. forcing all Anys to be immediately typed).

I agree that #2854 seems too narrowly focused. I like @ddfisher's suggestion, with a few tweaks:

  • Any should probably be okay as a function argument type if the argument type is Any (or object).
  • Assigning an Any value to a variable with an Any / object type seems okay as well.
  • Narrowing down from an Any value using isinstance or cast should be allowed.
@ddfisher
Copy link
Collaborator

Those sound like good tweaks to me, though I'd consider limiting the exception to argument/variables of Any type (i.e. not including object) for simplicity of understanding.

@miedzinski
Copy link
Contributor

I'd like to clarify my statements from #2905 without opening new issue. In case of generators, it is clear that --warn-return-any should warn when we are returning Any and the function return type is Generator[YieldType, SendType, ReturnType] where ReturnType is other than Any. The remaining issues are what to do when ReturnType is Any or we are declaring generator function to return Iterable[YieldType] or Iterator[YieldType] (typing docs suggest doing this). In the first scenario, I think mypy should look for ReturnType inside Generator and warn if needed (instead of always warning, because technically we are returning Generator, not ReturnType). In the latter, mypy could assume that ReturnType (and SendType, but it won't be possible to use generator methods anyway) is Any and don't warn.

@ddfisher, you said that it's not possible to access return value from generator function. Since Python 3.3 this is possible and PEP 380 describes it.

@ddfisher
Copy link
Collaborator

I'm not sure I completely followed your comment, but to clarify what I said earlier: attempting to access the return value of a generator function which is typed as Iterator is a type error. I wasn't referring to accessing the return type of Generator functions in general.

@pkch
Copy link
Contributor

pkch commented Feb 27, 2017

@miedzinski can you clarify what you're proposing? I tried to provide an example below but I think I'm missing something:

# --warn-return-any flag on
from typing import *

# warning: Returning Any from generator with declared return type "int"
def gen1(v: Any) -> Generator[int, Any, int]:
    yield 1
    return v

# "mypy should look inside Generator and warn if needed" -- not sure what you meant
def gen2(v: Any) -> Generator[int, Any, Any]:
    yield 1
    return v

# no warning; mypy assumes Iterator[int] is same Generator[int, Any, Any]
def gen3(v: Any) -> Iterator[int]:
    yield 1
    return v

Also, PEP implies Iterator[int] is equivalent to Generator[int, None, None]. Is it really the same as Generator[int, Any, Any]?

@miedzinski
Copy link
Contributor

@ddfisher I thought you meant generators in general. If it was about Iterator, then there is nothing to talk about. :)

@pkch, for example

def f(x: Any) -> Generator[int, Any, Any]:
    yield 0
    return x

If --warn-return-any was extended to generators as well, this shouldn't raise a warning like warning: Returning Any from function with declared return type "Generator[int, int, Any]", because the return type is actually Any - that's it. I can't find anything in PEP about Iterator[T] being equivalent to Generator[T, None, None] in PEP; it doesn't matter though since using these types would be illegal anyway.

@pkch
Copy link
Contributor

pkch commented Mar 30, 2017

What do we need to move this forward?

I had a couple bugs not discovered by mypy --strict only because I accidentally left out an argument to a generic type, and mypy silently interpreted it as [Any].

@gvanrossum
Copy link
Member

Honestly I don't personally understand this issue very well. Perhaps it would be helpful to provide some examples?

@pkch
Copy link
Contributor

pkch commented Mar 31, 2017

For example, let's say here:

T = TypeVar('T')

def f(x: T) -> List[T]:
    lst: List = []
    lst.append(1) # typo; I meant lst.append(x)
    return lst

f('a') # equals to [1], not ['a'] as intended

mypy --strict would type check this, but in runtime the program behaves incorrectly. If mypy --strict didn't auto-correct List to List[Any], I would have realized that my type annotation was accidentally too loose. Once I fix it to lst: List[T] = [], mypy would warn me of the bug in my program.

@gvanrossum
Copy link
Member

Oooh, subtle! I'm still not sure what Jukka's three initial bullets stand for.

@pkch
Copy link
Contributor

pkch commented Mar 31, 2017

Here's my understanding, please someone correct me if I missed something:

@ddfisher proposal was to add a flag (say, --disallow-implicit-any) that would completely disallow the use of Any-typed values except for one case (let's call it case 0) where Any is immediately and explicitly changed to something else.

@JukkaL suggested in the three bullet points the additional cases (let's call them cases 1, 2, 3) where --disallow-implicit-any should still permit the use of Any-typed values.

For example:

def f() -> Any:
   ...

def g(x: Any) -> None:
   if isinstance(x, int):
      x + 1 # case 3: ok in isinstance
   y = cast(int, x) # case 3: ok in cast

x: int = f() # case 0: ok because x is immediately typed to int
x: Any = f() # case 2: ok because x is explicitly declared as Any
g(x) # case 1: ok because g explicitly declared its argument as Any

@ddfisher proposal with the 4 exclusions would work perfectly for my use case.

Edit: changed y: int to y above.

@gvanrossum
Copy link
Member

And what are some examples that would be disallowed?

@pkch
Copy link
Contributor

pkch commented Mar 31, 2017

def f(x: int) -> Any:
   ...

def g(x: Any) -> None:
   y = x  # error
   f(x)  # error (immediate typing only applies to variable definitions, not arguments)

y = f(1)  # error

z: List = []  # error

And many other cases not obvious to me, such as overlapping @overload, etc. One way to find most of them is to look for return AnyType() in source code that is not preceded by a either an error message or a comment that says that an error message is already generated elsewhere.

@pkch
Copy link
Contributor

pkch commented Apr 7, 2017

This is closely related with #2696
I made #3141 that catches at least some of these implicit Any with mypy --warn-implicit-any.

If there's interest, I can work on it more, but my approach is very disorganized (I just caught cases that I personally wanted to fix in my own code).

@FuegoFro
Copy link
Contributor

FuegoFro commented Aug 1, 2017

Should this be closed now that #3470 is closed?

@ilevkivskyi
Copy link
Member

Should this be closed now that #3470 is closed?

I think so, we already have several flags for Any, if someone wants more, then one could open another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants