From 76f528c54951e0de799edc63888d09e0bed7bfe0 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 2 Feb 2022 12:38:08 -0500 Subject: [PATCH] Fix bug with optional parameters (#501) --- src/files/BrsFile.spec.ts | 4 ++-- src/files/BrsFile.ts | 8 ++++---- src/globalCallables.spec.ts | 30 +++++++++++++++++++++++++++++- src/types/FunctionType.spec.ts | 6 +++--- src/types/FunctionType.ts | 8 ++++---- 5 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 92d35774d..13b135393 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -1584,7 +1584,7 @@ describe('BrsFile', () => { (await program.getHover(file.pathAbsolute, Position.create(2, 30))).contents ).to.equal([ '```brightscript', - 'function UCase(s? as string) as string', + 'function UCase(s as string) as string', '```' ].join('\n')); }); @@ -1601,7 +1601,7 @@ describe('BrsFile', () => { ).to.equal([ '```brightscript', //TODO this really shouldn't be returning the global function, but it does...so make sure it doesn't crash right now. - 'function Instr(start? as integer, text? as string, substring? as string) as integer', + 'function Instr(start as integer, text as string, substring as string) as integer', '```' ].join('\n')); }); diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index e6f49717b..e7afb3aff 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -510,9 +510,9 @@ export class BrsFile { functionType.setName(assignment.name.text); for (let param of assignment.value.parameters) { - let isRequired = !param.defaultValue; + let isOptional = !!param.defaultValue; //TODO compute optional parameters - functionType.addParameter(param.name.text, param.type, isRequired); + functionType.addParameter(param.name.text, param.type, isOptional); } return functionType; @@ -573,8 +573,8 @@ export class BrsFile { isRestArgument: false }; params.push(callableParam); - let isRequired = !param.defaultValue; - functionType.addParameter(callableParam.name, callableParam.type, isRequired); + let isOptional = !!param.defaultValue; + functionType.addParameter(callableParam.name, callableParam.type, isOptional); } this.callables.push({ diff --git a/src/globalCallables.spec.ts b/src/globalCallables.spec.ts index 13cdde8ef..30c2145b4 100644 --- a/src/globalCallables.spec.ts +++ b/src/globalCallables.spec.ts @@ -1,7 +1,8 @@ -import { standardizePath as s } from './util'; +import util, { standardizePath as s } from './util'; import { Program } from './Program'; import { expectDiagnostics, expectZeroDiagnostics } from './testHelpers.spec'; import { DiagnosticMessages } from './DiagnosticMessages'; +import { expect } from 'chai'; let tmpPath = s`${process.cwd()}/.tmp`; let rootDir = s`${tmpPath}/rootDir`; @@ -43,6 +44,33 @@ describe('globalCallables', () => { ]); }); + it('handles optional params properly', () => { + program.addOrReplaceFile('source/main.brs', ` + sub main() + print Mid("value1", 1) 'third param is optional + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + }); + + it('hover shows correct for optional params', async () => { + const file = program.addOrReplaceFile('source/main.brs', ` + sub main() + print Mid("value1", 1) + end sub + `); + program.validate(); + const hover = await program.getHover(file.pathAbsolute, util.createPosition(2, 25)); + expect( + hover.contents.toString().replace('\r\n', '\n') + ).to.eql([ + '```brightscript', + 'function Mid(s as string, p as integer, n? as integer) as string', + '```' + ].join('\n')); + }); + describe('bslCore', () => { it('exists', () => { program.addOrReplaceFile('source/main.brs', ` diff --git a/src/types/FunctionType.spec.ts b/src/types/FunctionType.spec.ts index 69732e01d..463a95894 100644 --- a/src/types/FunctionType.spec.ts +++ b/src/types/FunctionType.spec.ts @@ -16,15 +16,15 @@ describe('FunctionType', () => { //different parameter count expect( - new FunctionType(new VoidType()).addParameter('a', new IntegerType(), true).isAssignableTo( + new FunctionType(new VoidType()).addParameter('a', new IntegerType(), false).isAssignableTo( new FunctionType(new VoidType()) ) ).to.be.false; //different parameter types expect( - new FunctionType(new VoidType()).addParameter('a', new IntegerType(), true).isAssignableTo( - new FunctionType(new VoidType()).addParameter('a', new StringType(), true) + new FunctionType(new VoidType()).addParameter('a', new IntegerType(), false).isAssignableTo( + new FunctionType(new VoidType()).addParameter('a', new StringType(), false) ) ).to.be.false; diff --git a/src/types/FunctionType.ts b/src/types/FunctionType.ts index c53748ac3..5dc5358ce 100644 --- a/src/types/FunctionType.ts +++ b/src/types/FunctionType.ts @@ -18,18 +18,18 @@ export class FunctionType implements BscType { */ public isSub = false; - public params = [] as Array<{ name: string; type: BscType; isRequired: boolean }>; + public params = [] as Array<{ name: string; type: BscType; isOptional: boolean }>; public setName(name: string) { this.name = name; return this; } - public addParameter(name: string, type: BscType, isRequired: boolean) { + public addParameter(name: string, type: BscType, isOptional: boolean) { this.params.push({ name: name, type: type, - isRequired: isRequired === false ? false : true + isOptional: isOptional === true ? true : false }); return this; } @@ -67,7 +67,7 @@ export class FunctionType implements BscType { public toString() { let paramTexts = []; for (let param of this.params) { - paramTexts.push(`${param.name}${param.isRequired ? '' : '?'} as ${param.type.toString()}`); + paramTexts.push(`${param.name}${param.isOptional ? '?' : ''} as ${param.type.toString()}`); } return `${this.isSub ? 'sub' : 'function'} ${this.name}(${paramTexts.join(', ')}) as ${this.returnType.toString()}`;