Use getRules and rule URL metadata where available

spec/rules-spec.js

@@ -0,0 +1,83 @@
+'use babel'
+
+import Rules from '../src/rules'
+
+describe('The Rules class', () => {
+  describe('replaceRules', () => {
+    const ruleArray = [
+      ['foo', { meta: { fixable: true } }],
+      ['bar', { meta: {} }]
+    ]
+
+    it('adds new rules', () => {
+      const rules = new Rules()
+      expect(rules.getRules()).toEqual(new Map())
+      rules.replaceRules(ruleArray)
+      expect(rules.getRules()).toEqual(new Map(ruleArray))
+    })
+
+    it('removes old rules', () => {
+      const rules = new Rules()
+      rules.replaceRules(ruleArray)
+      expect(rules.getRules()).toEqual(new Map(ruleArray))
+      rules.replaceRules([])
+      expect(rules.getRules()).toEqual(new Map())
+    })
+
+    it('updates the fixableRules list', () => {
+      const rules = new Rules()
+      expect(rules.getFixableRules()).toEqual([])
+      rules.replaceRules(ruleArray)
+      expect(rules.getFixableRules()).toEqual(['foo'])
+    })
+
+    it('validates new rules', () => {
+      const rules = new Rules()
+      const testRuleReplace = (value, throws = true) => {
+        const test = () => {
+          rules.replaceRules(value)
+        }
+        if (throws) {
+          expect(test).toThrow()
+        } else {
+          expect(test).not.toThrow()
+        }
+      }
+
+      // Invalid
+      testRuleReplace('foobar')
+      testRuleReplace({})
+      testRuleReplace(null)
+      testRuleReplace(undefined)
+      testRuleReplace([[]])
+      testRuleReplace([['foo']])
+      testRuleReplace([['foo', 'bar']])
+      testRuleReplace([['foo', []]])
+      testRuleReplace([['foo', null]])
+      testRuleReplace([['foo', undefined]])
+      // Valid
+      testRuleReplace([], false)
+      testRuleReplace([['foo', {}]], false)
+    })
+  })
+
+  describe('getRuleUrl', () => {
+    it('works with rules that define their own URL', () => {
+      const rules = new Rules()
+      rules.replaceRules([['foo', { meta: { docs: { url: 'bar' } } }]])
+      expect(rules.getRuleUrl('foo')).toBe('bar')
+    })
+
+    it('works with rules defined in eslint-rule-documentation', () => {
+      const rules = new Rules()
+      const url = 'https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-duplicates.md'
+      expect(rules.getRuleUrl('import/no-duplicates')).toBe(url)
+    })
+
+    it('gives a fallback URL on how to add a rule URL', () => {
+      const rules = new Rules()
+      const url = 'https://github.com/jfmengels/eslint-rule-documentation/blob/master/contributing.md'
+      expect(rules.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

@@ -1,14 +1,13 @@
 'use babel'
 
 import { join } from 'path'
-import ruleURI from 'eslint-rule-documentation'
 import { generateRange } from 'atom-linter'
 import cryptoRandomString from 'crypto-random-string'
-
 // eslint-disable-next-line import/no-extraneous-dependencies, import/extensions
 import { Range } from 'atom'
+import Rules from './rules'
 
-const fixableRules = new Set()
+export const rules = new Rules()
 
 /**
  * Start the worker process if it hasn't already been started
@@ -72,10 +71,6 @@ export async function sendJob(worker, config) {
   })
 }
 
-export function getFixableRules() {
-  return Array.from(fixableRules.values())
-}
-
 function validatePoint(textBuffer, line, col) {
   // Clip the given point to a valid one, and check if it equals the original
   if (!textBuffer.clipPosition([line, col]).isEqual([line, col])) {
@@ -286,7 +281,7 @@ export async function processESLintMessages(messages, textEditor, showRule, work
     }
 
     if (ruleId) {
-      ret.url = ruleURI(ruleId).url
+      ret.url = rules.getRuleUrl(ruleId)
     }
 
     let range
@@ -335,9 +330,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')) {
+    rules.replaceRules(response.updatedRules)
   }
   return processESLintMessages(response.messages, textEditor, showRule, worker)
 }

src/main.js

@@ -274,7 +274,7 @@ module.exports = {
         }
         if (textEditor.isModified() && ignoreFixableRulesWhileTyping) {
           // Note that this list will only contain rules after the first lint job
-          rules = idsToIgnoredRules(helpers.getFixableRules())
+          rules = idsToIgnoredRules(helpers.rules.getFixableRules())
         }
 
         if (!this.worker) {

src/rules.js

@@ -0,0 +1,88 @@
+import ruleURI from 'eslint-rule-documentation'
+
+// Private properties
+const rules = Symbol('rules')
+const validRulesArray = Symbol('validRulesArray')
+const validateRulesArray = Symbol('validateRulesArray')
+
+/**
+ * Stores a list of rules from ESLint
+ */
+export default class Rules {
+  /**
+   * Instantiates a Rules object, optionally with an existing list of rules
+   * @param {Array} newRules Array of Arrays of the rule and properties
+   */
+  constructor(newRules) {
+    if (Rules[validRulesArray](newRules)) {
+      this[rules] = new Map(newRules)
+    } else {
+      this[rules] = new Map()
+    }
+  }
+
+  /**
+   * Process the updated rules into the local Map and call further update functions
+   * @param  {Array} newRules Array of Arrays of the rule and properties
+   */
+  replaceRules(newRules) {
+    Rules[validateRulesArray](newRules)
+    this[rules] = new Map(newRules)
+  }
+
+  /**
+   * [getFixableRules description]
+   * @return {Array} The ruleIds of the currently known fixable rules
+   */
+  getFixableRules() {
+    return Array.from(this[rules]).reduce((fixable, [rule, props]) => {
+      if (props.meta && props.meta.fixable) {
+        return [...fixable, rule]
+      }
+      return fixable
+    }, [])
+  }
+
+  /**
+   * 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
+   */
+  getRuleUrl(ruleId) {
+    const props = this[rules].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
+  }
+
+  /**
+   * Return the known rules.
+   * @return {Map} The currently known rules
+   */
+  getRules() {
+    return new Map(this[rules])
+  }
+
+  static [validRulesArray](rulesArray) {
+    if (!Array.isArray(rulesArray)) {
+      return false
+    }
+    return rulesArray.every(elem =>
+      Array.isArray(elem) && elem.length === 2 &&
+      typeof elem[0] === 'string' && typeof elem[1] === 'object' &&
+      elem[1] !== null && !Array.isArray(elem[1]))
+  }
+
+  static [validateRulesArray](rulesArray) {
+    if (!Rules[validRulesArray](rulesArray)) {
+      throw new Error('Given rules are not valid!')
+    }
+  }
+}

src/worker-helpers.js

@@ -183,3 +183,41 @@ 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.
+ * NOTE: This only accounts for presence of the rules, changes to their metadata
+ * are not taken into account.
+ * @param  {Map} newRules     A Map of the new rules
+ * @param  {Map} currentRules A Map of the current rules
+ * @return {boolean}             Whether or not there were changes
+ */
+export function didRulesChange(currentRules, newRules) {
+  return !(currentRules.size === newRules.size &&
+    Array.from(currentRules.keys()).every(ruleId => newRules.has(ruleId)))
+}