pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.31k stars 1.14k forks source link

Crash ``Parsing Python code failed: expected an indented block after function definition`` #6301

Closed evgeni closed 2 years ago

evgeni commented 2 years ago

Bug description

When parsing the following file:

#!/usr/bin/python
import os

def bug():
    # pylint:disable=R
    if not os.path.exists('/bug'):
        os.mkdir("/bug")

pylint crashed with a AstroidSyntaxError and with the following stacktrace:

Traceback (most recent call last):
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/astroid/builder.py", line 168, in _data_build
    node, parser_module = _parse_string(data, type_comments=True)
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/astroid/builder.py", line 454, in _parse_string
    parsed = parser_module.parse(data + "\n", type_comments=type_comments)
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/astroid/_ast.py", line 49, in parse
    return parse_func(string)
  File "/usr/lib64/python3.10/ast.py", line 50, in parse
    return compile(source, filename, mode, flags,
  File "<unknown>", line 5

    ^
IndentationError: expected an indented block after function definition on line 4

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/pylint/lint/pylinter.py", line 1111, in _check_files
    self._check_file(get_ast, check_astroid_module, file)
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/pylint/lint/pylinter.py", line 1146, in _check_file
    check_astroid_module(ast_node)
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/pylint/lint/pylinter.py", line 1298, in check_astroid_module
    retval = self._check_astroid_module(
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/pylint/lint/pylinter.py", line 1341, in _check_astroid_module
    checker.process_module(node)
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/pylint/checkers/similar.py", line 832, in process_module
    self.append_stream(self.linter.current_name, stream, node.file_encoding)  # type: ignore[arg-type]
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/pylint/checkers/similar.py", line 373, in append_stream
    LineSet(
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/pylint/checkers/similar.py", line 663, in __init__
    self._stripped_lines = stripped_lines(
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/pylint/checkers/similar.py", line 566, in stripped_lines
    tree = astroid.parse("".join(lines))
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/astroid/builder.py", line 281, in parse
    return builder.string_build(code, modname=module_name, path=path)
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/astroid/builder.py", line 138, in string_build
    module, builder = self._data_build(data, modname, path)
  File "/home/evgeni/Devel/katello/katello-client-bootstrap/venv/lib64/python3.10/site-packages/astroid/builder.py", line 170, in _data_build
    raise AstroidSyntaxError(
astroid.exceptions.AstroidSyntaxError: Parsing Python code failed:
expected an indented block after function definition on line 4 (<unknown>, line 5)

If I remove the pylint:disable=R comment, things work as expected. If I call pylint without --ignore-imports=y, things work as expected. If I downgrade pylint (and astroid) to astroid-2.9.3 pylint-2.12.2, things work as expected.

Configuration

No response

Command used

python -m pylint --ignore-imports=y ./bootstrap.py

Pylint output

************* Module bootstrap
bootstrap.py:1:0: F0002: bootstrap.py: Fatal error while checking 'bootstrap.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/evgeni/.cache/pylint/pylint-crash-2022-04-14-09.txt'. (astroid-error)

Expected behavior

no astroid error ;-)

Pylint version

% pylint --version
pylint 2.13.5
astroid 2.11.2
Python 3.10.4 (main, Mar 25 2022, 00:00:00) [GCC 11.2.1 20220127 (Red Hat 11.2.1-9)]

OS / Environment

Fedora 35

Additional dependencies

astroid==2.11.2 dill==0.3.4 flake8==4.0.1 isort==4.3.21 lazy-object-proxy==1.7.1 mccabe==0.6.1 platformdirs==2.5.1 pycodestyle==2.8.0 pyflakes==2.4.0 pylint==2.13.5 toml==0.10.2 tomli==2.0.1 wrapt==1.13.3

cdce8p commented 2 years ago

Unfortunately, I cannot reproduce the error. Please make sure the indentation is correct. AstroidSyntaxError is usually emitted if the Python AST parsing failed.

evgeni commented 2 years ago

The syntax is correct (Python can execute the code just fine).

Did you call pylint with --ignore-imports=y?

evgeni commented 2 years ago

for some reason, lines in https://github.com/PyCQA/pylint/blob/85a480427c0f14dba9e26d56286b0a370057e792/pylint/checkers/similar.py#L566-L567 is only ['#!/usr/bin/python\n', 'import os\n', '\n', 'def bug():\n'] which indeed is invalid code.

It probably gets swallowed here: https://github.com/PyCQA/pylint/blob/85a480427c0f14dba9e26d56286b0a370057e792/pylint/checkers/similar.py#L368-L369

mbyrnepr2 commented 2 years ago

I can reproduce this with your specific versions of Pylint & astroid. It doesn't occur for:

pylint 2.14.0-dev0
astroid 2.11.2
Python 3.10.0b2 (v3.10.0b2:317314165a, May 31 2021, 10:02:22) [Clang 12.0.5 (clang-1205.0.22.9)]
evgeni commented 2 years ago

Right, upgrading pylint to latest git (47e168cf607e2069b103fc466edfe1c6522e2fb2) does fix it.

cdce8p commented 2 years ago

I was able to reproduce it now. Like @mbyrnepr2 said, this is already fixed in main. In particular #6271 seems to be the PR which resolved it. Tbh though, I'm not quite sure why.

Maybe you have an idea @DanielNoord? It seems to be related to the --ignore-imports=y option. Would we be able to backport the change?

DanielNoord commented 2 years ago

No, sorry! I have no immediate hunch for what could have been the fix (or previous bug)/

The only thing I can think of is the commenting of linter.load_command_line_configuration. Especially since the ignore-imports comes from the CLI. The way Similar (of which the options originates) interacts with the configuration parsing is quite intricate because it also has to do it semi-standalone. I worry I might have broken something while working on the transition that wasn't caught by our testsuite. It might be worth checking if we can also reproduce this with ignore-imports in a configuration file. If not, then we could add a notice about this in the release notes of 2.13.6.

Sorry guys! I hoped to avoid this! 😓

Pierre-Sassoulas commented 2 years ago

Sorry guys! I hoped to avoid this!

Don't worry, as we say in some python packaging circles it's impossible to migrate to argparse without breaking a few eggs.

DanielNoord commented 2 years ago

@evgeni Have you checked if the problem exists when using a configuration file as well? If not I'd say we can close this issue and (sadly) accept that this might be a bug in 2.13.6 and any other 2.13.x...

evgeni commented 2 years ago

Same thing with a config:

% cat pylintrc
[MASTER]
ignore-imports=y

% python -m pylint ./l2.py
************* Module l2
l2.py:1:0: F0002: l2.py: Fatal error while checking 'l2.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/evgeni/.cache/pylint/pylint-crash-2022-04-15-09.txt'. (astroid-error)
DanielNoord commented 2 years ago

😢

Sorry, I don't really have an easy solution here. Especially since this might have happened somewhere in the middle of the 30+ PRs that were necessary to transition from optparse to argparse... The only thing I can do is work extra hard to get 2.14 out sooner and have it work again 😄

evgeni commented 2 years ago

🤗

No worries, I can just pin to <2.13 for now.

For the mighty search engines: on Python 3.8 the error reads SyntaxError: unexpected EOF while parsing, the rest is identical :)

DanielNoord commented 2 years ago

Btw, 2.13.0 should work. We didn't touch the configuration handling until after .0!

evgeni commented 2 years ago

Nope, this is failing for me with

% python -m pylint --version
pylint 2.13.0
astroid 2.11.2
Python 3.10.4 (main, Mar 25 2022, 00:00:00) [GCC 11.2.1 2022012
7 (Red Hat 11.2.1-9)]
DanielNoord commented 2 years ago

Then we might have actually fixed a bug we didn't know about by moving to argparse...

I don't know any more 😅

I'm going to keep this issue open for visibility and close it as we release 2.14. Thanks for helping triaging this and sorry for not being able to give a better solution!

jacobtylerwalls commented 2 years ago

I was curious, so I bisected it. : D

We have, unfortunately, two bugs where the second bug silenced the crash aspect of the first one.

Bug 1: 4ca885fd8c35e6781a359cda0e4de2c0910973b6 Block disables can be used on function defs. So by creating the functionality of block disables for duplicate-code, everything under the def line is empty, which will crash with AstroidError when parsed in checkers.similar.stripped_lines().

Bug 2: 03cfbf3df1d20ba1bfd445c59f18c906e8dd8a62 This silenced the other bug by just not respecting ignore-imports=y. Place a breakpoint before if if ignore_imports or ignore_signatures: in checkers.similar.stripped_lines() and see that ignore_imports is now False. Something to do with SimilarChecker registering options. I'll open a separate issue for that for 2.14. We can leave this issue open for the crash in 2.13.6.

evgeni commented 2 years ago

escalated

Originally I thought this was my odd codebase that was tripping over pylint and wanted to shrug it off…

jacobtylerwalls commented 2 years ago

:-) we love testers! second issue extracted into #6350 so we can make sure to do it. thanks for getting in touch!

DanielNoord commented 2 years ago

@jacobtylerwalls What do you think would the best way to solve this? I was thinking maybe add an ... to lines for every disable we encounter, but then Similar might start bugging users about duplicate ...?

jacobtylerwalls commented 2 years ago

I'm testing a patch that inserts ": ..." so that def a(): # pylint: disable=blah\n becomes def a(): ... # pylint: disable=blah\n etc