Use getRules and rule URL metadata where available

Refactor the worker to send the full metadata instead of just the
fixable rules, allowing additional functionality to be implemented in
the main package code.

Rule gathering from the ESLint instance has been expanded to take
advantage of the new `CLIEngine#getRules` method if it is available.

The rule URL is now pulled directly from the metadata if it is
available, falling back to the current method of using
`eslint-rule-documentation` if the rule doesn't specify this for itself.

spec/helpers-spec.js

@@ -0,0 +1,48 @@
+'use babel'
+
+import * as Helpers from '../src/helpers'
+
+fdescribe('The helper module', () => {
+  describe('updateRules', () => {
+    const ruleArray = [
+      ['foo', { meta: { fixable: true } }],
+      ['bar', { meta: {} }]
+    ]
+
+    it('adds new rules', () => {
+      expect(Helpers.getRules()).toEqual(new Map())
+      Helpers.updateRules(ruleArray)
+      expect(Helpers.getRules()).toEqual(new Map(ruleArray))
+    })
+
+    it('removes old rules', () => {
+      Helpers.updateRules(ruleArray)
+      expect(Helpers.getRules()).toEqual(new Map(ruleArray))
+      Helpers.updateRules([])
+      expect(Helpers.getRules()).toEqual(new Map())
+    })
+
+    it('updates the fixableRules list', () => {
+      expect(Helpers.getFixableRules()).toEqual([])
+      Helpers.updateRules(ruleArray)
+      expect(Helpers.getFixableRules()).toEqual(['foo'])
+    })
+  })
+
+  describe('getRuleUrl', () => {
+    it('works with rules that define their own URL', () => {
+      Helpers.updateRules([['foo', { meta: { docs: { url: 'bar' } } }]])
+      expect(Helpers.getRuleUrl('foo')).toBe('bar')
+    })
+
+    it('works with rules defined in eslint-rule-documentation', () => {
+      const url = 'https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-duplicates.md'
+      expect(Helpers.getRuleUrl('import/no-duplicates')).toBe(url)
+    })
+
+    it('gives a fallback URL on how to add a rule URL', () => {
+      const url = 'https://github.com/jfmengels/eslint-rule-documentation/blob/master/contributing.md'
+      expect(Helpers.getRuleUrl('foo/bar')).toBe(url)
+    })
+  })
+})

spec/worker-helpers-spec.js

@@ -211,4 +211,58 @@ describe('Worker Helpers', () => {
       rimraf.sync(tempDir)
     })
   })
+
+  describe('getRules', () => {
+    it('works with the getRules function introduced in ESLint v4.15.0', () => {
+      const cliEngine = {
+        getRules: () => 'foo'
+      }
+      expect(Helpers.getRules(cliEngine)).toBe('foo')
+    })
+
+    it('works with the hidden linter in ESLint v4 before v4.15.0', () => {
+      const cliEngine = {
+        linter: {
+          getRules: () => 'foo'
+        }
+      }
+      expect(Helpers.getRules(cliEngine)).toBe('foo')
+    })
+
+    it('returns an empty Map for old ESLint versions', () => {
+      const cliEngine = {}
+      expect(Helpers.getRules(cliEngine)).toEqual(new Map())
+    })
+  })
+
+  describe('didRulesChange', () => {
+    const emptyRules = new Map()
+    const rules1 = new Map([['rule1', {}]])
+    const rules2 = new Map([['rule1', {}], ['rule2', {}]])
+
+    it('returns false for empty Maps', () => {
+      const newRules = new Map()
+      expect(Helpers.didRulesChange(emptyRules, newRules)).toBe(false)
+    })
+
+    it('returns false when they are the same', () => {
+      expect(Helpers.didRulesChange(rules1, rules1)).toBe(false)
+    })
+
+    it('returns true when a new rule is added to an empty list', () => {
+      expect(Helpers.didRulesChange(emptyRules, rules1)).toBe(true)
+    })
+
+    it('returns true when the last rule is removed', () => {
+      expect(Helpers.didRulesChange(rules1, emptyRules)).toBe(true)
+    })
+
+    it('returns true when a new rule is added to an existing list', () => {
+      expect(Helpers.didRulesChange(rules1, rules2)).toBe(true)
+    })
+
+    it('returns true when a rule is removed from an existing list', () => {
+      expect(Helpers.didRulesChange(rules2, rules1)).toBe(true)
+    })
+  })
 })

