Skip to content

Commit

Permalink
Adds Callfunc Completion and Validation (#1352)
Browse files Browse the repository at this point in the history
* Fixed completion and hovers on callFUn invocations

* Callfunc return types are used for assignments

* Callfunc and @invocations return types are honored, as long as they are built in types

* Adds Callfunc invocation validation

* removed .onlys

* Adds component.createChild() typing

* fixed some comments

* Removed validation errors on generic roSgNode

* Perform validation on dependent types when ComponentType changes

* added one final test for for revalidation when callfunc type changes

* Re-validates references to a component when the callfunc signature changes

* Change some comments:

* removed commented out code

* removed commented out code

* Makes componentType checking with builtin components better

* Added changed symbols logging -- noticed there's way too many there

* Added cannotFindCallFuncFunction and many tests

* Found a crash and fixed it

* Ensures validation when the unresolved member of a resolved type changes

* Added coomments

* Better handling of completion and validation of nodes from component libraries

* Fixes callfunc on result of function call

* Revert change - there's a difference between built in symbols and built in types

* Fix build

* Fixed issue for callfuncs returning voidtype

* Adds count of changed symbols to the validation metrics

* Adds protection on not being able to decipher defining nodes for some symbols

---------

Co-authored-by: Bronley Plumb <[email protected]>
  • Loading branch information
markwpearce and TwitchBronBron authored Jan 15, 2025
1 parent 7fb92ff commit 1a27f76
Show file tree
Hide file tree
Showing 47 changed files with 2,117 additions and 237 deletions.
72 changes: 69 additions & 3 deletions src/AstValidationSegmenter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { DottedGetExpression, TypeExpression, VariableExpression } from './parser/Expression';
import { isAliasStatement, isBinaryExpression, isBlock, isBody, isClassStatement, isConditionalCompileStatement, isDottedGetExpression, isInterfaceStatement, isNamespaceStatement, isTypeExpression, isVariableExpression } from './astUtils/reflection';
import { isAliasStatement, isBinaryExpression, isBlock, isBody, isClassStatement, isConditionalCompileStatement, isDottedGetExpression, isInterfaceStatement, isNamespaceStatement, isTypecastStatement, isTypeExpression, isVariableExpression } from './astUtils/reflection';
import { ChildrenSkipper, WalkMode, createVisitor } from './astUtils/visitors';
import type { ExtraSymbolData, GetTypeOptions, TypeChainEntry } from './interfaces';
import type { AstNode, Expression } from './parser/AstNode';
Expand All @@ -9,6 +9,7 @@ import { SymbolTypeFlag } from './SymbolTypeFlag';
import type { Token } from './lexer/Token';
import type { BrsFile } from './files/BrsFile';
import { TokenKind } from './lexer/TokenKind';
import type { BscSymbol } from './SymbolTable';

// eslint-disable-next-line no-bitwise
export const InsideSegmentWalkMode = WalkMode.visitStatements | WalkMode.visitExpressions | WalkMode.recurseChildFunctions;
Expand Down Expand Up @@ -73,12 +74,20 @@ export class AstValidationSegmenter {
return this.checkExpressionForUnresolved(segment, expression.expression.left as VariableExpression, assignedSymbolsNames) ||
this.checkExpressionForUnresolved(segment, expression.expression.right as VariableExpression, assignedSymbolsNames);
}
if (isTypeExpression(expression)) {
const typeIntypeExpression = expression.getType({ flags: SymbolTypeFlag.typetime });
if (typeIntypeExpression.isResolvable()) {
return this.handleTypeCastTypeExpression(segment, expression);
}
}
return this.addUnresolvedSymbol(segment, expression, assignedSymbolsNames);
}

private addUnresolvedSymbol(segment: AstNode, expression: Expression, assignedSymbolsNames?: Set<string>) {
const flag = util.isInTypeExpression(expression) ? SymbolTypeFlag.typetime : SymbolTypeFlag.runtime;
let typeChain: TypeChainEntry[] = [];
const extraData = {} as ExtraSymbolData;
const options: GetTypeOptions = { flags: flag, onlyCacheResolvedTypes: true, typeChain: typeChain, data: extraData };

const nodeType = expression.getType(options);
if (!nodeType?.isResolvable()) {
let symbolsSet: Set<UnresolvedSymbol>;
Expand Down Expand Up @@ -126,6 +135,8 @@ export class AstValidationSegmenter {

private currentNamespaceStatement: NamespaceStatement;
private currentClassStatement: ClassStatement;
private unresolvedTypeCastTypeExpressions: TypeExpression[] = [];


checkSegmentWalk(segment: AstNode) {
if (isNamespaceStatement(segment) || isBody(segment)) {
Expand Down Expand Up @@ -161,6 +172,7 @@ export class AstValidationSegmenter {
return;
}


this.segmentsForValidation.push(segment);
this.validatedSegments.set(segment, false);
let foundUnresolvedInSegment = false;
Expand All @@ -169,6 +181,16 @@ export class AstValidationSegmenter {
const assignedSymbolsNames = new Set<string>();
this.currentClassStatement = segment.findAncestor(isClassStatement);

if (isTypecastStatement(segment)) {
if (this.checkExpressionForUnresolved(segment, segment.typecastExpression.typeExpression)) {
this.unresolvedTypeCastTypeExpressions.push(segment.typecastExpression.typeExpression);
}
}
let unresolvedTypeCastTypeExpression: TypeExpression;
if (this.unresolvedTypeCastTypeExpressions.length > 0) {
unresolvedTypeCastTypeExpression = this.unresolvedTypeCastTypeExpressions[this.unresolvedTypeCastTypeExpressions.length - 1];
}

segment.walk(createVisitor({
AssignmentStatement: (stmt) => {
if (stmt.tokens.equals.kind === TokenKind.Equal) {
Expand All @@ -186,7 +208,11 @@ export class AstValidationSegmenter {
assignedSymbolsNames.add(stmt.tokens.item.text.toLowerCase());
},
VariableExpression: (expr) => {
if (!assignedSymbolsNames.has(expr.tokens.name.text.toLowerCase())) {
const hasUnresolvedTypecastedM = unresolvedTypeCastTypeExpression && expr.tokens.name.text.toLowerCase() === 'm';
if (hasUnresolvedTypecastedM) {
this.addUnresolvedSymbol(segment, unresolvedTypeCastTypeExpression);

} else if (!assignedSymbolsNames.has(expr.tokens.name.text.toLowerCase())) {
const expressionIsUnresolved = this.checkExpressionForUnresolved(segment, expr, assignedSymbolsNames);
foundUnresolvedInSegment = expressionIsUnresolved || foundUnresolvedInSegment;
}
Expand All @@ -195,6 +221,18 @@ export class AstValidationSegmenter {
DottedGetExpression: (expr) => {
const expressionIsUnresolved = this.checkExpressionForUnresolved(segment, expr, assignedSymbolsNames);
foundUnresolvedInSegment = expressionIsUnresolved || foundUnresolvedInSegment;
if (!foundUnresolvedInSegment && unresolvedTypeCastTypeExpression) {
let startOfDottedGet: Expression = expr;
while (isDottedGetExpression(startOfDottedGet)) {
startOfDottedGet = startOfDottedGet.obj;
}
if (isVariableExpression(startOfDottedGet)) {
const hasUnresolvedTypeCastedM = unresolvedTypeCastTypeExpression && startOfDottedGet.tokens.name.text.toLowerCase() === 'm';
if (hasUnresolvedTypeCastedM) {
this.handleTypeCastTypeExpression(segment, unresolvedTypeCastTypeExpression);
}
}
}
skipper.skip();
},
TypeExpression: (expr) => {
Expand All @@ -212,7 +250,34 @@ export class AstValidationSegmenter {
}
this.currentClassStatement = undefined;
this.currentClassStatement = undefined;
}


private handleTypeCastTypeExpression(segment: AstNode, typecastTypeExpression: TypeExpression) {
const expression = typecastTypeExpression;
if (isTypeExpression(expression)) {
const typeIntypeExpression = expression.getType({ flags: SymbolTypeFlag.typetime });

if (typeIntypeExpression.isResolvable()) {
const memberSymbols = typeIntypeExpression.getMemberTable().getAllSymbols(SymbolTypeFlag.runtime);
const unresolvedMembers: BscSymbol[] = [];
for (const memberSymbol of memberSymbols) {
if (!memberSymbol.type.isResolvable()) {
unresolvedMembers.push(memberSymbol);
}
}
let addedSymbol = false;
for (const unresolvedMember of unresolvedMembers) {
if (unresolvedMember?.data?.definingNode) {
addedSymbol = this.addUnresolvedSymbol(segment, unresolvedMember.data.definingNode) || addedSymbol;
}

}
return addedSymbol;
}
return this.addUnresolvedSymbol(segment, expression);
}
return false;
}

getAllUnvalidatedSegments() {
Expand All @@ -228,6 +293,7 @@ export class AstValidationSegmenter {

getSegmentsWithChangedSymbols(changedSymbols: Map<SymbolTypeFlag, Set<string>>): AstNode[] {
const segmentsToWalkForValidation: AstNode[] = [];

for (const segment of this.segmentsForValidation) {
if (this.validatedSegments.get(segment)) {
continue;
Expand Down
4 changes: 2 additions & 2 deletions src/CrossScopeValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ export class CrossScopeValidator {
if (this.providedTreeMap.has(scope)) {
return this.providedTreeMap.get(scope);
}
const providedTree = new ProvidedNode('', this.componentsMap);
const providedTree = new ProvidedNode('');//, this.componentsMap);
const duplicatesMap = new Map<string, Set<FileSymbolPair>>();

const referenceTypesMap = new Map<{ symbolName: string; file: BscFile; symbolObj: ProvidedSymbol }, Array<{ name: string; namespacedName?: string }>>();
Expand Down Expand Up @@ -499,7 +499,7 @@ export class CrossScopeValidator {
const typeName = 'rosgnode' + componentName;
const component = this.program.getComponent(componentName);
const componentSymbol = this.program.globalScope.symbolTable.getSymbol(typeName, SymbolTypeFlag.typetime)?.[0];
if (componentSymbol && component) {
if (componentSymbol && component && componentSymbol.type?.isBuiltIn) {
this.componentsMap.set(typeName, { file: component.file, symbol: componentSymbol });
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/DiagnosticManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ export class DiagnosticManager {
isMatch = !!context.tags?.includes(filter.tag);
}
if (isMatch && needToMatch.scope) {
isMatch = context.scope === filter.scope;
isMatch = context.scope?.name === filter.scope.name;
}
if (isMatch && needToMatch.fileUri) {
isMatch = cachedData.diagnostic.location?.uri === filter.fileUri;
Expand Down
11 changes: 11 additions & 0 deletions src/DiagnosticMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,17 @@ export let DiagnosticMessages = {
legacyCode: 1151,
severity: DiagnosticSeverity.Error,
code: 'return-type-coercion-mismatch'
}),
cannotFindCallFuncFunction: (name: string, fullName: string, typeName: string) => ({
message: `Cannot find callfunc function '${name}' for type '${typeName}'`,
data: {
name: name,
fullName: fullName,
typeName: typeName,
isCallfunc: true
},
severity: DiagnosticSeverity.Error,
code: 'cannot-find-callfunc'
})
};
export const defaultMaximumTruncationLength = 160;
Expand Down
2 changes: 2 additions & 0 deletions src/PluginInterface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export default class PluginInterface<T extends CompilerPlugin = CompilerPlugin>
* Call `event` on plugins
*/
public emit<K extends keyof PluginEventArgs<T> & string>(event: K, ...args: PluginEventArgs<T>[K]) {
this.logger.debug(`Emitting plugin event: ${event}`);
for (let plugin of this.plugins) {
if ((plugin as any)[event]) {
try {
Expand All @@ -75,6 +76,7 @@ export default class PluginInterface<T extends CompilerPlugin = CompilerPlugin>
* Call `event` on plugins, but allow the plugins to return promises that will be awaited before the next plugin is notified
*/
public async emitAsync<K extends keyof PluginEventArgs<T> & string>(event: K, ...args: PluginEventArgs<T>[K]): Promise< PluginEventArgs<T>[K][0]> {
this.logger.debug(`Emitting async plugin event: ${event}`);
for (let plugin of this.plugins) {
if ((plugin as any)[event]) {
try {
Expand Down
157 changes: 156 additions & 1 deletion src/Program.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type { SinonSpy } from 'sinon';
import { createSandbox } from 'sinon';
import { SymbolTypeFlag } from './SymbolTypeFlag';
import { AssetFile } from './files/AssetFile';
import type { ProvideFileEvent, CompilerPlugin, BeforeProvideFileEvent, AfterProvideFileEvent, BeforeFileAddEvent, AfterFileAddEvent, BeforeFileRemoveEvent, AfterFileRemoveEvent } from './interfaces';
import type { ProvideFileEvent, CompilerPlugin, BeforeProvideFileEvent, AfterProvideFileEvent, BeforeFileAddEvent, AfterFileAddEvent, BeforeFileRemoveEvent, AfterFileRemoveEvent, ScopeValidationOptions } from './interfaces';
import { StringType, TypedFunctionType, DynamicType, FloatType, IntegerType, InterfaceType, ArrayType, BooleanType, DoubleType, UnionType } from './types';
import { AssociativeArrayType } from './types/AssociativeArrayType';
import { ComponentType } from './types/ComponentType';
Expand Down Expand Up @@ -584,6 +584,161 @@ describe('Program', () => {
program.validate();
expectZeroDiagnostics(program);
});

describe('changed symbols', () => {
it('includes components when component interface changes', () => {
program.setFile('components/widget.xml', trim`
<component name="Widget" extends="Group">
<interface>
<field id="foo" type="string" />
</interface>
</component>
`);
program.setFile('components/other.xml', trim`
<component name="Other" extends="Group">
<interface>
<field id="foo" type="string" />
</interface>
</component>
`);
program.setFile('source/main.bs', `
sub sourceScopeFunc()
end sub
`);
program.validate();
let options: ScopeValidationOptions = program['currentScopeValidationOptions'];
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodewidget')).to.be.true;
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodeother')).to.be.true;

expectZeroDiagnostics(program);
//change widget
program.setFile('components/widget.xml', trim`
<component name="Widget" extends="Group">
<interface>
<field id="foo" type="integer" />
</interface>
</component>
`);
program.validate();
expectZeroDiagnostics(program);
options = program['currentScopeValidationOptions'];
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodewidget')).to.be.true;
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodeother')).to.be.false;
});

it('includes components when component callfunc changes', () => {
program.setFile('components/widget.xml', trim`
<component name="Widget" extends="Group">
<script type="text/brightscript" uri="widget.bs" />
<interface>
<function name="foo" />
</interface>
</component>
`);
program.setFile('components/widget.bs', `
sub foo()
end sub
`);
program.setFile('components/other.xml', trim`
<component name="Other" extends="Group">
<interface>
<field id="foo" type="string" />
</interface>
</component>
`);
program.setFile('source/main.bs', `
sub sourceScopeFunc()
end sub
`);
program.validate();
let options: ScopeValidationOptions = program['currentScopeValidationOptions'];
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodewidget')).to.be.true;
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodeother')).to.be.true;

expectZeroDiagnostics(program);
//change [email protected]
program.setFile('components/widget.bs', `
sub foo(input)
print input
end sub
`);
program.validate();
expectZeroDiagnostics(program);
options = program['currentScopeValidationOptions'];
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodewidget')).to.be.true;
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodeother')).to.be.false;
});

it('includes types that depend on a changed component', () => {
program.setFile('components/widget.xml', trim`
<component name="Widget" extends="Group">
<script type="text/brightscript" uri="widget.bs" />
<interface>
<function name="foo" />
</interface>
</component>
`);
program.setFile('components/widget.bs', `
sub foo()
end sub
`);
program.setFile('components/other.xml', trim`
<component name="Other" extends="Group">
<interface>
<field id="foo" type="string" />
</interface>
</component>
`);
program.setFile('source/main.bs', `
interface IncludesWidget
widget as roSGNodeWidget
end interface
sub sourceScopeFunc()
end sub
`);
program.validate();
expectZeroDiagnostics(program);
let options: ScopeValidationOptions = program['currentScopeValidationOptions'];
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodewidget')).to.be.true;
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodeother')).to.be.true;
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('includeswidget')).to.be.true;

// change roSgNodeOther
program.setFile('components/other.xml', trim`
<component name="Other" extends="Group">
<interface>
<field id="foo" type="integer" />
</interface>
</component>
`);
program.validate();
expectZeroDiagnostics(program);
options = program['currentScopeValidationOptions'];

// only rosgnodewidget changes
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodewidget')).to.be.false;
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodeother')).to.be.true;
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('includeswidget')).to.be.false;

//change [email protected]
program.setFile('components/widget.bs', `
sub foo(input)
print input
end sub
`);
program.validate();
expectZeroDiagnostics(program);
options = program['currentScopeValidationOptions'];

// has rosgnodewidget AND IncludesWidget, because it depends on roSgnodeWidget
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodewidget')).to.be.true;
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('rosgnodeother')).to.be.false;
expect(options.changedSymbols.get(SymbolTypeFlag.typetime).has('includeswidget')).to.be.true;
});

});

});

describe('hasFile', () => {
Expand Down
Loading

0 comments on commit 1a27f76

Please sign in to comment.