1388721 - [flake8] Support --fix in the flake8 linter

Status: RESOLVED FIXED

(Firefox Build System :: Lint and Formatting, enhancement, P3)

Description

[Andrew Halberstadt [:ahal]]

Flake8 itself doesn't fix errors, but we can install the autopep8 module to handle the pycodestyle problems.

We'll need to figure out a way to get them both to use the same configuration files. This might depend on being a bit smarter with flake8 configs.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1388721 - [lint] Support --fix in the flake8 linter via autopep8,

This adds autopep8 to flake8_requirements.txt. When --fix is passed in,
autopep8 will run prior to the normal flake8 run, so only errors that couldn't
be fixed automatically will be shown.

There is one caveat, namely autopep8 will only use the root .flake8 file
(there's no support for passing in subconfigs).

Review commit: https://reviewboard.mozilla.org/r/246956/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246956/

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8981137 [details]
Bug 1388721 - [lint] Support --fix in the flake8 linter via autopep8,

https://reviewboard.mozilla.org/r/246956/#review253312

::: tools/lint/python/flake8.py:131
(Diff revision 1)
>                      '"column":%(col)s,"rule":"%(code)s","message":"%(text)s"}',
>      ]
>  
> +    fix_cmdargs = [
> +        os.path.join(bindir, 'autopep8'),
> +        '--global-config', os.path.join(lintargs['root'], '.flake8'),

This is going to take in account the max length = 99, right ?

::: tools/lint/test/test_flake8.py:43
(Diff revision 1)
> +
> +    results = lint([path.strpath])
> +    assert len(results) == 2
> +
> +    # Make sure the missing blank line is fixed, but the unused import isn't.
> +    results = lint([path.strpath], fix=True)

Maybe you could also add a test with path = a directory?

Comment 3

[Andrew Halberstadt [:ahal]]

Comment on attachment 8981137 [details]
Bug 1388721 - [lint] Support --fix in the flake8 linter via autopep8,

https://reviewboard.mozilla.org/r/246956/#review253312

> This is going to take in account the max length = 99, right ?

Yes, that's defined in the global topsrcdir/.flake8 config which this uses. Guess I can add a test just to make sure.

Comment 4

[Andrew Halberstadt [:ahal]]

Comment on attachment 8981137 [details]
Bug 1388721 - [lint] Support --fix in the flake8 linter via autopep8,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246956/diff/1-2/

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8981137 [details]
Bug 1388721 - [lint] Support --fix in the flake8 linter via autopep8,

https://reviewboard.mozilla.org/r/246956/#review253342

Great, thanks!

By the way; I fixed more components, I will push them asap

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be2690ae5c76
[lint] Support --fix in the flake8 linter via autopep8, r=sylvestre

Comment 7

[Andrei Ciure[:aciure]]

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