python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.32k stars 2.81k forks source link

Proposal: treat "obvious" return type as annotated #4409

Open elazarg opened 6 years ago

elazarg commented 6 years ago

(Not sure if it should be here or in typing)

Consider this function:

def f():
    return "hello"
def g(x):
    if x: return A(1)
    else: return A(2)

f obviously (without any deep analysis) returns str, and g returns A. Why not use this information? This pattern is very common, and taking advantage of it can help in precise checking and remove clutter (for example -> Union[Tuple[int, int, str], Tuple[int, str, bool]]).

I propose treating calls to functions whose return expressions consists solely of literals and constructor calls as if they were declared with the returned type (join or union of the return types).

refi64 commented 6 years ago

Well, I always felt that the main reason for this was to allow mypy to be used on already-existing codebases, so the unannotated functions don't need to be type-safe (yet). This would change that behavior.

ethanhs commented 6 years ago

@kirbyfan64 I think I agree with what you are saying, but I also think there isn't a reason something like this couldn't be behind a flag, I've heard from several people that this is a significant pain point, as people don't want to have to do a lot of work to annotate obvious functions, so big +1 from me.

elazarg commented 6 years ago

@kirbyfan64 the proposal does not require unannotated functions to be type-safe. It simply allows annotated functions that use them to be slightly more type safe (peek into the immediate return expressions, if there is one).

elazarg commented 6 years ago

To elaborate:

class A: pass

def f(): return A(1)  # no error here, even though A has no no-arg __init__

def g() -> None:
    f().bar()  # error here
gvanrossum commented 6 years ago

However when is it obvious enough? Where do you draw the line?

On Dec 25, 2017 13:43, "Elazar Gershuni" notifications@github.com wrote:

To elaborate:

class A: pass def f(): return A(1) # no error here, even though A has no no-arg init def g() -> None: f().bar() # error here

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/python/mypy/issues/4409#issuecomment-353889908, or mute the thread https://github.com/notifications/unsubscribe-auth/ACwrMlh2ba7M5ihwiKF6_8U6p7N_2f9Kks5tEAjhgaJpZM4RMQzD .

elazarg commented 6 years ago

Either a literal or a direct constructor call on a globally defined class. (Part of the idea is that the return statements are also the documentation).

ethanhs commented 6 years ago

Expression atoms would be straightforward enough (not certain about identifiers however). At some point this could probably be expanded to entire expressions I believe (down the road).

elazarg commented 6 years ago

Yes, that could be a nice path. Atoms, then (recursively) tuples/lists, then simple names. My hope is this could be done as part of semantic analysis, just like the analysis of the annotations themselves.

refi64 commented 6 years ago

Could a director constructor call really be safe though? e.g.:

def func():
    A = lambda x: x
    return A(None)

Or even:

def func():
    return A(1)

func.__globals__['A'] = lambda x: x

Of course, this isn't ideal, but it happens in unannotated code.

elazarg commented 6 years ago

I think it will be as safe as any other analysis performed by mypy. The first example is not a call to a global constructor; the second example defeats any other analysis.

elazarg commented 6 years ago

I guess the discussion can be separated to two orthogonal questions:

ilevkivskyi commented 6 years ago

Note there is an old proposal https://github.com/python/typing/issues/276 to make this explicit. During the discussion it became clear that inferring types for arguments can be very hard, but for return types it is probably OK.

Recently, additional interest in this appeared in the context of data classes (PEP 557), where annotations are syntactically required, but people may be lazy to write exact types, so that this could be allowed:

@dataclass
class C:
    x: ... = 5  # equivalent to 'x = 5' (without an annotation) for a type checker

while for function this will work as

def func() -> ...:
    return 5

reveal_type(func())  # Revealed type is 'builtins.int'

An additional argument in favour of ellipsis is that libraries in numeric stack use ellipsis for inferred number of dimensions.

elazarg commented 6 years ago

One problem with annotations is that they mean that the function will be checked. It also does not completely address the issue of clutter and laziness.

I agree that if this is the direction, then the ellipsis idea is nice. But two minor points should be noted: its meaning is unrelated to Tuple[int, ...], and ... everywhere might make the code look bad.

ilevkivskyi commented 6 years ago

@elazarg

One problem with annotations is that they mean that the function will be checked.

