1379151 - [mozlint] Add support for fixers

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement, P2)

Description

[Andrew Halberstadt [:ahal]]

Many external linters (e.g eslint, clang-tidy) have the ability to fix the errors that they report. We could also implement a python fixer with the autopep8 module.

It would be nice if mozlint had a best effort --fix argument that could call into these various fixer functions. Linters could declare that they support fixing in their mozlint definitions. Linters that do not declare this would get skipped when running ./mach lint --fix.

Comment 1

[Andrew Halberstadt [:ahal]]

I think it's better to copy eslint's behaviour here. So with --fix, anything that can be fixed is, and the rest is printed to stdout. This way there's no need for linters to declare that they have "fixing" capability or anything like that. They either make use of the "fix" config or they don't.

I have a further idea for --fix. I want to add an "edit" mode that will open failing files in an editor (one by one). Then there can be a "mixed" mode which will automatically fix what it can, and open the remaining files in an editor.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1379151 - Add --fix and --edit to mozlint,

While --fix previously worked with eslint, it is now more official (will show
up in the |mach lint --help|).  ESlint is still the only thing that implements
it, but we can implement it for flake8 using the `autopep8` module.

--edit is a new concept that will open an editor for each failing file to let
you fix the errors manually.  For now it is very naive (just opens the file),
and is only really useful if you have an editor integration for the linter(s).
But in the future I'd like to have editor-specific implementations for this.
For example, with vim, we can use -q to pass in an error file that will start
the editor pre-populated with a list of all errors that can then be easily
jumped to. Other editors may just open up to the line containing the error.

--fix and --edit can be used in conjunction with one another. Doing that means
only errors that can't be fixed automatically will show up in your editor.

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

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8893635 [details]
Bug 1379151 - Add --fix and --edit to mozlint,

https://reviewboard.mozilla.org/r/164730/#review171100

Looks good. We might want some sort of re-lint after all edits done option, but I think as a starter this is an improvement, especially the official --fix option.

Comment 4

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 611fd94b45a0 -d 8e991e3b80f6: rebasing 412354:611fd94b45a0 "Bug 1379151 - Add --fix and --edit to mozlint, r=standard8" (tip)
merging python/mozlint/mozlint/cli.py
warning: conflicts while merging python/mozlint/mozlint/cli.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 5

[Andrew Halberstadt [:ahal]]

Comment on attachment 8893635 [details]
Bug 1379151 - Add --fix and --edit to mozlint,

https://reviewboard.mozilla.org/r/164730/#review171100

Good idea, I'll add that in a follow-up, possibly bug 1387555

Comment 6

[Andrew Halberstadt [:ahal]]

Comment on attachment 8893635 [details]
Bug 1379151 - Add --fix and --edit to mozlint,

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d24be9fbef98
Add --fix and --edit to mozlint, r=standard8

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for breaking Windows builds:

https://hg.mozilla.org/integration/autoland/rev/0e8f9aafd08969a7c65cad18e128686479241e3c

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d24be9fbef9836204f401be62d8f4c00623a9b31&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=122282020&repo=autoland

14:21:15     INFO - ============================= test session starts =============================
14:21:15     INFO - platform win32 -- Python 2.7.11, pytest-3.1.3, py-1.4.34, pluggy-0.4.0 -- z:\build\build\src\obj-firefox\_virtualenv\Scripts\python.exe
14:21:15     INFO - cachedir: ..\python\mozlint\.cache
14:21:15     INFO - rootdir: z:\build\build\src\python\mozlint, inifile:
14:21:15     INFO - collecting ... collected 2 items
14:21:15     INFO - ..\python\mozlint\test\test_cli.py::test_cli_run_with_fix PASSED
14:21:15     INFO - ..\python\mozlint\test\test_cli.py::test_cli_run_with_edit FAILED
14:21:15     INFO - ================================== FAILURES ===================================
14:21:15     INFO - ___________________________ test_cli_run_with_edit ____________________________
14:21:15     INFO - run = <function inner at 0x03D4BBB0>
14:21:15     INFO - parser = MozlintParser(prog='test_cli.py', usage=None, description=None, version=None, ...lass=<class 'argparse.HelpFormatter'>, conflict_handler='error', add_help=True)
14:21:15     INFO - capfd = <_pytest.capture.CaptureFixture instance at 0x03D53288>
14:21:15     INFO -     def test_cli_run_with_edit(run, parser, capfd):
14:21:15     INFO -         os.environ['EDITOR'] = 'echo'
14:21:15     INFO -         ret = run(['-f', 'json', '--edit', '--linter', 'external'])
14:21:15     INFO -         out, err = capfd.readouterr()
14:21:15     INFO -         assert ret == 0
14:21:15     INFO - >       assert out.endswith('foobar.js\n')
14:21:15     INFO - E       AssertionError: assert False
14:21:15     INFO - E        +  where False = <built-in method endswith of unicode object at 0x035190E0>('foobar.js\n')
14:21:15     INFO - E        +    where <built-in method endswith of unicode object at 0x035190E0> = 'z:\build\build\src\python\mozlint\test\files\foobar.js
14:21:15     INFO - \n'.endswith
14:21:15     INFO - ..\python\mozlint\test\test_cli.py:47: AssertionError
14:21:15     INFO - ===================== 1 failed, 1 passed in 20.01 seconds =====================

Comment 9

[Andrew Halberstadt [:ahal]]

Comment on attachment 8893635 [details]
Bug 1379151 - Add --fix and --edit to mozlint,

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

Comment 10

[Andrew Halberstadt [:ahal]]

Sorry for the bustage, thought I had tested this on Windows, but turns out I had pushed it along with another patch and it was the other patch that was making this pass. Latest review push should fix this:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f746d4d45c992a41a9525124af468a449d5e2e3

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e9296f1a1ad
Add --fix and --edit to mozlint, r=standard8

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2e9296f1a1ad