Skip to content

Commit

Permalink
Augments handling of void returns (#1380)
Browse files Browse the repository at this point in the history 
* Augments handling of void returns

* Removed commented out code

* Built-in functions that are as void will return UninitializedType
  • Loading branch information
@markwpearce
markwpearce authored Jan 3, 2025
1 parent 4319d5c commit f38c476
Show file tree
Hide file tree
Showing 14 changed files with 272 additions and 102 deletions.
36 changes: 19 additions & 17 deletions src/Program.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { CodeAction, Position, Range, SignatureInformation, Location, Docum
import type { BsConfig, FinalizedBsConfig } from './BsConfig';
import { Scope } from './Scope';
import { DiagnosticMessages } from './DiagnosticMessages';
import type { FileObj, SemanticToken, FileLink, ProvideHoverEvent, ProvideCompletionsEvent, Hover, ProvideDefinitionEvent, ProvideReferencesEvent, ProvideDocumentSymbolsEvent, ProvideWorkspaceSymbolsEvent, BeforeFileAddEvent, BeforeFileRemoveEvent, PrepareFileEvent, PrepareProgramEvent, ProvideFileEvent, SerializedFile, TranspileObj, SerializeFileEvent } from './interfaces';
import type { FileObj, SemanticToken, FileLink, ProvideHoverEvent, ProvideCompletionsEvent, Hover, ProvideDefinitionEvent, ProvideReferencesEvent, ProvideDocumentSymbolsEvent, ProvideWorkspaceSymbolsEvent, BeforeFileAddEvent, BeforeFileRemoveEvent, PrepareFileEvent, PrepareProgramEvent, ProvideFileEvent, SerializedFile, TranspileObj, SerializeFileEvent, ExtraSymbolData } from './interfaces';
import { standardizePath as s, util } from './util';
import { XmlScope } from './XmlScope';
import { DependencyGraph } from './DependencyGraph';
Expand Down Expand Up @@ -146,9 +146,9 @@ export class Program {
nodeType.addBuiltInInterfaces();
if (nodeData.name === 'Node') {
// Add `roSGNode` as shorthand for `roSGNodeNode`
this.globalScope.symbolTable.addSymbol('roSGNode', { description: nodeData.description }, nodeType, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('roSGNode', { description: nodeData.description, isBuiltIn: true }, nodeType, SymbolTypeFlag.typetime);
}
this.globalScope.symbolTable.addSymbol(nodeName, { description: nodeData.description }, nodeType, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol(nodeName, { description: nodeData.description, isBuiltIn: true }, nodeType, SymbolTypeFlag.typetime);
} else {
nodeType = this.globalScope.symbolTable.getSymbolType(nodeName, { flags: SymbolTypeFlag.typetime }) as ComponentType;
}
Expand All @@ -161,35 +161,37 @@ export class Program {
private populateGlobalSymbolTable() {
//Setup primitive types in global symbolTable

this.globalScope.symbolTable.addSymbol('boolean', undefined, BooleanType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('double', undefined, DoubleType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('dynamic', undefined, DynamicType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('float', undefined, FloatType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('function', undefined, new FunctionType(), SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('integer', undefined, IntegerType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('longinteger', undefined, LongIntegerType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('object', undefined, new ObjectType(), SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('string', undefined, StringType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('void', undefined, VoidType.instance, SymbolTypeFlag.typetime);
const builtInSymbolData: ExtraSymbolData = { isBuiltIn: true };

this.globalScope.symbolTable.addSymbol('boolean', builtInSymbolData, BooleanType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('double', builtInSymbolData, DoubleType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('dynamic', builtInSymbolData, DynamicType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('float', builtInSymbolData, FloatType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('function', builtInSymbolData, new FunctionType(), SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('integer', builtInSymbolData, IntegerType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('longinteger', builtInSymbolData, LongIntegerType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('object', builtInSymbolData, new ObjectType(), SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('string', builtInSymbolData, StringType.instance, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol('void', builtInSymbolData, VoidType.instance, SymbolTypeFlag.typetime);

BuiltInInterfaceAdder.getLookupTable = () => this.globalScope.symbolTable;

for (const callable of globalCallables) {
this.globalScope.symbolTable.addSymbol(callable.name, { description: callable.shortDescription }, callable.type, SymbolTypeFlag.runtime);
this.globalScope.symbolTable.addSymbol(callable.name, { ...builtInSymbolData, description: callable.shortDescription }, callable.type, SymbolTypeFlag.runtime);
}

for (const ifaceData of Object.values(interfaces) as BRSInterfaceData[]) {
const nodeType = new InterfaceType(ifaceData.name);
nodeType.addBuiltInInterfaces();
this.globalScope.symbolTable.addSymbol(ifaceData.name, { description: ifaceData.description }, nodeType, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol(ifaceData.name, { ...builtInSymbolData, description: ifaceData.description }, nodeType, SymbolTypeFlag.typetime);
}

for (const componentData of Object.values(components) as BRSComponentData[]) {
const nodeType = new InterfaceType(componentData.name);
nodeType.addBuiltInInterfaces();
if (componentData.name !== 'roSGNode') {
// we will add `roSGNode` as shorthand for `roSGNodeNode`, since all roSgNode components are SceneGraph nodes
this.globalScope.symbolTable.addSymbol(componentData.name, { description: componentData.description }, nodeType, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol(componentData.name, { ...builtInSymbolData, description: componentData.description }, nodeType, SymbolTypeFlag.typetime);
}
}

Expand All @@ -200,7 +202,7 @@ export class Program {
for (const eventData of Object.values(events) as BRSEventData[]) {
const nodeType = new InterfaceType(eventData.name);
nodeType.addBuiltInInterfaces();
this.globalScope.symbolTable.addSymbol(eventData.name, { description: eventData.description }, nodeType, SymbolTypeFlag.typetime);
this.globalScope.symbolTable.addSymbol(eventData.name, { ...builtInSymbolData, description: eventData.description }, nodeType, SymbolTypeFlag.typetime);
}

}
Expand Down
13 changes: 13 additions & 0 deletions src/Scope.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2267,6 +2267,19 @@ describe('Scope', () => {
expectZeroDiagnostics(program);
});

it('should allow object = invalid test', () => {
program.setFile(`source/main.brs`, `
sub create(objName)
obj = createObject(objName)
if obj = invalid
print "not valid"
end if
end sub
`);
program.validate();
expectZeroDiagnostics(program);
});

it('should correctly validate formatJson', () => {
program.setFile(`source/main.brs`, `
sub main()
Expand Down
1 change: 1 addition & 0 deletions src/SymbolTable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ export class SymbolTable implements SymbolTypeGetter {
options.data.isAlias = data?.isAlias;
options.data.isInstance = data?.isInstance;
options.data.isFromDocComment = data?.isFromDocComment;
options.data.isBuiltIn = data?.isBuiltIn;
}
return resolvedType;
}
Expand Down
96 changes: 83 additions & 13 deletions src/bscPlugin/validation/ScopeValidator.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1247,27 +1247,40 @@ describe('ScopeValidator', () => {
expectTypeToBe(data.fieldMismatches[0].actualType, StringType);
});

it.skip('TODO: should correctly be able to use a string literal to set a enum value', () => {
program.setFile<BrsFile>('source/util.bs', `
sub goDirection(dir as Direction)
it('allows a non-built-in void function as an argument', () => {
program.setFile<BrsFile>('source/main.bs', `
sub voidFunc() as void
end sub
sub testStringAsEnumVal()
goDirection("North") ' no error
goDirection("inside out") ' error
sub doPrint(x)
print x
end sub
enum Direction
North = "North"
South = "South"
East = "East"
West = "West"
end enum
`);
sub useVoidAsArg()
doPrint(voidFunc()) ' will print "invalid"
end sub
`);
program.validate();
expectZeroDiagnostics(program);
});

it('validates a built-in void function as an argument', () => {
program.setFile<BrsFile>('source/main.bs', `
sub doPrint(x)
print x
end sub
sub useVoidAsArg()
arr = [1,2,3]
doPrint(arr.push(4))
end sub
`);
program.validate();
expectDiagnostics(program, [
DiagnosticMessages.argumentTypeMismatch('uninitialized', 'dynamic').message
]);
});

describe('default params', () => {
it('generalizes EnumMembers to their parent types', () => {
program.setFile('source/util.bs', `
Expand Down Expand Up @@ -2682,6 +2695,31 @@ describe('ScopeValidator', () => {
DiagnosticMessages.returnTypeMismatch('integer', 'void').message
]);
});

it('allows empty return when return as void', () => {
program.setFile('source/util.bs', `
function doNothing1() as void
return
end function
sub doNothing2() as void
return
end sub
sub doNothing3() as void
end sub
sub doNothing4()
return
end sub
function doNothing5()
return
end function
`);
program.validate();
expectZeroDiagnostics(program);
});
});

describe('returnTypeCoercionMismatch', () => {
Expand Down Expand Up @@ -3353,6 +3391,38 @@ describe('ScopeValidator', () => {
DiagnosticMessages.operatorTypeMismatch('+', 'boolean', 'invalid').message
]);
});

it('allows using return of a void func as a variable', () => {
program.setFile<BrsFile>('source/main.brs', `
sub voidFunc() as void
end sub
sub test()
x = voidFunc()
if x = invalid
print "invalid"
end if
end sub
`);
program.validate();
expectZeroDiagnostics(program);
});

it('validates using return of a built-in void func as a variable', () => {
program.setFile<BrsFile>('source/main.brs', `
sub test()
arr = [1,2,3]
x = arr.push(4)
if x = invalid
print "invalid"
end if
end sub
`);
program.validate();
expectDiagnostics(program, [
DiagnosticMessages.operatorTypeMismatch('=', 'uninitialized', 'invalid').message
]);
});
});

describe('memberAccessibilityMismatch', () => {
Expand Down
46 changes: 21 additions & 25 deletions src/bscPlugin/validation/ScopeValidator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ import type { XmlScope } from '../../XmlScope';
import type { XmlFile } from '../../files/XmlFile';
import { SGFieldTypes } from '../../parser/SGTypes';
import { DynamicType } from '../../types/DynamicType';
import { VoidType } from '../../types/VoidType';
import { BscTypeKind } from '../../types/BscTypeKind';
import type { BrsDocWithType } from '../../parser/BrightScriptDocParser';
import brsDocParser from '../../parser/BrightScriptDocParser';
import { InvalidType } from '../../types/InvalidType';
import { VoidType } from '../../types/VoidType';

/**
* The lower-case names of all platform-included scenegraph nodes
Expand Down Expand Up @@ -421,16 +422,25 @@ export class ScopeValidator {
const getTypeOptions = { flags: SymbolTypeFlag.runtime, data: data };
let funcType = returnStmt.findAncestor(isFunctionExpression)?.getType({ flags: SymbolTypeFlag.typetime });
if (isTypedFunctionType(funcType)) {
const actualReturnType = returnStmt?.value
let actualReturnType = returnStmt?.value
? this.getNodeTypeWrapper(file, returnStmt?.value, getTypeOptions)
: VoidType.instance;
const compatibilityData: TypeCompatibilityData = {};

if (funcType.returnType.isResolvable() && actualReturnType && !funcType.returnType.isTypeCompatible(actualReturnType, compatibilityData)) {
this.addMultiScopeDiagnostic({
...DiagnosticMessages.returnTypeMismatch(actualReturnType.toString(), funcType.returnType.toString(), compatibilityData),
location: returnStmt.value?.location ?? returnStmt.location
});
// `return` statement by itself in non-built-in function will actually result in `invalid`
const valueReturnType = isVoidType(actualReturnType) ? InvalidType.instance : actualReturnType;

if (funcType.returnType.isResolvable()) {
if (!returnStmt?.value && isVoidType(funcType.returnType)) {
// allow empty return when function is return `as void`
// eslint-disable-next-line no-useless-return
return;
} else if (!funcType.returnType.isTypeCompatible(valueReturnType, compatibilityData)) {
this.addMultiScopeDiagnostic({
...DiagnosticMessages.returnTypeMismatch(actualReturnType.toString(), funcType.returnType.toString(), compatibilityData),
location: returnStmt.value?.location ?? returnStmt.location
});
}
}
}
}
Expand Down Expand Up @@ -524,6 +534,7 @@ export class ScopeValidator {
// Can not find the type. error handled elsewhere
return;
}

let leftTypeToTest = leftType;
let rightTypeToTest = rightType;

Expand All @@ -539,24 +550,10 @@ export class ScopeValidator {
// Because you need to verify each combination of types
return;
}

const leftIsPrimitive = isPrimitiveType(leftTypeToTest);
const rightIsPrimitive = isPrimitiveType(rightTypeToTest);
const leftIsAny = isDynamicType(leftTypeToTest) || isObjectType(leftTypeToTest);
const rightIsAny = isDynamicType(rightTypeToTest) || isObjectType(rightTypeToTest);


if (leftIsAny && rightIsAny) {
// both operands are basically "any" type... ignore;
return;
} else if ((leftIsAny && rightIsPrimitive) || (leftIsPrimitive && rightIsAny)) {
// one operand is basically "any" type... ignore;
return;
}
const opResult = util.binaryOperatorResultType(leftTypeToTest, binaryExpr.tokens.operator, rightTypeToTest);

if (isDynamicType(opResult)) {
// if the result was dynamic, that means there wasn't a valid operation
if (!opResult) {
// if the result was dynamic or void, that means there wasn't a valid operation
this.addMultiScopeDiagnostic({
...DiagnosticMessages.operatorTypeMismatch(binaryExpr.tokens.operator.text, leftType.toString(), rightType.toString()),
location: binaryExpr.location
Expand All @@ -581,7 +578,6 @@ export class ScopeValidator {
rightTypeToTest = rightType.underlyingType;
}


if (isUnionType(rightTypeToTest)) {
// TODO: it is possible to validate based on innerTypes, but more complicated
// Because you need to verify each combination of types
Expand All @@ -591,7 +587,7 @@ export class ScopeValidator {

} else if (isPrimitiveType(rightType)) {
const opResult = util.unaryOperatorResultType(unaryExpr.tokens.operator, rightTypeToTest);
if (isDynamicType(opResult)) {
if (!opResult) {
this.addMultiScopeDiagnostic({
...DiagnosticMessages.operatorTypeMismatch(unaryExpr.tokens.operator.text, rightType.toString()),
location: unaryExpr.location
Expand Down
2 changes: 1 addition & 1 deletion src/globalCallables.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ let runtimeFunctions: GlobalCallable[] = [{
}, {
name: 'Type',
shortDescription: 'Returns the type of a variable and/or object. See the BrightScript Component specification for a list of types.',
type: new TypedFunctionType(new ObjectType()),
type: new TypedFunctionType(new StringType()),
file: globalFile,
params: [{
name: 'variable',
Expand Down
4 changes: 4 additions & 0 deletions src/interfaces.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,10 @@ export interface ExtraSymbolData {
* Is this type as defined in a doc comment?
*/
isFromDocComment?: boolean;
/**
* Is this symbol built in to Brightscript?
*/
isBuiltIn?: boolean;
}

export interface GetTypeOptions {
Expand Down
12 changes: 10 additions & 2 deletions src/parser/Expression.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { ArrayType } from '../types/ArrayType';
import { AssociativeArrayType } from '../types/AssociativeArrayType';
import type { ComponentType } from '../types/ComponentType';
import { createToken } from '../astUtils/creators';
import { TypedFunctionType } from '../types';
import { InvalidType, TypedFunctionType, UninitializedType } from '../types';
import { SymbolTypeFlag } from '../SymbolTypeFlag';
import { FunctionType } from '../types/FunctionType';
import type { BaseFunctionType } from '../types/BaseFunctionType';
Expand Down Expand Up @@ -93,7 +93,7 @@ export class BinaryExpression extends Expression {
return util.binaryOperatorResultType(
this.left.getType(options),
this.tokens.operator,
this.right.getType(options));
this.right.getType(options)) ?? DynamicType.instance;
}
return DynamicType.instance;
}
Expand Down Expand Up @@ -205,6 +205,14 @@ export class CallExpression extends Expression {
return specialCaseReturnType;
}
if (isCallableType(calleeType) && (!isReferenceType(calleeType.returnType) || calleeType.returnType?.isResolvable())) {
if (isVoidType(calleeType.returnType)) {
if (options.data?.isBuiltIn) {
// built in functions that return `as void` will not initialize the result
return UninitializedType.instance;
}
// non-built in functions with return type`as void` actually return `invalid`
return InvalidType.instance;
}
return calleeType.returnType;
}
if (!isReferenceType(calleeType) && (calleeType as BaseFunctionType)?.returnType?.isResolvable()) {
Expand Down
Loading

0 comments on commit f38c476

Please sign in to comment.