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 irc.mozilla.org.

Tagged: , , , ,

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

What’s this?

You are currently reading More eslint coverage for Firefox developers at JAWS.

meta