It is maybe a matter of taste, but I think it is rather a plus, since I like to keep the current separation between annotated and non-annotated functions.

elazarg commented 6 years ago

The idea is to not to make the human hands robotic, but to give them fingernails 😄

Behind a flag, the taste will be the user's.

ilevkivskyi commented 6 years ago

The problem with a flag is that there already lots of them, so I am not keen to have one more (and this also doesn't solve the @dataclass problem). Anyway, I think we need to hear opinions of others.

elliott-beach commented 6 years ago

I think this definitely should be behind a flag, if included, because it changes the spirit of mypy by not allowing typed functions. There is a similar discussion for typescript, although it's for function arguments rather than return type ( https://github.com/Microsoft/TypeScript/issues/15114).

@ethanhs who specifically said the lack of this feature was a pain point (just wondering)?

JukkaL commented 6 years ago

I agree with Guido. It seems that the dividing line will have to be kind of arbitrary.

Somewhat related: One potential longer-term workaround to the problem would be to provide an editor hook that could automatically create a signature for the current function being edited. It could infer simple enough types and leave placeholders for other types. Figuring out the types could be a feature of the mypy daemon mode. Basically this would integrate PyAnnotate-like functionality to an editor.

elazarg commented 6 years ago

@elliott-beach I don't think this is related. I suggest no inference at all, let alone cross-procedural one.

akdor1154 commented 6 years ago

Typescript does this by default and it works beautifully. They have an (encouraged) compiler flag 'noImplicitAny' which will cause an error if the return type cannot be inferred; in this case the dev should manually annotate. The end result is really nice - I get return type safety for free in most cases.

The question about "when is it obvious enough" is pushed under the carpet and answered with "when our static analyzer can understand it". The analyzer improves with most typescript releases, so things that used to require explicit annotation (remember, the user knows this because they will have 'noImplicitAny' turned on) may no longer need them in newer Typescript versions. This has not caused any problems that I am aware of.

It does clash with the current mypy use of type annotations to determine whether to check a function or not. To be honest I reckon this is a pain. I would rather just be able to tell mypy "check this whole file" or if really needed, just use a decorator on specific functions to opt in or out. Other languages (rust, new c++ features, new c# features, swift, ts) are all heavily embracing type inference everywhere, and as a dev I really find this direction to be a lot more ergonomic (safety with less typing [of the keyboard variety], what'd not to like?)

gvanrossum commented 6 years ago

Maybe we can add a marker to request this, e.g. -> infer. But I presume that even TypeScript doesn't infer the argument types, so this feature seems of limited value except for argument-less functions.

To answer your question "what's not to like", it depends on whether you're working on a small new program or trying to add annotations to millions of lines of legacy code. In the latter case, turning on checking for all code would cause too many false positives (since legacy code tends to use clever hacks that the type checker doesn't understand). If you're in the small-new-program camp, just turn on --check-untyped-defs and other strict flags.

ilevkivskyi commented 6 years ago

Maybe we can add a marker to request this, e.g. -> infer

and there is an old proposal for this https://github.com/python/typing/issues/276

ethanhs commented 5 years ago

Would it be weird to only do this when --check-untyped-defs is true? Then we can side-step the ambiguity of un-annotated functions and people can opt into this behavior instead of opt out (plus no new flag needed).

JukkaL commented 5 years ago

Would it be weird to only do this when --check-untyped-defs is true?

I like this idea!

ilevkivskyi commented 5 years ago

I like this too.

ethanhs commented 5 years ago

I like this idea!

I like this too.

Great! Perhaps @elazarg is still interested? If not I might take a shot at it if I can find the time.

elazarg commented 5 years ago

That is a good idea indeed.

I'm still interested (and haven't contributed anything for too long) but I cannot commit to it before mid-April.

ethanhs commented 5 years ago

I'm still interested (and haven't contributed anything for too long) but I cannot commit to it before mid-April.

Glad to hear you are still interested! I think this is definitely a "nice to have" feature so if you find time in the future, great, but no need to get it done soon :)

JukkaL commented 3 years ago

I'm removing the low priority and 'needs discussion' labels since this seems like a popular enough idea. We'd still need somebody to contribute an implementation.

I think that instead of special casing literals and constructor calls, this should use the normal type inference engine (perhaps with some limitations). Otherwise the behavior would be too ad hoc.

For example, these should work:

