Imports in `try/except` blocks are not analyzed correctly (bump the version requirement up)

[szabi]

Imports in `try/except` blocks are not analyzed correctly (bump the version requirement up)

[netromdk]

Thanks for the feedback. Vermin parses Python code into an abstract syntax tree via Python's own ast module. However, it cannot distinguish between which flows would be followed at runtime because it doesn't actually execute the code. It's all static analysis. Note that side effects would occur if Vermin actually executed code - it could yield destructive behavior.

Let's consider your code example:

try:
    # be ready for 3.11 when it drops
    from enum import StrEnum
except ImportError:
    # enabled by package https://pypi.org/project/backports.strenum/
    # Thus we can use `StrEnum` in Python < 3.11, >=3.8.6
    from backports.strenum import StrEnum

It is parsed into the following abstract syntax tree:

Module(
  body=[
    Try(
      body=[
        ImportFrom(module='enum', names=[alias(name='StrEnum')], level=0)
      ],
      handlers=[
        ExceptHandler(
          type=Name(id='ImportError', ctx=Load()),
          body=[ImportFrom(module='backports.strenum', names=[alias(name='StrEnum')], level=0)]
        )
      ],
      orelse=[],
      finalbody=[]
    )
  ],
  type_ignores=[]
)

At runtime an exception would occur in try if the import fails but Vermin cannot know this, unfortunately.
This is why it checks each block of the abstract syntax tree in sequence.

You can see the Analysis Exclusions section of the README for information about how to exclude certain things from analysis that would otherwise confuse the outcome.

As for the backport, that could be added. Currently, the https://pypi.org/project/enum34/ backport is known as a backport for enum. But I guess that does not cover StrEnum so it doesn't work.

[szabi]

Thank you for very thorough and detailed reply.

I think, however, that vermin could still correctly disregard / not report the analysis result of "'enum.StrEnum' member requires !2, 3.11" even with a static analysis. Exactly because the found condition/bound is (statically!) within a "try" block in the AST. If it's in a "try" block, especially if there is an "ImportError" excepthandler, it is clear that the member is not required, as it is handled somehow otherwise by the developer.

Can you think of a situation where having the rule "ignore bounds from 'import member requires ...'" when in a try block (with an ImportError handler) would lead to a wrong analysis result? Obviously, the branch in the except ImportError: or a finally-block must be kept in the list of considerations.

[netromdk]

You are welcome. Sorry for the slow reply.

There are some things that are unclear about how to handle incompatible versions in such check-and-import-or-import-other situations. What if there are multiple versions detected in both try: and except: branches? And let's say some of those aren't import-like statements. Should those additional findings also be disregarded based on the choice of branch?

Additionally, which findings should be the ones reported? If one of them is to be disregarded, Vermin would actively have to choose. It cannot know whether you prefer to keep the try: findings or the except: findings. Furthermore, it still depends on runtime whether it is 3.11 or ~3. So even if it reported ~3 as you mention, I would argue it could also be considered wrong not to report 3.11.

This is why such a feature has not been implemented before, and why it always reports all findings such that it doesn't accidentally suppress valuable information.

Maybe exclude the try:except: using # novermin, and explain external dependencies in your docs, since your feature request is about excluding certain findings?

But, just for the record, I do understand where you're coming from and I've been thinking about this situation many times before without coming up with a proper solution, unfortunately.

[netromdk]

Closing this for now.