(GH-1413) Update grammar parsing for vscode-textmate v4 module (#1414)

* (maint) Add a logging interface

Previously the logging object had no interface which made testing difficult.
This commit adds a new interface which new features can use, which makes mocking
the logging object possible.

* (GH-1413) Update grammar parsing for vscode-textmate v4 module

VS Code 1.25 has updated the imported vsode-textmate module from v3 to v4, which
introduced some breaking changes.  This commit updates the grammar loader;

* Uses a thenable grammar parser, as V4 of the module is all asynchronous
* Uses a simple method detection technique on the Registry object to determine
  if we're using v3 or v4+ of the textmate module, and then branch the code
  accordingly.

  Note that this functionality can only be tested manually due to how the
  vscode-textmate module is incorporated into VSCode and the test-suite.

* Update Folding.ts

Add newline between copyright header and body

Author: glennsarti

src/features/Folding.ts

@@ -2,14 +2,15 @@
  * Copyright (C) Microsoft Corporation. All rights reserved.
  *--------------------------------------------------------*/
 
+import fs = require("fs");
 import * as path from "path";
 import * as vscode from "vscode";
 import {
     DocumentSelector,
     LanguageClient,
 } from "vscode-languageclient";
 import { IFeature } from "../feature";
-import { Logger } from "../logging";
+import { ILogger } from "../logging";
 import * as Settings from "../settings";
 
 /**
@@ -497,23 +498,26 @@ export class FoldingFeature implements IFeature {
      * @param logger           The logging object to send messages to
      * @param documentSelector documentSelector object for this Folding Provider
      */
-    constructor(private logger: Logger, documentSelector: DocumentSelector) {
-        const grammar: IGrammar = this.grammar(logger);
-
+    constructor(private logger: ILogger, documentSelector: DocumentSelector) {
         const settings = Settings.load();
         if (!(settings.codeFolding && settings.codeFolding.enable)) { return; }
 
-        // If the PowerShell grammar is not available for some reason, don't register a folding provider,
-        // which reverts VSCode to the default indentation style folding
-        if (grammar == null) {
-            logger.writeWarning("Unable to load the PowerShell grammar file");
-            return;
-        }
+        this.loadPSGrammar(logger)
+            .then((grammar) => {
+                // If the PowerShell grammar is not available for some reason, don't register a folding provider,
+                // which reverts VSCode to the default indentation style folding
+                if (!grammar) {
+                    logger.writeWarning("Unable to load the PowerShell grammar file");
+                    return;
+                }
 
-        this.foldingProvider = new FoldingProvider(grammar);
-        vscode.languages.registerFoldingRangeProvider(documentSelector, this.foldingProvider);
+                this.foldingProvider = new FoldingProvider(grammar);
+                vscode.languages.registerFoldingRangeProvider(documentSelector, this.foldingProvider);
 
-        logger.write("Syntax Folding Provider registered");
+                logger.write("Syntax Folding Provider registered");
+            }, (err) => {
+                this.logger.writeError(`Failed to load grammar file - error: ${err}`);
+            });
     }
 
     /* dispose() is required by the IFeature interface, but is not required by this feature */
@@ -527,7 +531,7 @@ export class FoldingFeature implements IFeature {
      * @param logger The logging object to send messages to
      * @returns      A grammar parser for the PowerShell language is succesful or undefined if an error occured
      */
-    public grammar(logger: Logger): IGrammar {
+    public loadPSGrammar(logger: ILogger): Thenable<IGrammar> {
         const tm = this.getCoreNodeModule("vscode-textmate", logger);
         if (tm == null) { return undefined; }
         logger.writeDiagnostic(`Loaded the vscode-textmate module`);
@@ -537,10 +541,18 @@ export class FoldingFeature implements IFeature {
         const grammarPath = this.powerShellGrammarPath();
         if (grammarPath == null) { return undefined; }
         logger.writeDiagnostic(`PowerShell grammar file specified as ${grammarPath}`);
-        try {
-            return registry.loadGrammarFromPathSync(grammarPath);
-        } catch (err) {
-            logger.writeError(`Error while loading the PowerShell grammar file at ${grammarPath}`, err);
+
+        // Branching for the different vscode-textmate modules
+        if ("loadGrammarFromPathSync" in registry) {
+            // V3 of the module allows synchronous loading of a grammar
+            return new Promise( (grammar) => {
+                return registry.loadGrammarFromPathSync(grammarPath);
+            });
+        } else {
+            // However in V4+ this is async only
+            const content = fs.readFileSync(grammarPath);
+            const rawGrammar = tm.parseRawGrammar(content.toString(), grammarPath);
+            return registry.addGrammar(rawGrammar);
         }
     }
 
@@ -552,7 +564,7 @@ export class FoldingFeature implements IFeature {
      * @param logger     The logging object to send messages to
      * @returns          The required module, or null if the module cannot be required
      */
-    private getCoreNodeModule(moduleName: string, logger: Logger) {
+    private getCoreNodeModule(moduleName: string, logger: ILogger) {
         // Attempt to load the module from known locations
         const loadLocations: string[] = [
             `${vscode.env.appRoot}/node_modules.asar/${moduleName}`,

src/logging.ts

@@ -16,7 +16,19 @@ export enum LogLevel {
     Error,
 }
 
-export class Logger {
+/** Interface for logging operations. New features should use this interface for the "type" of logger.
+ *  This will allow for easy mocking of the logger during unit tests.
+ */
+export interface ILogger {
+    write(message: string, ...additionalMessages: string[]);
+    writeDiagnostic(message: string, ...additionalMessages: string[]);
+    writeVerbose(message: string, ...additionalMessages: string[]);
+    writeWarning(message: string, ...additionalMessages: string[]);
+    writeAndShowWarning(message: string, ...additionalMessages: string[]);
+    writeError(message: string, ...additionalMessages: string[]);
+}
+
+export class Logger implements ILogger {
 
     public logBasePath: string;
     public logSessionPath: string;

test/features/folding.test.ts

@@ -24,12 +24,12 @@ function assertFoldingRegions(result, expected): void {
 
 suite("Features", () => {
 
-    suite("Folding Provider", () => {
+    suite("Folding Provider", async () => {
         const logger: MockLogger = new MockLogger();
         const mockSelector: DocumentSelector = [
             { language: "powershell", scheme: "file" },
         ];
-        const psGrammar = (new folding.FoldingFeature(logger, mockSelector)).grammar(logger);
+        const psGrammar = await (new folding.FoldingFeature(logger, mockSelector)).loadPSGrammar(logger);
         const provider = (new folding.FoldingProvider(psGrammar));
 
         test("Can detect the PowerShell Grammar", () => {

test/test_utils.ts

@@ -2,19 +2,9 @@
  * Copyright (C) Microsoft Corporation. All rights reserved.
  *--------------------------------------------------------*/
 
-import { Logger, LogLevel } from "../src/logging";
-
-export class MockLogger extends Logger {
-    // Note - This is not a true mock as the constructor is inherited and causes errors due to trying load
-    // the "PowerShell Extension Logs" multiple times.  Ideally logging should be via an interface and then
-    // we can mock correctly.
-
-    public dispose() { return undefined; }
-
-    public getLogFilePath(baseName: string): string { return "mock"; }
-
-    public writeAtLevel(logLevel: LogLevel, message: string, ...additionalMessages: string[]) { return undefined; }
+import { ILogger } from "../src/logging";
 
+export class MockLogger implements ILogger {
     public write(message: string, ...additionalMessages: string[]) { return undefined; }
 
     public writeDiagnostic(message: string, ...additionalMessages: string[]) { return undefined; }
@@ -28,6 +18,4 @@ export class MockLogger extends Logger {
     public writeError(message: string, ...additionalMessages: string[]) { return undefined; }
 
     public writeAndShowError(message: string, ...additionalMessages: string[]) { return undefined; }
-
-    public startNewLog(minimumLogLevel: string = "Normal") { return undefined; }
 }