Python version check is sensitive to comparison order

[paw-lu]

Bug Report

Whether mypy successfully detects a Python version check is dependent on the order of the comparison.

To Reproduce

"""This will not pass on Python 3.8."""
import sys

if (3, 8) <= sys.version_info:
    pass
else:
    import invalid_library

% mypy type.py --python-version 3.8
type.py:6: error: Cannot find implementation or library stub for module named 'invalid_library'
type.py:6: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 1 source file)

"""This will pass on Python 3.8."""
import sys

if sys.version_info >= (3, 8):
    pass
else:
    import invalid_library

% mypy type.py --python-version 3.8
Success: no issues found in 1 source file

Expected Behavior

I expected mypy to correctly parse the Python version check regardless of the order of the comparison.

Actual Behavior

(3, 8) <= sys.version_info is not correctly detected by mypy, while the logically equivalent sys.version_info >= (3, 8) is.

Your Environment

• Mypy version used: 0.812
• Mypy command-line flags: --python-version 3.8
• Mypy configuration options from mypy.ini (and other config files): Empty
• Python version used: 3.8.6
• Operating system and version: macOS 10.15.7

[yotaAI]

Hey, @paw-lu , I want to work on this issue. But I am new to open source projects. Is it also applicable for windows?

[paw-lu]

Hi @Pank3 !

While I admitedly haven't tested it on Windows—I expect it should be applicable!

[paw-lu]

@Pank3 I beleive (but am not sure) the problem orginates somewhere around consider_sys_version_info in mypy/reachability.py. If not exactly in that function then maybe somewhere it is called.

mypy/mypy/reachability.py

Lines 112 to 150 in bab4e22

	def consider_sys_version_info(expr: Expression, pyversion: Tuple[int, ...]) -> int:
	"""Consider whether expr is a comparison involving sys.version_info.
	Return ALWAYS_TRUE, ALWAYS_FALSE, or TRUTH_VALUE_UNKNOWN.
	"""
	# Cases supported:
	# - sys.version_info[<int>] <compare_op> <int>
	# - sys.version_info[:<int>] <compare_op> <tuple_of_n_ints>
	# - sys.version_info <compare_op> <tuple_of_1_or_2_ints>
	# (in this case <compare_op> must be >, >=, <, <=, but cannot be ==, !=)
	if not isinstance(expr, ComparisonExpr):
	return TRUTH_VALUE_UNKNOWN
	# Let's not yet support chained comparisons.
	if len(expr.operators) > 1:
	return TRUTH_VALUE_UNKNOWN
	op = expr.operators[0]
	if op not in ('==', '!=', '<=', '>=', '<', '>'):
	return TRUTH_VALUE_UNKNOWN
	thing = contains_int_or_tuple_of_ints(expr.operands[1])
	if thing is None:
	return TRUTH_VALUE_UNKNOWN
	index = contains_sys_version_info(expr.operands[0])
	if isinstance(index, int) and isinstance(thing, int):
	# sys.version_info[i] <compare_op> k
	if 0 <= index <= 1:
	return fixed_comparison(pyversion[index], op, thing)
	else:
	return TRUTH_VALUE_UNKNOWN
	elif isinstance(index, tuple) and isinstance(thing, tuple):
	lo, hi = index
	if lo is None:
	lo = 0
	if hi is None:
	hi = 2
	if 0 <= lo < hi <= 2:
	val = pyversion[lo:hi]
	if len(val) == len(thing) or len(val) > len(thing) and op not in ('==', '!='):
	return fixed_comparison(val, op, thing)
	return TRUTH_VALUE_UNKNOWN

[hack3r-0m]

@paw-lu do you think adding this case in that function will solve this issue or there are any other files that need to be changed too? are there any more such cases that need to be included?

[paw-lu]

do you think adding this case in that function will solve this issue or there are any other files that need to be changed too?

Honestly not sure! We'll have to play with it to find out. This was just an initial pointer to those interested in helping out.

are there any more such cases that need to be included?

For now, I think just taking into account the reverse order of the supported comparison operations is good. If people are interested in more complex operations support for that can be added later.

[PrasanthChettri]

@paw-lu
I wrote a small sample for doing the same

    l_value_sys = contains_sys_version_info(expr.operands[0])
    l_value_tuple_or_int = contains_int_or_tuple_of_ints(expr.operands[0]) 

    r_value_sys = contains_sys_version_info(expr.operands[1])
    r_value_tuple_or_int = contains_int_or_tuple_of_ints(expr.operands[1]) 

    index = l_value_sys or r_value_sys
    thing = r_value_tuple_or_int or l_value_tuple_or_int

I think it's adequate and correct, but it makes a bunch of test cases fail, mainly it gives out
I don't know what went wrong, can anyone help me with this?

[panse98]

Is this a problem related to only mac OS or it will arise on windows also?

[PrasanthChettri]

I believe it's for all the platforms

[alvaroq10]

Hi, first timer here and I have been following this problem and working on it for class in which I need to submit a pull request for an open-source project issue and I'm at a point where I expected to submit my PR for this in the next week or so. However, I see that another PR has already been submitted. Would I still be able to submit my own attempt at a solution in that timeframe and possibly receive some feedback even if its not chosen to be merged in? It would be really appreciated and save me from having to scrap my progress and restart elsewhere.

[JelleZijlstra]

@alvaroq10 we're having trouble keeping up with PRs as is, we're unlikely to look much at a PR for a problem that already has a solution.