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

syntax-error incorectly detected inside a type-comment with python 3.8 #3556

Open
exhuma opened this issue May 1, 2020 · 19 comments
Open

syntax-error incorectly detected inside a type-comment with python 3.8 #3556

exhuma opened this issue May 1, 2020 · 19 comments
Labels
Bug 🪲 Needs astroid update Needs an astroid update (probably a release too) before being mergable typing

Comments

@exhuma
Copy link

exhuma commented May 1, 2020

Steps to reproduce

"requirements.txt" file for the code-example below:

astroid==2.4.0
attrs==19.3.0
autopep8==1.5.2
isort==4.3.21
lazy-object-proxy==1.4.3
mccabe==0.6.1
mypy==0.770
mypy-extensions==0.4.3
psutil==5.7.0
psycopg2-binary==2.8.5
pycodestyle==2.5.0
pylint==2.5.0
six==1.14.0
SQLAlchemy==1.3.16
sqlalchemy-stubs==0.3
toml==0.10.0
typed-ast==1.4.1
typing-extensions==3.7.4.2
wrapt==1.12.1

Create a file with the following content:

# pylint: disable=too-few-public-methods, missing-docstring
from typing import Set

from sqlalchemy import Column, ForeignKeyConstraint, String
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import relationship

Base = declarative_base()


class Device(Base):  # type: ignore
    __tablename__ = 'device'
    hostname = Column(String, primary_key=True)
    ports = relationship(  # type: Set[Port]
        "Port",
        back_populates="device",
        collection_class=set
    )


class Port(Base):  # type: ignore
    __tablename__ = 'port'
    __table_args__ = (
        ForeignKeyConstraint(
            ['hostname'],
            ['device.hostname'],
            ondelete='CASCADE',
            onupdate='CASCADE',
        ),
    )
    hostname = Column(String, primary_key=True)
    label = Column(String, primary_key=True)
    device = relationship(  # type: Device
        "Device",
        back_populates="ports",
        uselist=False,
    )

Run pylint on that file.

Current behavior

pylint currently raises a syntax-error on line 14:

************* Module model
samemory/model.py:14:36: E0001: invalid syntax (<unknown>, line 14) (syntax-error)

Expected behavior

No syntax error should be raised

pylint --version output

› poetry run pylint --version                                                                                                                                                    ✗: 2
/usr/lib/python3.8/site-packages/html5lib/_trie/_base.py:3: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.9 it will stop working
  from collections import Mapping
/home/exhuma/work/sandbox/samemorytest/.venv/lib/python3.8/site-packages/astroid/interpreter/_import/spec.py:15: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
  import imp
pylint 2.5.0
astroid 2.4.0
Python 3.8.2 (default, Feb 26 2020, 22:21:03)
[GCC 9.2.1 20200130]
@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented May 1, 2020

I can reproduce this. Apparently, pylint expect the type to be after the declaration not in the middle of it. Ie:

    ports = relationship(
        "Port", back_populates="device", collection_class=set
    )  # type: Set[Port]

But it should not be a syntax error.

@Pierre-Sassoulas
Copy link
Member

I tried to add a test case, but I think fixing the version of various package is causing a problem. The problem does not exist with pylint 2.5.0 if you do not force other libs to be at a specific version.

Step to fix:

python3 -m venv venv
source venv/bin/activate
pip3 install SQLAlchemy==1.3.16 sqlalchemy-stubs==0.3 pylint==2.5.0
pylint test.py

Result :

************* Module test
test.py:8:0: E0602: Undefined variable 'Base' (undefined-variable)
test.py:11:13: E0602: Undefined variable 'Base' (undefined-variable)
test.py:12:4: E0602: Undefined variable '__tablename__' (undefined-variable)
test.py:13:4: E0602: Undefined variable 'hostname' (undefined-variable)
test.py:14:4: E0602: Undefined variable 'ports' (undefined-variable)
test.py:19:11: E0602: Undefined variable 'Base' (undefined-variable)
test.py:20:4: E0602: Undefined variable '__tablename__' (undefined-variable)
test.py:21:4: E0602: Undefined variable '__table_args__' (undefined-variable)
test.py:26:4: E0602: Undefined variable 'hostname' (undefined-variable)
test.py:27:4: E0602: Undefined variable 'label' (undefined-variable)
test.py:28:4: E0602: Undefined variable 'device' (undefined-variable)
test.py:2:0: W0611: Unused Set imported from typing (unused-import)

---------------------------------------------------------------------
Your code has been rated at -27.33/10 (previous run: 9.67/10, -37.00)

Try not settings the version of the dependencies of your dependencies, your requirements should be just "pylint".

@exhuma
Copy link
Author

exhuma commented May 2, 2020

@Pierre-Sassoulas I don't understand. I have pylint 2.5 and I get the error. But you say:

The problem does not exist with pylint 2.5.0

I also don't fix the versions in the dependencies. I am using poetry to manage dependencies and have this as my pyproject.toml:

[tool.poetry]
name = "samemorytest"
version = "0.1.0"
description = "Test Code to reproduce a memory issue from a larger application"
license = "MIT"

[tool.poetry.dependencies]
python = "^3.5"
sqlalchemy = "^1.3.16"
psycopg2-binary = "^2.8.5"
psutil = "^5.7.0"
attrs = "^19.3.0"

[tool.poetry.dev-dependencies]
autopep8 = "^1.5.2"
pylint = "^2.5.0"
mypy = "^0.770"
sqlalchemy-stubs = "^0.3"

[build-system]
requires = ["poetry>=0.12"]
build-backend = "poetry.masonry.api"

The output you see in my original post is the output of pip freeze which always outputs the file as hard-pinned versions. I added that to the original report in order to make it reproducible.

I still get the "Syntax Error" and I have pylint 2.5.0. So I don't understand why this was closed?

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented May 2, 2020

I'm sorry, I made the mistake of assuming I could make a quick guess to help you to fix your environment. Pylint 2.5.0 can process the file you gave correctly if you have the right package installed alongside it. I can even reproduce the bug with your faulty environment. I'm not going to debug your environement though.

But maybe it's a problem with python 3.8. Could you test what I suggested in my answer above and tell me if the problem still exists?

python3 -m venv venv
source venv/bin/activate
pip3 install SQLAlchemy==1.3.16 sqlalchemy-stubs==0.3 pylint==2.5.0
pylint test.py

@exhuma
Copy link
Author

exhuma commented May 2, 2020

Good call about the Python version. It seems to be an issue with Python 3.8. I created two fresh environments with 3.5 and 3.8. Here's the console output:

For Python 3.5

┌─[17:02:21] exhuma@displacer2
└─~/work/sandbox/pylint-bug-3556› ./env35/bin/python --version                                                                                                                                                ✗: 2
Python 3.5.9
┌─[17:02:28] exhuma@displacer2
└─~/work/sandbox/pylint-bug-3556› ./env35/bin/pylint test.py                                                                                                                                                     ✓
/home/exhuma/work/sandbox/pylint-bug-3556/env35/lib/python3.5/site-packages/astroid/interpreter/_import/spec.py:15: PendingDeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
  import imp
************* Module test
test.py:8:0: E0602: Undefined variable 'Base' (undefined-variable)
test.py:11:13: E0602: Undefined variable 'Base' (undefined-variable)
test.py:12:4: E0602: Undefined variable '__tablename__' (undefined-variable)
test.py:13:4: E0602: Undefined variable 'hostname' (undefined-variable)
test.py:14:4: E0602: Undefined variable 'ports' (undefined-variable)
test.py:21:11: E0602: Undefined variable 'Base' (undefined-variable)
test.py:22:4: E0602: Undefined variable '__tablename__' (undefined-variable)
test.py:23:4: E0602: Undefined variable '__table_args__' (undefined-variable)
test.py:31:4: E0602: Undefined variable 'hostname' (undefined-variable)
test.py:32:4: E0602: Undefined variable 'label' (undefined-variable)
test.py:33:4: E0602: Undefined variable 'device' (undefined-variable)
test.py:2:0: W0611: Unused Set imported from typing (unused-import)

----------------------------------------------------------------------
Your code has been rated at -27.33/10 (previous run: -27.33/10, +0.00)

And for Python 3.8

┌─[17:02:34] exhuma@displacer2
└─~/work/sandbox/pylint-bug-3556› ./env38/bin/python --version
Python 3.8.2
┌─[17:02:40] exhuma@displacer2
└─~/work/sandbox/pylint-bug-3556› ./env38/bin/pylint test.py
/home/exhuma/work/sandbox/pylint-bug-3556/env38/lib/python3.8/site-packages/astroid/interpreter/_import/spec.py:15: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
  import imp
************* Module test
test.py:14:36: E0001: invalid syntax (<unknown>, line 14) (syntax-error)

