939329 - (android-jslint) [meta] Enable JavaScript linting in mobile/android/

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Linters are very useful for catching bugs and maintaining order in dynamic languages, such as JavaScript, before the application is run and tested. This meta bug tracks the inclusion of linters in the mobile/android/ subdirectory.

Dependent bugs will include creating build system hooks, working around JS pre-processing, and fixing the current code to be run through the linter without errors.

JSHint looks like a good choice of linter as it already supports Mozilla specific extensions via the "moz" configuration variable [1].

[1]: http://jshint.org/docs/options/#moz

Comment 1

[:Margaret Leibovic]

I think the devtools folks use jshint with their code, it might be worth talking to them about how they've incorporated that, if you haven't already.

Comment 2

[:Margaret Leibovic]

(In reply to :Margaret Leibovic from comment #1)
> I think the devtools folks use jshint with their code, it might be worth
> talking to them about how they've incorporated that, if you haven't already.

Oh, I just saw you reference that in bug 939347 :D

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I asked a few other teams how they use linters:
  * devtools uses jshint but does not enforce its usage (see comment 0 [2]). They would be interested in a system like we're building
  * gaia uses both gjshint and jshint and strictly enforces its usage via their continuous integration system [1]. They also have a pre-commit hook [2] which prevents violating code from being committed in the first place. They manage the jshint dependency through npm and gjshint seems to be installed by hand (looking at the source of [2]).

I examined the pdfjs repo [3] who also appear to use jshint (it's a dependency in their package.json). I imagine their setup is similar to gaia's.

[1]: https://travis-ci.org/mozilla-b2g/gaia/builds
[2]: https://github.com/mozilla-b2g/gaia/blob/master/tools/pre-commit
[3]: https://github.com/mozilla/pdf.js

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Michael Comella (:mcomella) from comment #0)
> JSHint looks like a good choice of linter as it already supports Mozilla
> specific extensions via the "moz" configuration variable [1].
> 
> [1]: http://jshint.org/docs/options/#moz

I've been playing with JSHint with some /toolkit/ and /browser/ code too. Turns out we don't actually want the 'moz' option - as you can't have both that and the 'esnext' option (for ES6) enabled at the same time. From my experience, the ES6 support is far more useful - there's a small number of classes of edge-cases that aren't handled by not having 'moz', but they should be evaporating as SpiderMonkey gets better support for ES6 and we can port our code over to that.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

You might look as eslint, since it has great es6 support, and it allows you to write plugins to enforce local conventions.

Comment 6

[Mark Banner (:standard8)]

This is basically done, apart from a few more rules to enable, which bug 1424996/bug 1425047 are handling.