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

Phase 2 of Typing Improvements #827

Merged
merged 14 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion benchmarks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ let options = yargs
})
.option('config', {
type: 'string',
description: 'add additional BsConfig settings as JSON - eg. \'{"enableTypeValidation":true}\'',
description: 'add additional BsConfig settings as JSON - eg. \'{"removeParameterTypes":true}\'',
default: '{}'
})
.strict()
Expand Down
1 change: 0 additions & 1 deletion benchmarks/targets/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ module.exports = async (options: TargetOptions) => {
cwd: projectPath,
createPackage: false,
copyToStaging: false,
enableTypeValidation: false,
//disable diagnostic reporting (they still get collected)
diagnosticFilters: ['**/*'],
logLevel: 'error',
Expand Down
5 changes: 5 additions & 0 deletions src/DiagnosticMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,11 @@ export let DiagnosticMessages = {
message: `'${typeText}' cannot be used as a type`,
code: 1140,
severity: DiagnosticSeverity.Error
}),
argumentTypeMismatch: (actualTypeString: string, expectedTypeString: string) => ({
message: `Argument of type '${actualTypeString}' is not compatible with parameter of type '${expectedTypeString}'`,
code: 1141,
severity: DiagnosticSeverity.Error
})
};

Expand Down
2 changes: 1 addition & 1 deletion src/Program.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1616,7 +1616,7 @@ describe('Program', () => {
program.setFile('source/main.brs', `
sub A()
'call with wrong param count
B(1,2,3)
B("one", "two")

'call unknown function
C()
Expand Down
129 changes: 128 additions & 1 deletion src/Scope.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import { DynamicType } from './types/DynamicType';
import { ObjectType } from './types/ObjectType';
import { FloatType } from './types/FloatType';
import { NamespaceType } from './types/NamespaceType';
import { DoubleType } from './types/DoubleType';
import { UnionType } from './types/UnionType';
import { isFunctionStatement, isNamespaceStatement } from './astUtils/reflection';

describe('Scope', () => {
Expand Down Expand Up @@ -816,7 +818,7 @@ describe('Scope', () => {
sayMyName = function(name as string)
end function

sayMyName()
sayMyName("John Doe")
end sub`
);
program.validate();
Expand Down Expand Up @@ -2594,6 +2596,131 @@ describe('Scope', () => {
program.validate();
expectZeroDiagnostics(program);
});

describe('binary and unary expressions', () => {
it('should set symbols with correct types from binary expressions', () => {
let mainFile = program.setFile('source/main.bs', `
sub process()
s = "hello" + "world"
exp = 2^3
num = 3.14 + 3.14
bool = true or false
notEq = {} <> invalid
end sub
`);
program.validate();
expectZeroDiagnostics(program);
const processFnScope = mainFile.getFunctionScopeAtPosition(util.createPosition(2, 24));
const symbolTable = processFnScope.symbolTable;
const opts = { flags: SymbolTypeFlag.runtime };
expectTypeToBe(symbolTable.getSymbolType('s', opts), StringType);
expectTypeToBe(symbolTable.getSymbolType('exp', opts), IntegerType);
expectTypeToBe(symbolTable.getSymbolType('num', opts), FloatType);
expectTypeToBe(symbolTable.getSymbolType('bool', opts), BooleanType);
expectTypeToBe(symbolTable.getSymbolType('notEq', opts), BooleanType);
});

it('should set symbols with correct types from unary expressions', () => {
let mainFile = program.setFile('source/main.bs', `
sub process(boolVal as boolean, intVal as integer)
a = not boolVal
b = not true
c = not intVal
d = not 3.14

e = -34
f = -3.14
g = -intVal
h = - (-f)
end sub
`);
program.validate();
expectZeroDiagnostics(program);
const processFnScope = mainFile.getFunctionScopeAtPosition(util.createPosition(2, 24));
const symbolTable = processFnScope.symbolTable;
const opts = { flags: SymbolTypeFlag.runtime };
expectTypeToBe(symbolTable.getSymbolType('a', opts), BooleanType);
expectTypeToBe(symbolTable.getSymbolType('b', opts), BooleanType);
expectTypeToBe(symbolTable.getSymbolType('c', opts), IntegerType);
expectTypeToBe(symbolTable.getSymbolType('d', opts), IntegerType);

expectTypeToBe(symbolTable.getSymbolType('e', opts), IntegerType);
expectTypeToBe(symbolTable.getSymbolType('f', opts), FloatType);
expectTypeToBe(symbolTable.getSymbolType('g', opts), IntegerType);
expectTypeToBe(symbolTable.getSymbolType('h', opts), FloatType);
});
});

describe('assignment expressions', () => {
it('should set correct type on simple equals', () => {
let mainFile = program.setFile('source/main.bs', `
sub process(intVal as integer, dblVal as double, strVal as string)
a = intVal
b = dblVal
c = strVal
d = {}
f = m.foo
e = true
end sub
`);
program.validate();
expectZeroDiagnostics(program);
const processFnScope = mainFile.getFunctionScopeAtPosition(util.createPosition(2, 24));
const symbolTable = processFnScope.symbolTable;
const opts = { flags: SymbolTypeFlag.runtime };
expectTypeToBe(symbolTable.getSymbolType('a', opts), IntegerType);
expectTypeToBe(symbolTable.getSymbolType('b', opts), DoubleType);
expectTypeToBe(symbolTable.getSymbolType('c', opts), StringType);
expectTypeToBe(symbolTable.getSymbolType('d', opts), DynamicType);
expectTypeToBe(symbolTable.getSymbolType('f', opts), DynamicType);
expectTypeToBe(symbolTable.getSymbolType('e', opts), BooleanType);
});

it('should set correct type on compound equals', () => {
let mainFile = program.setFile('source/main.bs', `
sub process(intVal as integer, dblVal as double, strVal as string)
a = intVal
a += 4
b = dblVal
b *= 23
c = strVal
c += "hello world"
d = 3.14
d \= 3 ' integer division -> d could be either a float or int
end sub
`);
program.validate();
expectZeroDiagnostics(program);
const processFnScope = mainFile.getFunctionScopeAtPosition(util.createPosition(2, 24));
const symbolTable = processFnScope.symbolTable;
const opts = { flags: SymbolTypeFlag.runtime };
expectTypeToBe(symbolTable.getSymbolType('a', opts), IntegerType);
expectTypeToBe(symbolTable.getSymbolType('b', opts), DoubleType);
expectTypeToBe(symbolTable.getSymbolType('c', opts), StringType);
const dType = symbolTable.getSymbolType('d', opts);
expectTypeToBe(dType, UnionType);
expect((dType as UnionType).types).to.include(FloatType.instance);
expect((dType as UnionType).types).to.include(IntegerType.instance);
});

it('should work for a multiple binary expressions', () => {
let mainFile = program.setFile('source/main.bs', `
function process(intVal as integer)
x = (intVal * 2) + 1 + 3^8 + intVal + (3 - 9) ' should be int
result = 3.5 ' float
result *= (x + 1.123 * 3) ' -> float
return result
end function
`);
program.validate();
expectZeroDiagnostics(program);
const processFnScope = mainFile.getFunctionScopeAtPosition(util.createPosition(2, 24));
const symbolTable = processFnScope.symbolTable;
const opts = { flags: SymbolTypeFlag.runtime };
expectTypeToBe(symbolTable.getSymbolType('x', opts), IntegerType);
expectTypeToBe(symbolTable.getSymbolType('result', opts), FloatType);
});
});
});

describe('unlinkSymbolTable', () => {
Expand Down
38 changes: 0 additions & 38 deletions src/Scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,6 @@ export class Scope {

//do many per-file checks
this.enumerateBrsFiles((file) => {
this.diagnosticDetectFunctionCallsWithWrongParamCount(file, callableContainerMap);
this.diagnosticDetectShadowedLocalVars(file, callableContainerMap);
this.diagnosticDetectFunctionCollisions(file);
this.detectVariableNamespaceCollisions(file);
Expand Down Expand Up @@ -961,43 +960,6 @@ export class Scope {
this.diagnostics.push(...validator.diagnostics);
}

/**
* Detect calls to functions with the incorrect number of parameters
*/
private diagnosticDetectFunctionCallsWithWrongParamCount(file: BscFile, callableContainersByLowerName: CallableContainerMap) {
//validate all function calls
for (let expCall of file.functionCalls) {
let callableContainersWithThisName = callableContainersByLowerName.get(expCall.name.toLowerCase());

//use the first item from callablesByLowerName, because if there are more, that's a separate error
let knownCallableContainer = callableContainersWithThisName ? callableContainersWithThisName[0] : undefined;

if (knownCallableContainer) {
//get min/max parameter count for callable
let minParams = 0;
let maxParams = 0;
for (let param of knownCallableContainer.callable.params) {
maxParams++;
//optional parameters must come last, so we can assume that minParams won't increase once we hit
//the first isOptional
if (param.isOptional !== true) {
minParams++;
}
}
let expCallArgCount = expCall.args.length;
if (expCall.args.length > maxParams || expCall.args.length < minParams) {
let minMaxParamsText = minParams === maxParams ? maxParams : `${minParams}-${maxParams}`;
this.diagnostics.push({
...DiagnosticMessages.mismatchArgumentCount(minMaxParamsText, expCallArgCount),
range: expCall.nameRange,
//TODO detect end of expression call
file: file
});
}
}
}
}

/**
* Detect local variables (function scope) that have the same name as scope calls
*/
Expand Down
Loading