Upgrade to @rokucommunity/logger (#1137)

* Upgrade to @rokucommunity/logger

* Add original logger class back

* @rokucommunity/logger v0.3.4

package.json

@@ -108,7 +108,7 @@
     "node-run-cmd": "^1.0.1",
     "nyc": "^15.1.0",
     "object.pick": "^1.3.0",
-    "phin": "^3.5.1",
+    "phin": "^3.7.1",
     "rimraf": "^2.7.1",
     "semver-extra": "^3.0.0",
     "sinon": "^9.0.2",
@@ -126,6 +126,7 @@
   },
   "dependencies": {
     "@rokucommunity/bslib": "^0.1.1",
+    "@rokucommunity/logger": "^0.3.4",
     "@xml-tools/parser": "^1.0.7",
     "array-flat-polyfill": "^1.0.1",
     "chalk": "^2.4.2",

src/BsConfig.ts

@@ -1,4 +1,4 @@
-import type { LogLevel } from './Logger';
+import type { LogLevel } from './logging';
 
 export interface BsConfig {
     /**
@@ -173,7 +173,7 @@ export interface BsConfig {
      * The log level.
      * @default LogLevel.log
      */
-    logLevel?: LogLevel | 'error' | 'warn' | 'log' | 'info' | 'debug' | 'trace';
+    logLevel?: LogLevel | 'error' | 'warn' | 'log' | 'info' | 'debug' | 'trace' | 'off';
     /**
      * Override the path to source files in source maps. Use this if you have a preprocess step and want
      * to ensure the source maps point to the original location.

src/LanguageServer.ts

@@ -40,14 +40,14 @@ import { Deferred } from './deferred';
 import { DiagnosticMessages } from './DiagnosticMessages';
 import { ProgramBuilder } from './ProgramBuilder';
 import { standardizePath as s, util } from './util';
-import { Logger } from './Logger';
 import { Throttler } from './Throttler';
 import { KeyedThrottler } from './KeyedThrottler';
 import { DiagnosticCollection } from './DiagnosticCollection';
 import { isBrsFile } from './astUtils/reflection';
 import { encodeSemanticTokens, semanticTokensLegend } from './SemanticTokenUtils';
 import type { BusyStatus } from './BusyStatusTracker';
 import { BusyStatusTracker } from './BusyStatusTracker';
+import { logger } from './logging';
 
 export class LanguageServer {
     private connection = undefined as any as Connection;
@@ -112,8 +112,8 @@ export class LanguageServer {
         });
 
         //listen to all of the output log events and pipe them into the debug channel in the extension
-        this.loggerSubscription = Logger.subscribe((text) => {
-            this.connection.tracer.log(text);
+        this.loggerSubscription = logger.subscribe((message) => {
+            this.connection.tracer.log(message.argsText);
         });
 
         this.connection.onInitialize(this.onInitialize.bind(this));

src/Logger.spec.ts

@@ -1,5 +1,6 @@
 import { expect } from './chai-config.spec';
-import { Logger, LogLevel, noop } from './Logger';
+import type { Logger } from './logging';
+import { createLogger } from './logging';
 import chalk from 'chalk';
 import { createSandbox } from 'sinon';
 const sinon = createSandbox();
@@ -8,84 +9,83 @@ describe('Logger', () => {
     let logger: Logger;
 
     beforeEach(() => {
-        logger = new Logger(LogLevel.trace);
+        logger = createLogger({
+            logLevel: 'trace'
+        });
         sinon.restore();
         //disable chalk colors for testing
         sinon.stub(chalk, 'grey').callsFake((arg) => arg as any);
     });
 
-    it('noop does nothing', () => {
-        noop();
-    });
-
     it('loglevel setter converts string to enum', () => {
-        (logger as any).logLevel = 'error';
-        expect(logger.logLevel).to.eql(LogLevel.error);
-        (logger as any).logLevel = 'info';
-        expect(logger.logLevel).to.eql(LogLevel.info);
+        logger.logLevel = 'error';
+        expect(logger.logLevel).to.eql('error');
+        logger.logLevel = 'info';
+        expect(logger.logLevel).to.eql('info');
     });
 
-    it('uses LogLevel.log by default', () => {
-        logger = new Logger();
-        expect(logger.logLevel).to.eql(LogLevel.log);
+    it('uses "log" by default', () => {
+        logger = createLogger();
+        expect(logger.logLevel).to.eql('log');
     });
 
     describe('log methods call correct error type', () => {
         it('error', () => {
-            const stub = sinon.stub(logger as any, 'writeToLog').callsFake(() => { });
+            const stub = sinon.stub(logger as any, 'write').callsFake(() => { });
             logger.error();
-            expect(stub.getCalls()[0].args[0]).to.eql(console.error);
+            expect(stub.getCalls()[0].args[0]).to.eql('error');
         });
 
         it('warn', () => {
-            const stub = sinon.stub(logger as any, 'writeToLog').callsFake(() => { });
+            const stub = sinon.stub(logger as any, 'write').callsFake(() => { });
             logger.warn();
-            expect(stub.getCalls()[0].args[0]).to.eql(console.warn);
+            expect(stub.getCalls()[0].args[0]).to.eql('warn');
         });
 
         it('log', () => {
-            const stub = sinon.stub(logger as any, 'writeToLog').callsFake(() => { });
+            const stub = sinon.stub(logger as any, 'write').callsFake(() => { });
             logger.log();
-            expect(stub.getCalls()[0].args[0]).to.eql(console.log);
+            expect(stub.getCalls()[0].args[0]).to.eql('log');
         });
 
         it('info', () => {
-            const stub = sinon.stub(logger as any, 'writeToLog').callsFake(() => { });
+            const stub = sinon.stub(logger as any, 'write').callsFake(() => { });
             logger.info();
-            expect(stub.getCalls()[0].args[0]).to.eql(console.info);
+            expect(stub.getCalls()[0].args[0]).to.eql('info');
         });
 
         it('debug', () => {
-            const stub = sinon.stub(logger as any, 'writeToLog').callsFake(() => { });
+            const stub = sinon.stub(logger as any, 'write').callsFake(() => { });
             logger.debug();
-            expect(stub.getCalls()[0].args[0]).to.eql(console.debug);
+            expect(stub.getCalls()[0].args[0]).to.eql('debug');
         });
 
         it('trace', () => {
-            const stub = sinon.stub(logger as any, 'writeToLog').callsFake(() => { });
+            const stub = sinon.stub(logger as any, 'write').callsFake(() => { });
             logger.trace();
-            expect(stub.getCalls()[0].args[0]).to.eql(console.trace);
+            expect(stub.getCalls()[0].args[0]).to.eql('trace');
         });
     });
 
     it('skips all errors on error level', () => {
-        logger.logLevel = LogLevel.off;
-        const stub = sinon.stub(logger as any, 'writeToLog').callsFake(() => { });
+        let messages = [];
+        logger.subscribe((message) => {
+            messages.push(message);
+        });
+        logger.logLevel = 'off';
         logger.trace();
         logger.debug();
         logger.info();
         logger.log();
         logger.warn();
         logger.error();
 
-        expect(
-            stub.getCalls().map(x => x.args[0])
-        ).to.eql([]);
+        expect(messages).to.eql([]);
     });
 
     it('does not skip when log level is high enough', () => {
-        logger.logLevel = LogLevel.trace;
-        const stub = sinon.stub(logger as any, 'writeToLog').callsFake(() => { });
+        logger.logLevel = 'trace';
+        const stub = sinon.stub(logger as any, 'write').callsFake(() => { });
         logger.trace();
         logger.debug();
         logger.info();
@@ -96,50 +96,50 @@ describe('Logger', () => {
         expect(
             stub.getCalls().map(x => x.args[0])
         ).to.eql([
-            console.trace,
-            console.debug,
-            console.info,
-            console.log,
-            console.warn,
-            console.error
+            'trace',
+            'debug',
+            'info',
+            'log',
+            'warn',
+            'error'
         ]);
     });
 
     describe('time', () => {
         it('calls action even if logLevel is wrong', () => {
-            logger.logLevel = LogLevel.error;
+            logger.logLevel = 'error';
             const spy = sinon.spy();
-            logger.time(LogLevel.info, [], spy);
+            logger.time('info', [], spy);
             expect(spy.called).to.be.true;
         });
 
         it('runs timer when loglevel is right', () => {
-            logger.logLevel = LogLevel.log;
+            logger.logLevel = 'log';
             const spy = sinon.spy();
-            logger.time(LogLevel.log, [], spy);
+            logger.time('log', [], spy);
             expect(spy.called).to.be.true;
         });
 
         it('returns value', () => {
-            logger.logLevel = LogLevel.log;
+            logger.logLevel = 'log';
             const spy = sinon.spy(() => {
                 return true;
             });
             expect(
-                logger.time(LogLevel.log, [], spy)
+                logger.time('log', [], spy)
             ).to.be.true;
             expect(spy.called).to.be.true;
         });
 
         it('gives callable pause and resume functions even when not running timer', () => {
-            logger.time(LogLevel.info, [], (pause, resume) => {
+            logger.time('info', [], (pause, resume) => {
                 pause();
                 resume();
             });
         });
 
         it('waits for and returns a promise when a promise is returned from the action', () => {
-            expect(logger.time(LogLevel.info, ['message'], () => {
+            expect(logger.time('info', ['message'], () => {
                 return Promise.resolve();
             })).to.be.instanceof(Promise);
         });

src/Logger.ts

@@ -2,7 +2,10 @@ import chalk from 'chalk';
 import * as moment from 'moment';
 import { EventEmitter } from 'eventemitter3';
 import { Stopwatch } from './Stopwatch';
-
+import { LogLevelNumeric as LogLevel } from '@rokucommunity/logger';
+/**
+ * @deprecated use the `createLogger` function instead
+ */
 export class Logger {
 
     public static subscribe(callback) {
@@ -169,12 +172,4 @@ export function noop() {
 
 }
 
-export enum LogLevel {
-    off = 0,
-    error = 1,
-    warn = 2,
-    log = 3,
-    info = 4,
-    debug = 5,
-    trace = 6
-}
+export { LogLevelNumeric as LogLevel } from '@rokucommunity/logger';

src/PluginInterface.spec.ts

@@ -2,14 +2,14 @@
 /* eslint-disable @typescript-eslint/ban-ts-comment */
 import { expect } from './chai-config.spec';
 import * as sinon from 'sinon';
-import { Logger } from './Logger';
 import PluginInterface from './PluginInterface';
+import { createLogger } from './logging';
 
 describe('PluginInterface', () => {
     let pluginInterface: PluginInterface;
 
     beforeEach(() => {
-        pluginInterface = new PluginInterface([], { logger: new Logger() });
+        pluginInterface = new PluginInterface([], { logger: createLogger() });
     });
 
     it('allows adding a plugin', () => {

src/PluginInterface.ts

@@ -1,7 +1,5 @@
 import type { CompilerPlugin } from './interfaces';
-import type { Logger } from './Logger';
-import { LogLevel } from './Logger';
-
+import { LogLevel, createLogger, type Logger } from './logging';
 /*
  * we use `Required` everywhere here because we expect that the methods on plugin objects will
  * be optional, and we don't want to deal with `undefined`.
@@ -25,33 +23,38 @@ export type PluginEventArgs<T> = {
 
 export default class PluginInterface<T extends CompilerPlugin = CompilerPlugin> {
 
+    constructor();
     /**
      * @deprecated use the `options` parameter pattern instead
      */
     constructor(
-        plugins: CompilerPlugin[],
-        logger: Logger
+        plugins?: CompilerPlugin[],
+        logger?: Logger
     );
     constructor(
-        plugins: CompilerPlugin[],
-        options: {
-            logger: Logger;
+        plugins?: CompilerPlugin[],
+        options?: {
+            logger?: Logger;
             suppressErrors?: boolean;
         }
     );
     constructor(
-        private plugins: CompilerPlugin[],
-        options: {
-            logger: Logger;
+        private plugins?: CompilerPlugin[],
+        options?: {
+            logger?: Logger;
             suppressErrors?: boolean;
         } | Logger
     ) {
+        this.plugins ??= [];
         if (options?.constructor.name === 'Logger') {
             this.logger = options as unknown as Logger;
         } else {
             this.logger = (options as any)?.logger;
             this.suppressErrors = (options as any)?.suppressErrors === false ? false : true;
         }
+        if (!this.logger) {
+            this.logger = createLogger();
+        }
     }
 
     private logger: Logger;

src/Program.spec.ts

@@ -15,13 +15,13 @@ import type { FunctionStatement, PrintStatement } from './parser/Statement';
 import { EmptyStatement } from './parser/Statement';
 import { expectCompletionsExcludes, expectCompletionsIncludes, expectDiagnostics, expectHasDiagnostics, expectZeroDiagnostics, trim, trimMap } from './testHelpers.spec';
 import { doesNotThrow } from 'assert';
-import { Logger } from './Logger';
 import { createVisitor, WalkMode } from './astUtils/visitors';
 import { isBrsFile } from './astUtils/reflection';
 import type { LiteralExpression } from './parser/Expression';
 import type { AstEditor } from './astUtils/AstEditor';
 import { tempDir, rootDir, stagingDir } from './testHelpers.spec';
 import type { BsDiagnostic } from './interfaces';
+import { createLogger } from './logging';
 
 let sinon = sinonImport.createSandbox();
 
@@ -188,7 +188,7 @@ describe('Program', () => {
                 beforeFileParse: beforeFileParse,
                 afterFileParse: afterFileParse,
                 afterFileValidate: afterFileValidate
-            }], { logger: new Logger() });
+            }], { logger: createLogger() });
 
             //add a new source file
             program.setFile('source/main.brs', '');

src/Program.ts

@@ -13,7 +13,8 @@ import { standardizePath as s, util } from './util';
 import { XmlScope } from './XmlScope';
 import { DiagnosticFilterer } from './DiagnosticFilterer';
 import { DependencyGraph } from './DependencyGraph';
-import { Logger, LogLevel } from './Logger';
+import type { Logger } from './logging';
+import { LogLevel, createLogger } from './logging';
 import chalk from 'chalk';
 import { globalFile } from './globalCallables';
 import { parseManifest, getBsConst } from './preprocessor/Manifest';
@@ -65,7 +66,7 @@ export class Program {
         plugins?: PluginInterface
     ) {
         this.options = util.normalizeConfig(options);
-        this.logger = logger || new Logger(options.logLevel as LogLevel);
+        this.logger = logger ?? createLogger(options);
         this.plugins = plugins || new PluginInterface([], { logger: this.logger });
 
         //inject the bsc plugin as the first plugin in the stack.