More eslint coverage for Firefox developers

23 January, 2017 § Leave a comment

Since my last post on the eslint work that is happening in mozilla-central, we have increased coverage as well as enabling more rules.

The publication of that post spawned a few conversations, among them Florian Quèze has added a bunch of new Mozilla-specific rules to our eslint configuration and evilpies has started looking at what it would take to run eslint on the JS engine code.

The following rules have been added:

  • newURI originally had three arguments, of which the last two were sparsely used and often had null passed in. Florian converted newURI to make the last two arguments optional, and wrote a script to remove the unneeded null usages. This now has an eslint rule to enforce it.
  • removeObserver had many usages that passed an unused third argument of false. This was most likely done as people were writing code that used addObserver(..., ..., false), and thought that they needed to pass false when calling removeObserver. After that, it most likely got copy-pasted around the codebase. This also now has an eslint rule enforcing people don’t do this again.
  • addEventListener has a {once: true} argument that can be used to automatically remove the event listener after it is called the first time. This rule can be applied to any code that was indiscriminately calling removeEventListener. This work should land very soon, as it just got r+ today. This work is being tracked in bug 1331599.
  • no-undef work continues to make progress as Mark Banner is now focusing on /services subdirectory. We also have a few more globals defined in our configuration and have more testing globals that are imported automatically through a new rule.
  • quotes is now enabled, requiring double-quotes for strings except in cases where double-quotes are used within the string or if the string is a template literal.

While working on no-undef, Mark noticed that the /services directory wasn’t being scanned by eslint even though it wasn’t listed in the .eslintignore file. The directory was missing an .eslintrc.js file. We now have one added in the directory, though there remain 59 warnings which should be fixed (these are listed as warnings temporarily and will be converted to errors once fixed).

Also of note is that we now have automated tests for the custom rules that we have implemented. You can view these tests in the /tools/lint/eslint/eslint-plugin-mozilla/tests directory.

If you’ve got ideas for new rules that we can add to help clean up the codebase, please comment on this post or contact any of us on IRC on the #eslint channel of

eslint updates for Firefox developers

10 January, 2017 § 1 Comment

In the past week there has been quite a lot of progress made on the eslint front.

Last week I enabled the following rules for the default mozilla-central eslint configuration (/toolkit/.eslintrc.js):

Mark Banner has continued to work on fixing the remaining no-undef errors. This work is on-going and is being tracked by a meta bug.

Florian Quèze just landed a patch yesterday to simplify calls to so the two trailing arguments are optional. Previously 99% of the calls to the function passed in null for the trailing arguments. Florian is planning on cleaning up some addEventListener code as well and I am pushing for him to implement special eslint rules along with them to help enforce these changes going forward.

I enabled most of the rules for eslint and gathered counts of the number of errors related to each rule. The following list shows each disabled rule along with the number of associated errors as of mozilla-central revision f13abb8ba9f3:

  • array-callback-return = 3
  • no-new-func = 13
  • no-useless-concat = 14
  • no-void = 14
  • no-multi-str = 15
  • no-new-wrappers = 18
  • no-array-constructor = 20
  • no-eval = 20
  • no-await-in-loop = 21
  • no-sequences = 22
  • no-inner-declarations = 23
  • no-unmodified-loop-condition = 24
  • wrap-iife = 25
  • no-constant-condition = 28
  • no-template-curly-in-string = 39
  • no-loop-func = 44
  • no-fallthrough = 51
  • no-new = 56
  • no-throw-literal = 134
  • no-prototype-builtins = 158
  • no-caller = 165
  • no-unused-expressions = 171
  • no-useless-escape = 194
  • complexity = 208
  • no-case-declarations = 238
  • guard-for-in = 284
  • radix = 342
  • no-shadow = 356
  • no-eq-null = 442
  • dot-notation = 459
  • default-case = 485
  • block-scoped-var = 749
  • no-empty-function = 1144
  • dot-location = 2327
  • no-extra-parens = 2464
  • no-invalid-this = 2947

If you would like to work on fixing any of these, please file a bug in the Toolkit :: General component of Bugzilla and request review from myself, Mossop, or Standard8.

If you’d like eslint to run on a directory that you work in, remove the reference to it from the .eslintignore file located at the mozilla-central root and add a .eslintrc.js file. This will now allow eslint to scan that directory.

Also, another project that someone can pick up is to help us move towards a single rule definition. We would like to move to a single set of rules which will help for consistent coding styling. You can look at this listing of .eslintrc.js files to see the differences between them. Some define globals that are unique to the directory or have different include paths to the root configuration, but some also define extra rules. We would like to get those rules added to the root configuration, though we haven’t determined how to settle rule conflicts yet.

Where Am I?

You are currently browsing entries tagged with eslint at JAWS.