Skip to content

Commit

Permalink
Use in-memory source code for diagnostic printing (#217)
Browse files Browse the repository at this point in the history
* improves diagnostic printing, by going to program to retrieve file contents, and allowing in-memory files to be used, not failing when there is no content, and improving efficiency

* fix many lint errors

* simplify tests.

* remove unnecessary beforeEach and afterEach for tests

Co-authored-by: Bronley Plumb <bronley@gmail.com>
  • Loading branch information
georgejecook and TwitchBronBron authored Oct 15, 2020
1 parent bc223c6 commit 91194a3
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 11 deletions.
107 changes: 107 additions & 0 deletions src/ProgramBuilder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import { Program } from './Program';
import { ProgramBuilder } from './ProgramBuilder';
import { standardizePath as s, util } from './util';
import { Logger, LogLevel } from './Logger';
import * as diagnosticUtils from './diagnosticUtils';
import { Range, BscFile, BsDiagnostic } from '.';
import { DiagnosticSeverity } from './astUtils';
import { BrsFile } from './files/BrsFile';

let tmpPath = s`${process.cwd()}/.tmp`;
let rootDir = s`${tmpPath}/rootDir`;
Expand Down Expand Up @@ -172,4 +176,107 @@ describe('ProgramBuilder', () => {
expect(beforeProgramValidate.callCount).to.equal(1);
expect(afterProgramValidate.callCount).to.equal(1);
});


describe('printDiagnostics', () => {

it('prints no diagnostics when showDiagnosticsInConsole is false', () => {
builder.options.showDiagnosticsInConsole = false;

let stub = sinon.stub(builder, 'getDiagnostics').returns([]);
expect(stub.called).to.be.false;
builder['printDiagnostics']();
});

it('prints nothing when there are no diagnostics', () => {
builder.options.showDiagnosticsInConsole = true;

sinon.stub(builder, 'getDiagnostics').returns([]);
let printStub = sinon.stub(diagnosticUtils, 'printDiagnostic');

builder['printDiagnostics']();

expect(printStub.called).to.be.false;
});

it('prints diagnostic, when file is present in project', () => {
builder.options.showDiagnosticsInConsole = true;

let diagnostics = createBsDiagnostic('p1', ['m1']);
let f1 = diagnostics[0].file as BrsFile;
f1.fileContents = `l1\nl2\nl3`;
sinon.stub(builder, 'getDiagnostics').returns(diagnostics);

sinon.stub(builder.program, 'getFileByPathAbsolute').returns(f1);

let printStub = sinon.stub(diagnosticUtils, 'printDiagnostic');

builder['printDiagnostics']();

expect(printStub.called).to.be.true;
});
});

it('prints diagnostic, when file has no lines', () => {
builder.options.showDiagnosticsInConsole = true;

let diagnostics = createBsDiagnostic('p1', ['m1']);
let f1 = diagnostics[0].file as BrsFile;
f1.fileContents = null;
sinon.stub(builder, 'getDiagnostics').returns(diagnostics);

sinon.stub(builder.program, 'getFileByPathAbsolute').returns(f1);

let printStub = sinon.stub(diagnosticUtils, 'printDiagnostic');

builder['printDiagnostics']();

expect(printStub.called).to.be.true;
});

it('prints diagnostic, when no file present', () => {
builder.options.showDiagnosticsInConsole = true;

let diagnostics = createBsDiagnostic('p1', ['m1']);
sinon.stub(builder, 'getDiagnostics').returns(diagnostics);

sinon.stub(builder.program, 'getFileByPathAbsolute').returns(null);

let printStub = sinon.stub(diagnosticUtils, 'printDiagnostic');

builder['printDiagnostics']();

expect(printStub.called).to.be.true;
});
});

function createBsDiagnostic(filePath: string, messages: string[]): BsDiagnostic[] {
let file = new BrsFile(filePath, filePath, null);
let diagnostics = [];
for (let message of messages) {
let d = createDiagnostic(file, 1, message);
d.file = file;
diagnostics.push(d);
}
return diagnostics;
}
function createDiagnostic(
bscFile: BscFile,
code: number,
message: string,
startLine = 0,
startCol = 99999,
endLine = 0,
endCol = 99999,
severity: DiagnosticSeverity = DiagnosticSeverity.Error
) {
const diagnostic = {
code: code,
message: message,
range: Range.create(startLine, startCol, endLine, endCol),
file: bscFile,
severity: severity
};
return diagnostic;
}

21 changes: 10 additions & 11 deletions src/ProgramBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ import { standardizePath as s, util, loadPlugins } from './util';
import { Watcher } from './Watcher';
import { DiagnosticSeverity } from 'vscode-languageserver';
import { Logger, LogLevel } from './Logger';
import { getPrintDiagnosticOptions, printDiagnostic } from './diagnosticUtils';
import PluginInterface from './PluginInterface';

import * as diagnosticUtils from './diagnosticUtils';
/**
* A runner class that handles
*/
Expand Down Expand Up @@ -80,7 +79,7 @@ export class ProgramBuilder {
//if this is not a diagnostic, something else is wrong...
throw e;
}
await this.printDiagnostics();
this.printDiagnostics();

//we added diagnostics, so hopefully that draws attention to the underlying issues.
//For now, just use a default options object so we have a functioning program
Expand Down Expand Up @@ -205,7 +204,7 @@ export class ProgramBuilder {
return runPromise;
}

private async printDiagnostics() {
private printDiagnostics() {
if (this.options.showDiagnosticsInConsole === false) {
return;
}
Expand All @@ -221,7 +220,7 @@ export class ProgramBuilder {
}

//get printing options
const options = getPrintDiagnosticOptions(this.options);
const options = diagnosticUtils.getPrintDiagnosticOptions(this.options);
const { cwd, emitFullPaths } = options;

let pathsAbsolute = Object.keys(diagnosticsByFile).sort();
Expand All @@ -239,16 +238,16 @@ export class ProgramBuilder {
if (!emitFullPaths) {
filePath = path.relative(cwd, filePath);
}

//load the file text
let fileText = await util.getFileContents(pathAbsolute);
let lines = util.getLines(fileText);
const file = this.program.getFileByPathAbsolute(pathAbsolute);
//get the file's in-memory contents if available
const lines = file?.fileContents?.split(/\r?\n/g) ?? [];

for (let diagnostic of sortedDiagnostics) {
//default the severity to error if undefined
let severity = typeof diagnostic.severity === 'number' ? diagnostic.severity : DiagnosticSeverity.Error;
//format output
printDiagnostic(options, severity, filePath, lines, diagnostic);
diagnosticUtils.printDiagnostic(options, severity, filePath, lines, diagnostic);
}
}
}
Expand All @@ -274,7 +273,7 @@ export class ProgramBuilder {
return -1;
}

await this.printDiagnostics();
this.printDiagnostics();
wereDiagnosticsPrinted = true;
let errorCount = this.getDiagnostics().filter(x => x.severity === DiagnosticSeverity.Error).length;

Expand All @@ -297,7 +296,7 @@ export class ProgramBuilder {
return 0;
} catch (e) {
if (wereDiagnosticsPrinted === false) {
await this.printDiagnostics();
this.printDiagnostics();
}
throw e;
}
Expand Down

0 comments on commit 91194a3

Please sign in to comment.