Simplify suppression of spurious 'unused-ignore's

[ikonst]

In #15164 we've made it so that # type: ignores in "unreachable" code are not flagged as unused.

Here I'm removing the unreachable_lines attribute (and related setters). It appears that the same effect can be achieved by pruning the lines from the existing ignored_lines dict.

This change also has the advantage (IMO) of directly spelling out what we're doing here. In contrast, when assigning to unreachable_lines one has to follow the code to determine what it's being used for. And yes, it's more open-ended to keep "unreachable_lines" but then (a) it's not so simple because not all "unreachabilities" are the same, (b) unused open-end is unwarranted complexity.

Motivation

While looking at #12409, it became clear to me that the term "unreachable" became somewhat overloaded:

• code that cannot be soundly reached

  if False:
    a = 42  # E: Statement is unreachable
  if True:
    raise Exception
    a = 42  # E: Statement is unreachable
  assert False
  a = 42  # E: Statement is unreachable

• code unreachable on current version/platform

  if sys.version_info < (3,):
     a = 42
  if sys.platform == 'foobar':
     a = 42
  assert sys.platform == 'foobar'
  a = 42

IMO it would be helpful to disambiguate and I suggest we stop referring to the latter as "reachability".

[ikonst]

@ilevkivskyi since you authored #15164

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

[ilevkivskyi]

FWIW I am not a fan of this idea: in general I try to avoid modifying parsed part of AST unless really necessary, and ignored_lines comes almost directly from parser.

Another thing, I suspect this may break daemon mode, and --warn-unused-ignores is already quite fragile/tricky in daemon mode.

[ikonst]

Wouldn't del file.defs[i + 1 :] similarly modify parsed AST?

(Also, WDYT about at the very least renaming unreachable_lines to skipped_lines? Trying to disambiguate the concept of "reachability".)

[ikonst]

^ @ilevkivskyi

[ilevkivskyi]

Wouldn't del file.defs[i + 1 :] similarly modify parsed AST?

Yes, and I think it is a bad idea, but maybe it was needed for some reason (like an alternative would be even worse).

WDYT about at the very least renaming unreachable_lines to skipped_lines

Yes, this is a good idea.

[ikonst]

Closed in favor of #15483.