Add support for literal addition #18004
Conversation
This comment has been minimized.
This comment has been minimized.
|
Can you look at the new errors reported by mypy_primer? If we infer literal types more eagerly, they could generate new false positives. |
The primer results look quite promising IMO. For from typing import Literal
class SessionMiddleware:
def __init__(
self,
same_site: Literal["lax", "strict", "none"] = "lax",
https_only: bool = False,
) -> None:
...
self.security_flags = "httponly; samesite=" + same_site
if https_only: # Secure flag can be used with HTTPS only
self.security_flags += "; secure" # <-- errorIt can be resolved by annotation -- class CaptureMethodBase:
description = ""
class BitBltCaptureMethod(CaptureMethodBase):
description = (
"\nThe best option when compatible. But it cannot properly record "
+ "\nOpenGL, Hardware Accelerated or Exclusive Fullscreen windows. "
+ "\nThe smaller the selected region, the more efficient it is. "
)
class ForceFullContentRenderingCaptureMethod(BitBltCaptureMethod):
description = (
"\nUses BitBlt behind the scene, but passes a special flag "
+ "\nto PrintWindow to force rendering the entire desktop. "
+ "\nAbout 10-15x slower than BitBlt based on original window size "
+ "\nand can mess up some applications' rendering pipelines. "
)Just removing the -- Not completely sure whats going on with |
I think implicit and explicit string concatenation should behave the same. Otherwise it would seem inconsistent. I also know that some people prefer explicit string concatenation as a style thing, since it's easy to accidentally use implicit string concatenation (e.g. omit a comma at the end). |
|
What about inferring a literal addition only if one of the operands is not a literal expression? We have tried to adhere to an informal policy where literal types are not inferred for variables "out of thin air", i.e. an explicit |
|
In particular, this feels inconsistent to me: class A:
a = ""
class B(A):
a = "a" + "b"
class C:
a = "a" + "b"
reveal_type(A.a) # Revealed type is "builtins.str"
reveal_type(B.a) # Revealed type is "Literal['ab']" (why literal here?)
reveal_type(C.a) # Revealed type is "builtins.str" |
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
Makes sense. I missed the |
This comment has been minimized.
This comment has been minimized.
|
Related issue: #11616 |
mypy/checkexpr.py
Outdated
|
|
||
| values: list[int | str] = sorted( | ||
| { | ||
| val[0] + val[1] # type: ignore[operator] |
There was a problem hiding this comment.
Can you avoid the type: ignore here? In the future we will probably disallow many ignores in code compiled with mypyc, since some type ignores can cause erratic behavior, such as C compilation errors.
There was a problem hiding this comment.
Can you avoid the
type: ignorehere?
Not in an elegant way. The values could be either str or int (for int and bytes). With the comparison of the fallback types we know that left and right values have the same type but not which one. If necessary, it would be possible to split these into two cases with cast and set comprehensions for each one. Don't think that it would be an improvement though.
| reveal_type(d + 'foo') # N: Revealed type is "builtins.str" | ||
| reveal_type('foo' + d) # N: Revealed type is "builtins.str" | ||
| reveal_type(d + 'bar') # N: Revealed type is "Literal['foobar']" | ||
| reveal_type('bar' + d) # N: Revealed type is "Literal['barfoo']" | ||
|
|
||
| reveal_type(a.__add__(b)) # N: Revealed type is "builtins.int" | ||
| reveal_type(b.__add__(a)) # N: Revealed type is "builtins.int" |
There was a problem hiding this comment.
Check a.__add__(a). It would be nice if it was consistent with a + a, though it's probably not important.
There was a problem hiding this comment.
The result will be int even if a: Literal[3]. The narrowing is only applied for the a + a case currently. The method call to __add__ is handled by self.check_op as the fallback case which I haven't modified here.
Test case added in 36f5e2e
| bytes_a: Literal[b"a"] | ||
| bytes_b: Literal[b"b"] | ||
| bytes_union_1: Literal[b"a", b"b"] | ||
| bytes_union_2: Literal[b"d", b"c"] |
There was a problem hiding this comment.
Test union with literal type and non-literal type -- e.g. user-defined class that defines __add__ that accepts str. Also test with union of literal type and Any.
There was a problem hiding this comment.
The narrowing is only applied if all Union items are Literals itself. Thus the inference for these cases didn't change.
Test cases added in 36f5e2e
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Related issue: #11990 Are there any guards against combinatorial explosion? E.g. from typing import Literal
type HexDigit = Literal["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "A", "B", "C", "D", "E", "F"]
def foo(a: HexDigit, b: HexDigit, c: HexDigit) -> None:
reveal_type(a + b + c) |
That's a good point. Thanks for bringing it to my attention! Implemented a guard in 24e2f21. For now, I've set it to a max of |
This comment has been minimized.
This comment has been minimized.
|
Rebased and added one more commit (c22d563) to remove the type ignore. |
|
Diff from mypy_primer, showing the effect of this PR on open source code: starlette (https://github.com/encode/starlette)
+ starlette/middleware/sessions.py:34: error: Incompatible types in assignment (expression has type "Literal['httponly; samesite=lax; secure', 'httponly; samesite=none; secure', 'httponly; samesite=strict; secure']", variable has type "Literal['httponly; samesite=lax', 'httponly; samesite=none', 'httponly; samesite=strict']") [assignment]
+ starlette/middleware/sessions.py:36: error: Incompatible types in assignment (expression has type "str", variable has type "Literal['httponly; samesite=lax', 'httponly; samesite=none', 'httponly; samesite=strict']") [assignment]
|
Add support for addition of Literal values and str, int and bytes expressions. This is especially useful when working with TypedDicts. Previously this code would have emitted a
literal-requirederror.Refs #11616