Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use in-memory source code for diagnostic printing #217

Merged
merged 5 commits into from
Oct 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loads the file contents from the in-memory BrsFile/XmlFile. Previously the contents were being loaded from disk, which does not work for in-memory-only files injected by plugins.

//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