1405304 - Provide Unix report formatter

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Andreas Tolfsen ❲:ato❳]

Many editors (ed, Emacs, vi, Acme) recognise the Unix output format
frequently employed by preprocessors and compilers.  Interacting
with the hyperlink allows the user to jump to a location in a file
instantaneously.

The output format looks like this:

	some/file.c:42: something went wrong

It would be useful for the mach linter to support this output
format.

The output format is already supported by grep(1), eslint, and
jshint.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1405304 - Add Unix formatter for mozlint.

This patch introduces a new report formatter for the mozlint
framework used by "./mach lint" that respects Unix output conventions,
popularised by grep(1), compilers, and preprocessors.

The output format looks like this:

	testing/marionette/driver.js:1153:48: no-unused-vars error: 'resp' is defined but never used.

Many editors (ed, Emacs, vi, Acme) recognise this format, allowing
users to interact with the output like a hyperlink to jump to the
specified location in a file.

DONTBUILD

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

Comment 2

[Andrew Halberstadt [:ahal]]

Comment on attachment 8914744 [details]
Bug 1405304 - Add Unix formatter for mozlint.

https://reviewboard.mozilla.org/r/186030/#review191058

::: commit-message-11fe0:15
(Diff revision 1)
> +
> +Many editors (ed, Emacs, vi, Acme) recognise this format, allowing
> +users to interact with the output like a hyperlink to jump to the
> +specified location in a file.
> +
> +DONTBUILD

This would normally be ok, but I'd like if you could add a test for this, and that actually does run as part of the builds.

To add a test you just need to create a new key/value here:
http://searchfox.org/mozilla-central/source/python/mozlint/test/test_formatters.py#17

Can run locally with `mach python-test python/mozlint`

::: python/mozlint/mozlint/formatters/unix.py:18
(Diff revision 1)
> +    """Formatter that respects Unix output conventions frequently
> +    employed by preprocessors and compilers.  The format is
> +    "FILENAME:LINE:COL: MESSAGE".
> +
> +    """
> +    fmt = "{path}:{lineno}:{column}: {rule} {level}: {message}"

I'm ok with this, but just fyi this differs from the eslint "unix" format:
https://eslint.org/docs/user-guide/formatters/#unix

If you wanted to be consistent, the rule/level should show up at the end in `[]`. I'll leave it up to you whether to fix or drop this.

Comment 3

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8914744 [details]
Bug 1405304 - Add Unix formatter for mozlint.

https://reviewboard.mozilla.org/r/186030/#review191058

> This would normally be ok, but I'd like if you could add a test for this, and that actually does run as part of the builds.
> 
> To add a test you just need to create a new key/value here:
> http://searchfox.org/mozilla-central/source/python/mozlint/test/test_formatters.py#17
> 
> Can run locally with `mach python-test python/mozlint`

Thanks for the tips.  Added a test and removed DONTBUILD.

> I'm ok with this, but just fyi this differs from the eslint "unix" format:
> https://eslint.org/docs/user-guide/formatters/#unix
> 
> If you wanted to be consistent, the rule/level should show up at the end in `[]`. I'll leave it up to you whether to fix or drop this.

I think this output is better because it groups the fields
hierarchically, starting with filename followed by buffer position,
then error type.  This makes the output easier to parse with awk(1).

For example if you want to extract the error or warning type you
would define the field separator and print the desired column:

> % ./mach lint -funix testing/marionette | awk -F: '{ print $4 }'
>  eslint error

With the eslint format you would have to parse the line using
regular expressions, which doable with sed(1), but is more
computationally expensive and leaves you with less flexibility to do
further postprocessing because the output will be irregular.

If we imagine you want to count the number of different error types
per file, we could lexicographically sort the output and count the
different errors:

> % ./mach lint -funix testing/marionette | awk -F: '{ print $1, $4 }' | uniq -c | sort
>       4 testing/marionette/driver.js  semi error
>       2 testing/marionette/driver.js  comma-dangle error
>       1 testing/marionette/listener.js  semi error

Comment 4

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8914744 [details]
Bug 1405304 - Add Unix formatter for mozlint.

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

Comment 5

[Andrew Halberstadt [:ahal]]

Comment on attachment 8914744 [details]
Bug 1405304 - Add Unix formatter for mozlint.

https://reviewboard.mozilla.org/r/186030/#review191058

> I think this output is better because it groups the fields
> hierarchically, starting with filename followed by buffer position,
> then error type.  This makes the output easier to parse with awk(1).
> 
> For example if you want to extract the error or warning type you
> would define the field separator and print the desired column:
> 
> > % ./mach lint -funix testing/marionette | awk -F: '{ print $4 }'
> >  eslint error
> 
> With the eslint format you would have to parse the line using
> regular expressions, which doable with sed(1), but is more
> computationally expensive and leaves you with less flexibility to do
> further postprocessing because the output will be irregular.
> 
> If we imagine you want to count the number of different error types
> per file, we could lexicographically sort the output and count the
> different errors:
> 
> > % ./mach lint -funix testing/marionette | awk -F: '{ print $1, $4 }' | uniq -c | sort
> >       4 testing/marionette/driver.js  semi error
> >       2 testing/marionette/driver.js  comma-dangle error
> >       1 testing/marionette/listener.js  semi error

Makes sense, thanks for the explanation!

Comment 6

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8914744 [details]
Bug 1405304 - Add Unix formatter for mozlint.

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b5d8e8c5258
Add Unix formatter for mozlint. r=ahal

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/1b5d8e8c5258