Closed Bug 1405304 Opened 7 years ago Closed 7 years ago

Provide Unix report formatter

Categories

(Developer Infrastructure :: Lint and Formatting, enhancement)

3 Branch
enhancement
Not set
normal

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: ato, Assigned: ato)

Details

Attachments

(1 file)

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.
Assignee: nobody → ato
Status: NEW → ASSIGNED
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.
Attachment #8914744 - Flags: review?(ahalberstadt) → review+
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
Attachment #8914744 - Flags: review+ → review?(ahalberstadt)
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!
Attachment #8914744 - Flags: review?(ahalberstadt) → review+
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b5d8e8c5258
Add Unix formatter for mozlint. r=ahal
https://hg.mozilla.org/mozilla-central/rev/1b5d8e8c5258
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: