diff --git a/src/Program.ts b/src/Program.ts index d50052cef..456d316dc 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -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); diff --git a/src/bscPlugin/hover/HoverProcessor.spec.ts b/src/bscPlugin/hover/HoverProcessor.spec.ts index ff0c7383e..a6b65a076 100644 --- a/src/bscPlugin/hover/HoverProcessor.spec.ts +++ b/src/bscPlugin/hover/HoverProcessor.spec.ts @@ -219,5 +219,62 @@ 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'); + expect(hover.contents).to.contain('Returns the absolute value of the argument.'); + }); + + 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 + + // 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'); + expect(hover.contents).to.contain('Returns the absolute value of the argument.'); + }); + + it('finds hover for global function with both shortDescription and documentation', () => { + const file = program.setFile('source/main.brs', ` + sub main() + result = Atn(-5) + end sub + `); + program.validate(); + + // hover over the `Atn` 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 Atn(x as float) as float'); + expect(hover.contents).to.contain('Returns the arctangent (in radians) of the argument.'); + expect(hover.contents).to.contain('ATN(X)` returns "the angle whose tangent is X"'); + }); }); }); diff --git a/src/bscPlugin/hover/HoverProcessor.ts b/src/bscPlugin/hover/HoverProcessor.ts index 3c2052c6d..ef2fca145 100644 --- a/src/bscPlugin/hover/HoverProcessor.ts +++ b/src/bscPlugin/hover/HoverProcessor.ts @@ -1,7 +1,7 @@ import { isBrsFile, isFunctionType, isXmlFile } from '../../astUtils/reflection'; import type { BrsFile } from '../../files/BrsFile'; import type { XmlFile } from '../../files/XmlFile'; -import type { Hover, ProvideHoverEvent } from '../../interfaces'; +import type { Hover, ProvideHoverEvent, Callable } from '../../interfaces'; import type { Token } from '../../lexer/Token'; import { TokenKind } from '../../lexer/TokenKind'; import { BrsTranspileState } from '../../parser/BrsTranspileState'; @@ -39,6 +39,24 @@ export class HoverProcessor { return parts.join('\n'); } + private buildCallableContents(callable: Callable, fence: (code: string) => string) { + const parts = [fence(callable.type.toString())]; + + // Use shortDescription and documentation for all callables (both global and user-defined) + const docs = []; + if (callable.shortDescription) { + docs.push(callable.shortDescription); + } + if (callable.documentation) { + docs.push(callable.documentation); + } + if (docs.length > 0) { + parts.push('***', docs.join('\n\n')); + } + + return parts.join('\n'); + } + private getBrsFileHover(file: BrsFile): Hover | undefined { const scope = this.event.scopes[0]; const fence = (code: string) => util.mdFence(code, 'brightscript'); @@ -115,7 +133,7 @@ export class HoverProcessor { if (callable) { return { range: token.range, - contents: this.buildContentsWithDocs(fence(callable.type.toString()), callable.functionStatement?.func?.functionType) + contents: this.buildCallableContents(callable, fence) }; } } @@ -125,31 +143,7 @@ export class HoverProcessor { * Combine all the documentation found before a token (i.e. comment tokens) */ private getTokenDocumentation(tokens: Token[], token?: Token) { - const comments = [] as Token[]; - if (!token) { - return undefined; - } - const idx = tokens?.indexOf(token); - if (!idx || idx === -1) { - return undefined; - } - for (let i = idx - 1; i >= 0; i--) { - const token = tokens[i]; - //skip whitespace and newline chars - if (token.kind === TokenKind.Comment) { - comments.push(token); - } else if (token.kind === TokenKind.Newline || token.kind === TokenKind.Whitespace) { - //skip these tokens - continue; - - //any other token means there are no more comments - } else { - break; - } - } - if (comments.length > 0) { - return comments.reverse().map(x => x.text.replace(/^('|rem)/i, '')).join('\n'); - } + return util.getTokenDocumentation(tokens, token); } private getXmlFileHover(file: XmlFile): Hover | undefined { diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 24b613729..cb114d986 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -1987,7 +1987,9 @@ describe('BrsFile', () => { ).to.equal([ '```brightscript', 'function UCase(s as string) as string', - '```' + '```', + '***', + 'Converts the string to all upper case.' ].join('\n')); }); @@ -2004,7 +2006,9 @@ describe('BrsFile', () => { '```brightscript', //TODO this really shouldn't be returning the global function, but it does...so make sure it doesn't crash right now. 'function Instr(startOrText as dynamic, textOrSubstring as string, substring? as string) as integer', - '```' + '```', + '***', + 'Returns the position of the first instances of substring within text, starting at the specified start position.\nReturns 0 if the substring is not found. Unlike the ifString.Instr() method, the first position is 1.' ].join('\n')); }); diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index d8b86cf45..536993e4b 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -616,6 +616,9 @@ export class BrsFile { functionType.addParameter(callableParam.name, callableParam.type, isOptional); } + // Extract documentation from comment tokens + const documentation = util.getTokenDocumentation(this.parser.tokens, statement.func.functionType); + this.callables.push({ isSub: statement.func.functionType.text.toLowerCase() === 'sub', name: statement.name.text, @@ -626,7 +629,8 @@ export class BrsFile { type: functionType, getName: statement.getName.bind(statement), hasNamespace: !!statement.findAncestor(isNamespaceStatement), - functionStatement: statement + functionStatement: statement, + shortDescription: documentation }); } } diff --git a/src/globalCallables.spec.ts b/src/globalCallables.spec.ts index aae6160dc..521b95993 100644 --- a/src/globalCallables.spec.ts +++ b/src/globalCallables.spec.ts @@ -63,7 +63,9 @@ describe('globalCallables', () => { ).to.eql([ '```brightscript', 'function Mid(s as string, p as integer, n? as integer) as string', - '```' + '```', + '***', + 'Returns a substring of s with length n and starting at position p.\nn may be omitted, in which case the string starting at p and ending at the end of the string is returned.\nUnlike the ifString.Mid() method, the first character in the string is position 1.' ].join('\n')); }); diff --git a/src/util.ts b/src/util.ts index db38cfd28..56b25dd7e 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1769,6 +1769,37 @@ export class Util { //just return empty string so log functions don't crash with undefined project numbers return ''; } + + /** + * Combine all the documentation found before a token (i.e. comment tokens) + */ + public getTokenDocumentation(tokens: Token[], token?: Token) { + const comments = [] as Token[]; + if (!token) { + return undefined; + } + const idx = tokens?.indexOf(token); + if (!idx || idx === -1) { + return undefined; + } + for (let i = idx - 1; i >= 0; i--) { + const currentToken = tokens[i]; + //skip whitespace and newline chars + if (currentToken.kind === TokenKind.Comment) { + comments.push(currentToken); + } else if (currentToken.kind === TokenKind.Newline || currentToken.kind === TokenKind.Whitespace) { + //skip these tokens + continue; + + //any other token means there are no more comments + } else { + break; + } + } + if (comments.length > 0) { + return comments.reverse().map(x => x.text.replace(/^('|rem)/i, '')).join('\n'); + } + } } /**