diff --git a/docs/named-arguments.md b/docs/named-arguments.md new file mode 100644 index 000000000..1fcfc7d06 --- /dev/null +++ b/docs/named-arguments.md @@ -0,0 +1,233 @@ +# Named Arguments + +BrighterScript supports calling functions with named arguments, allowing you to pass arguments by parameter name instead of by position. + +## Basic Usage + +Use `paramName: value` syntax inside a function call: + +```brighterscript +sub greet(name as string, excited as boolean) + if excited + print "Hello, " + name + "!!!" + else + print "Hello, " + name + end if +end sub + +sub main() + greet(name: "Bob", excited: true) +end sub +``` + +
+ View the transpiled BrightScript code + +```brightscript +sub greet(name as string, excited as boolean) + if excited + print "Hello, " + name + "!!!" + else + print "Hello, " + name + end if +end sub + +sub main() + greet("Bob", true) +end sub +``` + +
+ +## Supported Call Sites + +Named arguments are supported for regular functions, namespace functions, and class constructors: + +```brighterscript +namespace MyNs + sub greet(name as string, excited as boolean) + end sub +end namespace + +class Point + function new(x as integer, y as integer) + end function +end class + +sub main() + MyNs.greet(name: "Bob", excited: true) + p = new Point(x: 1, y: 2) +end sub +``` + +## Out-of-Order Arguments + +Named arguments must be passed in the same order as the function declaration. Calling a function with named arguments out of declaration order is a compile-time error: + +```brighterscript +sub createUser(name as string, age as integer, admin as boolean) +end sub + +sub main() + ' error: Named argument 'name' is out of order + createUser(admin: false, name: "Alice", age: 30) +end sub +``` + +### Quick fix + +In editors with BrighterScript language support, a `Reorder named arguments to match function declaration` quick fix is offered for this diagnostic. Applying it rewrites the call to match the function signature: + +```brighterscript +sub createUser(name as string, age as integer, admin as boolean) +end sub + +sub main() + createUser(name: "Alice", age: 30, admin: false) +end sub +``` + +## Mixed Positional and Named Arguments + +Positional arguments can be combined with named arguments. Positional arguments must come first: + +```brighterscript +sub greet(name as string, title as string, excited as boolean) +end sub + +sub main() + greet("Bob", excited: true, title: "Mr") +end sub +``` + +
+ View the transpiled BrightScript code + +```brightscript +sub greet(name as string, title as string, excited as boolean) +end sub + +sub main() + greet("Bob", "Mr", true) +end sub +``` + +
+ +## Skipping Optional Parameters + +Named arguments let you skip optional middle parameters. BrighterScript inserts the default value for any skipped parameter: + +```brighterscript +sub greet(name as string, title = "Mr", excited = false) +end sub + +sub main() + ' Skip "title", pass "excited" by name + greet(name: "Bob", excited: true) +end sub +``` + +
+ View the transpiled BrightScript code + +```brightscript +sub greet(name as string, title = "Mr", excited = false) +end sub + +sub main() + ' Skip "title", pass "excited" by name + greet("Bob", "Mr", true) +end sub +``` + +
+ +If a skipped optional parameter has no default value, `invalid` is inserted: + +```brighterscript +sub greet(name as string, title = invalid, excited = false) +end sub + +sub main() + greet(name: "Bob", excited: true) +end sub +``` + +
+ View the transpiled BrightScript code + +```brightscript +sub greet(name as string, title = invalid, excited = false) +end sub + +sub main() + greet("Bob", invalid, true) +end sub +``` + +
+ +## Validation + +BrighterScript validates named argument calls and reports the following errors: + +### Unknown argument name + +```brighterscript +sub greet(name as string) +end sub + +greet(nope: "Bob") ' error: Unknown named argument 'nope' for function 'greet' +``` + +### Duplicate named argument + +```brighterscript +greet(name: "Bob", name: "Alice") ' error: Named argument 'name' was already provided +``` + +### Positional argument after named argument + +```brighterscript +greet(name: "Bob", true) ' error: Positional arguments cannot follow named arguments +``` + +### Named arguments on an unknown function + +Named arguments require the function definition to be visible so BrighterScript can verify parameter names at compile time: + +```brighterscript +unknownFunc(param: true) ' error: Cannot use named arguments when calling 'unknownFunc' because the function definition cannot be found +``` + +### Cross-scope parameter conflict + +When a `.bs` file is shared across multiple component scopes and the same function name resolves to different parameter signatures in those scopes, named arguments cannot be safely transpiled (the same call site would need different positional output per scope). BrighterScript reports this as an error: + +```brighterscript +' shared.bs (included by both CompA.xml and CompB.xml) +sub callFoo() + ' error: Named arguments for 'foo' are ambiguous: the function has different + ' parameter signatures across component scopes that share this file. + foo(a: 2, b: 1) +end sub +``` + +```brighterscript +' helperA.bs (included by CompA.xml) +sub foo(a, b) +end sub +``` + +```brighterscript +' helperB.bs (included by CompB.xml) +sub foo(b, a) +end sub +``` + +To resolve, give the conflicting functions distinct names or align their parameter order across scopes. + +## BrighterScript Only + +Named argument syntax is a BrighterScript feature and is not valid in plain BrightScript (`.brs`) files. diff --git a/docs/readme.md b/docs/readme.md index 6bb8e9961..bd867ebf5 100644 --- a/docs/readme.md +++ b/docs/readme.md @@ -50,6 +50,15 @@ enum RemoteButton end enum ``` +## [Named Arguments](named-arguments.md) +```brighterscript +sub greet(name as string, excited as boolean) +end sub + +' pass args by name, in any order +greet(excited: true, name: "Bob") +``` + ## [Namespaces](namespaces.md) ```brighterscript namespace util diff --git a/src/DiagnosticMessages.ts b/src/DiagnosticMessages.ts index 12357a6a3..637fbe09a 100644 --- a/src/DiagnosticMessages.ts +++ b/src/DiagnosticMessages.ts @@ -770,6 +770,41 @@ export let DiagnosticMessages = { message: `'${featureName}' requires Roku firmware version ${minimumVersion} or higher (current target is ${configuredVersion})`, code: 1146, severity: DiagnosticSeverity.Error + }), + duplicateFunctionParameter: (paramName: string) => ({ + message: `Duplicate parameter name '${paramName}'`, + code: 1147, + severity: DiagnosticSeverity.Warning + }), + unknownNamedArgument: (argName: string, funcName: string) => ({ + message: `Unknown named argument '${argName}' for function '${funcName}'`, + code: 1148, + severity: DiagnosticSeverity.Error + }), + namedArgDuplicate: (argName: string) => ({ + message: `Named argument '${argName}' was already provided`, + code: 1149, + severity: DiagnosticSeverity.Error + }), + positionalArgAfterNamedArg: () => ({ + message: `Positional arguments cannot follow named arguments`, + code: 1150, + severity: DiagnosticSeverity.Error + }), + namedArgsNotAllowedForUnknownFunction: (funcName: string) => ({ + message: `Cannot use named arguments when calling '${funcName}' because the function definition cannot be found`, + code: 1151, + severity: DiagnosticSeverity.Error + }), + namedArgsCrossScopeConflict: (funcName: string) => ({ + message: `Named arguments for '${funcName}' are ambiguous: the function has different parameter signatures across component scopes that share this file. Named arguments cannot be safely transpiled.`, + code: 1152, + severity: DiagnosticSeverity.Error + }), + namedArgOutOfOrder: (argName: string) => ({ + message: `Named argument '${argName}' is out of order`, + code: 1153, + severity: DiagnosticSeverity.Error }) }; diff --git a/src/Scope.ts b/src/Scope.ts index 176cd39b4..6c6f659ac 100644 --- a/src/Scope.ts +++ b/src/Scope.ts @@ -16,7 +16,7 @@ import { Cache } from './Cache'; import { URI } from 'vscode-uri'; import type { BrsFile } from './files/BrsFile'; import type { DependencyGraph, DependencyChangedEvent } from './DependencyGraph'; -import { isBrsFile, isMethodStatement, isCustomType, isFunctionType, isXmlFile, isNamespaceStatement, isEnumMemberStatement } from './astUtils/reflection'; +import { isBrsFile, isMethodStatement, isCustomType, isFunctionType, isXmlFile, isNamespaceStatement, isEnumMemberStatement, isNamedArgumentExpression } from './astUtils/reflection'; import { SymbolTable } from './SymbolTable'; import type { Statement } from './parser/AstNode'; import { LogLevel } from './logging'; @@ -947,6 +947,11 @@ export class Scope { private diagnosticDetectFunctionCallsWithWrongParamCount(file: BscFile, callableContainersByLowerName: CallableContainerMap) { //validate all function calls for (let expCall of file?.functionCalls ?? []) { + // Skip calls that use named arguments — those are fully validated by validateNamedArgCalls() + if (expCall.args.some(a => isNamedArgumentExpression(a.expression))) { + continue; + } + let callableContainersWithThisName = callableContainersByLowerName.get(expCall.name.toLowerCase()); //use the first item from callablesByLowerName, because if there are more, that's a separate error diff --git a/src/astUtils/reflection.ts b/src/astUtils/reflection.ts index b5f5e9144..f8acc804f 100644 --- a/src/astUtils/reflection.ts +++ b/src/astUtils/reflection.ts @@ -1,5 +1,5 @@ import type { Body, AssignmentStatement, Block, ExpressionStatement, CommentStatement, ExitForStatement, ExitWhileStatement, FunctionStatement, IfStatement, IncrementStatement, PrintStatement, GotoStatement, LabelStatement, ReturnStatement, EndStatement, StopStatement, ForStatement, ForEachStatement, WhileStatement, DottedSetStatement, IndexedSetStatement, LibraryStatement, NamespaceStatement, ImportStatement, ClassFieldStatement, ClassMethodStatement, ClassStatement, InterfaceFieldStatement, InterfaceMethodStatement, InterfaceStatement, EnumStatement, EnumMemberStatement, TryCatchStatement, CatchStatement, ThrowStatement, MethodStatement, FieldStatement, ConstStatement, ContinueStatement, DimStatement, TypecastStatement, AliasStatement, TypeStatement } from '../parser/Statement'; -import type { LiteralExpression, BinaryExpression, CallExpression, FunctionExpression, NamespacedVariableNameExpression, DottedGetExpression, XmlAttributeGetExpression, IndexedGetExpression, GroupingExpression, EscapedCharCodeLiteralExpression, ArrayLiteralExpression, AALiteralExpression, UnaryExpression, VariableExpression, SourceLiteralExpression, NewExpression, CallfuncExpression, TemplateStringQuasiExpression, TemplateStringExpression, TaggedTemplateStringExpression, AnnotationExpression, FunctionParameterExpression, AAMemberExpression, AAIndexedMemberExpression, TypeCastExpression, TernaryExpression, NullCoalescingExpression } from '../parser/Expression'; +import type { LiteralExpression, BinaryExpression, CallExpression, FunctionExpression, NamespacedVariableNameExpression, DottedGetExpression, XmlAttributeGetExpression, IndexedGetExpression, GroupingExpression, EscapedCharCodeLiteralExpression, ArrayLiteralExpression, AALiteralExpression, UnaryExpression, VariableExpression, SourceLiteralExpression, NewExpression, CallfuncExpression, TemplateStringQuasiExpression, TemplateStringExpression, TaggedTemplateStringExpression, AnnotationExpression, FunctionParameterExpression, AAMemberExpression, AAIndexedMemberExpression, TypeCastExpression, TernaryExpression, NullCoalescingExpression, NamedArgumentExpression } from '../parser/Expression'; import type { BrsFile } from '../files/BrsFile'; import type { XmlFile } from '../files/XmlFile'; import type { BscFile, File, TypedefProvider } from '../interfaces'; @@ -213,6 +213,9 @@ export function isBinaryExpression(element: AstNode | undefined): element is Bin export function isCallExpression(element: AstNode | undefined): element is CallExpression { return element?.constructor.name === 'CallExpression'; } +export function isNamedArgumentExpression(element: AstNode | undefined): element is NamedArgumentExpression { + return element?.constructor.name === 'NamedArgumentExpression'; +} export function isFunctionExpression(element: AstNode | undefined): element is FunctionExpression { return element?.constructor.name === 'FunctionExpression'; } diff --git a/src/bscPlugin/codeActions/CodeActionsProcessor.spec.ts b/src/bscPlugin/codeActions/CodeActionsProcessor.spec.ts index aad2feea8..7e1132d3d 100644 --- a/src/bscPlugin/codeActions/CodeActionsProcessor.spec.ts +++ b/src/bscPlugin/codeActions/CodeActionsProcessor.spec.ts @@ -473,6 +473,41 @@ describe('CodeActionsProcessor', () => { }); }); + describe('named argument ordering', () => { + it('offers quick fix for out-of-order named arguments', () => { + const file = program.setFile('source/main.bs', ` + sub greet(name as string, excited as boolean) + end sub + + sub main() + greet(excited: true, name: "Bob") + end sub + `); + + testGetCodeActions(file, util.createRange(5, 41, 5, 41), [ + 'Reorder named arguments to match function declaration' + ]); + }); + + it('reorders mixed positional and named args to declaration order', () => { + const file = program.setFile('source/main.bs', ` + sub greet(name as string, title = "Mr" as string, excited = false as boolean) + end sub + + sub main() + greet("Bob", excited: true, title: "Dr") + end sub + `); + + program.validate(); + const actions = program.getCodeActions(file.srcPath, util.createRange(5, 50, 5, 50)); + const action = actions.find(x => x.title === 'Reorder named arguments to match function declaration'); + const edit = Object.values(action.edit.changes)[0][0]; + + expect(edit.newText).to.equal(`"Bob", title: "Dr", excited: true`); + }); + }); + it('suggests imports at very start and very end of diagnostic', () => { program.setFile('source/first.bs', ` namespace alpha diff --git a/src/bscPlugin/codeActions/CodeActionsProcessor.ts b/src/bscPlugin/codeActions/CodeActionsProcessor.ts index aee5a60ef..844bb2a5a 100644 --- a/src/bscPlugin/codeActions/CodeActionsProcessor.ts +++ b/src/bscPlugin/codeActions/CodeActionsProcessor.ts @@ -8,10 +8,11 @@ import type { BrsFile } from '../../files/BrsFile'; import type { XmlFile } from '../../files/XmlFile'; import type { BscFile, BsDiagnostic, OnGetCodeActionsEvent } from '../../interfaces'; import { ParseMode } from '../../parser/Parser'; +import type { Expression } from '../../parser/AstNode'; import { util } from '../../util'; -import { isBrsFile, isFunctionExpression, isMethodStatement, isXmlFile } from '../../astUtils/reflection'; -import type { FunctionExpression } from '../../parser/Expression'; -import type { MethodStatement } from '../../parser/Statement'; +import { isBrsFile, isCallExpression, isDottedGetExpression, isFunctionExpression, isMethodStatement, isNamedArgumentExpression, isNamespaceStatement, isNewExpression, isVariableExpression, isXmlFile } from '../../astUtils/reflection'; +import type { CallExpression, FunctionExpression } from '../../parser/Expression'; +import type { MethodStatement, NamespaceStatement } from '../../parser/Statement'; import { WalkMode } from '../../astUtils/visitors'; import { TokenKind } from '../../lexer/TokenKind'; import { getMissingExtendsInsertPosition } from './codeActionHelpers'; @@ -53,6 +54,8 @@ export class CodeActionsProcessor { this.suggestMissingOverrideQuickFixes([diagnostic]); } else if (diagnostic.code === DiagnosticCodeMap.cannotUseOverrideKeywordOnConstructorFunction) { this.suggestRemoveOverrideFromConstructorQuickFixes([diagnostic]); + } else if (diagnostic.code === DiagnosticCodeMap.namedArgOutOfOrder) { + this.suggestNamedArgOrderQuickFix(diagnostic as DiagnosticMessageType<'namedArgOutOfOrder'>); } } @@ -482,6 +485,114 @@ export class CodeActionsProcessor { } } + /** + * Suggests reordering named arguments to declaration order. + */ + private suggestNamedArgOrderQuickFix(diagnostic: DiagnosticMessageType<'namedArgOutOfOrder'>) { + if (!isBrsFile(this.event.file)) { + return; + } + const closestExpression = this.event.file.getClosestExpression(diagnostic.range.start); + const callExpression = ( + isCallExpression(closestExpression) + ? closestExpression + : closestExpression?.findAncestor(isCallExpression) + ); + if (!callExpression || callExpression.args.length < 2) { + return; + } + + const parameters = this.getCallExpressionParameters(callExpression); + if (!parameters) { + return; + } + + const argsWithParamIndex = [] as Array<{ arg: Expression; paramIndex: number }>; + let expectedParamIndex = 0; + let hasNamedArg = false; + + for (const arg of callExpression.args) { + if (!isNamedArgumentExpression(arg)) { + // Quick-fix can only reorder arguments; it cannot repair invalid syntax/shape such as + // positional args after named args or positional args beyond parameter count. + if (hasNamedArg || expectedParamIndex >= parameters.length) { + return; + } + argsWithParamIndex.push({ arg: arg, paramIndex: expectedParamIndex }); + expectedParamIndex++; + continue; + } + + hasNamedArg = true; + const namedArg = arg; + const paramIndex = parameters.findIndex(p => p.name.text.toLowerCase() === namedArg.name.text.toLowerCase()); + // Unknown names and duplicate parameter targets are invalid; do not offer a reorder fix. + if (paramIndex < 0 || argsWithParamIndex.some(x => x.paramIndex === paramIndex)) { + return; + } + argsWithParamIndex.push({ arg: namedArg, paramIndex: paramIndex }); + expectedParamIndex = paramIndex + 1; + } + + const sortedArgs = [...argsWithParamIndex].sort((a, b) => a.paramIndex - b.paramIndex); + const sortedArgText = sortedArgs.map(x => util.getTextForRange(this.event.file.fileContents, x.arg.range)).join(', '); + const firstArg = callExpression.args[0]; + const lastArg = callExpression.args[callExpression.args.length - 1]; + if (!firstArg?.range || !lastArg?.range) { + return; + } + + this.event.codeActions.push( + codeActionUtil.createCodeAction({ + title: 'Reorder named arguments to match function declaration', + diagnostics: [diagnostic], + kind: CodeActionKind.QuickFix, + changes: [{ + type: 'replace', + filePath: this.event.file.srcPath, + range: util.createRangeFromPositions(firstArg.range.start, lastArg.range.end), + newText: sortedArgText + }] + }) + ); + } + + private getCallExpressionParameters(callExpression: CallExpression) { + const scope = this.event.program.getFirstScopeForFile(this.event.file); + if (!scope) { + return undefined; + } + if (isNewExpression(callExpression.parent)) { + const newExpression = callExpression.parent; + const className = newExpression.className.getName(ParseMode.BrighterScript); + const containingNamespace = newExpression.findAncestor(isNamespaceStatement)?.getName(ParseMode.BrighterScript)?.toLowerCase(); + const classLink = scope.getClassFileLink(className, containingNamespace); + if (!classLink) { + return undefined; + } + return util.getClassConstructorParams(classLink.item, scope, containingNamespace); + } + if (isVariableExpression(callExpression.callee)) { + return this.getCallableParametersByName(callExpression.callee.name.text.toLowerCase()); + } + if (isDottedGetExpression(callExpression.callee)) { + const brsName = util.resolveNamespaceCallableName(callExpression.callee, scope); + if (!brsName) { + return undefined; + } + return this.getCallableParametersByName(brsName.toLowerCase()); + } + } + + private getCallableParametersByName(lowerName: string) { + const scope = this.event.program.getFirstScopeForFile(this.event.file); + if (!scope) { + return undefined; + } + const callable = scope.getCallableByName(lowerName); + return callable?.functionStatement?.func?.parameters; + } + /** * Adds code actions to insert a missing `extends` attribute on an XML component tag. * Offers Group, Task, and ContentNode as common choices. diff --git a/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts b/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts index 503f3cf81..6f1d8e4c3 100644 --- a/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts +++ b/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts @@ -1,12 +1,12 @@ -import { createAssignmentStatement, createBlock, createDottedSetStatement, createIfStatement, createIndexedSetStatement, createToken } from '../../astUtils/creators'; -import { isAssignmentStatement, isBinaryExpression, isBlock, isBody, isBrsFile, isDottedGetExpression, isDottedSetStatement, isGroupingExpression, isIndexedGetExpression, isIndexedSetStatement, isLiteralExpression, isNamespaceStatement, isUnaryExpression, isVariableExpression } from '../../astUtils/reflection'; +import { createAssignmentStatement, createBlock, createDottedSetStatement, createIfStatement, createIndexedSetStatement, createInvalidLiteral, createToken } from '../../astUtils/creators'; +import { isAssignmentStatement, isBinaryExpression, isBlock, isBody, isBrsFile, isDottedGetExpression, isDottedSetStatement, isFunctionExpression, isGroupingExpression, isIndexedGetExpression, isIndexedSetStatement, isLiteralExpression, isNamedArgumentExpression, isNamespaceStatement, isUnaryExpression, isVariableExpression } from '../../astUtils/reflection'; import { createVisitor, WalkMode } from '../../astUtils/visitors'; import type { BrsFile } from '../../files/BrsFile'; import type { BeforeFileTranspileEvent } from '../../interfaces'; import type { Token } from '../../lexer/Token'; import { TokenKind } from '../../lexer/TokenKind'; import type { Expression, Statement } from '../../parser/AstNode'; -import type { TernaryExpression } from '../../parser/Expression'; +import type { CallExpression, FunctionExpression, FunctionParameterExpression, NamedArgumentExpression, TernaryExpression } from '../../parser/Expression'; import { LiteralExpression } from '../../parser/Expression'; import { ParseMode } from '../../parser/Parser'; import type { ConstStatement, IfStatement, NamespaceStatement } from '../../parser/Statement'; @@ -21,10 +21,108 @@ export class BrsFilePreTranspileProcessor { public process() { if (isBrsFile(this.event.file)) { + this.processNamedArgumentCalls(); this.iterateExpressions(); } } + /** + * Rewrite calls that use named arguments into positional calls. + * Named arguments are expected to already be validated to match function parameter order. + */ + private processNamedArgumentCalls() { + const scope = this.event.program.getFirstScopeForFile(this.event.file); + if (!scope) { + return; + } + const callableContainerMap = util.getCallableContainersByLowerName(scope.getAllCallables()); + + for (const func of this.event.file.parser.references.functionExpressions) { + for (const callExpr of func.callExpressions) { + if (!callExpr.args.some(isNamedArgumentExpression)) { + continue; + } + + let params: FunctionParameterExpression[] | undefined; + + if (isVariableExpression(callExpr.callee)) { + const funcName = callExpr.callee.name.text; + params = callableContainerMap.get(funcName.toLowerCase())?.[0]?.callable?.functionStatement?.func?.parameters; + } else if (isDottedGetExpression(callExpr.callee)) { + // Namespace function call (e.g. MyNs.myFunc(a: 1)) + const brsName = util.resolveNamespaceCallableName(callExpr.callee, scope); + if (brsName) { + params = callableContainerMap.get(brsName.toLowerCase())?.[0]?.callable?.functionStatement?.func?.parameters; + } + } + + if (!params) { + continue; + } + + this.rewriteNamedArgCall(callExpr, params); + } + + // Constructor CallExpressions are not in func.callExpressions, so handle them separately. + for (const newExpr of this.event.file.parser.references.newExpressions) { + const callExpr = newExpr.call; + if (!callExpr.args.some(isNamedArgumentExpression)) { + continue; + } + // Only process constructor calls that belong to this function + if (newExpr.findAncestor(isFunctionExpression) !== func) { + continue; + } + + const className = newExpr.className.getName(ParseMode.BrighterScript); + const containingNamespace = newExpr.findAncestor(isNamespaceStatement)?.getName(ParseMode.BrighterScript)?.toLowerCase(); + const classLink = scope.getClassFileLink(className, containingNamespace); + if (!classLink) { + continue; + } + + const params = util.getClassConstructorParams(classLink.item, scope, containingNamespace); + this.rewriteNamedArgCall(callExpr, params); + } + } + } + + /** + * Rewrite a single named-argument call expression in place. + * Leaves invalid calls unchanged (diagnostics are emitted during validation). + */ + private rewriteNamedArgCall(callExpr: CallExpression, params: FunctionParameterExpression[]) { + const finalArgs = [] as Expression[]; + let expectedParamIdx = 0; + let seenNamedArg = false; + + for (const arg of callExpr.args) { + if (!isNamedArgumentExpression(arg)) { + if (seenNamedArg || expectedParamIdx >= params.length) { + return; + } + finalArgs.push(arg); + expectedParamIdx++; + continue; + } + + seenNamedArg = true; + const namedArg = arg as NamedArgumentExpression; + const paramIdx = params.findIndex(p => p.name.text.toLowerCase() === namedArg.name.text.toLowerCase()); + if (paramIdx < expectedParamIdx) { + return; + } + + for (let i = expectedParamIdx; i < paramIdx; i++) { + finalArgs.push(params[i]?.defaultValue?.clone() ?? createInvalidLiteral()); + } + finalArgs.push(namedArg.value); + expectedParamIdx = paramIdx + 1; + } + + this.event.editor.arraySplice(callExpr.args, 0, callExpr.args.length, ...finalArgs); + } + private iterateExpressions() { const scope = this.event.program.getFirstScopeForFile(this.event.file); //TODO move away from this loop and use a visitor instead diff --git a/src/bscPlugin/validation/BrsFileValidator.ts b/src/bscPlugin/validation/BrsFileValidator.ts index 9d99ae8fc..325aa37e5 100644 --- a/src/bscPlugin/validation/BrsFileValidator.ts +++ b/src/bscPlugin/validation/BrsFileValidator.ts @@ -140,6 +140,7 @@ export class BrsFileValidator { node.symbolTable.addSymbol('m', undefined, DynamicType.instance); } this.validateFunctionParameterCount(node); + this.validateFunctionParameterNames(node); }, FunctionParameterExpression: (node) => { if (node.name) { @@ -237,6 +238,24 @@ export class BrsFileValidator { } } + private validateFunctionParameterNames(func: FunctionExpression) { + const seen = new Set(); + for (const param of func.parameters) { + const nameLower = param.name?.text?.toLowerCase(); + if (!nameLower) { + continue; + } + if (seen.has(nameLower)) { + this.event.file.addDiagnostic({ + ...DiagnosticMessages.duplicateFunctionParameter(param.name.text), + range: param.name.range + }); + } else { + seen.add(nameLower); + } + } + } + private validateEnumDeclaration(stmt: EnumStatement) { const members = stmt.getMembers(); //the enum data type is based on the first member value diff --git a/src/bscPlugin/validation/ScopeValidator.ts b/src/bscPlugin/validation/ScopeValidator.ts index b5692b3fd..ac13767d7 100644 --- a/src/bscPlugin/validation/ScopeValidator.ts +++ b/src/bscPlugin/validation/ScopeValidator.ts @@ -1,9 +1,9 @@ import { URI } from 'vscode-uri'; -import { isBrsFile, isCallExpression, isDottedGetExpression, isLiteralExpression, isNamespaceStatement, isVariableExpression, isXmlScope } from '../../astUtils/reflection'; +import { isBrsFile, isCallExpression, isDottedGetExpression, isLiteralExpression, isNamespaceStatement, isVariableExpression, isXmlScope, isNamedArgumentExpression } from '../../astUtils/reflection'; import { Cache } from '../../Cache'; import { DiagnosticMessages } from '../../DiagnosticMessages'; import type { BrsFile } from '../../files/BrsFile'; -import type { BscFile, BsDiagnostic, OnScopeValidateEvent } from '../../interfaces'; +import type { BscFile, BsDiagnostic, OnScopeValidateEvent, CallableContainerMap, Callable } from '../../interfaces'; import type { ConstStatement, EnumStatement, NamespaceStatement } from '../../parser/Statement'; import util from '../../util'; import { nodes, components } from '../../roku-types'; @@ -13,7 +13,7 @@ import { TokenKind } from '../../lexer/TokenKind'; import type { Scope } from '../../Scope'; import type { DiagnosticRelatedInformation } from 'vscode-languageserver'; import type { Expression } from '../../parser/AstNode'; -import type { VariableExpression, DottedGetExpression } from '../../parser/Expression'; +import type { FunctionParameterExpression, VariableExpression, DottedGetExpression, NamedArgumentExpression } from '../../parser/Expression'; import { ParseMode } from '../../parser/Parser'; import { createVisitor, WalkMode } from '../../astUtils/visitors'; @@ -54,6 +54,7 @@ export class ScopeValidator { this.iterateFileExpressions(file); this.validateCreateObjectCalls(file); this.validateComputedAAKeys(file); + this.validateNamedArgCalls(file); } }); } @@ -510,6 +511,257 @@ export class ScopeValidator { this.event.scope.addDiagnostics(diagnostics); } + /** + * Validate calls that use named arguments. + * Supports: simple variable calls, namespace function calls, and constructor calls (new MyClass()). + */ + private validateNamedArgCalls(file: BrsFile) { + const { scope } = this.event; + const callableContainerMap: CallableContainerMap = util.getCallableContainersByLowerName(scope.getAllCallables()); + + // Validate regular and namespace function calls + for (const func of file.parser.references.functionExpressions) { + for (const callExpr of func.callExpressions) { + if (!callExpr.args.some(isNamedArgumentExpression)) { + continue; + } + + let funcName: string; + let params: FunctionParameterExpression[]; + + if (isVariableExpression(callExpr.callee)) { + funcName = callExpr.callee.name.text; + const callable = callableContainerMap.get(funcName.toLowerCase())?.[0]?.callable; + if (!callable) { + this.addDiagnostic({ + ...DiagnosticMessages.namedArgsNotAllowedForUnknownFunction(funcName), + range: callExpr.callee.range, + file: file + }); + continue; + } + params = callable.functionStatement.func.parameters; + this.checkNamedArgCrossScopeConflict(funcName, callable, params, callExpr, file, scope); + } else if (isDottedGetExpression(callExpr.callee)) { + // Namespace function call (e.g. MyNs.myFunc(a: 1)) + const brsName = util.resolveNamespaceCallableName(callExpr.callee, scope); + if (!brsName) { + this.addDiagnostic({ + ...DiagnosticMessages.namedArgsNotAllowedForUnknownFunction((callExpr.callee as DottedGetExpression).name.text), + range: callExpr.callee.range, + file: file + }); + continue; + } + const callable = callableContainerMap.get(brsName.toLowerCase())?.[0]?.callable; + if (!callable) { + const displayName = brsName.replace(/_/g, '.'); + this.addDiagnostic({ + ...DiagnosticMessages.namedArgsNotAllowedForUnknownFunction(displayName), + range: callExpr.callee.range, + file: file + }); + continue; + } + funcName = brsName; + params = callable.functionStatement.func.parameters; + } else { + this.addDiagnostic({ + ...DiagnosticMessages.namedArgsNotAllowedForUnknownFunction((callExpr.callee as any)?.name?.text ?? '?'), + range: callExpr.openingParen.range, + file: file + }); + continue; + } + + this.validateNamedArgCallArgs(callExpr, params, funcName, file); + } + } + + // Validate constructor calls with named args (new MyClass(a: 1)) + for (const newExpr of file.parser.references.newExpressions) { + const callExpr = newExpr.call; + if (!callExpr.args.some(isNamedArgumentExpression)) { + continue; + } + + const className = newExpr.className.getName(ParseMode.BrighterScript); + const containingNamespace = callExpr.findAncestor(isNamespaceStatement)?.getName(ParseMode.BrighterScript)?.toLowerCase(); + const classLink = scope.getClassFileLink(className, containingNamespace); + if (!classLink) { + this.addDiagnostic({ + ...DiagnosticMessages.namedArgsNotAllowedForUnknownFunction(className), + range: callExpr.callee.range, + file: file + }); + continue; + } + + const params = util.getClassConstructorParams(classLink.item, scope, containingNamespace); + this.validateNamedArgCallArgs(callExpr, params, className, file); + } + } + + /** + * Validate the named argument list for a single call expression against the resolved parameter list. + */ + private validateNamedArgCallArgs(callExpr: { args: Expression[]; callee: Expression; openingParen?: { range: any } }, params: FunctionParameterExpression[], funcName: string, file: BrsFile) { + const assignedParams = new Set(); + const minParams = params.filter(p => !p.defaultValue).length; + const maxParams = params.length; + const hasTooManyArgs = callExpr.args.length > maxParams; + let seenNamedArg = false; + let expectedParamIdx = 0; + let hasError = false; + + for (const arg of callExpr.args) { + if (!isNamedArgumentExpression(arg)) { + if (seenNamedArg) { + this.addDiagnostic({ + ...DiagnosticMessages.positionalArgAfterNamedArg(), + range: arg.range, + file: file + }); + hasError = true; + continue; + } + if (expectedParamIdx < params.length) { + assignedParams.add(expectedParamIdx); + } + expectedParamIdx++; + } else { + seenNamedArg = true; + const namedArg = arg as NamedArgumentExpression; + const paramName = namedArg.name.text.toLowerCase(); + const paramIdx = params.findIndex(p => p.name.text.toLowerCase() === paramName); + + if (paramIdx < 0) { + this.addDiagnostic({ + ...DiagnosticMessages.unknownNamedArgument(namedArg.name.text, funcName), + range: namedArg.name.range, + file: file + }); + hasError = true; + continue; + } + + if (assignedParams.has(paramIdx)) { + this.addDiagnostic({ + ...DiagnosticMessages.namedArgDuplicate(namedArg.name.text), + range: namedArg.name.range, + file: file + }); + hasError = true; + continue; + } + + if (paramIdx < expectedParamIdx) { + this.addDiagnostic({ + ...DiagnosticMessages.namedArgOutOfOrder(namedArg.name.text), + range: namedArg.name.range, + file: file + }); + hasError = true; + continue; + } + + assignedParams.add(paramIdx); + expectedParamIdx = paramIdx + 1; + } + } + + if (hasError) { + return; + } + + if (hasTooManyArgs) { + this.addDiagnostic({ + ...DiagnosticMessages.mismatchArgumentCount( + minParams === maxParams ? maxParams : `${minParams}-${maxParams}`, + callExpr.args.length + ), + range: callExpr.callee.range, + file: file + }); + return; + } + + // Validate all required params are provided + for (let i = 0; i < params.length; i++) { + if (!assignedParams.has(i) && !params[i].defaultValue) { + this.addDiagnostic({ + ...DiagnosticMessages.mismatchArgumentCount( + minParams === maxParams ? maxParams : `${minParams}-${maxParams}`, + callExpr.args.length + ), + range: callExpr.callee.range, + file: file + }); + break; + } + } + } + + /** + * Emit a diagnostic if the same function name resolves to different parameter signatures + * in other component scopes that share this file. Named arg transpilation is per-file + * (using the first scope), so an ambiguous signature would produce incorrect output. + */ + private checkNamedArgCrossScopeConflict(funcName: string, callable: Callable, params: FunctionParameterExpression[], callExpr: { callee: Expression }, file: BrsFile, scope: Scope) { + for (const otherScope of this.event.program.getScopesForFile(file)) { + if (otherScope === scope) { + continue; + } + const otherCallable = otherScope.getCallableByName(funcName); + if (!otherCallable) { + continue; + } + // Guard against namespace functions that share a short name with our target + if (otherCallable.getName(ParseMode.BrightScript).toLowerCase() !== funcName.toLowerCase()) { + continue; + } + const otherParams = otherCallable.functionStatement?.func?.parameters; + if (!otherParams) { + continue; + } + if (!this.haveSameParamSignature(params, otherParams)) { + const relatedInformation: DiagnosticRelatedInformation[] = [ + { + message: `'${funcName}' defined in scope '${scope.name}'`, + location: util.createLocation( + URI.file(callable.file.srcPath).toString(), + callable.nameRange ?? callable.range + ) + }, + { + message: `'${funcName}' defined in scope '${otherScope.name}'`, + location: util.createLocation( + URI.file(otherCallable.file.srcPath).toString(), + otherCallable.nameRange ?? otherCallable.range + ) + } + ]; + this.addDiagnosticOnce({ + ...DiagnosticMessages.namedArgsCrossScopeConflict(funcName), + range: callExpr.callee.range, + file: file, + relatedInformation: relatedInformation + }); + break; + } + } + } + + /** + * Returns true if both parameter lists have the same names in the same order. + */ + private haveSameParamSignature(p1: FunctionParameterExpression[], p2: FunctionParameterExpression[]): boolean { + if (p1.length !== p2.length) { + return false; + } + return p1.every((p, i) => p.name.text.toLowerCase() === p2[i].name.text.toLowerCase()); + } + /** * Adds a diagnostic to the first scope for this key. Prevents duplicate diagnostics * for diagnostics where scope isn't important. (i.e. CreateObject validations) diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 89d86f1a1..2092dbb82 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -5038,6 +5038,51 @@ describe('BrsFile', () => { }]); }); + it('warns on duplicate parameter names in a function', () => { + program.setFile('source/main.bs', ` + function test(a, b, a) + end function + `); + program.validate(); + expectDiagnostics(program, [{ + ...DiagnosticMessages.duplicateFunctionParameter('a'), + range: util.createRange(1, 32, 1, 33) + }]); + }); + + it('warns on duplicate parameter names in a sub', () => { + program.setFile('source/main.bs', ` + sub test(name as string, name as string) + end sub + `); + program.validate(); + expectDiagnostics(program, [{ + ...DiagnosticMessages.duplicateFunctionParameter('name'), + range: util.createRange(1, 37, 1, 41) + }]); + }); + + it('does not warn when all parameter names are unique', () => { + program.setFile('source/main.bs', ` + function test(a, b, c) + end function + `); + program.validate(); + expectZeroDiagnostics(program); + }); + + it('duplicate parameter check is case-insensitive', () => { + program.setFile('source/main.bs', ` + function test(myParam, MyParam) + end function + `); + program.validate(); + expectDiagnostics(program, [{ + ...DiagnosticMessages.duplicateFunctionParameter('MyParam'), + range: util.createRange(1, 35, 1, 42) + }]); + }); + describe('getClosestExpression', () => { it('returns undefined for missing Position', () => { const file = program.setFile('source/main.bs', ` diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index 139f81cd4..0002d34e1 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -25,7 +25,7 @@ import { standardizePath as s, util } from '../util'; import { BrsTranspileState } from '../parser/BrsTranspileState'; import { Preprocessor } from '../preprocessor/Preprocessor'; import { serializeError } from 'serialize-error'; -import { isCallExpression, isMethodStatement, isClassStatement, isDottedGetExpression, isFunctionExpression, isFunctionStatement, isFunctionType, isLiteralExpression, isNamespaceStatement, isStringType, isVariableExpression, isImportStatement, isFieldStatement, isEnumStatement, isConstStatement } from '../astUtils/reflection'; +import { isCallExpression, isMethodStatement, isClassStatement, isDottedGetExpression, isFunctionExpression, isFunctionStatement, isFunctionType, isLiteralExpression, isNamespaceStatement, isStringType, isVariableExpression, isImportStatement, isFieldStatement, isEnumStatement, isConstStatement, isNamedArgumentExpression } from '../astUtils/reflection'; import type { BscType } from '../types/BscType'; import { createVisitor, WalkMode } from '../astUtils/visitors'; import type { DependencyGraph } from '../DependencyGraph'; @@ -742,33 +742,35 @@ export class BrsFile { let args = [] as CallableArg[]; //TODO convert if stmts to use instanceof instead for (let arg of expression.args as any) { + // For named arguments, delegate to the inner value expression for type analysis + const innerArg = isNamedArgumentExpression(arg) ? arg.value : arg; //is a literal parameter value - if (isLiteralExpression(arg)) { + if (isLiteralExpression(innerArg)) { args.push({ range: arg.range, - type: arg.type, - text: arg.token.text, + type: innerArg.type, + text: innerArg.token.text, expression: arg, typeToken: undefined }); //is variable being passed into argument - } else if (arg.name) { + } else if (innerArg.name) { args.push({ range: arg.range, //TODO - look up the data type of the actual variable type: new DynamicType(), - text: arg.name.text, + text: innerArg.name.text, expression: arg, typeToken: undefined }); - } else if (arg.value) { + } else if (innerArg.value) { let text = ''; /* istanbul ignore next: TODO figure out why value is undefined sometimes */ - if (arg.value.value) { - text = arg.value.value.toString(); + if (innerArg.value.value) { + text = innerArg.value.value.toString(); } let callableArg = { range: arg.range, diff --git a/src/parser/Expression.ts b/src/parser/Expression.ts index 344836697..4d2b9884e 100644 --- a/src/parser/Expression.ts +++ b/src/parser/Expression.ts @@ -146,6 +146,43 @@ export class CallExpression extends Expression { } } +export class NamedArgumentExpression extends Expression { + constructor( + readonly name: Identifier, + readonly colon: Token, + readonly value: Expression + ) { + super(); + this.range = util.createBoundingRange(this.name, this.colon, this.value); + } + + public readonly range: Range | undefined; + + transpile(state: BrsTranspileState) { + // Named arguments are resolved to positional order during validation. + // If reached during transpile (e.g. for an unresolvable call), emit the value only. + return this.value.transpile(state); + } + + walk(visitor: WalkVisitor, options: WalkOptions) { + // eslint-disable-next-line no-bitwise + if (options.walkMode & InternalWalkMode.walkExpressions) { + walk(this, 'value', visitor, options); + } + } + + public clone() { + return this.finalizeClone( + new NamedArgumentExpression( + util.cloneToken(this.name), + util.cloneToken(this.colon), + this.value?.clone() + ), + ['value'] + ); + } +} + export class FunctionExpression extends Expression implements TypedefProvider { constructor( readonly parameters: FunctionParameterExpression[], diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts index 368c33668..d40b613df 100644 --- a/src/parser/Parser.ts +++ b/src/parser/Parser.ts @@ -90,7 +90,8 @@ import { TypeCastExpression, UnaryExpression, VariableExpression, - XmlAttributeGetExpression + XmlAttributeGetExpression, + NamedArgumentExpression } from './Expression'; import type { Diagnostic, Range } from 'vscode-languageserver'; import type { Logger } from '../logging'; @@ -2807,7 +2808,20 @@ export class Parser { throw this.lastDiagnosticAsError(); } try { - args.push(this.expression()); + // In BrighterScript mode, detect named argument syntax: `paramName: value` + if ( + this.options.mode === ParseMode.BrighterScript && + this.checkAny(TokenKind.Identifier, ...this.allowedLocalIdentifiers) && + this.checkNext(TokenKind.Colon) + ) { + const name = this.advance() as Identifier; + name.kind = TokenKind.Identifier; + const colon = this.advance(); + const value = this.expression(); + args.push(new NamedArgumentExpression(name, colon, value)); + } else { + args.push(this.expression()); + } } catch (error) { this.rethrowNonDiagnosticError(error); // we were unable to get an expression, so don't continue diff --git a/src/parser/tests/expression/NamedArgument.spec.ts b/src/parser/tests/expression/NamedArgument.spec.ts new file mode 100644 index 000000000..957a33ab8 --- /dev/null +++ b/src/parser/tests/expression/NamedArgument.spec.ts @@ -0,0 +1,479 @@ +import { expect } from '../../../chai-config.spec'; +import { Program } from '../../../Program'; +import { ParseMode, Parser } from '../../Parser'; +import { DiagnosticMessages } from '../../../DiagnosticMessages'; +import { expectDiagnostics, expectDiagnosticsIncludes, expectZeroDiagnostics, getTestTranspile, rootDir, trim } from '../../../testHelpers.spec'; +import { isNamedArgumentExpression } from '../../../astUtils/reflection'; +import type { ExpressionStatement } from '../../Statement'; +import type { CallExpression } from '../../Expression'; + +describe('named argument expressions', () => { + + describe('parsing', () => { + it('parses a single named argument', () => { + const { statements, diagnostics } = Parser.parse( + 'myFunc(paramOne: true)', + { mode: ParseMode.BrighterScript } + ); + expectZeroDiagnostics({ diagnostics: diagnostics }); + const call = (statements[0] as ExpressionStatement).expression as CallExpression; + expect(call.args).to.be.lengthOf(1); + expect(isNamedArgumentExpression(call.args[0])).to.be.true; + expect((call.args[0] as any).name.text).to.equal('paramOne'); + }); + + it('parses mixed positional and named arguments', () => { + const { statements, diagnostics } = Parser.parse( + 'myFunc(first, paramThree: true)', + { mode: ParseMode.BrighterScript } + ); + expectZeroDiagnostics({ diagnostics: diagnostics }); + const call = (statements[0] as ExpressionStatement).expression as CallExpression; + expect(call.args).to.be.lengthOf(2); + expect(isNamedArgumentExpression(call.args[0])).to.be.false; + expect(isNamedArgumentExpression(call.args[1])).to.be.true; + }); + + it('parses multiple named arguments', () => { + const { statements, diagnostics } = Parser.parse( + 'myFunc(paramTwo: true, paramOne: false)', + { mode: ParseMode.BrighterScript } + ); + expectZeroDiagnostics({ diagnostics: diagnostics }); + const call = (statements[0] as ExpressionStatement).expression as CallExpression; + expect(call.args).to.be.lengthOf(2); + expect(isNamedArgumentExpression(call.args[0])).to.be.true; + expect(isNamedArgumentExpression(call.args[1])).to.be.true; + }); + + it('does not treat identifier:value as named arg in BrightScript mode', () => { + const { diagnostics } = Parser.parse( + 'myFunc(paramOne: true)', + { mode: ParseMode.BrightScript } + ); + // In BrightScript mode the colon is an unexpected token + expect(diagnostics.length).to.be.greaterThan(0); + }); + + it('accepts complex expressions as named arg values', () => { + const { statements, diagnostics } = Parser.parse( + 'myFunc(count: 1 + 2, name: "hello")', + { mode: ParseMode.BrighterScript } + ); + expectZeroDiagnostics({ diagnostics: diagnostics }); + const call = (statements[0] as ExpressionStatement).expression as CallExpression; + expect(call.args).to.be.lengthOf(2); + }); + }); + + describe('validation', () => { + let program: Program; + + beforeEach(() => { + program = new Program({ rootDir: rootDir }); + }); + + afterEach(() => { + program.dispose(); + }); + + it('validates a correctly named argument call', () => { + program.setFile('source/main.bs', ` + sub greet(name as string, excited as boolean) + end sub + sub main() + greet(name: "Bob", excited: true) + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + }); + + it('gives diagnostic for unknown named argument', () => { + program.setFile('source/main.bs', ` + sub greet(name as string) + end sub + sub main() + greet(nope: "Bob") + end sub + `); + program.validate(); + expectDiagnostics(program, [ + DiagnosticMessages.unknownNamedArgument('nope', 'greet') + ]); + }); + + it('gives diagnostic for duplicate named argument', () => { + program.setFile('source/main.bs', ` + sub greet(name as string) + end sub + sub main() + greet(name: "Bob", name: "Alice") + end sub + `); + program.validate(); + expectDiagnostics(program, [ + DiagnosticMessages.namedArgDuplicate('name') + ]); + }); + + it('gives diagnostic for positional arg after named arg', () => { + program.setFile('source/main.bs', ` + sub greet(name as string, excited as boolean) + end sub + sub main() + greet(name: "Bob", true) + end sub + `); + program.validate(); + expectDiagnostics(program, [ + DiagnosticMessages.positionalArgAfterNamedArg() + ]); + }); + + it('gives diagnostic when named args are out of declaration order', () => { + program.setFile('source/main.bs', ` + sub greet(name as string, excited as boolean) + end sub + sub main() + greet(excited: true, name: "Bob") + end sub + `); + program.validate(); + expectDiagnostics(program, [ + DiagnosticMessages.namedArgOutOfOrder('name') + ]); + }); + + it('gives diagnostic for named arg to unknown function', () => { + program.setFile('source/main.bs', ` + sub main() + unknownFunc(param: true) + end sub + `); + program.validate(); + expectDiagnosticsIncludes(program, [ + DiagnosticMessages.namedArgsNotAllowedForUnknownFunction('unknownFunc') + ]); + }); + + it('gives diagnostic for missing required parameter', () => { + program.setFile('source/main.bs', ` + sub greet(name as string, excited as boolean) + end sub + sub main() + greet(excited: true) + end sub + `); + program.validate(); + expectDiagnostics(program, [ + DiagnosticMessages.mismatchArgumentCount('2', 1) + ]); + }); + + it('allows skipping middle optional params', () => { + program.setFile('source/main.bs', ` + sub greet(name as string, title = "Mr", excited = false) + end sub + sub main() + greet(name: "Bob", excited: true) + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + }); + + describe('namespace calls', () => { + it('validates a correctly named argument call to a namespace function', () => { + program.setFile('source/main.bs', ` + namespace MyNs + sub greet(name as string, excited as boolean) + end sub + end namespace + sub main() + MyNs.greet(name: "Bob", excited: true) + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + }); + + it('gives diagnostic for unknown named argument on a namespace function', () => { + program.setFile('source/main.bs', ` + namespace MyNs + sub greet(name as string) + end sub + end namespace + sub main() + MyNs.greet(nope: "Bob") + end sub + `); + program.validate(); + expectDiagnostics(program, [ + DiagnosticMessages.unknownNamedArgument('nope', 'MyNs_greet') + ]); + }); + + it('gives diagnostic for named args on a non-namespace dotted call', () => { + program.setFile('source/main.bs', ` + sub main() + obj = {} + obj.method(name: "Bob") + end sub + `); + program.validate(); + expectDiagnosticsIncludes(program, [ + DiagnosticMessages.namedArgsNotAllowedForUnknownFunction('method') + ]); + }); + }); + + describe('constructor calls', () => { + it('validates correctly named argument call to a class constructor', () => { + program.setFile('source/main.bs', ` + class Point + x as integer + y as integer + function new(x as integer, y as integer) + m.x = x + m.y = y + end function + end class + sub main() + p = new Point(x: 1, y: 2) + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + }); + + it('gives diagnostic for unknown named argument on a constructor', () => { + program.setFile('source/main.bs', ` + class Point + x as integer + y as integer + function new(x as integer, y as integer) + end function + end class + sub main() + p = new Point(z: 99) + end sub + `); + program.validate(); + expectDiagnosticsIncludes(program, [ + DiagnosticMessages.unknownNamedArgument('z', 'Point') + ]); + }); + + it('gives diagnostic for missing required constructor parameter', () => { + program.setFile('source/main.bs', ` + class Point + x as integer + y as integer + function new(x as integer, y as integer) + end function + end class + sub main() + p = new Point(x: 1) + end sub + `); + program.validate(); + expectDiagnosticsIncludes(program, [ + DiagnosticMessages.mismatchArgumentCount('2', 1) + ]); + }); + + it('uses inherited constructor params when subclass has no constructor', () => { + program.setFile('source/main.bs', ` + class Shape + color as string + function new(color as string) + m.color = color + end function + end class + class Circle extends Shape + end class + sub main() + c = new Circle(color: "red") + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + }); + }); + + describe('cross-scope conflict', () => { + it('gives diagnostic when same function name has different parameter signatures across component scopes', () => { + // shared.bs calls foo() with named args; each component imports a different + // definition of foo() with the same param names but in different order. + program.setFile('components/shared.bs', ` + sub callFoo() + foo(a: 2, b: 1) + end sub + `); + program.setFile('components/helperA.bs', ` + sub foo(a, b) + end sub + `); + program.setFile('components/helperB.bs', ` + sub foo(b, a) + end sub + `); + program.setFile('components/CompA.xml', trim` + + +