Skip to content

Commit

Permalink
Move file validation into program.validate() (#504)
Browse files Browse the repository at this point in the history
  • Loading branch information
@TwitchBronBron
TwitchBronBron authored Feb 3, 2022
1 parent 71887d1 commit 925d8d0
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 53 deletions.
2 changes: 2 additions & 0 deletions src/Program.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,7 @@ describe('Program', () => {
};
program.plugins.add(plugin);
program.addOrReplaceFile('source/main.brs', '');
program.validate();
expect(plugin.beforeFileValidate.callCount).to.equal(1);
expect(plugin.onFileValidate.callCount).to.equal(1);
expect(plugin.afterFileValidate.callCount).to.equal(1);
Expand All @@ -2420,6 +2421,7 @@ describe('Program', () => {
};
program.plugins.add(plugin);
program.addOrReplaceFile('components/main.xml', '');
program.validate();
expect(plugin.beforeFileValidate.callCount).to.equal(1);
expect(plugin.onFileValidate.callCount).to.equal(1);
expect(plugin.afterFileValidate.callCount).to.equal(1);
Expand Down
70 changes: 30 additions & 40 deletions src/Program.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,20 +404,6 @@ export class Program {

brsFile.attachDependencyGraph(this.dependencyGraph);

this.plugins.emit('beforeFileValidate', {
program: this,
file: file
});

file.validate();

//emit an event to allow plugins to contribute to the file validation process
this.plugins.emit('onFileValidate', {
program: this,
file: file
});

this.plugins.emit('afterFileValidate', brsFile);
} else if (
//is xml file
fileExtension === '.xml' &&
Expand Down Expand Up @@ -450,22 +436,6 @@ export class Program {
//register this compoent now that we have parsed it and know its component name
this.registerComponent(xmlFile, scope);


//emit an event before starting to validate this file
this.plugins.emit('beforeFileValidate', {
file: file,
program: this
});

xmlFile.validate();

//emit an event to allow plugins to contribute to the file validation process
this.plugins.emit('onFileValidate', {
file: xmlFile,
program: this
});

this.plugins.emit('afterFileValidate', xmlFile);
} else {
//TODO do we actually need to implement this? Figure out how to handle img paths
// let genericFile = this.files[pathAbsolute] = <any>{
Expand Down Expand Up @@ -587,17 +557,10 @@ export class Program {
this.diagnostics = [];
this.plugins.emit('beforeProgramValidate', this);

this.logger.time(LogLevel.info, ['Validate all scopes'], () => {
for (let scopeName in this.scopes) {
let scope = this.scopes[scopeName];
scope.validate();
}
});

//find any files NOT loaded into a scope
for (let filePath in this.files) {
let file = this.files[filePath];
//validate every file
for (const file of Object.values(this.files)) {

//find any files NOT loaded into a scope
if (!this.fileIsIncludedInAnyScope(file)) {
this.logger.debug('Program.validate(): fileNotReferenced by any scope', () => chalk.green(file?.pkgPath));
//the file is not loaded in any scope
Expand All @@ -607,8 +570,35 @@ export class Program {
range: util.createRange(0, 0, 0, Number.MAX_VALUE)
});
}

//for every unvalidated file, validate it
if (!file.isValidated) {
this.plugins.emit('beforeFileValidate', {
program: this,
file: file
});

//emit an event to allow plugins to contribute to the file validation process
this.plugins.emit('onFileValidate', {
program: this,
file: file
});
//call file.validate() IF the file has that function defined
file.validate?.();
file.isValidated = true;

this.plugins.emit('afterFileValidate', file);
}
}


this.logger.time(LogLevel.info, ['Validate all scopes'], () => {
for (let scopeName in this.scopes) {
let scope = this.scopes[scopeName];
scope.validate();
}
});

this.detectDuplicateComponentNames();

this.plugins.emit('afterProgramValidate', this);
Expand Down
3 changes: 2 additions & 1 deletion src/bscPlugin/codeActions/CodeActionsProcessor.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe('CodeActionsProcessor', () => {
<component name="comp1">
</component>
`);
program.validate();
expectCodeActions(() => {
program.getCodeActions(
file.pathAbsolute,
Expand Down Expand Up @@ -69,12 +70,12 @@ describe('CodeActionsProcessor', () => {
<component name="comp1" attr2="attr3" attr3="attr3">
</component>
`);
program.validate();
const codeActions = program.getCodeActions(
file.pathAbsolute,
//<comp|onent name="comp1">
util.createRange(1, 5, 1, 5)
);

expect(
codeActions[0].edit.changes[URI.file(s`${rootDir}/components/comp1.xml`).toString()][0].range
).to.eql(
Expand Down
20 changes: 12 additions & 8 deletions src/files/BrsFile.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -996,41 +996,45 @@ describe('BrsFile', () => {
});

it('adds error for library statements NOT at top of file', () => {
let file = program.addOrReplaceFile('source/main.bs', `
program.addOrReplaceFile('source/file.brs', ``);
program.addOrReplaceFile('source/main.bs', `
sub main()
end sub
import "file.brs"
`);
expectDiagnostics(file, [
program.validate();
expectDiagnostics(program, [
DiagnosticMessages.importStatementMustBeDeclaredAtTopOfFile()
]);
});

it('supports library imports', () => {
file.parse(`
program.addOrReplaceFile('source/main.brs', `
Library "v30/bslCore.brs"
`);
expectZeroDiagnostics(file);
expectZeroDiagnostics(program);
});

it('adds error for library statements NOT at top of file', () => {
let file = program.addOrReplaceFile('source/main.brs', `
program.addOrReplaceFile('source/main.brs', `
sub main()
end sub
Library "v30/bslCore.brs"
`);
expectDiagnostics(file, [
program.validate();
expectDiagnostics(program, [
DiagnosticMessages.libraryStatementMustBeDeclaredAtTopOfFile()
]);
});

it('adds error for library statements inside of function body', () => {
let file = program.addOrReplaceFile('source/main.brs', `
program.addOrReplaceFile('source/main.brs', `
sub main()
Library "v30/bslCore.brs"
end sub
`);
expectDiagnostics(file, [
program.validate();
expectDiagnostics(program, [
DiagnosticMessages.libraryStatementMustBeDeclaredAtTopOfFile()
]);
});
Expand Down
9 changes: 9 additions & 0 deletions src/files/BrsFile.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ export class BrsFile {
* The key used to identify this file in the dependency graph
*/
public dependencyGraphKey: string;

/**
* Indicates whether this file needs to be validated.
* Files are only ever validated a single time
*/
public isValidated = false;

/**
* The all-lowercase extension for this file (including the leading dot)
*/
Expand Down Expand Up @@ -223,6 +230,8 @@ export class BrsFile {

//if we have a typedef file, skip parsing this file
if (this.hasTypedef) {
//skip validation since the typedef is shadowing this file
this.isValidated = true;
return;
}

Expand Down
9 changes: 5 additions & 4 deletions src/files/XmlFile.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ describe('XmlFile', () => {
});

it('Adds error when no component is declared in xml', () => {
file = program.addOrReplaceFile('components/comp.xml', '<script type="text/brightscript" uri="ChildScene.brs" />');
program.addOrReplaceFile('components/comp.xml', '<script type="text/brightscript" uri="ChildScene.brs" />');
program.validate();
expectDiagnostics(program, [
{
...DiagnosticMessages.xmlUnexpectedTag('script'),
Expand Down Expand Up @@ -572,7 +573,7 @@ describe('XmlFile', () => {
});

it('adds warning when no "extends" attribute is found', () => {
file = program.addOrReplaceFile<XmlFile>(
program.addOrReplaceFile<XmlFile>(
{
src: `${rootDir}/components/comp1.xml`,
dest: `components/comp1.xml`
Expand All @@ -583,8 +584,8 @@ describe('XmlFile', () => {
</component>
`
);

expectDiagnostics(file, [
program.validate();
expectDiagnostics(program, [
DiagnosticMessages.xmlComponentMissingExtendsAttribute()
]);
});
Expand Down
6 changes: 6 additions & 0 deletions src/files/XmlFile.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ export class XmlFile {
*/
private unsubscribeFromDependencyGraph: () => void;

/**
* Indicates whether this file needs to be validated.
* Files are only ever validated a single time
*/
public isValidated = false;

/**
* The extension for this file
*/
Expand Down

0 comments on commit 925d8d0

Please sign in to comment.