Skip to content

Fix hover for global functions by adding global scope fallback #1541

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

Draft
Copilot wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Fix hover for global functions by adding global scope fallback #1541

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fc8c8bf
Initial plan
Copilot Jul 18, 2025
2465506
Fix hover for global functions by adding global scope fallback
Copilot Jul 18, 2025
79ebcfd
Fix hover descriptions for global functions and remove dead code
Copilot Jul 18, 2025
b50df8d
Unify documentation handling for all callables by populating shortDes…
Copilot Jul 18, 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
8 changes: 7 additions & 1 deletion src/Program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,11 +1027,17 @@ export class Program {
let file = this.getFile(srcPath);
let result: Hover[];
if (file) {
//find the scopes for this file
let scopes = this.getScopesForFile(file);

//if there are no scopes, include the global scope so we at least get the built-in functions
scopes = scopes.length > 0 ? scopes : [this.globalScope];

const event = {
program: this,
file: file,
position: position,
scopes: this.getScopesForFile(file),
scopes: scopes,
hovers: []
} as ProvideHoverEvent;
this.plugins.emit('beforeProvideHover', event);
Expand Down
39 changes: 39 additions & 0 deletions src/bscPlugin/hover/HoverProcessor.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,5 +219,44 @@ describe('HoverProcessor', () => {
expect(hover?.range).to.eql(util.createRange(2, 40, 2, 50));
expect(hover?.contents).to.eql(fence('const name.sp.a.c.e.SOME_VALUE = true'));
});

it('finds hover for global function', () => {
const file = program.setFile('source/main.brs', `
sub main()
result = Abs(-5)
end sub
`);
program.validate();

// hover over the `Abs` function call
let hover = program.getHover(file.pathAbsolute, util.createPosition(2, 29))[0];
expect(hover).to.exist;
expect(hover.range).to.eql(util.createRange(2, 29, 2, 32));
expect(hover.contents).to.contain('function Abs(x as float) as float');
});

it('finds hover for global function when file has no scopes', () => {
// Create a program with no manifest and no scopes to ensure global scope fallback
const testProgram = new Program({ rootDir: rootDir });

const file = testProgram.setFile('standalone/main.brs', `
sub main()
result = Abs(-5)
end sub
`);

// Don't validate - this simulates a file that might not be in any scopes
const scopes = testProgram.getScopesForFile(file);
Copy link
Member

@TwitchBronBron TwitchBronBron Jul 18, 2025

Choose a reason for hiding this comment

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

This is dead code. please remove. (you could find this out by running the lint rules before committing).

Copy link
Contributor Author

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

Fixed by removing the unused variable. The build now passes without TypeScript errors.


// hover over the `Abs` function call
let hover = testProgram.getHover(file.pathAbsolute, util.createPosition(2, 29))[0];

testProgram.dispose();

// After the fix, this should now work even when file has no scopes
expect(hover).to.exist;
expect(hover.range).to.eql(util.createRange(2, 29, 2, 32));
expect(hover.contents).to.contain('function Abs(x as float) as float');
Copy link
Member

@TwitchBronBron TwitchBronBron Jul 18, 2025

Choose a reason for hiding this comment

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

We need to also see the description of the function. Looking at globalCallables.ts it shows the description for this function should be something like "Returns the absolute value of the argument.".

Here's what I'm expecting.

'a test function
sub test()
end sub

sub init()
    test() 'hover over this, it includes "a test function"
end sub

But for the global functions, it doesn't. It's not even related to files with zero scopes. I can reproduce this in my source/main.brs file.

Copy link
Contributor Author

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

Fixed by modifying HoverProcessor to show shortDescription and documentation from global callables. The hover now includes descriptions like "Returns the absolute value of the argument." for global functions. Updated tests to verify the descriptions are shown.

});
});
});