Community
Participate
Working Groups
https://dev.eclipse.org/ipzilla/show_bug.cgi?id=8420 Due to lack of a CLA we have been unable to use the rules from ESLint in Orion. However, we can apply for an exception. This bug is to capture the pros and cons of doing so.
On the Orion dev mailing list, Max noted that Eclipse approved usage of momemt.js and Bootstrap UI which had similar(ish?) IP issues. Mike Milinkovich also pointed out the exceptions in the IP process. Pros: - Less code to maintain in Orion - Get any bug fixes and new rules for 'free'. Cons: - The eslint rules are delivered in separate files. This caused timeout issues for us previously when loading all the rule files using require.js. - We have solid test coverage for our existing rules. Our rules are integrated into other tooling (settings page and the content assist proposals for inline eslint settings for example). - We would need to more frequently do the CQ process to update ESLint versions. - Some (many?) of the rules we don't currently implement are formatting rules which we may not want to use (or at least will require a quickfix).
pro: + larger ruleset than Orion's + good ES6 support across rules (Orion's is patchy) con: - upstream ESLint's rules are not externalized for i18n - their context#report() API doesn't support flagging the problematic token. (I believe you are limited to flagging an AST node, though this may've changed since I last looked.) - IIRC some Orion rules instrument their reported problems to make it easier to provide quick fixes.. this would be lost
All of the above, plus: not really a pro or con - we would still have to edit eslint to accept ASTs rather than text, leading to... cons - eslint now uses their own parser, so we have to shake out (fix) incompatibility issues (espree vs. esprima, if any).
Assigning to Mike to determine priority.
(In reply to Carolyn MacLeod from comment #4) > Assigning to Mike to determine priority. I'm going to leave this as normal priority. To be honest I am undecided about the benefits of using the upstream rules. 1. We have built a lot of features around our rules, which we would lose, 2. We would have to tackle the timeout problems again (for loading N rule files vs. our one 'rule' file) 3. we would still have to wire all of the rules up for prefs, etc (or they will not be used) 4. we have already implemented most of the rules we think would be useful in the IDE (we are always adding more though) 5. either way we will still have to modify eslint to make it work in Orion
Any reason for why we haven't contributed back to eslint esprima support and/or have a way to bundle up the rules in one file automatically ? wouldn't those things back in eslint reduce the overhead orion would have to maintain ?
(In reply to Max Rydahl Andersen from comment #6) > Any reason for why we haven't contributed back to eslint esprima support > and/or have a way to bundle up the rules in one file automatically ? > ESlint used to run on Esprima, but now they use their own parser Espree (which is a fork of Esprima, and still Mozilla-spec compliant). The rule-loading-timeout issue is really only prevalent in target sites, making it more of an annoyance than a critical problem (its all minified and combined in our build anyway), so to be honest, there hasn't been anything for us to contribute back in this space, which is why we haven't. We have opened an issue for allowing ASTs to be used rather than source (https://github.com/eslint/eslint/issues/1025), and we should probably open issues for the context.report() API update as well.
The CQ for ESLint 1.9.0 (bug 482505) also gave us the green light to directly consume the upstream rules. We should start by trying out a few of the upstream rules the we wrote our own version for and run the unit tests + smoke tests to see whats happening.
I am preparing a first set of new rules for: 'accessor-pairs' 'no-control-regex' 'no-duplicate-case' 'no-empty-character-class' 'no-extra-boolean-cast' 'no-extra-parens' 'no-invalid-regexp' 'no-negated-in-lhs' 'no-obj-calls' 'no-eq-null' 'no-else-return'
New Gerrit change created: https://git.eclipse.org/r/62771
New Gerrit change created: https://git.eclipse.org/r/62774
(In reply to Eclipse Genie from comment #10) > New Gerrit change created: https://git.eclipse.org/r/62771 This is abandonned. I forgot to enable all the tests again. I was running only the validator tests when adding new tests for the new rules.
New Gerrit change created: https://git.eclipse.org/r/62830
Gerrit change https://git.eclipse.org/r/62830 was merged to [master]. Commit: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=194790e72101239a03941a9ad11eab6b97760152
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=194790e72101239a03941a9ad11eab6b97760152 Gerrit patch has been merged with master Great work Olivier. We now exceed 2000 tests in our suite. The consumption of the rules looks good and the change removes the need for an Orion change in ESLint. I have some minor concerns with the rules themselves which I will bring up in another bug if substantiated. Also, the wiki page needs to be updated with the new rules. https://wiki.eclipse.org/Orion/ESLint As we add more rules from ESLint we will open new bugs as necessary.
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=ede415417c37de69bff53990fe03a00dcefff0b9 Had to change how we loaded the rules.
