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

Improve pylama linter output handling #4106

Merged
merged 2 commits into from
Apr 23, 2022
Merged

Conversation

favadi
Copy link
Contributor

@favadi favadi commented Mar 6, 2022

The current pattern for matching pylama output is:

'\v^.{-}:([0-9]+):([0-9]+): +%(([A-Z][0-9]+):? +)?(.*)$'

By default, pylama output format looks like:

samples/pylama.py:99:101 E501 line too long (189 > 100 characters) [pycodestyle]
samples/pylama.py:104:101 E501 line too long (189 > 100 characters) [pycodestyle]
samples/pylama.py:11:9  f-string is missing placeholders [pyflakes]
samples/pylama.py:12:101 E501 line too long (150 > 100 characters) [pycodestyle]

Notice the there is no : in the first part of the output. It looks
like the linter expects pylama to a non-default --format option, or
pylama changed the default format at some point. pylama support
--format json that will produce following output:

{
  "source": "pycodestyle",
  "col": 101,
  "lnum": 99,
  "etype": "E",
  "message": "line too long (189 > 100 characters)",
  "filename": "samples/pylama.py",
  "number": "E501"
},
{
  "source": "pycodestyle",
  "col": 101,
  "lnum": 104,
  "etype": "E",
  "message": "line too long (189 > 100 characters)",
  "filename": "samples/pylama.py",
  "number": "E501"
},
{
  "source": "pyflakes",
  "col": 9,
  "lnum": 11,
  "etype": "E",
  "message": "f-string is missing placeholders",
  "filename": "samples/pylama.py",
  "number": ""
},
{
  "source": "pycodestyle",
  "col": 101,
  "lnum": 12,
  "etype": "E",
  "message": "line too long (150 > 100 characters)",
  "filename": "samples/pylama.py",
  "number": "E501"
}

This commit introduces following changes:

  • forces pylama command to always use --format json. Not sure if this
    will cause any issues.

  • use json_decode instead of regex to parse the errors

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just double-checked the changelog and the JSON format option was added late last year: https://github.com/klen/pylama/blob/develop/Changelog

To support this we'll have to check which version of pylama is being used and only use the JSON format if the version is new enough. Have a look at how we check the version for flake8 as an example.

@favadi
Copy link
Contributor Author

favadi commented Mar 21, 2022

@w0rp I added version check to support both older and newer version of pylama, please take another look.

Btw, I referred to flake8 check and this line looks suspicious. I think it will execute poetry --version and pipenv --version instead of poetry run flake8 --version and pipenv run flake8 --version respectively.

@favadi favadi requested a review from w0rp March 21, 2022 14:58
@hsanson hsanson merged commit 6c51bb1 into dense-analysis:master Apr 23, 2022
cyyever pushed a commit to cyyever/ale that referenced this pull request Jul 11, 2022
* Use JSON format for newer pylama version

https://github.com/klen/pylama/blob/develop/Changelog

The --format json option is added in pylama version 8.1.4.

* Fix linting warnings for pylama
cyyever pushed a commit to cyyever/ale that referenced this pull request Jul 11, 2022
* Use JSON format for newer pylama version

https://github.com/klen/pylama/blob/develop/Changelog

The --format json option is added in pylama version 8.1.4.

* Fix linting warnings for pylama
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants