astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
30.78k stars 1.02k forks source link

SIM108 autofix removes comments #1719

Closed twoertwein closed 1 year ago

twoertwein commented 1 year ago
def test(x: int) -> int:
    if x > 0:
        # test test
        abc = x
    else:
        # test test test
        abc = -x
    return abc

ruff --fix # 0.0.213

def test(x: int) -> int:
    abc = x if x > 0 else -x
    return abc
messense commented 1 year ago

I'm not sure what to do, merge comments together doesn't make much sense to me.

twoertwein commented 1 year ago

Maybe something like:

def test(x: int) -> int:
    abc = (
        # test test
        x if x > 0 else
        # test test test
        -x
    )
    return abc
charliermarsh commented 1 year ago

My gut reaction is that this is quite hard to get right in the general case. We could try to come up with some formal rules, but it might be easier to just avoid fixing in the event that the snippet contains comments (or make that an option).

spaceone commented 1 year ago

Maybe just ignore the check if there are multiple lines involved? I got this fix also for this code:

                   if parser.errno == BAD_FIRST_LINE:
                      req = wrappers.Request(sock, server=self._server)
                  else:
                      req = wrappers.Request(
                          sock,
                          parser.get_method(),
                          parser.get_scheme() or _scheme,
                          parser.get_path(),
                          parser.get_version(),
                          parser.get_query_string(),
                          server=self._server,
                      ) 

and it didn't make the code cleaner.

Duplicate: https://github.com/charliermarsh/ruff/issues/1791#issue-1529813542

spaceone commented 1 year ago

See also https://github.com/charliermarsh/ruff/issues/1766

spaceone commented 1 year ago

REOPEN:

ruff 0.0.237 just changed this, which ends with a comment:

-            if 200 <= exitcode < 300:
-                exitcode = 0
-            else:
-                exitcode = 1  # dooh, sys.exit returns a short, mapping 400 to 144
+            exitcode = 0 if 200 <= exitcode < 300 else 1  # dooh, sys.exit returns a short, mapping 400 to 144