@Pierre-Sassoulas Pierre-Sassoulas changed the title syntax-error detected inside a type-comment (false-positive) syntax-error incorectly detected inside a type-comment with python 3.8 May 2, 2020
@PCManticore
Copy link
Contributor

I believe this is in issue with type comments parsing on Python 3.8:

In [3]: ast.parse(open('a.py').read(), type_comments=True)
Traceback (most recent call last):

  File "/Users/claudiu/.pyenv/versions/3.8.1/lib/python3.8/site-packages/IPython/core/interactiveshell.py", line 3331, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)

  File "<ipython-input-3-50ff69f324d7>", line 1, in <module>
    ast.parse(open('a.py').read(), type_comments=True)

  File "/Users/claudiu/.pyenv/versions/3.8.1/lib/python3.8/ast.py", line 47, in parse
    return compile(source, filename, mode, flags,

  File "<unknown>", line 14
    ports = relationship(  # type: Set[Port]
                                   ^
SyntaxError: invalid syntax

We do have some code to reparse the file if the syntax errors were caused by misplaced type annotations, but in this case the error does not indicate anything related to the type comments at all: https://github.com/PyCQA/astroid/blob/master/astroid/builder.py#L446
I confirm it works fine when we're not trying to parse the type comments, so maybe we should remove that condition and retry the parsing in all cases.

@dbaty
Copy link
Contributor

dbaty commented Sep 7, 2020

See also #3757 which reports the same syntax error from Astroid on the following function definition:

def func(
    # type: () -> int
):
    return 2

Here also, the type comment is probably misplaced. Both errors are probably related, if not the same.

@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 2.7.0, 2.7.x Feb 20, 2021
@cdce8p cdce8p added the typing label Oct 17, 2021
@ewerybody
Copy link

ewerybody commented Oct 25, 2021

Hello! I'm on

pylint 2.11.1
astroid 2.8.3
Python 3.9.7

and also experience some syntax errors on typing comments like:

# type: (SourceDepotPathUI, cadtlib.source_item._CadtPath | None) -> None

image

# type: (str) -> dict[str, str]

image

each highlighting the first argument listed in the typing comment within the parenthesis with a popup:

invalid syntax (<unknown>, line 23)pylint(syntax-error)

How is this still an issue!? Why does this break on comments at ALL!? ☹️

Is there a way to prevent this? I'm actually using Pylance for these kinds of inspections and it works superb. Pylance is just not interested in a lot of coding style issues that pylint nicely covers...

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Oct 25, 2021

Hey could you provide the example as text instead of screenshot@ewerybody ? (Edit : not ina new issue, this one is still opened, sorry)

@ewerybody
Copy link

OK. Amended with code snippets👍 but kept the shots for context.

(Edit : not ina new issue, this one is still opened, sorry)

What do you mean? You want me to open a new issue with this?

Btw:
I "fixed" it by adding syntax-error to my now huge --disable= list 🙈

@ewerybody
Copy link

OK I just noticed that "fixing" it by adding syntax-error to the --disable= list just makes it skip the entire file.
That's really not acceptable ☹

@ewerybody
Copy link

OK. I just digged into this as well.

So. this is an astroid issue! What should we ask for there?

  • ask to expose type_comments?
  • make pylint set type_comments to False?

In the function @PCManticore was linking it seems that astroid IS actually dealing with broken parsing
when type_comments == True! (see) But there is only 1 type of error message checked:

MISPLACED_TYPE_ANNOTATION_ERROR = "misplaced type annotation"

The one that I'm having is 'invalid syntax'. Well. should this break anyway? The comment there kind of speaks what I'm thinking:

        # If the type annotations are misplaced for some reason, we do not want
        # to fail the entire parsing of the file, so we need to retry the parsing without
        # type comment support.

After all: astroid would probably needed to be fixed to accept these kinds of type comments!
Well ... OK. I'll try to open an issue there.

bhack added a commit to bhack/tensorflow that referenced this issue Mar 10, 2022
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 2.x, 2.15.0 May 9, 2022
@DanielNoord DanielNoord added the Needs astroid update Needs an astroid update (probably a release too) before being mergable label Jun 30, 2022
@DanielNoord
Copy link
Collaborator

This is very similar to #3757.

However, while that issues is about ignoring incorrect type comments this one is about recognising a different set of incorrect type comments and handling those accordingly. They should probably be fixed at the same time.

@Pierre-Sassoulas Pierre-Sassoulas removed this from the 2.15.0 milestone Aug 25, 2022
@exhuma
Copy link
Author

exhuma commented Oct 29, 2022

I've tested this against newer versions of Python (3.10.4) and Pylint (2.15.5) and it is still reproducible.

Here's the new requirements.txt file:

astroid==2.12.12
attrs==19.3.0
autopep8==1.5.2
dill==0.3.6
isort==4.3.21
lazy-object-proxy==1.4.3
mccabe==0.6.1
mypy==0.770
mypy-extensions==0.4.3
platformdirs==2.5.2
psutil==5.7.0
psycopg2-binary==2.8.5
pycodestyle==2.5.0
pylint==2.15.5
six==1.14.0
SQLAlchemy==1.3.16
sqlalchemy-stubs==0.3
toml==0.10.0
tomli==2.0.1
tomlkit==0.11.6
typed-ast==1.5.4
typing-extensions==3.7.4.2
wrapt==1.12.1

And the diff against the previous file:

--- requirements.txt    2022-10-29 11:18:27.580688144 +0200
+++ requirements-2022-10-29.txt 2022-10-29 11:17:57.039770141 +0200
@@ -1,19 +1,23 @@
-astroid==2.4.0
+astroid==2.12.12
 attrs==19.3.0
 autopep8==1.5.2
+dill==0.3.6
 isort==4.3.21
 lazy-object-proxy==1.4.3
 mccabe==0.6.1
 mypy==0.770
 mypy-extensions==0.4.3
+platformdirs==2.5.2
 psutil==5.7.0
 psycopg2-binary==2.8.5
 pycodestyle==2.5.0
-pylint==2.5.0
+pylint==2.15.5
 six==1.14.0
 SQLAlchemy==1.3.16
 sqlalchemy-stubs==0.3
 toml==0.10.0
-typed-ast==1.4.1
+tomli==2.0.1
+tomlkit==0.11.6
+typed-ast==1.5.4
 typing-extensions==3.7.4.2
 wrapt==1.12.1

Interestingly, mypy caused a syntax error as well on the same lines. This could be fixed by upgrading mypy. The pylint issue remains however.

For Python 3 projects it is anyway possible to write proper annotations for type-hinting instead of comments. Which does not suffer from the same problem:

class Foo:
    var = {10}  # type: Set[int]           <- Syntax error
    var: Set[int] = {10}  # ok

@ngie-eign
Copy link
Contributor

ngie-eign commented Feb 8, 2023

This is still an issue with python2.7 style type hints. Here's a trivial repro:

# Output looks like this:
#
#     type: Bogus_Type;

Software versions used when reproducing this issue:

$ pkg query "%dn %dv" pylint-py39
py39-tomlkit 0.11.6
py39-tomli 2.0.1
python39 3.9.16
py39-typing-extensions 4.4.0
py39-setuptools 63.1.0
py39-platformdirs 2.5.4
py39-mccabe 0.7.0
py39-isort 5.10.1
py39-dill 0.3.4
py39-astroid 2.12.13

This was a false positive flagged in some code that copy-pasted output from a comment (I changed the type).

@ngie-eign
Copy link
Contributor

I honestly think that the best path forward for addressing this is to either make this issue a warning or remove pre-python3.6 compatible type hint support entirely, given that <python3.6 is no longer supported.

@exhuma
Copy link
Author

exhuma commented Feb 9, 2023

I don't have an issue with dropping 3.6 considering that it's out of support.

@Pierre-Sassoulas
Copy link
Member

Code with python 2 type hint is still valid in python 3.11. The support is being dropped on some tools like autoflake, I know because I have some big code base that are typed this way and autoflake started to remove the typing import used in the py2 type hint. But I don't have any trouble with pylint so far, so I suppose this issue is a corner case (or I would have fixed it myself). Ultimately we might have to drop the issue if no one is interested by working on it (even if theoretically it's still useful).

@yilei
Copy link
Contributor

yilei commented Feb 28, 2023

Pylint also doesn't like # type: Any comments after a module import:

from collections import abc  # type: Any
E0001: Parsing failed: 'invalid syntax (<unknown>, line 1)' (syntax-error)
$ pylint --version
pylint 2.16.2
astroid 2.14.2
Python 3.11.0 (main, Feb 21 2023, 11:59:45) [GCC 12.2.0]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Needs astroid update Needs an astroid update (probably a release too) before being mergable typing
Projects
None yet
Development

No branches or pull requests

9 participants