if sys.version_info blocks without an else get rewritten via the pre-commit hook

[jvavrek]

In the section on Python2 and old Python3.x blocks, the README states:

Note that if blocks without an else will not be rewritten as it could introduce a syntax error.

However I have noticed that some of my else-less if blocks for checking old Python versions get deleted by the pyupgrade pre-commit hook.

Minimal steps to replicate:

mkdir pre-commit-bug
cd pre-commit-bug
git init
# create test.py and .pre-commit-config.yaml as below
git add test.py .pre-commit-config.yaml
pyupgrade --py36-plus test.py  # no change
pre-commit install
pre-commit run --all  # this rewrites the if block test.py

# test.py
import sys
from setuptools import setup

name = "my_app"

# Check python version
if sys.version_info < (3, 6):
    raise Exception(f"{name} doesn't support python<3.6")

# run setup
setup(name=name)

# .pre-commit-config.yaml
repos:
  - repo: https://github.com/asottile/pyupgrade
    rev: v3.15.0
    hooks:
    - id: pyupgrade
      args: [--py36-plus]

Not only does the pre-commit hook rewrite (delete) the if block unexpectedly, it also deletes the next comment (# run setup)! As a result, my test.py after pre-commit looks like

import sys
from setuptools import setup

name = "my_app"

# Check python version
setup(name=name)

where the remaining comment now pertains to the wrong line of code.

Version info: pyupgrade 3.3.1 pre-commit 2.20.0 python 3.9.13

[asottile]

this was intentionally changed at some point. there are a few cases where it still leaves an else less one but when it deems it safe it can

[jvavrek]

this was intentionally changed at some point. there are a few cases where it still leaves an else less one but when it deems it safe it can

Is it intentional that the behavior differs between the pre-commit hook and the command line, though?

Similarly, is it intentional/safe that it deletes the next comment?

[asottile]

you are comparing 3.3.1 vs 3.15.0 what do you expect?

[jvavrek]

you are comparing 3.3.1 vs 3.15.0 what do you expect?

Ok, if I update my local pyupgrade to 3.15.0, I get consistent behavior between the two---both delete the if block. However I still submit that there are two issues:

1. The documentation is out of date
2. pyupgrade incorrectly deletes the next comment after the if sys.version_info block

[asottile]

the end of the block is not well defined when there's trialing comments -- pyupgrade picks the approach based on the information the tokenization hands it -- there isn't a correct choice because people do all sorts of flavors of:

if ...:
    code
    # comment

etc.

the docs aren't really out of date since there are still cases where it will leave an elseless branch alone -- but enumerating them in the docs isn't useful

[asottile]

the case the docs is specifically referring to (where it will introduce a syntax error) is:

if True:
    if sys.version_info ...:
        ...

[jvavrek]

Fair points, thanks.