Add protolint.lint command (#21)

plexsystems · Jun 2, 2021 · dd2f327 · dd2f327
1 parent e6dc9f5
commit dd2f327
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 88 deletions.
diff --git a/package.json b/package.json
@@ -31,6 +31,14 @@
     "onLanguage:proto"
   ],
   "main": "./out/src/extension",
+  "contributes": {
+    "commands": [
+      {
+        "command": "protolint.lint",
+        "title": "Protolint: Lint protobuf file"
+      }
+    ]
+  },
   "scripts": {
     "vscode:prepublish": "npm run compile",
     "compile": "tsc",

diff --git a/src/extension.ts b/src/extension.ts
@@ -2,34 +2,48 @@ import * as vscode from 'vscode';
 import * as cp from 'child_process';
 import Linter, { LinterError } from './linter';
 
+const diagnosticCollection = vscode.languages.createDiagnosticCollection("protolint");
+
 export function activate(context: vscode.ExtensionContext) {
+
+  // Verify that protolint can be successfully executed on the host machine by running the version command.
+  // In the event the binary cannot be executed, tell the user where to download protolint from.
   const result = cp.spawnSync('protolint', ['version']);
   if (result.status !== 0) {
     vscode.window.showErrorMessage("protolint was not detected. Download from: https://github.com/yoheimuta/protolint");
     return;
   }
 
-  const commandId = 'extension.protobuflint';
-  const diagnosticCollection = vscode.languages.createDiagnosticCollection(commandId);
-  let events = vscode.commands.registerCommand(commandId, () => {
-    vscode.workspace.onDidSaveTextDocument((document: vscode.TextDocument) => {
-      doLint(document, diagnosticCollection);
-    });
+  vscode.commands.registerCommand('protolint.lint', runLint);
 
-    vscode.workspace.onDidOpenTextDocument((document: vscode.TextDocument) => {
-      doLint(document, diagnosticCollection);
-    });
+  vscode.workspace.onDidSaveTextDocument((document: vscode.TextDocument) => {
+    vscode.commands.executeCommand('protolint.lint');
   });
 
-  vscode.commands.executeCommand(commandId);
-  context.subscriptions.push(events);
+  // Run the linter when the user changes the file that they are currently viewing
+  // so that the lint results show up immediately.
+  vscode.window.onDidChangeActiveTextEditor((e: vscode.TextEditor | undefined) => {
+    vscode.commands.executeCommand('protolint.lint');
+  });
 }
 
-async function doLint(codeDocument: vscode.TextDocument, collection: vscode.DiagnosticCollection): Promise<void> {
-  if(codeDocument.languageId === 'proto3' || codeDocument.languageId === 'proto') {
+function runLint() {
+  let editor = vscode.window.activeTextEditor;
+  if (!editor) {
+      return;
+  }
+
+  // We only want to run protolint on documents that are known to be
+  // protocol buffer files.
+  const doc = editor.document;
+  if(doc.languageId !== 'proto3' && doc.languageId !== 'proto') {
     return;
   }
 
+  doLint(doc, diagnosticCollection);
+}
+
+async function doLint(codeDocument: vscode.TextDocument, collection: vscode.DiagnosticCollection): Promise<void> {
   const linter = new Linter(codeDocument);
   const errors: LinterError[] = await linter.lint();
   const diagnostics = errors.map(error => {

diff --git a/src/linter.ts b/src/linter.ts
@@ -16,65 +16,61 @@ export default class Linter {
   }
 
   public async lint(): Promise<LinterError[]> {
-    const errors = await this.runProtoLint();
-    if (!errors) {
+    const result = await this.runProtoLint();
+    if (!result) {
       return [];
     }
 
-    // protolint returns "not found config file by searching" when it is
-    // executed and a configuration file cannot be found.
-    if (errors.includes("not found config")) {
-      vscode.window.showErrorMessage(errors);
+    const lintingErrors: LinterError[] = this.parseErrors(result);
+
+    // When errors exist, but no linting errors were returned show the error window
+    // in VSCode as it is most likely an issue with the binary itself such as not being
+    // able to find a configuration or a file to lint.
+    if (lintingErrors.length === 0) {
+      vscode.window.showErrorMessage("protolint: " + result);
       return [];
     }
 
-    const lintingErrors: LinterError[] = this.parseErrors(errors);
-    return lintingErrors;
-  }
 
-  private parseErrors(errorStr: string): LinterError[] {
-    let errors = errorStr.split('\n') || [];
-
-    var result = errors.reduce((errors: LinterError[], currentError: string) => {
-      const parsedError = parseProtoError(currentError);
-      if (!parsedError.reason) {
-        return errors;
-      }
-
-      const linterError: LinterError = this.createLinterError(parsedError);
-      return errors.concat(linterError);
-    }, []);
-
-    return result;
+    return lintingErrors;
   }
 
   private async runProtoLint(): Promise<string> {
     if (!vscode.workspace.workspaceFolders) {
       return "";
     }
 
-    const currentFile = this.codeDocument.uri.fsPath;
     let workspaceFolder: vscode.WorkspaceFolder = vscode.workspace.getWorkspaceFolder(this.codeDocument.uri) || vscode.workspace.workspaceFolders[0];
-    const cmd = `protolint lint -config_dir_path="${workspaceFolder.uri.fsPath}" "${currentFile}"`;
-
-    const exec = util.promisify(cp.exec);
+    const cmd = `protolint lint -config_dir_path="${workspaceFolder.uri.fsPath}" "${this.codeDocument.uri.fsPath}"`;
 
     let lintResults: string = "";
+
+    // Execute the protolint binary and store the output from standard error.
+    // The output could either be an error from using the binary improperly, such as unable to find
+    // a configuration, or linting errors.
+    const exec = util.promisify(cp.exec);
     await exec(cmd).catch((error: any) => lintResults = error.stderr);
 
     return lintResults;
   }
 
-  private createLinterError(error: ProtoError): LinterError {
-    const linterError: LinterError = {
-      proto: error,
-      range: this.getErrorRange(error)
-    };
+  private parseErrors(errorStr: string): LinterError[] {
+    let errors = errorStr.split('\n') || [];
 
-    return linterError;
-  }
+    var result = errors.reduce((errors: LinterError[], currentError: string) => {
+      const parsedError = parseProtoError(currentError);
+      if (!parsedError.reason) {
+        return errors;
+      }
+
+      const linterError: LinterError = {
+        proto: parsedError,
+        range: this.codeDocument.lineAt(parsedError.line - 1).range
+      };
 
-  private getErrorRange(error: ProtoError): vscode.Range {
-    return this.codeDocument.lineAt(error.line - 1).range;
+      return errors.concat(linterError);
+    }, []);
+
+    return result;
   }
 }
diff --git a/src/protoError.ts b/src/protoError.ts
@@ -3,6 +3,11 @@ export interface ProtoError {
   reason: string;
 }
 
+// parseProtoError takes the an error message from protolint
+// and attempts to parse it as a linting error.
+//
+// Linting errors are in the format:
+// [path/to/file.proto:line:column] an error message is here
 export function parseProtoError(error: string): ProtoError {
   if (!error) {
     return getEmptyProtoError();

diff --git a/test/protoError.test.ts b/test/protoError.test.ts
@@ -1,39 +1,37 @@
-import * as assert from 'assert';
-import { ProtoError, parseProtoError, getEmptyProtoError } from '../src/protoError';
-
-// Proto errors are in the format:
-// [path/to/file.proto:line:column] an error message is here
-describe('parseProtoError', () => {
-  it('should return empty protoerror when there is no error', () => {
-    const expected: ProtoError = getEmptyProtoError();
-    const actual = parseProtoError("");
-
-    assert.deepEqual(actual, expected);
-  });
-
-  describe('should return the correct values', () => {
-    it('when parsing a valid error', () => {
-      const expected: ProtoError = {
-        line: 1,
-        reason: "test error"
-      };
-
-      const error: string = "[path/to/file.proto:1:5] test error";
-      const actual = parseProtoError(error);
-
-      assert.deepEqual(actual, expected);
-    });
-
-    it('when file directory contains a space', () => {
-      const expected: ProtoError = {
-        line: 1,
-        reason: "test error"
-      };
-
-      const error: string = "[path/to /file.proto:1:5] test error";
-      const actual = parseProtoError(error);
-
-      assert.deepEqual(actual, expected);
-    });
-  });
-});
+import * as assert from 'assert';
+import { ProtoError, parseProtoError, getEmptyProtoError } from '../src/protoError';
+
+describe('parseProtoError', () => {
+  it('should return empty protoerror when there is no error', () => {
+    const expected: ProtoError = getEmptyProtoError();
+    const actual = parseProtoError("");
+
+    assert.deepStrictEqual(actual, expected);
+  });
+
+  describe('should return the correct values', () => {
+    it('when parsing a valid error', () => {
+      const expected: ProtoError = {
+        line: 1,
+        reason: "test error"
+      };
+
+      const error: string = "[path/to/file.proto:1:5] test error";
+      const actual = parseProtoError(error);
+
+      assert.deepStrictEqual(actual, expected);
+    });
+
+    it('when file directory contains a space', () => {
+      const expected: ProtoError = {
+        line: 1,
+        reason: "test error"
+      };
+
+      const error: string = "[path/to /file.proto:1:5] test error";
+      const actual = parseProtoError(error);
+
+      assert.deepStrictEqual(actual, expected);
+    });
+  });
+});