def f():
    return sys.platform.startswith('win')   # Infer bool as the return type

def g():
    return f()  # Infer bool as the return type

def h():
    return [g(), g()]  # Infer list[bool] as the return type

The type checker already has the concept of 'deferring' type checking of functions if we refer to something without a ready type. In cases like the above, the type of the function would not be ready until we've type checked the body of the function. After type checking we'd infer the return type from the return statements.

The implementation could slow down type inference a lot, so we may need to be more clever about the order of type checking. A minor performance hit would be fine, however.

DevilXD commented 3 years ago

Not sure if this is the best place to suggest this, or should I make a new issue, but I've recently tried using MyPy with the --disallow-incomplete-defs flag on my codebase, and the results I got were quite off-putting, after a bunch of functions that had typed input paremeters, but no -> None return type defined, were returned as problems. While I get the meaning behind the flag (and the results I got), there are cases where a function doesn't even have a return statement inside and isn't used in any expression either (only simple call) - some examples of those are __init__ or helper-setter methods on a class, for example. I find having to add -> None to every function like that quite pointless, as MyPy should be able to detect that easily.

My suggestion would be to infer the return type as None (something that seems to fit within this issue), if the body of the function contains a bare return statement, or no return statement at all. Right now, this is how it looks like:

# mypy: disallow-incomplete-defs

from typing import List

strings: List[str] = []

def add(text: str):  # error: Function is missing a return type annotation
    strings.append(text)
djmattyg007 commented 3 years ago

It really does feel ridiculous that I have to manually add -> None to hundreds of pytest test functions despite them having precisely zero return statements inside of them. Why can mypy not infer this automatically despite it being completely obvious to the human eye? Is there a config setting or command-line flag I can turn on to get this behaviour? I want to turn on strict mode, but I have no interest in adding a totally useless void return type to hundreds of functions before being able to do so.

I even tried looking into what it would take to create a plugin to deal with this problem for pytest, but from what I've been able to find there isn't a plugin hook for this. Nothing seems to be called before checking the type of a standalone function that isn't called by my own code.

kstauffer commented 2 years ago

@djmattyg007 worte:

Is there a config setting or command-line flag I can turn on to get this behaviour? I want to turn on strict mode, but I have no interest in adding a totally useless void return type to hundreds of functions before being able to do so.

Create a mypy.ini config file. At the top level of my FancyPackage git repo is mypy.ini:

[mypy]
strict = True
files = FancyPackage

[mypy-FancyPackage._c_wrapper]
ignore_errors = True

[mypy-FancyPackage.test.*]
disallow_untyped_defs = False
disallow_untyped_calls = False

When I run "mypy" with no arguments, the FancyPackage/ subdirectory is checked in strict mode (except for the auto-generated FancyPackage/_c_wrapper.py, a python shim for a C library), but FancyPackage/test/ are only checked for correct usage of FancyPackage without checking the validity of the test functions & classes defined in FancyPackage/test/ per se.

djmattyg007 commented 2 years ago

@kstauffer That doesn't sound like a useful solution at all. It disables a whole bunch of warnings for all affected code, rather than one specific warning for just the functions where it is necessary. All it will lead to is other legitimate warnings being missed.

kstauffer commented 2 years ago

@djmattyg007

That doesn't sound like

(Emphasis added.) Did you actually try it? It disables precisely the warning you don't care about and only in the files where you don't care about it (your test files). My config was just an example. You can tailor yours to your needs. https://mypy.readthedocs.io/en/stable/config_file.html

djmattyg007 commented 2 years ago

I don't want to disable those settings on a per-file basis. I want to disable them on a per-function basis.

michaeloliverx commented 2 years ago

TypeScript's inference is really great in this regard, with strict mode enabled you don't have to annotated the return types of functions. I would love some level of support for this in mypy.

ryanc-bs commented 11 months ago

It may be obvious to most humans that f should be marked as returning str, but it would be more precise to say it returns Literal["hello"]. Maybe a slightly more complex example could be said to return an enum of possible string literals rather than any arbitrary str. It is also not incorrect to say that f returns Any or object. Automatically inferring the return type could be problematic when the developer intentions are unclear.

ikokostya commented 11 months ago

@ryanc-bs

It may be obvious to most humans that f should be marked as returning str, but it would be more precise to say it returns Literal["hello"]

The following definitions can be considered as equal:

