Repository navigation
Allow returning inferred None from functions - #5663
Conversation
JukkaL
left a comment
JukkaL
left a comment
There was a problem hiding this comment.
Using NoneTyp attribute to handle an edge case such as this seems like an overkill. This increases complexity and adds some overhead with a pretty small gain.
Instead, I'd rather rewrite the None return check to look at whether the callable refers to something with an explicit declared None return type (look for the annotation in the node, not the inferred type). It's fine if the semantics of the check change slightly as this is not required for correctness.
|
OK, I have rewritten it without a flag and I like the new semantics even more, it looks more consistent. This however required fixing an unrelated issue: previously all |
JukkaL
left a comment
JukkaL
left a comment
There was a problem hiding this comment.
Thanks for the update, I like how the new check works. There seems to be a problem with methods though (see my comment).
| o = a() # E: Function does not return a value | ||
| o = A().g(a) # E: "g" of "A" does not return a value | ||
| o = a() | ||
| o = A().g(a) |
There was a problem hiding this comment.
I think that this should still generate an error, since A.g has an explicit None return type.
| a = f() # E: "f" does not return a value | ||
| o = a() # E: Function does not return a value | ||
| o = A().g(a) # E: "g" of "A" does not return a value | ||
| o = a() |
There was a problem hiding this comment.
It would be okay for this to still generate an error, since __call__ has an explicit None return, but since this is an edge case it's not that important. Might be worth adding an issues about this though.
|
@JukkaL |
JukkaL
left a comment
JukkaL
left a comment
There was a problem hiding this comment.
I think that this is enough special casing for now. Looks good now!
|
This seems to have introduced a regression where we will emit the produces an error |
|
Hm, interesting, I will check this. |
Fixes a regression introduced in #5663.
3 participants
Fixes #4214
As discussed in the issue, assigning an inferred (from generics, overloads, or lambdas)
Nonetype should be always allowed, while prohibiting assigning function calls that have an explicitNonereturn type may have some value (at least people don't complain much about this).Note: this error is quite nasty for dataclasses, where it flags a legitimate typical use case (see tests).