Skip to content
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.

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

Adds Callfunc Completion and Validation #1352

Open
wants to merge 25 commits into
base: release-1.0.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
cc42bfd
Fixed completion and hovers on callFUn invocations
markwpearce Nov 18, 2024
0871ad7
Callfunc return types are used for assignments
markwpearce Nov 18, 2024
ac8299d
Callfunc and @invocations return types are honored, as long as they a…
markwpearce Nov 19, 2024
03c9a82
Adds Callfunc invocation validation
markwpearce Nov 21, 2024
eed31a7
removed .onlys
markwpearce Nov 21, 2024
332332a
Adds component.createChild() typing
markwpearce Nov 22, 2024
74ab909
fixed some comments
markwpearce Nov 22, 2024
a320d93
Removed validation errors on generic roSgNode
markwpearce Nov 22, 2024
d9b2255
Perform validation on dependent types when ComponentType changes
markwpearce Nov 25, 2024
c68cdbe
added one final test for for revalidation when callfunc type changes
markwpearce Nov 25, 2024
0ff8001
Re-validates references to a component when the callfunc signature ch…
markwpearce Nov 25, 2024
940af94
Change some comments:
markwpearce Nov 25, 2024
4f5b5a7
removed commented out code
markwpearce Nov 25, 2024
adf264a
removed commented out code
markwpearce Nov 25, 2024
bab2489
Makes componentType checking with builtin components better
markwpearce Nov 26, 2024
9d1b21d
Added changed symbols logging -- noticed there's way too many there
markwpearce Nov 26, 2024
346e205
Added cannotFindCallFuncFunction and many tests
markwpearce Nov 29, 2024
b1d7d72
Found a crash and fixed it
markwpearce Nov 29, 2024
e13b70a
Ensures validation when the unresolved member of a resolved type changes
markwpearce Dec 2, 2024
13e0a90
Merge branch 'release-1.0.0' into callfunc_completion_and_validation
TwitchBronBron Dec 12, 2024
2e0ca5c
Merge branch 'release-1.0.0' into callfunc_completion_and_validation
TwitchBronBron Dec 13, 2024
0ce303f
Merge branch 'release-1.0.0' into callfunc_completion_and_validation
markwpearce Dec 15, 2024
caa88ec
Added coomments
markwpearce Dec 15, 2024
2572c30
Better handling of completion and validation of nodes from component …
markwpearce Dec 15, 2024
ecf13cb
Fixes callfunc on result of function call
markwpearce Dec 15, 2024
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
70 changes: 67 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)) {
markwpearce marked this conversation as resolved.
Show resolved Hide resolved
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,32 @@ 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) {
addedSymbol = this.addUnresolvedSymbol(segment, unresolvedMember?.data?.definingNode) || addedSymbol;

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

getAllUnvalidatedSegments() {
Expand All @@ -228,6 +291,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 @@ -234,7 +234,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;
TwitchBronBron marked this conversation as resolved.
Show resolved Hide resolved
}
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 @@ -822,6 +822,17 @@ export let DiagnosticMessages = {
message: `Expected return statement in function`,
code: 1155,
severity: DiagnosticSeverity.Error
}),
cannotFindCallFuncFunction: (name: string, fullName: string, typeName: string) => ({
message: `Cannot find callfunc function '${name}' for type '${typeName}'`,
code: 1156,
data: {
name: name,
fullName: fullName,
typeName: typeName,
isCallfunc: true
},
severity: DiagnosticSeverity.Error
})
};
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
Loading