def f():
    return "hello"

def f():
    x = "hello"
    reveal_type(x) #  Revealed type is "builtins.str"
    return x

mypy already infers type for local variable, and I don't see why rule for inferring return type should be different. For comparison, in both cases pyright infers return type of f as Literal["hello"] (see https://microsoft.github.io/pyright/#/type-inference?id=return-type-inference and https://microsoft.github.io/pyright/#/type-inference?id=literals).

markshannon commented 10 months ago

Having to write -> None on every function that can only return None because it has no explicit return statements is quite annoying.

In general, if MyPy can infer the return type accurately, then it should. There is no benefit to not doing so. It doesn't have to report the error, if not in --strict mode, but there seem no reason not to use the information that MyPy already knows.

For example:

def x():
     return 1

def y() -> str:
    return "hello " + x()

The above code is clearly wrong, yet without --strict, x() has the type Any and "hello " + x() isn't checked. With --strict, MyPy complains that x() is not annotated, which is a distraction from the actual error.

bluenote10 commented 10 months ago

In general, if MyPy can infer the return type accurately, then it should. There is no benefit to not doing so.

There probably is a benefit: performance. I'd assume inferring the return types where missing requires a bit of extra work, because currently mypy can just skip checking all returns and their types, which should be faster.

But nonetheless it would be a very nice feature. If performance is a factor it could simply be made an opt-in feature.

ethanhs commented 5 months ago

Ok I think I am going to sit down and commit time to work on an implementation of this. It's a prerequisite to #6646 IMO, and I think fixing that is very important.

I think I will try an implementation with the full inference engine, but I also want to try a more minimal approach, as I think it is less risky.

Avasam commented 4 months ago

I was gonna ask for some sort of no-untyped-def check (disallow_incomplete_defs or disallow_untyped_defs) that only raises for methods that mypy won't analyze (ie: 0 non-self parameters w/ missing return type, or missing parameter annotations).

But this request would alleviate some of the pain with disallow_incomplete/untyped_defs and reduce a lot of Function is missing a return type annotation

pranavrajpal commented 3 months ago

I think one potential problem that might come up with adding a configuration option for turning this on is that this changes how a given type signature is interpreted by mypy, unlike most of the other configuration options, so if a library you were using turned that option on while you had it off or you turned it on while the library turned it off then mypy would do something that wasn't intended. In the former case, mypy would silently infer an Any return type if the library authors were relying on the return type being inferred, while in the latter case mypy would, if I'm understanding the proposal correctly, start emitting errors relating to that inferred return type being used incorrectly, even if the library authors intentionally avoided annotating that since they were relying on it getting inferred as Any.

In my experience, I've definitely run into errors similar to the latter case with pyright, since (if I recall correctly) I usually have reasonably strict settings turned on while I sometimes use libraries without type annotations, and libraries that have complicated return types tend to result in pyright inferring a return type that leads to a false positive error. To some extent that's an understandable design decision since someone turning on stricter settings might prefer false positives over false negatives, but I think it's a risk to keep in mind here.

As far as I can tell, the only precedent for a config option that changes how types are interpreted by mypy is implicit_optional, and I think the historical context behind that makes it reasonably different than this case, since if you and a library disagree on whether to set that flag, there is a "right" option, while in the case of this option, libraries might be expected to support both it being set and unset, which would end up with them getting the worst of both worlds for their external API at least (you'd need to add return types for functions with obvious return types to make sure mypy/other type checkers inferred them correctly, but you'd also probably want to annotate non-obvious return types with Any explicitly so that mypy didn't infer the wrong thing).

eternal-sorrow commented 3 months ago

This proposal does not require the full inference of the return type. Only the obvious one in simple cases, like if a function does not have a return statement, infer it as -> None, or if it has only one return statement, that's the return type of the function.

ikokostya commented 3 months ago

@eternal-sorrow

or if it has only one return statement,

It looks like artificial limitation. For example, in the following example

def func1(val: int):
    if val > 3:
        return ""
    elif val < 1:
        return True

return type is str | bool | None. It seems fairly obvious. Pyright and TypeScript can handle such cases.

elazarg commented 3 months ago

@ikokostya while this function is similar to the one I have described at the beginning of this issue, I am against inferring its type. This feature should be about (a) performing zero computation and (b) having the return expression itself serve as an explicit annotation.