Use executeOnText for fix jobs

This prevents ESLint from deleting the users cache file, because `executeOnText` bypasses the cache logic in ESLint entirely.
AtomLinter · May 2, 2017 · 62f416f · 62f416f
1 parent 0bdcfa7
commit 62f416f
Show file tree

Hide file tree

Showing 8 changed files with 94 additions and 39 deletions.
diff --git a/lib/main.js b/lib/main.js
@@ -115,6 +115,10 @@ module.exports = {
           if (!workerHelpers) {
             workerHelpers = require('./worker-helpers');
           }
+          const text = editor.getText();
+          if (text.length === 0) {
+            return;
+          }
           const filePath = editor.getPath();
           const projectPath = atom.project.relativizePath(filePath)[0];
 
@@ -135,6 +139,7 @@ module.exports = {
           _this.worker.request('job', {
             type: 'fix',
             config: atom.config.get('linter-eslint'),
+            contents: text,
             rules,
             filePath,
             projectPath
@@ -175,17 +180,20 @@ module.exports = {
             yield waitOnIdle();
           }
           const textEditor = atom.workspace.getActiveTextEditor();
+          const text = textEditor.getText();
+          if (text.length === 0) {
+            return;
+          }
           const filePath = textEditor.getPath();
           const projectPath = atom.project.relativizePath(filePath)[0];
 
-          if (!textEditor || textEditor.isModified()) {
+          if (!textEditor) {
             // Abort for invalid or unsaved text editors
-            atom.notifications.addError('Linter-ESLint: Please save before fixing');
             return;
           }
 
           let rules = {};
-          if (textEditor.isModified() && Object.keys(ignoredRulesWhenFixing).length > 0) {
+          if (Object.keys(ignoredRulesWhenFixing).length > 0) {
             rules = ignoredRulesWhenFixing;
           }
 
@@ -195,6 +203,7 @@ module.exports = {
           _this.worker.request('job', {
             type: 'fix',
             config: atom.config.get('linter-eslint'),
+            contents: text,
             rules,
             filePath,
             projectPath

diff --git a/lib/worker-helpers.js b/lib/worker-helpers.js
@@ -159,10 +159,11 @@ function getRelativePath(fileDir, filePath, config) {
 }
 
 function getCLIEngineOptions(type, config, rules, filePath, fileDir, givenConfigPath) {
+  const isFix = type === 'fix';
   const cliEngineConfig = {
     rules,
     ignore: !config.disableEslintIgnore,
-    fix: type === 'fix'
+    fix: isFix
   };
 
   const ignoreFile = config.disableEslintIgnore ? null : (0, _atomLinter.findCached)(fileDir, '.eslintignore');

diff --git a/lib/worker.js b/lib/worker.js
@@ -46,16 +46,16 @@ function lintJob(_ref) {
       filePath = _ref.filePath;
 
   const cliEngine = new eslint.CLIEngine(cliEngineOptions);
-
-  return typeof contents === 'string' ? cliEngine.executeOnText(contents, filePath) : cliEngine.executeOnFiles([filePath]);
+  return cliEngine.executeOnText(contents, filePath);
 }
 
 function fixJob(_ref2) {
   let cliEngineOptions = _ref2.cliEngineOptions,
+      contents = _ref2.contents,
       eslint = _ref2.eslint,
       filePath = _ref2.filePath;
 
-  const report = lintJob({ cliEngineOptions, eslint, filePath });
+  const report = lintJob({ cliEngineOptions, contents, eslint, filePath });
 
   eslint.CLIEngine.outputFixes(report);
 
@@ -91,7 +91,7 @@ function fixJob(_ref2) {
     const report = lintJob({ cliEngineOptions, contents, eslint, filePath });
     job.response = report.results.length ? report.results[0].messages.filter(shouldBeReported) : [];
   } else if (type === 'fix') {
-    job.response = fixJob({ cliEngineOptions, eslint, filePath });
+    job.response = fixJob({ cliEngineOptions, contents, eslint, filePath });
   } else if (type === 'debug') {
     const modulesDir = _path2.default.dirname((0, _atomLinter.findCached)(fileDir, 'node_modules/eslint') || '');
     job.response = Helpers.findESLintDirectory(modulesDir, config);

diff --git a/spec/fixtures/files/.eslintcache b/spec/fixtures/files/.eslintcache
diff --git a/spec/linter-eslint-spec.js b/spec/linter-eslint-spec.js
@@ -15,6 +15,7 @@ const badPath = path.join(fixturesDir, 'files', 'bad.js')
 const badInlinePath = path.join(fixturesDir, 'files', 'badInline.js')
 const emptyPath = path.join(fixturesDir, 'files', 'empty.js')
 const fixPath = path.join(fixturesDir, 'files', 'fix.js')
+const cachePath = path.join(fixturesDir, 'files', '.eslintcache')
 const configPath = path.join(fixturesDir, 'configs', '.eslintrc.yml')
 const importingpath = path.join(fixturesDir,
   'import-resolution', 'nested', 'importing.js')
@@ -43,6 +44,12 @@ function copyFileToDir(fileToCopyPath, destinationDir) {
   })
 }
 
+/**
+ * Utility helper to copy a file into the OS temp directory.
+ *
+ * @param  {string} fileToCopyPath  Path of the file to be copied
+ * @return {string}                 Full path of the file in copy destination
+ */
 function copyFileToTempDir(fileToCopyPath) {
   return new Promise(async (resolve) => {
     const tempFixtureDir = fs.mkdtempSync(tmpdir() + path.sep)
@@ -63,6 +70,25 @@ async function getNotification() {
   })
 }
 
+async function makeFixes(textEditor) {
+  return new Promise(async (resolve) => {
+    // Subscribe to the file reload event
+    const editorReloadSub = textEditor.getBuffer().onDidReload(async () => {
+      editorReloadSub.dispose()
+      // File has been reloaded in Atom, notification checking will happen
+      // async either way, but should already be finished at this point
+      resolve()
+    })
+
+    // Now that all the required subscriptions are active, send off a fix request
+    atom.commands.dispatch(atom.views.getView(textEditor), 'linter-eslint:fix-file')
+    const notification = await getNotification()
+
+    expect(notification.getMessage()).toBe('Linter-ESLint: Fix complete.')
+    expect(notification.getType()).toBe('success')
+  })
+}
+
 describe('The eslint provider for Linter', () => {
   const linterProvider = linterEslint.provideLinter()
   const lint = linterProvider.lint
@@ -205,25 +231,6 @@ describe('The eslint provider for Linter', () => {
       expect(messages.length).toBe(2)
     }
 
-    async function makeFixes(textEditor) {
-      return new Promise(async (resolve) => {
-        // Subscribe to the file reload event
-        const editorReloadSub = textEditor.getBuffer().onDidReload(async () => {
-          editorReloadSub.dispose()
-          // File has been reloaded in Atom, notification checking will happen
-          // async either way, but should already be finished at this point
-          resolve()
-        })
-
-        // Now that all the required subscriptions are active, send off a fix request
-        atom.commands.dispatch(atom.views.getView(textEditor), 'linter-eslint:fix-file')
-        const notification = await getNotification()
-
-        expect(notification.getMessage()).toBe('Linter-ESLint: Fix complete.')
-        expect(notification.getType()).toBe('success')
-      })
-    }
-
     it('should fix linting errors', async () => {
       await firstLint(editor)
       await makeFixes(editor)
@@ -248,6 +255,35 @@ describe('The eslint provider for Linter', () => {
     })
   })
 
+  describe('when an eslint cache file is present', () => {
+    let editor
+    let tempDir
+
+    beforeEach(async () => {
+      // Copy the file to a temporary folder
+      const tempFixturePath = await copyFileToTempDir(fixPath)
+      editor = await atom.workspace.open(tempFixturePath)
+      tempDir = path.dirname(tempFixturePath)
+      // Copy the config to the same temporary directory
+      await copyFileToDir(configPath, tempDir)
+    })
+
+    afterEach(() => {
+      // Remove the temporary directory
+      rimraf.sync(tempDir)
+    })
+
+    it('does not delete the cache file when performing fixes', async () => {
+      const tempCacheFile = await copyFileToDir(cachePath, tempDir)
+      const checkCachefileExists = () => {
+        fs.statSync(tempCacheFile)
+      }
+      expect(checkCachefileExists).not.toThrow()
+      await makeFixes(editor)
+      expect(checkCachefileExists).not.toThrow()
+    })
+  })
+
   describe('Ignores specified rules when editing', () => {
     const expected = 'Trailing spaces not allowed. ' +
       '(<a href="http://eslint.org/docs/rules/no-trailing-spaces">no-trailing-spaces</a>)'

diff --git a/src/main.js b/src/main.js
@@ -109,6 +109,10 @@ module.exports = {
           if (!workerHelpers) {
             workerHelpers = require('./worker-helpers')
           }
+          const text = editor.getText()
+          if (text.length === 0) {
+            return
+          }
           const filePath = editor.getPath()
           const projectPath = atom.project.relativizePath(filePath)[0]
 
@@ -129,6 +133,7 @@ module.exports = {
           this.worker.request('job', {
             type: 'fix',
             config: atom.config.get('linter-eslint'),
+            contents: text,
             rules,
             filePath,
             projectPath
@@ -162,17 +167,20 @@ module.exports = {
           await waitOnIdle()
         }
         const textEditor = atom.workspace.getActiveTextEditor()
+        const text = textEditor.getText()
+        if (text.length === 0) {
+          return
+        }
         const filePath = textEditor.getPath()
         const projectPath = atom.project.relativizePath(filePath)[0]
 
-        if (!textEditor || textEditor.isModified()) {
+        if (!textEditor) {
           // Abort for invalid or unsaved text editors
-          atom.notifications.addError('Linter-ESLint: Please save before fixing')
           return
         }
 
         let rules = {}
-        if (textEditor.isModified() && Object.keys(ignoredRulesWhenFixing).length > 0) {
+        if (Object.keys(ignoredRulesWhenFixing).length > 0) {
           rules = ignoredRulesWhenFixing
         }
 
@@ -182,6 +190,7 @@ module.exports = {
         this.worker.request('job', {
           type: 'fix',
           config: atom.config.get('linter-eslint'),
+          contents: text,
           rules,
           filePath,
           projectPath

diff --git a/src/worker-helpers.js b/src/worker-helpers.js
@@ -135,11 +135,14 @@ export function getRelativePath(fileDir, filePath, config) {
   return Path.basename(filePath)
 }
 
-export function getCLIEngineOptions(type, config, rules, filePath, fileDir, givenConfigPath) {
+export function getCLIEngineOptions(
+  type, config, rules, filePath, fileDir, givenConfigPath
+) {
+  const isFix = type === 'fix'
   const cliEngineConfig = {
     rules,
     ignore: !config.disableEslintIgnore,
-    fix: type === 'fix'
+    fix: isFix,
   }
 
   const ignoreFile = config.disableEslintIgnore ? null : findCached(fileDir, '.eslintignore')

diff --git a/src/worker.js b/src/worker.js
@@ -30,14 +30,11 @@ function shouldBeReported(problem) {
 
 function lintJob({ cliEngineOptions, contents, eslint, filePath }) {
   const cliEngine = new eslint.CLIEngine(cliEngineOptions)
-
-  return typeof contents === 'string'
-    ? cliEngine.executeOnText(contents, filePath)
-    : cliEngine.executeOnFiles([filePath])
+  return cliEngine.executeOnText(contents, filePath)
 }
 
-function fixJob({ cliEngineOptions, eslint, filePath }) {
-  const report = lintJob({ cliEngineOptions, eslint, filePath })
+function fixJob({ cliEngineOptions, contents, eslint, filePath }) {
+  const report = lintJob({ cliEngineOptions, contents, eslint, filePath })
 
   eslint.CLIEngine.outputFixes(report)
 
@@ -68,7 +65,7 @@ create().onRequest('job', ({ contents, type, config, filePath, projectPath, rule
     const report = lintJob({ cliEngineOptions, contents, eslint, filePath })
     job.response = report.results.length ? report.results[0].messages.filter(shouldBeReported) : []
   } else if (type === 'fix') {
-    job.response = fixJob({ cliEngineOptions, eslint, filePath })
+    job.response = fixJob({ cliEngineOptions, contents, eslint, filePath })
   } else if (type === 'debug') {
     const modulesDir = Path.dirname(findCached(fileDir, 'node_modules/eslint') || '')
     job.response = Helpers.findESLintDirectory(modulesDir, config)