{"action":"created","issue":{"url":"https://api.github.com/repos/python/mypy/issues/5963","repository_url":"https://api.github.com/repos/python/mypy","labels_url":"https://api.github.com/repos/python/mypy/issues/5963/labels{/name}","comments_url":"https://api.github.com/repos/python/mypy/issues/5963/comments","events_url":"https://api.github.com/repos/python/mypy/issues/5963/events","html_url":"https://github.com/python/mypy/issues/5963","id":385065521,"node_id":"MDU6SXNzdWUzODUwNjU1MjE=","number":5963,"title":"Small bug: `if` expression doesn't short circuit for --always-false/--always-true","user":{"login":"samertm","id":969377,"node_id":"MDQ6VXNlcjk2OTM3Nw==","avatar_url":"https://avatars1.githubusercontent.com/u/969377?v=4","gravatar_id":"","url":"https://api.github.com/users/samertm","html_url":"https://github.com/samertm","followers_url":"https://api.github.com/users/samertm/followers","following_url":"https://api.github.com/users/samertm/following{/other_user}","gists_url":"https://api.github.com/users/samertm/gists{/gist_id}","starred_url":"https://api.github.com/users/samertm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/samertm/subscriptions","organizations_url":"https://api.github.com/users/samertm/orgs","repos_url":"https://api.github.com/users/samertm/repos","events_url":"https://api.github.com/users/samertm/events{/privacy}","received_events_url":"https://api.github.com/users/samertm/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-11-28T01:53:56Z","updated_at":"2018-11-28T04:16:52Z","closed_at":null,"author_association":"NONE","body":"`if` expressions don't correctly short-circuit for variables that are set as \"always true/false\" constants with the `--always-false` and `--always-true` flags, even though they work for other \"always true/false\" conditions, like ones that use `sys.platform`.\r\n\r\nHere's a test case:\r\n```\r\n# flags: --platform linux --always-false IS_THING\r\nimport sys\r\n\r\nif sys.platform == 'darwin':\r\n    darwin_only = 1\r\n\r\nif sys.platform == 'darwin' and darwin_only:\r\n    # This works because `sys.platform == 'darwin'` is inferred as `ALWAYS_FALSE`, and\r\n    # that causes the short-circuit logic to run.\r\n    pass\r\n\r\nIS_THING = False\r\n\r\nif IS_THING:\r\n    thing_only = 1\r\n\r\nif IS_THING and thing_only:\r\n    # Blows up with the error `error: Name 'thing_only' is not defined` since\r\n    # IS_THING is inferred as `MYPY_FALSE`, so the short-circuiting logic\r\n    # doesn't run\r\n    pass\r\n```\r\n\r\nThe short-circuit logic is scattered throughout the codebase, but here's an [example](https://github.com/python/mypy/blob/3c311c24323d20bbf7e13084ba16bcb99e8934d7/mypy/semanal.py#L3074):\r\n\r\n```\r\n        if expr.op in ('and', 'or'):\r\n            inferred = infer_condition_value(expr.left, self.options)\r\n            if ((inferred == ALWAYS_FALSE and expr.op == 'and') or\r\n                    (inferred == ALWAYS_TRUE and expr.op == 'or')):\r\n                expr.right_unreachable = True\r\n                return\r\n            elif ((inferred == ALWAYS_TRUE and expr.op == 'and') or\r\n                    (inferred == ALWAYS_FALSE and expr.op == 'or')):\r\n                expr.right_always = True\r\n```\r\n\r\nNotice that it checks for ALWAYS_FALSE and ALWAYS_TRUE, but not MYPY_FALSE or MYPY_TRUE>\r\n\r\nAnd the --always-true/false flags are translated to MYPY_TRUE/FALSE [here](https://github.com/python/mypy/blob/3c311c24323d20bbf7e13084ba16bcb99e8934d7/mypy/semanal.py#L3856).\r\n\r\nThere are two potential fixes:\r\n\r\n1. Include two MYPY_TRUE/FALSE in all of the `if` short circuit checks.\r\n\r\nI think that works, because even when using the MYPY variable directly, it's probably surprising that it doesn't short-circuit.\r\n\r\n2. Use ALWAYS_TRUE/FALSE with the --always-true/false flags instead of MYPY_TRUE/FALSE.\r\n\r\n@gvanrossum might have more context on why the --always-* flags use the MYPY_* constants instead of the ALWAYS_* constants, since he added them. FWIW, using ALWAYS_* makes more semantic sense to me, since I'm using the flags for variables that will have the same value at runtime. (e.g. I'm setting `--always-true=IS_WINDOWS` to typecheck my app for windows, so IS_WINDOWS will also be true at runtime.) That's distinct from how `MYPY` works, where it's always False at runtime and always True for typechecking.\r\n\r\nThoughts?\r\n\r\n* What are the versions of mypy and Python you are using?\r\n\r\npython 3.5.2, latest master for mypy\r\n"},"comment":{"url":"https://api.github.com/repos/python/mypy/issues/comments/442313843","html_url":"https://github.com/python/mypy/issues/5963#issuecomment-442313843","issue_url":"https://api.github.com/repos/python/mypy/issues/5963","id":442313843,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0MjMxMzg0Mw==","user":{"login":"gvanrossum","id":2894642,"node_id":"MDQ6VXNlcjI4OTQ2NDI=","avatar_url":"https://avatars3.githubusercontent.com/u/2894642?v=4","gravatar_id":"","url":"https://api.github.com/users/gvanrossum","html_url":"https://github.com/gvanrossum","followers_url":"https://api.github.com/users/gvanrossum/followers","following_url":"https://api.github.com/users/gvanrossum/following{/other_user}","gists_url":"https://api.github.com/users/gvanrossum/gists{/gist_id}","starred_url":"https://api.github.com/users/gvanrossum/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gvanrossum/subscriptions","organizations_url":"https://api.github.com/users/gvanrossum/orgs","repos_url":"https://api.github.com/users/gvanrossum/repos","events_url":"https://api.github.com/users/gvanrossum/events{/privacy}","received_events_url":"https://api.github.com/users/gvanrossum/received_events","type":"User","site_admin":false},"created_at":"2018-11-28T04:16:52Z","updated_at":"2018-11-28T04:16:52Z","author_association":"MEMBER","body":"Huh, how quickly we forget. :-)\r\n\r\nIt sounds like a fix might be straightforward: just make sure both are tested everywhere else."}}
{"actor":{"display_login":"gvanrossum"}}