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

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

Closed
evgeni opened this issue Apr 14, 2022 · 20 comments · Fixed by #6357
Closed

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

evgeni opened this issue Apr 14, 2022 · 20 comments · Fixed by #6357
Assignees
Labels
Configuration Related to configuration Crash 💥 A bug that makes pylint crash duplicate-code Related to code duplication checker Regression
Milestone

Comments

@evgeni
Copy link

evgeni commented Apr 14, 2022

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

@evgeni evgeni added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Apr 14, 2022
@cdce8p cdce8p added Cannot reproduce 🤷 Crash 💥 A bug that makes pylint crash and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Apr 14, 2022
@cdce8p
Copy link
Member

cdce8p commented Apr 14, 2022

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

@evgeni
Copy link
Author

evgeni commented Apr 14, 2022

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

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

@evgeni
Copy link
Author

evgeni commented Apr 14, 2022

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
Copy link
Member

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
Copy link
Author

evgeni commented Apr 14, 2022

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

@cdce8p
Copy link
Member

cdce8p commented Apr 14, 2022

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?

@cdce8p cdce8p added the Configuration Related to configuration label Apr 14, 2022
@cdce8p cdce8p added this to the 2.14.0 milestone Apr 14, 2022
@DanielNoord
Copy link
Collaborator

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
Copy link
Member

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
Copy link
Collaborator

@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
Copy link
Author

evgeni commented Apr 15, 2022

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
Copy link
Collaborator

DanielNoord commented Apr 15, 2022

😢

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
Copy link
Author

evgeni commented Apr 15, 2022

🤗

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
Copy link
Collaborator

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

@evgeni
Copy link
Author

evgeni commented Apr 15, 2022

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
Copy link
Collaborator

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
Copy link
Member

jacobtylerwalls commented Apr 15, 2022

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: 4ca885f
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: 03cfbf3
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.

@jacobtylerwalls jacobtylerwalls modified the milestones: 2.14.0, 2.13.6 Apr 15, 2022
@jacobtylerwalls jacobtylerwalls added the Blocker 🙅 Blocks the next release label Apr 15, 2022
@evgeni
Copy link
Author

evgeni commented Apr 15, 2022

escalated

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

@jacobtylerwalls
Copy link
Member

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

@jacobtylerwalls jacobtylerwalls added Regression duplicate-code Related to code duplication checker labels Apr 15, 2022
@DanielNoord
Copy link
Collaborator

@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
Copy link
Member

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

@jacobtylerwalls jacobtylerwalls self-assigned this Apr 16, 2022
evgeni added a commit to Katello/katello-client-bootstrap that referenced this issue Apr 17, 2022
pylint 2.13+ has a bug with disable=R and ignore-imports=y, which we
use.

See pylint-dev/pylint#6301 for details
evgeni added a commit to Katello/katello-client-bootstrap that referenced this issue Apr 19, 2022
pylint 2.13+ has a bug with disable=R and ignore-imports=y, which we
use.

See pylint-dev/pylint#6301 for details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Configuration Related to configuration Crash 💥 A bug that makes pylint crash duplicate-code Related to code duplication checker Regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants