fix: detect ESLint >= 8 and tell the user about linter-eslint-node (#…

…1464)

Co-Authored-By: Tony Brix <tony@brix.ninja>

README.md

@@ -4,9 +4,11 @@
 [![Dependency Status](https://david-dm.org/AtomLinter/linter-eslint.svg)](https://david-dm.org/AtomLinter/linter-eslint)
 
 This linter plugin for [Linter](https://github.com/AtomLinter/Linter) provides
-an interface to [eslint](http://eslint.org). It will be used with files that
+an interface to [eslint](http://eslint.org) versions 7 and below. It will be used with files that
 have the "JavaScript" syntax.
 
+**For linting in projects that use ESLint v8 and above, install [linter-eslint-node](https://atom.io/packages/linter-eslint-node).**
+
 ## Installation
 
 ```ShellSession
@@ -24,7 +26,7 @@ This package requires an `eslint` of at least v1.0.0.
 
 If you do not have the `linter` package installed, it will be
 installed
-for you. If you are using an alternative `linter-*` consumer, 
+for you. If you are using an alternative `linter-*` consumer,
 the `linter` package can be disabled.
 
 If you wish to lint files in JavaScript-derivative languages (like Typescript,

package.json

@@ -2,7 +2,7 @@
   "name": "linter-eslint",
   "main": "./dist/main.js",
   "version": "9.0.0",
-  "description": "Lint JavaScript on the fly, using ESLint",
+  "description": "Lint JavaScript on the fly, using ESLint (v7 or older)",
   "repository": "https://github.com/AtomLinter/linter-eslint.git",
   "license": "MIT",
   "engines": {
@@ -155,6 +155,13 @@
           "type": "string",
           "default": "",
           "order": 5
+        },
+        "showIncompatibleVersionNotification": {
+          "title": "Notify when incompatible ESLint is detected",
+          "description": "When enabled, will show a notification if this package loads inside a project using ESLint version 8 or greater _and_ the user has not already installed the newer `linter-eslint-node` package. Uncheck if you don't want these notifications.",
+          "type": "boolean",
+          "default": true,
+          "order": 6
         }
       }
     }

spec/worker-helpers-spec.js

@@ -81,7 +81,7 @@ describe('Worker Helpers', () => {
         advanced: { localNodeModules: path }
       })
       const eslint = Helpers.getESLintInstance('', config)
-      expect(eslint).toBe('located')
+      expect(eslint.type).toBe('located')
     })
 
     it('tries to find an indirect local eslint using a relative path', () => {
@@ -93,13 +93,13 @@ describe('Worker Helpers', () => {
       })
       const eslint = Helpers.getESLintInstance('', config, projectPath)
 
-      expect(eslint).toBe('located')
+      expect(eslint.type).toBe('located')
     })
 
     it('tries to find a local eslint', () => {
       const config = createConfig()
       const eslint = Helpers.getESLintInstance(getFixturesPath('local-eslint'), config)
-      expect(eslint).toBe('located')
+      expect(eslint.type).toBe('located')
     })
 
     it('cries if local eslint is not found', () => {
@@ -109,13 +109,20 @@ describe('Worker Helpers', () => {
       }).toThrow()
     })
 
+    it('cries if incompatible eslint is found', () => {
+      expect(() => {
+        const config = createConfig()
+        Helpers.getESLintInstance(getFixturesPath('incompatible-eslint'), config)
+      }).toThrow()
+    })
+
     it('tries to find a global eslint if config is specified', () => {
       const config = createConfig({
         global: { useGlobalEslint: true, globalNodePath }
       })
       console.log({ config })
       const eslint = Helpers.getESLintInstance(getFixturesPath('local-eslint'), config)
-      expect(eslint).toBe('located')
+      expect(eslint.type).toBe('located')
     })
 
     it('cries if global eslint is not found', () => {
@@ -133,7 +140,7 @@ describe('Worker Helpers', () => {
       const fileDir = Path.join(getFixturesPath('local-eslint'), 'lib', 'foo.js')
       const config = createConfig()
       const eslint = Helpers.getESLintInstance(fileDir, config)
-      expect(eslint).toBe('located')
+      expect(eslint.type).toBe('located')
     })
   })
 

src/helpers.js

@@ -4,12 +4,16 @@ import { randomBytes } from 'crypto'
 import { promisify } from 'util'
 // eslint-disable-next-line import/no-extraneous-dependencies, import/extensions
 import { Range, Task } from 'atom'
+// eslint-disable-next-line import/no-unresolved
+import { shell } from 'electron'
 import Rules from './rules'
 import { throwIfInvalidPoint } from './validate/editor'
 
 const asyncRandomBytes = promisify(randomBytes)
 export const rules = new Rules()
 let worker = null
+let isIncompatibleEslintVersion = false
+let seenIncompatibleVersionNotification = false
 
 /**
  * Start the worker process if it hasn't already been started
@@ -48,6 +52,10 @@ export function killWorker() {
   }
 }
 
+export function isIncompatibleEslint() {
+  return isIncompatibleEslintVersion
+}
+
 /**
  * Send a job to the worker and return the results
  * @param  {Object} config Configuration for the job to send to the worker
@@ -74,11 +82,12 @@ export async function sendJob(config) {
     // All worker errors are caught and re-emitted along with their associated
     // emitKey, so that we do not create multiple listeners for the same
     // 'task:error' event
-    const errSub = worker.on(`workerError:${config.emitKey}`, ({ msg, stack }) => {
+    const errSub = worker.on(`workerError:${config.emitKey}`, ({ msg, stack, name }) => {
       // Re-throw errors from the task
       const error = new Error(msg)
       // Set the stack to the one given to us by the worker
       error.stack = stack
+      error.name = name
       errSub.dispose()
       // eslint-disable-next-line no-use-before-define
       responseSub.dispose()
@@ -189,6 +198,44 @@ export function generateUserMessage(textEditor, options) {
   }]
 }
 
+function isNewPackageInstalled() {
+  return atom.packages.isPackageLoaded('linter-eslint-node')
+   || atom.packages.isPackageDisabled('linter-eslint-node')
+}
+
+function showIncompatibleVersionNotification(message) {
+  const notificationEnabled = atom.config.get('linter-eslint.advanced.showIncompatibleVersionNotification')
+  if (!notificationEnabled || seenIncompatibleVersionNotification || isNewPackageInstalled()) {
+    return
+  }
+
+  // Show this message only once per session.
+  seenIncompatibleVersionNotification = true
+  const notification = atom.notifications.addWarning(
+    'linter-eslint: Incompatible version',
+    {
+      description: message,
+      dismissable: true,
+      buttons: [
+        {
+          text: 'Install linter-eslint-node',
+          onDidClick() {
+            shell.openExternal('https://atom.io/packages/linter-eslint-node')
+            notification.dismiss()
+          }
+        },
+        {
+          text: 'Don\'t show this notification again',
+          onDidClick() {
+            atom.config.set('linter-eslint.advanced.showIncompatibleVersionNotification', false)
+            notification.dismiss()
+          }
+        }
+      ]
+    }
+  )
+}
+
 /**
  * Generates a message to the user in order to nicely display the Error being
  * thrown instead of depending on generic error handling.
@@ -197,10 +244,19 @@ export function generateUserMessage(textEditor, options) {
  * @return {import("atom/linter").Message[]} Message to user generated from the Error
  */
 export function handleError(textEditor, error) {
-  const { stack, message } = error
+  const { stack, message, name } = error
+  // We want this specific worker error to show up as a notification so that we
+  // can include a button for installing the new package.
+  if (name === 'IncompatibleESLintError') {
+    isIncompatibleEslintVersion = true
+    killWorker()
+    showIncompatibleVersionNotification(message)
+    return
+  }
   // Only show the first line of the message as the excerpt
   const excerpt = `Error while running ESLint: ${message.split('\n')[0]}.`
   const description = `<div style="white-space: pre-wrap">${message}\n<hr />${stack}</div>`
+  // eslint-disable-next-line consistent-return
   return generateUserMessage(textEditor, { severity: 'error', excerpt, description })
 }
 

src/main.js

@@ -182,6 +182,13 @@ module.exports = {
           return null
         }
 
+        if (helpers.isIncompatibleEslint()) {
+          // The project's version of ESLint doesn't work with this package. Once
+          // this is detected, we won't try to send any jobs until the window is
+          // reloaded.
+          return null
+        }
+
         const filePath = textEditor.getPath()
         if (!filePath) {
           // The editor currently has no path, we can't report messages back to
@@ -246,6 +253,13 @@ module.exports = {
       return
     }
 
+    if (helpers.isIncompatibleEslint()) {
+      // The project's version of ESLint doesn't work with this package. Once
+      // this is detected, we won't try to send any jobs until the window is
+      // reloaded.
+      return
+    }
+
     if (textEditor.isModified()) {
       // Abort for invalid or unsaved text editors
       const message = 'Linter-ESLint: Please save before fixing'
@@ -280,6 +294,9 @@ module.exports = {
         atom.notifications.addSuccess(response)
       }
     } catch (err) {
+      if (err.name === 'IncompatibleESLintError') {
+        return
+      }
       atom.notifications.addWarning(err.message)
     }
   },

src/worker-helpers.js

@@ -14,6 +14,14 @@ const Cache = {
   LAST_MODULES_PATH: null
 }
 
+class IncompatibleESLintError extends Error {
+  constructor(version) {
+    // eslint-disable-next-line max-len
+    super(`The version of ESLint used in this project is ${version}, which is incompatible with this package. The \`linter-eslint-node\` Atom package provides support for ESLint versions 8 and higher.\n\nYou can disable this notice in the linter-eslint package settings under **Uncommon → Notify when incompatible ESLint is detected**.`)
+    this.name = 'IncompatibleESLintError'
+  }
+}
+
 /**
  * Takes a path and translates `~` to the user's home directory, and replaces
  * all environment variables with their value.
@@ -112,6 +120,27 @@ export function findESLintDirectory(modulesDir, config, projectPath, fallbackFor
   }
 }
 
+// Given an ESLint module path, checks its version and throws if the version is
+// too new for this package to support.
+function checkForIncompatibleESLint(directory) {
+  let packageMeta
+  try {
+    // eslint-disable-next-line import/no-dynamic-require
+    packageMeta = require(Path.join(directory, 'package.json'))
+    if (!packageMeta || !packageMeta.version) {
+      return
+    }
+  } catch (_) {
+    return
+  }
+  // We don't need sophisticated parsing logic here; we just need to look at
+  // the major version.
+  const m = packageMeta.version.match(/^([\d]+)\./)
+  if (m && Number(m[1]) > 7) {
+    throw new IncompatibleESLintError(packageMeta.version)
+  }
+}
+
 /**
  * @param {string} modulesDir
  * @param {object} config
@@ -120,10 +149,19 @@ export function findESLintDirectory(modulesDir, config, projectPath, fallbackFor
  */
 export function getESLintFromDirectory(modulesDir, config, projectPath) {
   const { path: ESLintDirectory } = findESLintDirectory(modulesDir, config, projectPath)
+  let eslint
   try {
     // eslint-disable-next-line import/no-dynamic-require
-    return require(ESLintDirectory)
+    eslint = require(ESLintDirectory)
+    if (!('CLIEngine' in eslint)) {
+      checkForIncompatibleESLint(ESLintDirectory)
+    }
+    return eslint
   } catch (e) {
+    // If this is the result of an incompatible ESLint, an error will be
+    // thrown; otherwise we should proceed with the local-path fallback.
+    checkForIncompatibleESLint(ESLintDirectory)
+
     if (config.global.useGlobalEslint && e.code === 'MODULE_NOT_FOUND') {
       throw new Error('ESLint not found, try restarting Atom to clear caches.')
     }

src/worker.js

@@ -99,7 +99,11 @@ module.exports = async () => {
       }
       emit(emitKey, response)
     } catch (workerErr) {
-      emit(`workerError:${emitKey}`, { msg: workerErr.message, stack: workerErr.stack })
+      emit(`workerError:${emitKey}`, {
+        msg: workerErr.message,
+        stack: workerErr.stack,
+        name: workerErr.name
+      })
     }
   })
 }