Skip to content

Fix files added in beforeProgramValidate hook not being validated #1

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.

Sign up for GitHub

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

Closed
Copilot wants to merge 2 commits into from
Closed

Fix files added in beforeProgramValidate hook not being validated #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a5b7fc
Initial plan
Copilot Aug 27, 2025
f4e56bf
Fix issue #1450: Files set in beforeProgramValidate hook are now vali…
Copilot Aug 27, 2025
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
76 changes: 76 additions & 0 deletions src/Program.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,44 @@ describe('Program', () => {
// await program.loadOrReloadFile('components', '')
});

it('validates files added in beforeProgramValidate hook', () => {
const afterFileValidate = sinon.spy();

program.plugins = new PluginInterface([{
name: 'test plugin that adds files during beforeProgramValidate',
beforeProgramValidate: (program: Program) => {
// Add a file that defines an enum
const fileContent = `enum Tasks
MyTask = "_AsyncTask"
end enum`;
program.setFile('source/AsyncTask/Tasks.bs', fileContent);
},
afterFileValidate: afterFileValidate
}], { logger: createLogger() });

// Add a main file that imports the enum
program.setFile('source/main.bs', `
import "pkg:/source/AsyncTask/Tasks.bs"

sub main()
print Tasks.MyTask
end sub
`);

program.validate();

// Verify that the Tasks.bs file was validated
const tasksFile = program.getFile('source/AsyncTask/Tasks.bs');
expect(tasksFile).to.not.be.undefined;
expect(tasksFile.isValidated).to.be.true;

// Verify afterFileValidate was called for both files (main.bs and Tasks.bs)
expect(afterFileValidate.callCount).to.equal(2);

// Should have no diagnostics (Tasks should be found)
expectZeroDiagnostics(program);
});

it(`emits events for scope and file creation`, () => {
const beforeProgramValidate = sinon.spy();
const afterProgramValidate = sinon.spy();
Expand Down Expand Up @@ -224,6 +262,44 @@ describe('Program', () => {
expect(afterFileParse.callCount).to.equal(2);
expect(afterFileValidate.callCount).to.equal(2);
});

it('validates files added in beforeProgramValidate hook', () => {
const afterFileValidate = sinon.spy();

program.plugins = new PluginInterface([{
name: 'test plugin that adds files during beforeProgramValidate',
beforeProgramValidate: (program: Program) => {
// Add a file that defines an enum
const fileContent = `enum Tasks
MyTask = "_AsyncTask"
end enum`;
program.setFile('source/AsyncTask/Tasks.bs', fileContent);
},
afterFileValidate: afterFileValidate
}], { logger: createLogger() });

// Add a main file that imports the enum
program.setFile('source/main.bs', `
import "pkg:/source/AsyncTask/Tasks.bs"

sub main()
print Tasks.MyTask
end sub
`);

program.validate();

// Verify that the Tasks.bs file was validated
const tasksFile = program.getFile('source/AsyncTask/Tasks.bs');
expect(tasksFile).to.not.be.undefined;
expect(tasksFile.isValidated).to.be.true;

// Verify afterFileValidate was called for both files (main.bs and Tasks.bs)
expect(afterFileValidate.callCount).to.equal(2);

// Should have no diagnostics (Tasks should be found)
expectZeroDiagnostics(program);
});
});

describe('validate', () => {
Expand Down
38 changes: 21 additions & 17 deletions src/Program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,23 +727,27 @@ export class Program {
this.plugins.emit('beforeProgramValidate', this);
beforeProgramValidateWasEmitted = true;
})
.forEach(Object.values(this.files), (file) => {
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);
.once(() => {
Copy link

@TwitchBronBron TwitchBronBron Aug 27, 2025

Choose a reason for hiding this comment

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

This isn't just a normal .forEach. It's part of a class called Sequencer that runs the validations in small batches, yielding to the event loop every so often to allow the LanguageServer to cancel validations if necessary. A tight loop iterating over every file would block the event loop for the entire list.

In v1, we've modified the sequencer to accept a factory function for the items array instead of predetermining them. This would possibly solve this issue, because the factory could return the list of files right when the .foreach is processed instead of doing it ahead of time.

https://github.com/rokucommunity/brighterscript/blob/v1/src/common/Sequencer.ts

And if that's still not quite flexible enough, that factory could return a generator, so you could constantly be looking up the next file to return even if devs have added a new file during the process. Something like this:

public *allFiles(): Generator<[string, BrsFile | XmlFile], void, unknown> {
    const yielded = new Set<string>();
    while (true) {
        let foundNew = false;
        for (const [key, file] of this.files) {
            if (!yielded.has(key)) {
                yielded.add(key);
                foundNew = true;
                yield [key, file];
            }
        }
        if (!foundNew) {
            break;
        }
    }
}

// Capture files AFTER beforeProgramValidate event to include any files added by plugins
const filesToValidate = Object.values(this.files);
for (const file of filesToValidate) {
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);
}
}
})
.forEach(Object.values(this.scopes), (scope) => {
Expand Down