<div dir="ltr"><div><div>We've been using ESLint for the past 6 months on the devtools codebase, but for now it has only been a way to help reducing mistakes and having a consistent style when coding.<br>Indeed, we haven't yet made it run automatically, so it's up to developers to integrate it in their workflows.<br></div>For instance, I use Sublime Text (and I know a lot of people on the team do too), and it's integrated nicely in this editor (see <a href="https://wiki.mozilla.org/DevTools/CodingStandards">our doc here</a>).<br></div><div>We'd be really really interested in having it run automatically.<br></div><div><br></div><div>ESLint does work with ES6, but does *not* work with spidermonkey non-standard syntax.<br>We've removed all instances of this from the devtools codebase (see bug 887895).<br> That's because the first thing it does when checking a file is parse the code in it using <a href="https://github.com/eslint/espree">espree</a>. Espree supports es5/6, and it it meets non-valid syntax, it just fails and ESLint can't run the rules on that file.<br><br></div><div>For the same reason, ESLint does *not* work when a file has pre-processor instructions either. Luckily, we don't have too many of those in devtools, so we can just<a href="http://eslint.org/docs/user-guide/configuring.html#ignoring-files-and-directories"> ignore them</a>.<br>I guess we could always try and hack on a parser of our own since ESLint can be <a href="http://eslint.org/docs/user-guide/configuring.html#specifying-parser">configured with one</a>.<br></div><div><br>To answer one of Gijs' questions, yes it does catch  non-existing variables, "there's a rule for that"! In fact, the most time-intensive thing when using ESLint is deciding which <a href="http://eslint.org/docs/rules/">rules </a>you want to enable and with which level.<br></div><div>I agree with Mark on that by the way, better start with almost all rules disabled (or set to warning instead of error), and first get the infra in place.<br><br></div><div>Last, it's easy to write new rules for ESLint, and we started to create a few useful ones that you can find here: <a href="https://dxr.mozilla.org/mozilla-central/source/testing/eslint-plugin-mozilla">https://dxr.mozilla.org/mozilla-central/source/testing/eslint-plugin-mozilla</a><br></div><div>In fact, './mach eslint --setup' installs them locally so you can then use them in your own .eslintrc config file.<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Nov 28, 2015 at 5:25 AM, Kris Maglione <span dir="ltr"><<a href="mailto:kmaglione@mozilla.com" target="_blank">kmaglione@mozilla.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Nov 27, 2015 at 08:02:35AM -0800, Dave Townsend wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
On Thu, Nov 26, 2015 at 3:05 PM, Mark Banner <<a href="mailto:mbanner@mozilla.com" target="_blank">mbanner@mozilla.com</a>> wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
On 26/11/2015 19:58, Dave Townsend wrote:<br>
On Thu, Nov 26, 2015 at 5:56 AM, Mark Banner <<a href="mailto:mbanner@mozilla.com" target="_blank">mbanner@mozilla.com</a>> wrote:<br></span><span class="">
- `catch(ex if ex instanceOf Error)` this isn't on a standards track, and we<br>
might have to write a plugin if we want to keep it.<br>
- Array Comprehensions: `[ x for (x of a) if (x.color === ‘blue’) ]`<br>
however, these might not get standized, so we'd probably need to write our<br>
own if we keep using them (xref<br>
<a href="https://github.com/eslint/espree/issues/125" rel="noreferrer" target="_blank">https://github.com/eslint/espree/issues/125</a>).<br>
</span></blockquote><span class="">
<br>
My understanding is that array comprehensions have been dropped. So if<br>
neither of these are on the standards track maybe we shouldn't be<br>
using them. The catch case seems more difficult to switch away from<br>
though.<br>
</span></blockquote>
<br>
It's not much more difficult than switching away from comprehensions. I had to remove catch guards from a few places to run coverage tools, and just changing `catch (e if guard)` to `catch (e) { if (!guard) throw e; }` worked just fine. Since then, every time I've seen a catch guard, I've thought about how easy it would be to remove it, and the answer has always been "very".<br>
<br>
The only real snag I can think of is the debugger's "ignore caught exceptions" feature, but it doesn't know about catch guards anyway.<span class="HOEnZb"><font color="#888888"><br>
<br>
-- <br>
Kris Maglione<br>
Firefox Add-ons Engineer<br>
Mozilla Corporation<br>
<br>
Lying to ourselves is more deeply ingrained than lying to others.<br>
        --Fyodor Dostoevsky<br>
<br>
</font></span></blockquote></div><br></div>