#14213

Committed 22 Apr 2025 02:21PM UTC coverage: 88.976% (-0.1%) from 89.104%

Build Type

push

Committed by

Commit Message

Flag incorrect return statements in functions and subs (#1463)

Coverage Stats

7615 of 9030 branches covered (84.33%)

Branch coverage included in aggregate %.

43 of 48 new or added lines in 4 files covered. (89.58%)

9892 of 10646 relevant lines covered (92.92%)

1849.7 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

73.78

/src/bscPlugin/codeActions/CodeActionsProcessor.ts

import type { Diagnostic } from 'vscode-languageserver';
import { CodeActionKind } from 'vscode-languageserver';
import { codeActionUtil } from '../../CodeActionUtil';
import type { DiagnosticMessageType } from '../../DiagnosticMessages';
import { DiagnosticCodeMap } from '../../DiagnosticMessages';
import type { BrsFile } from '../../files/BrsFile';
import type { XmlFile } from '../../files/XmlFile';
import type { BscFile, OnGetCodeActionsEvent } from '../../interfaces';
import { ParseMode } from '../../parser/Parser';
import { util } from '../../util';
import { isBrsFile, isFunctionExpression } from '../../astUtils/reflection';
import type { FunctionExpression } from '../../parser/Expression';
import { TokenKind } from '../../lexer/TokenKind';

export class CodeActionsProcessor {
    public constructor(
        public event: OnGetCodeActionsEvent
    ) {

    }

    public process() {
        for (const diagnostic of this.event.diagnostics) {
            if (diagnostic.code === DiagnosticCodeMap.cannotFindName || diagnostic.code === DiagnosticCodeMap.cannotFindFunction) {
                this.suggestCannotFindName(diagnostic as any);
            } else if (diagnostic.code === DiagnosticCodeMap.classCouldNotBeFound) {
                this.suggestClassImports(diagnostic as any);
            } else if (diagnostic.code === DiagnosticCodeMap.xmlComponentMissingExtendsAttribute) {
                this.addMissingExtends(diagnostic as any);
            } else if (diagnostic.code === DiagnosticCodeMap.voidFunctionMayNotReturnValue) {
                this.addVoidFunctionReturnActions(diagnostic);
            } else if (diagnostic.code === DiagnosticCodeMap.nonVoidFunctionMustReturnValue) {
                this.addNonVoidFunctionReturnActions(diagnostic);
            }
        }
    }

    private suggestedImports = new Set<string>();

    /**
     * Generic import suggestion function. Shouldn't be called directly from the main loop, but instead called by more specific diagnostic handlers
     */
    private suggestImports(diagnostic: Diagnostic, key: string, files: BscFile[]) {
        //skip if we already have this suggestion
        if (this.suggestedImports.has(key)) {
            return;
        }

        this.suggestedImports.add(key);
        const importStatements = (this.event.file as BrsFile).parser.references.importStatements;
        //find the position of the first import statement, or the top of the file if there is none
        const insertPosition = importStatements[importStatements.length - 1]?.importToken.range?.start ?? util.createPosition(0, 0);

        //find all files that reference this function
        for (const file of files) {
            const pkgPath = util.getRokuPkgPath(file.pkgPath);
            this.event.codeActions.push(
                codeActionUtil.createCodeAction({
                    title: `import "${pkgPath}"`,
                    diagnostics: [diagnostic],
                    isPreferred: false,
                    kind: CodeActionKind.QuickFix,
                    changes: [{
                        type: 'insert',
                        filePath: this.event.file.srcPath,
                        position: insertPosition,
                        newText: `import "${pkgPath}"\n`
                    }]
                })
            );
        }
    }

    private suggestCannotFindName(diagnostic: DiagnosticMessageType<'cannotFindName'>) {
        //skip if not a BrighterScript file
        if ((diagnostic.file as BrsFile).parseMode !== ParseMode.BrighterScript) {
            return;
        }
        const lowerName = (diagnostic.data.fullName ?? diagnostic.data.name).toLowerCase();

        this.suggestImports(
            diagnostic,
            lowerName,
            [
                ...this.event.file.program.findFilesForFunction(lowerName),
                ...this.event.file.program.findFilesForClass(lowerName),
                ...this.event.file.program.findFilesForNamespace(lowerName),
                ...this.event.file.program.findFilesForEnum(lowerName)
            ]
        );
    }

    private suggestClassImports(diagnostic: DiagnosticMessageType<'classCouldNotBeFound'>) {
        //skip if not a BrighterScript file
        if ((diagnostic.file as BrsFile).parseMode !== ParseMode.BrighterScript) {
            return;
        }
        const lowerClassName = diagnostic.data.className.toLowerCase();
        this.suggestImports(
            diagnostic,
            lowerClassName,
            this.event.file.program.findFilesForClass(lowerClassName)
        );
    }

    private addMissingExtends(diagnostic: DiagnosticMessageType<'xmlComponentMissingExtendsAttribute'>) {
        const srcPath = this.event.file.srcPath;
        const { component } = (this.event.file as XmlFile).parser.ast;
        //inject new attribute after the final attribute, or after the `<component` if there are no attributes
        const pos = (component.attributes[component.attributes.length - 1] ?? component.tag).range.end;
        this.event.codeActions.push(
            codeActionUtil.createCodeAction({
                title: `Extend "Group"`,
                diagnostics: [diagnostic],
                isPreferred: true,
                kind: CodeActionKind.QuickFix,
                changes: [{
                    type: 'insert',
                    filePath: srcPath,
                    position: pos,
                    newText: ' extends="Group"'
                }]
            })
        );
        this.event.codeActions.push(
            codeActionUtil.createCodeAction({
                title: `Extend "Task"`,
                diagnostics: [diagnostic],
                kind: CodeActionKind.QuickFix,
                changes: [{
                    type: 'insert',
                    filePath: srcPath,
                    position: pos,
                    newText: ' extends="Task"'
                }]
            })
        );
        this.event.codeActions.push(
            codeActionUtil.createCodeAction({
                title: `Extend "ContentNode"`,
                diagnostics: [diagnostic],
                kind: CodeActionKind.QuickFix,
                changes: [{
                    type: 'insert',
                    filePath: srcPath,
                    position: pos,
                    newText: ' extends="ContentNode"'
                }]
            })
        );
    }

    private addVoidFunctionReturnActions(diagnostic: Diagnostic) {
        this.event.codeActions.push(
            codeActionUtil.createCodeAction({
                title: `Remove return value`,
                diagnostics: [diagnostic],
                kind: CodeActionKind.QuickFix,
                changes: [{
                    type: 'delete',
                    filePath: this.event.file.srcPath,
                    range: util.createRange(
                        diagnostic.range.start.line,
                        diagnostic.range.start.character + 'return'.length,
                        diagnostic.range.end.line,
                        diagnostic.range.end.character
                    )
                }]
            })
        );
        if (isBrsFile(this.event.file)) {
            const expression = this.event.file.getClosestExpression(diagnostic.range.start);
            const func = expression.findAncestor<FunctionExpression>(isFunctionExpression);

            //if we're in a sub and we do not have a return type, suggest converting to a function
            if (func.functionType.kind === TokenKind.Sub && !func.returnTypeToken) {
                //find the first function in a file that uses the `function` keyword
                const referenceFunction = this.event.file.parser.ast.findChild<FunctionExpression>((node) => {
                    return isFunctionExpression(node) && node.functionType.kind === TokenKind.Function;
                });
                const functionTypeText = referenceFunction?.functionType.text ?? 'function';
                const endFunctionTypeText = referenceFunction?.end?.text ?? 'end function';
                this.event.codeActions.push(
                    codeActionUtil.createCodeAction({
                        title: `Convert ${func.functionType.text} to ${functionTypeText}`,
                        diagnostics: [diagnostic],
                        kind: CodeActionKind.QuickFix,
                        changes: [
                            //function
                            {
                                type: 'replace',
                                filePath: this.event.file.srcPath,
                                range: func.functionType.range,
                                newText: functionTypeText
                            },
                            //end function
                            {
                                type: 'replace',
                                filePath: this.event.file.srcPath,
                                range: func.end.range,
                                newText: endFunctionTypeText
                            }
                        ]
                    })
                );
            }

            //function `as void` return type. Suggest removing the return type
            if (func.functionType.kind === TokenKind.Function && func.returnTypeToken?.kind === TokenKind.Void) {
                this.event.codeActions.push(
                    codeActionUtil.createCodeAction({
                        title: `Remove return type from function declaration`,
                        diagnostics: [diagnostic],
                        kind: CodeActionKind.QuickFix,
                        changes: [{
                            type: 'delete',
                            filePath: this.event.file.srcPath,
                            // )| as void|
                            range: util.createRange(
                                func.rightParen.range.start.line,
                                func.rightParen.range.start.character + 1,
                                func.returnTypeToken.range.end.line,
                                func.returnTypeToken.range.end.character
                            )
                        }]
                    })
                );
            }
        }
    }

    private addNonVoidFunctionReturnActions(diagnostic: Diagnostic) {
        if (isBrsFile(this.event.file)) {
            const expression = this.event.file.getClosestExpression(diagnostic.range.start);
            const func = expression.findAncestor<FunctionExpression>(isFunctionExpression);

            //`sub as <non-void type>`, suggest removing the return type
            if (func.functionType.kind === TokenKind.Sub && func.returnTypeToken && func.returnTypeToken?.kind !== TokenKind.Void) {
                this.event.codeActions.push(
                    codeActionUtil.createCodeAction({
                        title: `Remove return type from sub declaration`,
                        diagnostics: [diagnostic],
                        kind: CodeActionKind.QuickFix,
                        changes: [{
                            type: 'delete',
                            filePath: this.event.file.srcPath,
                            // )| as void|
                            range: util.createRange(
                                func.rightParen.range.start.line,
                                func.rightParen.range.start.character + 1,
                                func.returnTypeToken.range.end.line,
                                func.returnTypeToken.range.end.character
                            )
                        }]
                    })
                );
            }

            //function with no return type.
            if (func.functionType.kind === TokenKind.Function && !func.returnTypeToken) {
                //find tokens for `as` and `void` in the file if possible
                let asText: string;
                let voidText: string;
                let subText: string;
                let endSubText: string;
                for (const token of this.event.file.parser.tokens) {
                    if (asText && voidText && subText && endSubText) {
                        break;
                    }
                    if (token?.kind === TokenKind.As) {
                        asText = token?.text;
                    } else if (token?.kind === TokenKind.Void) {
                        voidText = token?.text;
                    } else if (token?.kind === TokenKind.Sub) {
                        subText = token?.text;
                    } else if (token?.kind === TokenKind.EndSub) {
                        endSubText = token?.text;
                    }
                }

                //suggest converting to `as void`
                this.event.codeActions.push(
                    codeActionUtil.createCodeAction({
                        title: `Add void return type to function declaration`,
                        diagnostics: [diagnostic],
                        kind: CodeActionKind.QuickFix,
                        changes: [{
                            type: 'insert',
                            filePath: this.event.file.srcPath,
                            position: func.rightParen.range.end,
                            newText: ` ${asText ?? 'as'} ${voidText ?? 'void'}`
                        }]
                    })
                );
                //suggest converting to sub
                this.event.codeActions.push(
                    codeActionUtil.createCodeAction({
                        title: `Convert function to sub`,
                        diagnostics: [diagnostic],
                        kind: CodeActionKind.QuickFix,
                        changes: [{
                            type: 'replace',
                            filePath: this.event.file.srcPath,
                            range: func.functionType.range,
                            newText: subText ?? 'sub'
                        }, {
                            type: 'replace',
                            filePath: this.event.file.srcPath,
                            range: func.end.range,
                            newText: endSubText ?? 'end sub'
                        }]
                    })
                );
            }
        }
    }
}

1	import type { Diagnostic } from 'vscode-languageserver';
2	import { CodeActionKind } from 'vscode-languageserver';	1✔
3	import { codeActionUtil } from '../../CodeActionUtil';	1✔
4	import type { DiagnosticMessageType } from '../../DiagnosticMessages';
5	import { DiagnosticCodeMap } from '../../DiagnosticMessages';	1✔
6	import type { BrsFile } from '../../files/BrsFile';
7	import type { XmlFile } from '../../files/XmlFile';
8	import type { BscFile, OnGetCodeActionsEvent } from '../../interfaces';
9	import { ParseMode } from '../../parser/Parser';	1✔
10	import { util } from '../../util';	1✔
11	import { isBrsFile, isFunctionExpression } from '../../astUtils/reflection';	1✔
12	import type { FunctionExpression } from '../../parser/Expression';
13	import { TokenKind } from '../../lexer/TokenKind';	1✔
14
15	export class CodeActionsProcessor {	1✔
16	public constructor(
17	public event: OnGetCodeActionsEvent	14✔
18	) {
19
20	}
21
22	public process() {
23	for (const diagnostic of this.event.diagnostics) {	14✔
24	if (diagnostic.code === DiagnosticCodeMap.cannotFindName \|\| diagnostic.code === DiagnosticCodeMap.cannotFindFunction) {	14✔
25	this.suggestCannotFindName(diagnostic as any);	8✔
26	} else if (diagnostic.code === DiagnosticCodeMap.classCouldNotBeFound) {	6!
27	this.suggestClassImports(diagnostic as any);	×
28	} else if (diagnostic.code === DiagnosticCodeMap.xmlComponentMissingExtendsAttribute) {	6✔
29	this.addMissingExtends(diagnostic as any);	2✔
30	} else if (diagnostic.code === DiagnosticCodeMap.voidFunctionMayNotReturnValue) {	4✔
31	this.addVoidFunctionReturnActions(diagnostic);	2✔
32	} else if (diagnostic.code === DiagnosticCodeMap.nonVoidFunctionMustReturnValue) {	2!
33	this.addNonVoidFunctionReturnActions(diagnostic);	2✔
34	}
35	}
36	}
37
38	private suggestedImports = new Set<string>();	14✔
39
40	/**
41	* Generic import suggestion function. Shouldn't be called directly from the main loop, but instead called by more specific diagnostic handlers
42	*/
43	private suggestImports(diagnostic: Diagnostic, key: string, files: BscFile[]) {
44	//skip if we already have this suggestion
45	if (this.suggestedImports.has(key)) {	7!
46	return;	×
47	}
48
49	this.suggestedImports.add(key);	7✔
50	const importStatements = (this.event.file as BrsFile).parser.references.importStatements;	7✔
51	//find the position of the first import statement, or the top of the file if there is none
52	const insertPosition = importStatements[importStatements.length - 1]?.importToken.range?.start ?? util.createPosition(0, 0);	7✔
53
54	//find all files that reference this function
55	for (const file of files) {	7✔
56	const pkgPath = util.getRokuPkgPath(file.pkgPath);	9✔
57	this.event.codeActions.push(	9✔
58	codeActionUtil.createCodeAction({
59	title: `import "${pkgPath}"`,
60	diagnostics: [diagnostic],
61	isPreferred: false,
62	kind: CodeActionKind.QuickFix,
63	changes: [{
64	type: 'insert',
65	filePath: this.event.file.srcPath,
66	position: insertPosition,
67	newText: `import "${pkgPath}"\n`
68	}]
69	})
70	);
71	}
72	}
73
74	private suggestCannotFindName(diagnostic: DiagnosticMessageType<'cannotFindName'>) {
75	//skip if not a BrighterScript file
76	if ((diagnostic.file as BrsFile).parseMode !== ParseMode.BrighterScript) {	8✔
77	return;	1✔
78	}
79	const lowerName = (diagnostic.data.fullName ?? diagnostic.data.name).toLowerCase();	7!
80
81	this.suggestImports(	7✔
82	diagnostic,
83	lowerName,
84	[
85	...this.event.file.program.findFilesForFunction(lowerName),
86	...this.event.file.program.findFilesForClass(lowerName),
87	...this.event.file.program.findFilesForNamespace(lowerName),
88	...this.event.file.program.findFilesForEnum(lowerName)
89	]
90	);
91	}
92
93	private suggestClassImports(diagnostic: DiagnosticMessageType<'classCouldNotBeFound'>) {
94	//skip if not a BrighterScript file
95	if ((diagnostic.file as BrsFile).parseMode !== ParseMode.BrighterScript) {	×
96	return;	×
97	}
98	const lowerClassName = diagnostic.data.className.toLowerCase();	×
99	this.suggestImports(	×
100	diagnostic,
101	lowerClassName,
102	this.event.file.program.findFilesForClass(lowerClassName)
103	);
104	}
105
106	private addMissingExtends(diagnostic: DiagnosticMessageType<'xmlComponentMissingExtendsAttribute'>) {
107	const srcPath = this.event.file.srcPath;	2✔
108	const { component } = (this.event.file as XmlFile).parser.ast;	2✔
109	//inject new attribute after the final attribute, or after the `<component` if there are no attributes
110	const pos = (component.attributes[component.attributes.length - 1] ?? component.tag).range.end;	2!
111	this.event.codeActions.push(	2✔
112	codeActionUtil.createCodeAction({
113	title: `Extend "Group"`,
114	diagnostics: [diagnostic],
115	isPreferred: true,
116	kind: CodeActionKind.QuickFix,
117	changes: [{
118	type: 'insert',
119	filePath: srcPath,
120	position: pos,
121	newText: ' extends="Group"'
122	}]
123	})
124	);
125	this.event.codeActions.push(	2✔
126	codeActionUtil.createCodeAction({
127	title: `Extend "Task"`,
128	diagnostics: [diagnostic],
129	kind: CodeActionKind.QuickFix,
130	changes: [{
131	type: 'insert',
132	filePath: srcPath,
133	position: pos,
134	newText: ' extends="Task"'
135	}]
136	})
137	);
138	this.event.codeActions.push(	2✔
139	codeActionUtil.createCodeAction({
140	title: `Extend "ContentNode"`,
141	diagnostics: [diagnostic],
142	kind: CodeActionKind.QuickFix,
143	changes: [{
144	type: 'insert',
145	filePath: srcPath,
146	position: pos,
147	newText: ' extends="ContentNode"'
148	}]
149	})
150	);
151	}
152
153	private addVoidFunctionReturnActions(diagnostic: Diagnostic) {
154	this.event.codeActions.push(	2✔
155	codeActionUtil.createCodeAction({
156	title: `Remove return value`,
157	diagnostics: [diagnostic],
158	kind: CodeActionKind.QuickFix,
159	changes: [{
160	type: 'delete',
161	filePath: this.event.file.srcPath,
162	range: util.createRange(
163	diagnostic.range.start.line,
164	diagnostic.range.start.character + 'return'.length,
165	diagnostic.range.end.line,
166	diagnostic.range.end.character
167	)
168	}]
169	})
170	);
171	if (isBrsFile(this.event.file)) {	2!
172	const expression = this.event.file.getClosestExpression(diagnostic.range.start);	2✔
173	const func = expression.findAncestor<FunctionExpression>(isFunctionExpression);	2✔
174
175	//if we're in a sub and we do not have a return type, suggest converting to a function
176	if (func.functionType.kind === TokenKind.Sub && !func.returnTypeToken) {	2✔
177	//find the first function in a file that uses the `function` keyword
178	const referenceFunction = this.event.file.parser.ast.findChild<FunctionExpression>((node) => {	1✔
179	return isFunctionExpression(node) && node.functionType.kind === TokenKind.Function;	6✔
180	});
181	const functionTypeText = referenceFunction?.functionType.text ?? 'function';	1!
182	const endFunctionTypeText = referenceFunction?.end?.text ?? 'end function';	1!
183	this.event.codeActions.push(	1✔
184	codeActionUtil.createCodeAction({
185	title: `Convert ${func.functionType.text} to ${functionTypeText}`,
186	diagnostics: [diagnostic],
187	kind: CodeActionKind.QuickFix,
188	changes: [
189	//function
190	{
191	type: 'replace',
192	filePath: this.event.file.srcPath,
193	range: func.functionType.range,
194	newText: functionTypeText
195	},
196	//end function
197	{
198	type: 'replace',
199	filePath: this.event.file.srcPath,
200	range: func.end.range,
201	newText: endFunctionTypeText
202	}
203	]
204	})
205	);
206	}
207
208	//function `as void` return type. Suggest removing the return type
209	if (func.functionType.kind === TokenKind.Function && func.returnTypeToken?.kind === TokenKind.Void) {	2!
210	this.event.codeActions.push(	1✔
211	codeActionUtil.createCodeAction({
212	title: `Remove return type from function declaration`,
213	diagnostics: [diagnostic],
214	kind: CodeActionKind.QuickFix,
215	changes: [{
216	type: 'delete',
217	filePath: this.event.file.srcPath,
218	// )\| as void\|
219	range: util.createRange(
220	func.rightParen.range.start.line,
221	func.rightParen.range.start.character + 1,
222	func.returnTypeToken.range.end.line,
223	func.returnTypeToken.range.end.character
224	)
225	}]
226	})
227	);
228	}
229	}
230	}
231
232	private addNonVoidFunctionReturnActions(diagnostic: Diagnostic) {
233	if (isBrsFile(this.event.file)) {	2!
234	const expression = this.event.file.getClosestExpression(diagnostic.range.start);	2✔
235	const func = expression.findAncestor<FunctionExpression>(isFunctionExpression);	2✔
236
237	//`sub as <non-void type>`, suggest removing the return type
238	if (func.functionType.kind === TokenKind.Sub && func.returnTypeToken && func.returnTypeToken?.kind !== TokenKind.Void) {	2!
239	this.event.codeActions.push(	1✔
240	codeActionUtil.createCodeAction({
241	title: `Remove return type from sub declaration`,
242	diagnostics: [diagnostic],
243	kind: CodeActionKind.QuickFix,
244	changes: [{
245	type: 'delete',
246	filePath: this.event.file.srcPath,
247	// )\| as void\|
248	range: util.createRange(
249	func.rightParen.range.start.line,
250	func.rightParen.range.start.character + 1,
251	func.returnTypeToken.range.end.line,
252	func.returnTypeToken.range.end.character
253	)
254	}]
255	})
256	);
257	}
258
259	//function with no return type.
260	if (func.functionType.kind === TokenKind.Function && !func.returnTypeToken) {	2✔
261	//find tokens for `as` and `void` in the file if possible
262	let asText: string;
263	let voidText: string;
264	let subText: string;
265	let endSubText: string;
266	for (const token of this.event.file.parser.tokens) {	1✔
267	if (asText && voidText && subText && endSubText) {	13!
NEW 268	break;	×
269	}
270	if (token?.kind === TokenKind.As) {	13!
NEW 271	asText = token?.text;	×
272	} else if (token?.kind === TokenKind.Void) {	13!
NEW 273	voidText = token?.text;	×
274	} else if (token?.kind === TokenKind.Sub) {	13!
NEW 275	subText = token?.text;	×
276	} else if (token?.kind === TokenKind.EndSub) {	13!
NEW 277	endSubText = token?.text;	×
278	}
279	}
280
281	//suggest converting to `as void`
282	this.event.codeActions.push(	1✔
283	codeActionUtil.createCodeAction({
284	title: `Add void return type to function declaration`,
285	diagnostics: [diagnostic],
286	kind: CodeActionKind.QuickFix,
287	changes: [{
288	type: 'insert',
289	filePath: this.event.file.srcPath,
290	position: func.rightParen.range.end,
291	newText: ` ${asText ?? 'as'} ${voidText ?? 'void'}`	6!
292	}]
293	})
294	);
295	//suggest converting to sub
296	this.event.codeActions.push(	1✔
297	codeActionUtil.createCodeAction({
298	title: `Convert function to sub`,
299	diagnostics: [diagnostic],
300	kind: CodeActionKind.QuickFix,
301	changes: [{
302	type: 'replace',
303	filePath: this.event.file.srcPath,
304	range: func.functionType.range,
305	newText: subText ?? 'sub'	3!
306	}, {
307	type: 'replace',
308	filePath: this.event.file.srcPath,
309	range: func.end.range,
310	newText: endSubText ?? 'end sub'	3!
311	}]
312	})
313	);
314	}
315	}
316	}
317	}

rokucommunity / brighterscript / #14213

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous