codeql icon indicating copy to clipboard operation
codeql copied to clipboard

False positive – "Statement has no effect" for Python type hint ellipsis

Open maxfischer2781 opened this issue 3 years ago • 3 comments
trafficstars

Description of the false positive

The ellipsis ... (three dots) is commonly used in Python to omit bodies for type hinting declarations. Since technically a lone ... is an expression statement, LGTM/CodeQL marks it as a statement without effect.

For example, we could have something like this:

@overload
def some_func(a: float) -> str:
    ...   # body is omitted since this is just for type hinting

@overload
def some_func(a: int) -> int:
   ...

def some_func(a):
    return a if isinstance(a, int) else str(a)

In this case, both @overload definitions exist only for type hinting. Only the final definition is actually executable at runtime. Emitting a warning for the type hint bodies is misleading; they have no effect on purpose.

Roughly, it should be fine to suppress the warning for def blocks if:

  • the entire body is ...
  • it is in a type-declaration context such as @overload or Protocol

See PEP 8 on "Function annotations …" for reference.

Code samples or links to source code

https://github.com/maxfischer2781/asyncstdlib/blob/b4b3a557bf7d0c411495d9fd7bd8e8f500ab6b97/asyncstdlib/builtins.py#L328-L335

https://github.com/maxfischer2781/asyncstdlib/blob/57f372b3ea8367b9cb448647664e582e783e4e75/asyncstdlib/_lrucache.py#L77-L79

URL to the alert on GitHub code scanning (optional)

https://github.com/maxfischer2781/asyncstdlib/security/code-scanning/33

https://github.com/maxfischer2781/asyncstdlib/security/code-scanning/98

maxfischer2781 avatar Nov 21 '22 16:11 maxfischer2781

Also seeing lot (several pages) of these spurious false positives: e.g. https://github.com/bokeh/bokeh/security/code-scanning/339

Besides pep8 it's worth noting the Mypy docs explicitly demonstrate this usage for @overload specifically: https://mypy.readthedocs.io/en/stable/more_types.html#function-overloading

bryevdv avatar Nov 27 '22 23:11 bryevdv

I've just updated the description since I noticed that not only @overload is a frequent situation for this false positive but also Protocol. The docs also explicitly list an example using ellipsis:

class Proto(Protocol):
    def meth(self) -> int:
        ...

maxfischer2781 avatar Jan 05 '24 16:01 maxfischer2781

Is there any workaround for this issue? I'm facing the same problem (example), and it's making PRs much harder to review.

JasonGrace2282 avatar Jul 15 '24 14:07 JasonGrace2282