Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linting code containing sqlalchemy FunctionElement and compiler.compiles crashes pylint with an InferenceError exception #5679

Closed
davekirby opened this issue Jan 14, 2022 · 5 comments · Fixed by #5725
Assignees
Labels
Crash 💥 A bug that makes pylint crash
Milestone

Comments

@davekirby
Copy link

davekirby commented Jan 14, 2022

Bug description

After upgrading pylint from 2.2.2 to 2.12.2 and astroid from 2.1.0 to 2.9.3, a file that contained sqlalchemy user function definitions caused pylint to crash with an InferenceError exception. I have cut the file down to the example code below, and it still crashes. Curiously if you remove either the FunctionElement subclass OR the expression.compiles decorator it works, even though they are completely separate bits of code.

from sqlalchemy.ext.compiler import compiles
from sqlalchemy.sql.expression import FunctionElement


class concat(FunctionElement):  # pylint: disable=invalid-name,too-many-ancestors
    """Simple string concatenation function (variable length arguments one)."""
    name = "compat"


@compiles(concat, "postgresql")
def compile_concat_for_postgresql(element, compiler, **kw):
    """Implementation of compat for PostgreSQL."""
    return f"compat({compiler.process(element.clauses, **kw)})"


class months_between(
    FunctionElement
):  # pylint: disable=invalid-name,too-many-ancestors
    """Amount of months between two dates represented as integer value."""
    name = "months_between"

    def __init__(self, *clauses, **kwargs):
        super().__init__(*clauses, **kwargs)

Command used

pylint test.py

Pylint output

pylint crashed with a ``InferenceError`` and with the following stacktrace:


Traceback (most recent call last):
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/pylint/lint/pylinter.py", line 1034, in _check_files
    self._check_file(get_ast, check_astroid_module, file)
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/pylint/lint/pylinter.py", line 1069, in _check_file
    check_astroid_module(ast_node)
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/pylint/lint/pylinter.py", line 1204, in check_astroid_module
    ast_node, walker, rawcheckers, tokencheckers
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/pylint/lint/pylinter.py", line 1250, in _check_astroid_module
    walker.walk(node)
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/pylint/utils/ast_walker.py", line 75, in walk
    self.walk(child)
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/pylint/utils/ast_walker.py", line 75, in walk
    self.walk(child)
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/pylint/utils/ast_walker.py", line 75, in walk
    self.walk(child)
  [Previous line repeated 2 more times]
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/pylint/utils/ast_walker.py", line 72, in walk
    callback(astroid)
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/pylint/checkers/typecheck.py", line 1041, in visit_attribute
    for n in owner.getattr(node.attrname)
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/astroid/objects.py", line 210, in getattr
    return list(self.igetattr(name, context=context))
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/astroid/objects.py", line 176, in igetattr
    for inferred in bases._infer_stmts([cls[name]], context, frame=self):
  File "/opt/dbconda/envs/dbsleuth/lib/python3.6/site-packages/astroid/bases.py", line 170, in _infer_stmts
    context=context,
astroid.exceptions.InferenceError: Inference failed for all members of [<FunctionDef.__init__ l.None at 0x7f4606fdd0f0>].

Expected behavior

Pylint to run to completion and report any errors in the file.

Pylint version

pylint 2.12.2
astroid 2.9.3
Python 3.6.7 | packaged by conda-forge | (default, Nov 6 2019, 16:19:42)

OS / Environment

RHEL 8

Additional dependencies

SQLAlchemy==1.4.29

@davekirby davekirby added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jan 14, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Crash 💥 A bug that makes pylint crash and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jan 14, 2022
@Pierre-Sassoulas
Copy link
Member

Thank you for opening the bug. This is quite an upgrade you did there !

@DanielNoord
Copy link
Collaborator

The following diff would fix this issue:

diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py
index bb9c3de52..25837c343 100644
--- a/pylint/checkers/typecheck.py
+++ b/pylint/checkers/typecheck.py
@@ -1087,6 +1087,8 @@ accessed. Python regular expressions are accepted.",
                     continue
                 missingattr.add((owner, name))
                 continue
+            except astroid.InferenceError:
+                continue
             # stop on the first found
             break
         else:

However, I'd like to investigate if we can't prevent this InferenceError. It seems this might be preventable. Similarly, we might want to add the try...except to getattr in astroid instead of pylint as getattr does not really reflect that it using inference and that it should be guarded by an except.

@DanielNoord DanielNoord self-assigned this Jan 21, 2022
@DanielNoord
Copy link
Collaborator

I'm trying to narrow down the issue. I think I have found a solution that goes beyond catching the exception, but I wonder if my new fix isn't masking a deeper issue.

@davekirby I got the reproducible example down to:

from sqlalchemy.sql.expression import FunctionElement


def compile_concat_for_postgresql(compiler, **kw):
    """Implementation of compat for PostgreSQL."""
    return f"compat({compiler.process(**kw)})"


class months_between(FunctionElement):
    """Amount of months between two dates represented as integer value."""

    def __init__(self):
        super().__init__()

I don't really understand why compile_concat_for_postgresql is necessary for this to crash. I have no experience with sqlalchemy and I grepped for the function name but couldn't find anything. Could you try and explain what the function does? I don't immediately see why its definition interferes with astroid's inference of months_between but it seems to have some effect...

@DanielNoord
Copy link
Collaborator

DanielNoord commented Jan 25, 2022

Brought the reproducible example down to:
test.py:

from sqlalchemy.sql import roles
from test2 import FromClause

class HasMemoized(object):
    ...

class Generative(HasMemoized):
    ...

class ColumnElement(
    roles.ColumnArgumentOrKeyRole,
    roles.BinaryElementRole,
    roles.OrderByRole,
    roles.ColumnsClauseRole,
    roles.LimitOffsetRole,
    roles.DMLColumnRole,
    roles.DDLConstraintColumnRole,
    roles.StatementRole,
    Generative,
):
    ...

class FunctionElement(ColumnElement, FromClause):
    ...

class months_between(FunctionElement):
    """Amount of months between two dates represented as integer value."""
    def __init__(self):
        super().__init__()

test2.py:

from operator import attrgetter
from sqlalchemy.sql import roles

class HasCacheKey(object):
    ...

class HasMemoized(object):
    ...

class MemoizedHasCacheKey(HasCacheKey, HasMemoized):
    ...

class ClauseElement(MemoizedHasCacheKey):
    ...

class ReturnsRows(roles.ReturnsRowsRole, ClauseElement):
    ...

class Selectable(ReturnsRows):
    ...

class FromClause(roles.AnonymizedFromClauseRole, Selectable):
    c = property(attrgetter("columns"))

sqlalchemy.sql.roles:

from operator import attrgetter
from sqlalchemy.sql import roles

class HasCacheKey(object):
    ...

class HasMemoized(object):
    ...

class MemoizedHasCacheKey(HasCacheKey, HasMemoized):
    ...

class ClauseElement(MemoizedHasCacheKey):
    ...

class ReturnsRows(roles.ReturnsRowsRole, ClauseElement):
    ...

class Selectable(ReturnsRows):
    ...

class FromClause(roles.AnonymizedFromClauseRole, Selectable):
    c = property(attrgetter("columns"))

I believe the issue is related to the imports in roles. There are probably too many ancestors for the cache and therefore astroid trips up.

@DanielNoord
Copy link
Collaborator

I have found a fix and submitted a PR to astroid. I expect it to land in 2.10 which I hope we can release before pylint 2.13 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Crash 💥 A bug that makes pylint crash
Projects
None yet
3 participants