src/helpers.js

@@ -8,6 +8,7 @@ import cryptoRandomString from 'crypto-random-string'
 // eslint-disable-next-line import/no-extraneous-dependencies, import/extensions
 import { Range } from 'atom'
 
+const lintRules = new Map()
 const fixableRules = new Set()
 
 /**
@@ -221,6 +222,25 @@ const generateInvalidTrace = async ({
   }
 }
 
+/**
+ * Get the URL of the documentation for a rule, either from the rule's own
+ * metadata, from eslint-rule-documentation's known rules, or the fallback URL
+ * on how to add it to eslint-rule-documentation.
+ * @param  {String} ruleId The rule ID to get the documentation URL for
+ * @return {String}        URL of the rule documentation
+ */
+export function getRuleUrl(ruleId) {
+  const props = lintRules.get(ruleId)
+  if (props && props.meta && props.meta.docs && props.meta.docs.url) {
+    // The rule has a documentation URL specified in its metadata
+    return props.meta.docs.url
+  }
+
+  // The rule didn't specify a URL in its metadata, or was not currently known
+  // somehow. Attempt to determine a URL using eslint-rule-documentation.
+  return ruleURI(ruleId).url
+}
+
 /**
  * Given a raw response from ESLint, this processes the messages into a format
  * compatible with the Linter API.
@@ -279,7 +299,7 @@ export async function processESLintMessages(messages, textEditor, showRule, work
     }
 
     if (ruleId) {
-      ret.url = ruleURI(ruleId).url
+      ret.url = getRuleUrl(ruleId)
     }
 
     let range
@@ -319,6 +339,39 @@ export async function processESLintMessages(messages, textEditor, showRule, work
   }))
 }
 
+/**
+ * Update the list of fixable rules
+ */
+function updateFixableRules() {
+  fixableRules.clear()
+  lintRules.forEach((props, rule) => {
+    if (
+      Object.prototype.hasOwnProperty.call(props, 'meta') &&
+      Object.prototype.hasOwnProperty.call(props.meta, 'fixable')
+    ) {
+      fixableRules.add(rule)
+    }
+  })
+}
+
+/**
+ * Process the updated rules into the local Map and call further update functions
+ * @param  {Array} updatedRules Array of Arrays of the rule and properties
+ */
+export function updateRules(updatedRules) {
+  lintRules.clear()
+  updatedRules.forEach(([rule, props]) => lintRules.set(rule, props))
+  updateFixableRules()
+}
+
+/**
+ * Return the known list of rules.
+ * @return {Map} The currently known rules
+ */
+export function getRules() {
+  return lintRules
+}
+
 /**
  * Processes the response from the lint job
  * @param  {Object}     response   The raw response from the job
@@ -328,9 +381,8 @@ export async function processESLintMessages(messages, textEditor, showRule, work
  * @return {Promise}               The messages transformed into Linter messages
  */
 export async function processJobResponse(response, textEditor, showRule, worker) {
-  if (Object.prototype.hasOwnProperty.call(response, 'fixableRules')) {
-    fixableRules.clear()
-    response.fixableRules.forEach(rule => fixableRules.add(rule))
+  if (Object.prototype.hasOwnProperty.call(response, 'updatedRules')) {
+    updateRules(response.updatedRules)
   }
   return processESLintMessages(response.messages, textEditor, showRule, worker)
 }

src/worker-helpers.js

@@ -184,3 +184,56 @@ export function getCLIEngineOptions(type, config, rules, filePath, fileDir, give
 
   return cliEngineConfig
 }
+
+/**
+ * Gets the list of rules used for a lint job
+ * @param  {Object} cliEngine The CLIEngine instance used for the lint job
+ * @return {Map}              A Map of the rules used, rule names as keys, rule
+ *                            properties as the contents.
+ */
+export function getRules(cliEngine) {
+  // Pull the list of rules used directly from the CLIEngine
+  // Added in https://github.com/eslint/eslint/pull/9782
+  if (Object.prototype.hasOwnProperty.call(cliEngine, 'getRules')) {
+    return cliEngine.getRules()
+  }
+
+  // Attempt to use the internal (undocumented) `linter` instance attached to
+  // the CLIEngine to get the loaded rules (including plugin rules).
+  // Added in ESLint v4
+  if (Object.prototype.hasOwnProperty.call(cliEngine, 'linter')) {
+    return cliEngine.linter.getRules()
+  }
+
+  // Older versions of ESLint don't (easily) support getting a list of rules
+  return new Map()
+}
+
+/**
+ * Given an exiting rule list and a new rule list, determines whether there
+ * have been changes.
+ * @param  {[type]} newRules     A map of the new rules
+ * @param  {[type]} currentRules A Map of the current rules
+ * @return {boolean}             Whether or not there were changes
+ */
+export function didRulesChange(currentRules, newRules) {
+  let rulesChanged = false
+  const currentRuleIds = new Set(currentRules.keys())
+  const newRuleIds = new Set(newRules.keys())
+
+  // Check for new rules added since the last time we sent currentRules
+  const newRulesIds = new Set(newRules.keys())
+  currentRuleIds.forEach(rule => newRulesIds.delete(rule))
+  if (newRulesIds.size > 0) {
+    rulesChanged = true
+  }
+
+  // Check for rules that were removed since the last time we sent currentRules
+  const removedRuleIds = new Set(currentRuleIds)
+  newRuleIds.forEach(rule => removedRuleIds.delete(rule))
+  if (removedRuleIds.size > 0) {
+    rulesChanged = true
+  }
+
+  return rulesChanged
+}

src/worker.js

@@ -9,64 +9,19 @@ import isConfigAtHomeRoot from './is-config-at-home-root'
 
 process.title = 'linter-eslint helper'
 
-const fixableRules = new Set()
+const rulesMetadata = new Map()
 let sendRules = false
 
-/**
- * Modifies the closed-over fixableRules variable when called _if_ there are
- * newly-loaded fixable rules or fixable rules are removed from the set of all
- * loaded rules, according to the eslint `linter` instance that is passed in.
- *
- * @param  {Object} linter eslint 'linter' instance
- * @return {void}
- */
-function updateFixableRules(linter) {
-  if (linter === undefined) {
-    // ESLint < v4 doesn't support this property
-    return
-  }
-
-  // Build a set of fixable rules based on the rules loaded in the provided linter
-  const currentRules = new Set()
-  linter.getRules().forEach((props, rule) => {
-    if (
-      Object.prototype.hasOwnProperty.call(props, 'meta') &&
-      Object.prototype.hasOwnProperty.call(props.meta, 'fixable')
-    ) {
-      currentRules.add(rule)
-    }
-  })
-
-  // Unless something has changed, we won't need to send updated set of fixableRules
-  sendRules = false
-
-  // Check for new fixable rules added since the last time we sent fixableRules
-  const newRules = new Set(currentRules)
-  fixableRules.forEach(rule => newRules.delete(rule))
-  if (newRules.size > 0) {
-    sendRules = true
-  }
-
-  // Check for fixable rules that were removed since the last time we sent fixableRules
-  const removedRules = new Set(fixableRules)
-  currentRules.forEach(rule => removedRules.delete(rule))
-  if (removedRules.size > 0) {
-    sendRules = true
-  }
-
-  if (sendRules) {
-    // Rebuild fixableRules
-    fixableRules.clear()
-    currentRules.forEach(rule => fixableRules.add(rule))
-  }
-}
-
 function lintJob({ cliEngineOptions, contents, eslint, filePath }) {
   const cliEngine = new eslint.CLIEngine(cliEngineOptions)
   const report = cliEngine.executeOnText(contents, filePath)
-  // Use the internal (undocumented) `linter` instance attached to the cliEngine
-  // to check the loaded rules (including plugin rules) and update our list of fixable rules.
-  updateFixableRules(cliEngine.linter)
+  const rules = Helpers.getRules(cliEngine)
+  sendRules = Helpers.didRulesChange(rulesMetadata, rules)
+  if (sendRules) {
+    // Rebuild rulesMetadata
+    rulesMetadata.clear()
+    rules.forEach((properties, rule) => rulesMetadata.set(rule, properties))
+  }
   return report
 }
 
@@ -111,7 +66,10 @@ module.exports = async () => {
         messages: report.results.length ? report.results[0].messages : []
       }
       if (sendRules) {
-        response.fixableRules = Array.from(fixableRules.keys())
+        // You can't emit Maps, convert to Array of Arrays to send back.
+        response.updatedRules = []
+        rulesMetadata.forEach((props, rule) =>
+          response.updatedRules.push([rule, props]))
       }
     } else if (type === 'fix') {
       response = fixJob({ cliEngineOptions, contents, eslint, filePath })