Merge pull request #197 from AtomLinter/arcanemagus/rewrite-parsing

Re-write the parsing engine

Author: Arcanemagus

lib/main.js

@@ -6,7 +6,77 @@ import { dirname, extname } from 'path';
 
 let helpers = null;
 let clangFlags = null;
-const regex = /(.+):(\d+):(\d+):(?:{(\d+):(\d+)-(\d+):(\d+)}.*:)? ([\w \\-]+): (.*)/g;
+
+const regex = new RegExp([
+  '^(<stdin>|.+):', // Path, usually <stdin>
+  '(\\d+):(\\d+):', // Base line and column
+  '(?:({.+}):)?', // Range position(s), if present
+  ' ([\\w \\\\-]+):', // Message type
+  ' ([^[\\n\\r]+)', // The message
+  '(?: \\[(.+)\\])?\\r?$', // -W flag, if any
+  '(?:\\r?\\n^ .+$)+', // The visual caret diagnostics, necessary to include in output for fix-its
+  '(?:\\r?\\n^fix-it:".+":', // Start of fix-it block
+  '{(\\d+):(\\d+)-(\\d+):(\\d+)}:', // fix-it range
+  '"(.+)"', // fix-it replacement text
+  '$)?', // End of fix-it block
+].join(''), 'gm');
+
+/**
+ * Given a set of ranges in clangs format, determine the range encompasing all points
+ * @param  {String} ranges The raw range string to parse
+ * @return {Range}        An Atom Range object encompasing all given ranges
+ */
+const parseClangRanges = (ranges) => {
+  const rangeRE = /{(\d+):(\d+)-(\d+):(\d+)}/g;
+  let lineStart;
+  let colStart;
+  let lineEnd;
+  let colEnd;
+
+  let match = rangeRE.exec(ranges);
+  while (match !== null) {
+    const rangeLineStart = Number.parseInt(match[1], 10) - 1;
+    const rangeColStart = Number.parseInt(match[2], 10) - 1;
+    const rangeLineEnd = Number.parseInt(match[3], 10) - 1;
+    const rangeColEnd = Number.parseInt(match[4], 10) - 1;
+    if (lineStart === undefined) {
+      // First match
+      lineStart = rangeLineStart;
+      colStart = rangeColStart;
+      lineEnd = rangeLineEnd;
+      colEnd = rangeColEnd;
+    } else {
+      if (rangeLineStart > lineEnd) {
+        // Higher starting line
+        lineEnd = rangeLineStart;
+        colEnd = rangeColStart;
+      }
+      if (rangeLineEnd > lineEnd) {
+        // Higher ending line
+        lineEnd = rangeLineEnd;
+        colEnd = rangeColEnd;
+      }
+      if (rangeColEnd > colEnd) {
+        // Higher ending column
+        colEnd = rangeColEnd;
+      }
+    }
+    match = rangeRE.exec(ranges);
+  }
+  return [[lineStart, colStart], [lineEnd, colEnd]];
+};
+
+/**
+ * Determine if a given path is open in an existing TextEditor
+ * @param  {String} filePath The file path to search for an editor of
+ * @return {TextEditor | false}      The TextEditor or false if none found
+ */
+const findTextEditor = (filePath) => {
+  const allEditors = atom.workspace.getTextEditors();
+  const matchingEditor = allEditors.find(
+    textEditor => textEditor.getPath() === filePath);
+  return matchingEditor || false;
+};
 
 export default {
   activate() {
@@ -84,7 +154,7 @@ export default {
         }
 
         const filePath = editor.getPath();
-        if (filePath === undefined) {
+        if (typeof filePath === 'undefined') {
           // The editor has no path, meaning it hasn't been saved. Although
           // clang could give us results for this, Linter needs a path
           return [];
@@ -95,8 +165,9 @@ export default {
 
         const args = [
           '-fsyntax-only',
-          '-fno-caret-diagnostics',
-          '-fno-diagnostics-fixit-info',
+          '-fno-color-diagnostics',
+          '-fdiagnostics-absolute-paths',
+          '-fdiagnostics-parseable-fixits',
           '-fdiagnostics-print-source-range-info',
           '-fexceptions',
           `-ferror-limit=${this.clangErrorLimit}`,
@@ -168,35 +239,62 @@ export default {
 
         if (editor.getText() !== fileText) {
           // Editor contents have changed, tell Linter not to update results
-          // eslint-disable-next-line no-console
-          console.warn('linter-clang: Editor contents changed, not updating results');
           return null;
         }
 
         const toReturn = [];
-        let match = regex.exec(output);
 
+        let match = regex.exec(output);
         while (match !== null) {
+          const isCurrentFile = match[1] === '<stdin>';
+          // If the "file" is stdin, override to the current editor's path
+          const file = isCurrentFile ? filePath : match[1];
           let position;
-          if (match[4] !== undefined) {
-            const lineStart = Number.parseInt(match[4], 10) - 1;
-            const colStart = Number.parseInt(match[5], 10) - 1;
-            const lineEnd = Number.parseInt(match[6], 10) - 1;
-            const colEnd = Number.parseInt(match[7], 10) - 1;
-            position = [[lineStart, colStart], [lineEnd, colEnd]];
+          if (match[4]) {
+            // Clang gave us a range, use that
+            position = parseClangRanges(match[4]);
           } else {
+            // Generate a range based on the single point
             const line = Number.parseInt(match[2], 10) - 1;
             const col = Number.parseInt(match[3], 10) - 1;
-            position = helpers.generateRange(editor, line, col);
+            if (!isCurrentFile) {
+              const fileEditor = findTextEditor(match[1]);
+              if (fileEditor !== false) {
+                // Found an open editor for the file
+                position = helpers.generateRange(fileEditor, line, col);
+              } else {
+                // Generate a one character range in the file
+                position = [[line, col], [line, col + 1]];
+              }
+            } else {
+              position = helpers.generateRange(editor, line, col);
+            }
           }
-          const severity = /error/.test(match[8]) ? 'error' : 'warning';
-          // Handle paths other than the file being linted still
-          const file = match[1] === '<stdin>' ? filePath : match[1];
-          toReturn.push({
+          const severity = /error/.test(match[5]) ? 'error' : 'warning';
+          let excerpt;
+          if (match[7]) {
+            // There is a -Wflag specified, for now just re-insert that into the excerpt
+            excerpt = `${match[6]} [${match[7]}]`;
+          } else {
+            excerpt = match[6];
+          }
+          const message = {
             severity,
             location: { file, position },
-            excerpt: match[9],
-          });
+            excerpt,
+          };
+          if (match[8]) {
+            // We have a suggested fix available
+            const fixLineStart = Number.parseInt(match[8], 10) - 1;
+            const fixColStart = Number.parseInt(match[9], 10) - 1;
+            const fixLineEnd = Number.parseInt(match[10], 10) - 1;
+            const fixColEnd = Number.parseInt(match[11], 10) - 1;
+            message.solutions = [{
+              position: [[fixLineStart, fixColStart], [fixLineEnd, fixColEnd]],
+              replaceWith: match[12],
+            }];
+          }
+          toReturn.push(message);
           match = regex.exec(output);
         }
 

spec/files/fixit.hpp

@@ -0,0 +1,4 @@
+#ifndef _FIXIT_H
+#define _FIXIT_H
+
+#endif FIXIT_H

spec/files/multi-range.cpp

@@ -0,0 +1,6 @@
+int main(int argc, char const *argv[]) {
+  double foo = 1;
+  double bar = 2;
+  int foobar = foo % bar;
+  return 0;
+}

spec/files/otherFile/a.cpp

@@ -0,0 +1 @@
+#include "a.hpp"

spec/files/otherFile/a.hpp

@@ -0,0 +1,3 @@
+#pragma once
+
+#include "b.hpp"

spec/files/otherFile/b.hpp

@@ -0,0 +1,3 @@
+#pragma once
+
+int foo += 1;

spec/linter-clang-spec.js

@@ -2,13 +2,19 @@
 
 // eslint-disable-next-line import/no-extraneous-dependencies
 import { beforeEach, it } from 'jasmine-fix';
+// Note, when testing if using fit you must import it!
 import { join } from 'path';
 
 const lint = require('../lib/main').provideLinter().lint;
 
 const miPath = join(__dirname, 'files', 'missing_import');
 const poPath = join(__dirname, 'files', 'pragma', 'pragma_once');
 const validPath = join(__dirname, 'files', 'valid.c');
+const multiRangePath = join(__dirname, 'files', 'multi-range.cpp');
+const fixitPath = join(__dirname, 'files', 'fixit.hpp');
+const otherPath = join(__dirname, 'files', 'otherFile');
+const otherAPath = join(otherPath, 'a.cpp');
+const otherBPath = join(otherPath, 'b.hpp');
 const fileText = `// This is a comment, this will not return any errors.
 #include "nothing.h"
 
@@ -119,4 +125,50 @@ describe('The Clang provider for AtomLinter', () => {
     expect(messages[0].location.file).toBe(validPath);
     expect(messages[0].location.position).toEqual([[1, 9], [1, 20]]);
   });
+
+  it('handles multiple ranges', async () => {
+    const editor = await atom.workspace.open(multiRangePath);
+    const messages = await lint(editor);
+    expect(messages.length).toBe(1);
+    expect(messages[0].severity).toBe('error');
+    expect(messages[0].excerpt).toBe("invalid operands to binary expression ('double' and 'double')");
+    expect(messages[0].location.file).toBe(multiRangePath);
+    expect(messages[0].location.position).toEqual([[3, 15], [3, 24]]);
+  });
+
+  it('handles suggested fixes', async () => {
+    const editor = await atom.workspace.open(fixitPath);
+    const messages = await lint(editor);
+    expect(messages.length).toBe(1);
+    expect(messages[0].severity).toBe('warning');
+    expect(messages[0].excerpt).toBe('extra tokens at end of #endif directive [-Wextra-tokens]');
+    expect(messages[0].location.file).toBe(fixitPath);
+    expect(messages[0].location.position).toEqual([[3, 7], [3, 14]]);
+    expect(messages[0].solutions.length).toBe(1);
+    expect(messages[0].solutions[0].position).toEqual([[3, 7], [3, 7]]);
+    expect(messages[0].solutions[0].replaceWith).toBe('//');
+  });
+
+  describe('handles messages in other files', () => {
+    it('without another editor open', async () => {
+      const editorA = await atom.workspace.open(otherAPath);
+      const messages = await lint(editorA);
+      expect(messages.length).toBe(1);
+      expect(messages[0].severity).toBe('error');
+      expect(messages[0].excerpt).toBe("invalid '+=' at end of declaration; did you mean '='?");
+      expect(messages[0].location.file).toBe(otherBPath);
+      expect(messages[0].location.position).toEqual([[2, 8], [2, 9]]);
+    });
+
+    it('with another editor open', async () => {
+      const editorA = await atom.workspace.open(otherAPath);
+      await atom.workspace.open(otherBPath);
+      const messages = await lint(editorA);
+      expect(messages.length).toBe(1);
+      expect(messages[0].severity).toBe('error');
+      expect(messages[0].excerpt).toBe("invalid '+=' at end of declaration; did you mean '='?");
+      expect(messages[0].location.file).toBe(otherBPath);
+      expect(messages[0].location.position).toEqual([[2, 8], [2, 12]]);
+    });
+  });
 });