python-trio / flake8-async

Highly opinionated linter for Trio code
https://flake8-async.readthedocs.io
MIT License
17 stars 2 forks source link

How to handle unparseable/incorrect code #147

Closed jakkdl closed 1 year ago

jakkdl commented 1 year ago

Moving this out from #145 so it can be merged. I'll make it 1 for now for simplicity of implementation

What should the program/plugin do when encountering the following code:

for _ in range(1, 10, 0):
  ...

Note that this is only when all parameters are literals, so there should literally be no way this is correct - other than when somebody is intentionally writing incorrect code in a test.

None of the linters of ours catches it (could be added to bugbear I suppose, but it doesn't seem worth given how rare it is), but we happen to be able to catch it as a side-effect of checking whether loops are guaranteed to run. Options:

  1. Treat it as not guaranteed to iterate
    • Not Our Business:tm: to warn about it. Program will crash at runtime as expected.
    • I've been bitten in the past when developing this when I've wrapped stuff in except Exception and mistyped code inside the try: - so I somewhat dislike it for that reason. But that should not really be a thing here, and I rewrote it to have minimal code within the try.
  2. Add error code that can be used in the future for various clearly incorrect code that's checked as a side effect of the plugin.
    • Needs an extra error code, and might actually need some added infrastructure since it's bound to a helper - not a visitor/error class.
  3. Print to stdout
    • Should be rare enough that sidestepping the proper error infra and just printing to stdout should be fine. Will warrant a separate test, but that's easy to write. Downside is that you can't ignore it, nor will affect return status of the check.

Example implementation of 3:

try:
    evaluated_range = range(*values)
except Exception:
    parameters = ", ".join(map(str, values))
    print(f'Malformed range expression: `range({parameters})` crashes.')
    return False
return len(evaluated_range) > 0

I'm not coming up with any other similar cases in other helpers that this could also apply to, but I could imagine similar cases coming up again. But it's really minor and this case should be exceedingly rare, only real reason would be if somebody is messing up the order of the parameters in a 2/3-parameter range expression.

Zac-HD commented 1 year ago

raise RuntimeError(explanation) or print-and-ignore both seem fine to me.