1623024 - mozlint: Add pylint as a new linter

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

it finds issues that flake8 doesn't find
For example, the missing coma here:
https://searchfox.org/mozilla-central/source/dom/bindings/parser/WebIDL.py#3483-3484
https://searchfox.org/mozilla-central/source/dom/bindings/parser/WebIDL.py#5470-5473

dom/bindings/parser/WebIDL.py:3428:26: E1120: No value for argument 'locations' in constructor call (no-value-for-parameter)
dom/bindings/parser/WebIDL.py:5430:22: E1120: No value for argument 'locations' in constructor call (no-value-for-parameter)

Comment 1

[Sylvestre Ledru [:Sylvestre]]

To evaluate it, we should run it on directory on which we are running flake8 already. See if it does find interesting stuff
https://searchfox.org/mozilla-central/source/.flake8 (the exclude list)

Then we could add a new linter:
https://firefox-source-docs.mozilla.org/code-quality/lint/create.html

Adding a good first bug which is probably good second bug for someone who knows python already

Comment 2

[Andrew Halberstadt [:ahal] (PTO Until Apr 3rd)]

IMO pylint is way too strict by default, we'd need to be very conscious of the rules we enable. Ideally we wouldn't want overlap between pylint / flake8 either so devs don't see duplicated issues.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Yeah, we should enable it on stuff that flake8 doesn't cover (at least no-value-for-parameter)

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Here is what code is doing:
https://code.visualstudio.com/docs/python/linting#_pylint

Should be pretty easy to add.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1623024 - mozlint: Add pylint as new linter r?#linter-reviewers

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1623024 - mozlint: Add pylint to the CI r?#linter-reviewers

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1623024 - mozlint: run pylint at review phase r?#linter-reviewers

Depends on D79099

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba8087e38e6c
mozlint: Add pylint as new linter r=linter-reviewers,ahal
https://hg.mozilla.org/integration/autoland/rev/2ba0929ca2b5
mozlint: Add pylint to the CI r=linter-reviewers,ahal
https://hg.mozilla.org/integration/autoland/rev/ef81294f5db7
mozlint: run pylint at review phase r=linter-reviewers,ahal

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/ba8087e38e6c
https://hg.mozilla.org/mozilla-central/rev/2ba0929ca2b5
https://hg.mozilla.org/mozilla-central/rev/ef81294f5db7

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1623024 - mozlint/pylint - follow up: fix a syntax issue in the rst file

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e16a500ce946
mozlint/pylint - follow up: fix a syntax issue in the rst file

Comment 12

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/e16a500ce946