From 062faf795204da2e8ff856e91900eec165ab9988 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Mon, 25 Nov 2024 16:11:16 -0500 Subject: [PATCH] Fixes missing prefixes in .bs and d.bs files for interfaces, enums, and constants (#80) * Add prefixing support for enums/consts/interfaces. * brighterscript@0.68.0 * Fix prefixing of enums/classes/interfaces in fields * Refactor prefixable declarations to unify handling of classes, enums, consts, and interfaces --- package-lock.json | 14 +-- package.json | 2 +- src/prefixer/File.ts | 168 ++++++++++++++++++++++------- src/prefixer/ModuleManager.spec.ts | 141 +++++++++++++++++++++++- src/prefixer/RopmModule.ts | 8 +- 5 files changed, 281 insertions(+), 52 deletions(-) diff --git a/package-lock.json b/package-lock.json index 765b84f..e45ec89 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "dependencies": { "@xml-tools/ast": "^5.0.5", "@xml-tools/parser": "1.0.10", - "brighterscript": "^0.67.8", + "brighterscript": "^0.68.0", "del": "6.0.0", "fs-extra": "9.1.0", "glob-all": "3.2.1", @@ -1727,9 +1727,9 @@ } }, "node_modules/brighterscript": { - "version": "0.67.8", - "resolved": "https://registry.npmjs.org/brighterscript/-/brighterscript-0.67.8.tgz", - "integrity": "sha512-2pQcPzW/NCEEofojLuHRDcLuQTmHssNpj1xW2oRiSFMo0YHR5cBIHrG/I71lpJwpyw/HzzK0vcEbthlOW4QVuw==", + "version": "0.68.0", + "resolved": "https://registry.npmjs.org/brighterscript/-/brighterscript-0.68.0.tgz", + "integrity": "sha512-UKxzMHJo9QhOmcIrfjB0ICjWoK3b6PGJ4AnKPCg8GYF1FR6IW+03Q8MinTR0iTjlC0DbiXTvc9GS+1tUTAOi4Q==", "dependencies": { "@rokucommunity/bslib": "^0.1.1", "@rokucommunity/logger": "^0.3.9", @@ -9409,9 +9409,9 @@ } }, "brighterscript": { - "version": "0.67.8", - "resolved": "https://registry.npmjs.org/brighterscript/-/brighterscript-0.67.8.tgz", - "integrity": "sha512-2pQcPzW/NCEEofojLuHRDcLuQTmHssNpj1xW2oRiSFMo0YHR5cBIHrG/I71lpJwpyw/HzzK0vcEbthlOW4QVuw==", + "version": "0.68.0", + "resolved": "https://registry.npmjs.org/brighterscript/-/brighterscript-0.68.0.tgz", + "integrity": "sha512-UKxzMHJo9QhOmcIrfjB0ICjWoK3b6PGJ4AnKPCg8GYF1FR6IW+03Q8MinTR0iTjlC0DbiXTvc9GS+1tUTAOi4Q==", "requires": { "@rokucommunity/bslib": "^0.1.1", "@rokucommunity/logger": "^0.3.9", diff --git a/package.json b/package.json index e39d4c1..a2dd12b 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "dependencies": { "@xml-tools/ast": "^5.0.5", "@xml-tools/parser": "1.0.10", - "brighterscript": "^0.67.8", + "brighterscript": "^0.68.0", "del": "6.0.0", "fs-extra": "9.1.0", "glob-all": "3.2.1", diff --git a/src/prefixer/File.ts b/src/prefixer/File.ts index b664a28..5d65f54 100644 --- a/src/prefixer/File.ts +++ b/src/prefixer/File.ts @@ -5,8 +5,15 @@ import { buildAst } from '@xml-tools/ast'; import type { RopmOptions } from '../util'; import { util } from '../util'; import * as path from 'path'; -import type { BrsFile, Position, Program, Range, XmlFile } from 'brighterscript'; -import { ParseMode, createVisitor, isCallExpression, isCustomType, isDottedGetExpression, isDottedSetStatement, isIndexedGetExpression, isIndexedSetStatement, WalkMode, util as bsUtil } from 'brighterscript'; +import type { BrsFile, Position, Program, Range, XmlFile, NamespaceStatement } from 'brighterscript'; +import { ParseMode, createVisitor, isCallExpression, isCustomType, isDottedGetExpression, isDottedSetStatement, isIndexedGetExpression, isIndexedSetStatement, WalkMode, util as bsUtil, isNamespaceStatement, DeclarableTypes } from 'brighterscript'; + +/** + * List of all declaralbe types in BrighterScript (i.e. the native types), stored in lower case for reference + */ +const nativeTypes = new Set( + DeclarableTypes.map(x => x.toLowerCase()) +); export class File { constructor( @@ -79,7 +86,7 @@ export class File { endOffset: number; }>; - public classDeclarations = [] as Array<{ + public prefixableDeclarations = [] as Array<{ name: string; nameOffset: number; hasNamespace: boolean; @@ -94,9 +101,9 @@ export class File { }>; /** - * Anywhere that a class is used as a type (like in class `extends` or function parameters) + * Anywhere that a prefixable reference (i.e. class, enum, etc...) is used as a type (like in class `extends` or function parameters) */ - public classReferences = [] as Array<{ + public prefixableReferences = [] as Array<{ fullyQualifiedName: string; offsetBegin: number; offsetEnd: number; @@ -241,24 +248,54 @@ export class File { this.findComponentInterfaceFunctions(); this.findComponentFieldOnChangeFunctions(); } - } - private addClassRef(className: string, containingNamespace: string | undefined, range: Range) { - //look up the class. If we can find it, use it - const cls = (this.bscFile as BrsFile).getClassFileLink(className, containingNamespace)?.item; + private tryAddPrefixableRef(options: { + name: string | undefined; + containingNamespace: string | undefined; + range: Range | undefined; + }) { + //skip if we don't have a name or a range + if (!options.name || !options.range) { + return; + } - let fullyQualifiedName: string; - if (cls) { - fullyQualifiedName = bsUtil.getFullyQualifiedClassName(cls.getName(ParseMode.BrighterScript), cls.namespaceName?.getName(ParseMode.BrighterScript)); - } else { - fullyQualifiedName = bsUtil.getFullyQualifiedClassName(className, containingNamespace); + //skip if this is a native type, don't prefix it + if (options.name.trim().length > 0 && nativeTypes.has(options.name.toLowerCase())) { + return; } - this.classReferences.push({ - fullyQualifiedName: fullyQualifiedName, - offsetBegin: this.positionToOffset(range.start), - offsetEnd: this.positionToOffset(range.end) + const lowerName = options.name.toLowerCase(); + const lowerContainingNamespace = options.containingNamespace?.toLowerCase(); + + const scopes = this.bscFile.program.getScopesForFile(this.bscFile); + let fullyQualifiedName: string | undefined; + + //find the first item in the first scope that has it + for (let scope of scopes) { + const enumLink = scope.getEnumFileLink(lowerName, lowerContainingNamespace); + if (enumLink) { + fullyQualifiedName = enumLink.item.fullName; + break; + } + + const link = + scope.getClassFileLink(lowerName, lowerContainingNamespace) ?? + scope.getInterfaceFileLink(lowerName, lowerContainingNamespace) ?? + scope.getConstFileLink(lowerName, lowerContainingNamespace); + if (link) { + fullyQualifiedName = bsUtil.getFullyQualifiedClassName( + link.item.getName(ParseMode.BrighterScript), + link.item.namespaceName?.getName(ParseMode.BrighterScript) + ); + break; + } + } + + this.prefixableReferences.push({ + fullyQualifiedName: fullyQualifiedName ?? bsUtil.getFullyQualifiedClassName(options.name, options.containingNamespace), + offsetBegin: this.positionToOffset(options.range.start), + offsetEnd: this.positionToOffset(options.range.end) }); } @@ -306,7 +343,7 @@ export class File { //track class declarations (.bs and .d.bs only) ClassStatement: (cls) => { const annotations = cls.annotations ?? []; - this.classDeclarations.push({ + this.prefixableDeclarations.push({ name: cls.name.text, nameOffset: this.positionToOffset(cls.name.range.start), hasNamespace: !!cls.namespaceName, @@ -317,32 +354,84 @@ export class File { endOffset: this.positionToOffset(cls.end.range.end) }); - if (cls.parentClassName) { - this.addClassRef( - cls.parentClassName.getName(ParseMode.BrighterScript), - cls.namespaceName?.getName(ParseMode.BrighterScript), - cls.parentClassName.range! - ); - } + this.tryAddPrefixableRef({ + name: cls.parentClassName?.getName(ParseMode.BrighterScript), + containingNamespace: cls.namespaceName?.getName(ParseMode.BrighterScript), + range: cls.parentClassName?.range + }); + }, + FieldStatement: (node) => { + this.tryAddPrefixableRef({ + name: node.type?.text, + containingNamespace: node.findAncestor(isNamespaceStatement)?.getName(ParseMode.BrighterScript), + range: node.type?.range + }); + }, + InterfaceFieldStatement: (node) => { + this.tryAddPrefixableRef({ + name: node.tokens.type?.text, + containingNamespace: node.findAncestor(isNamespaceStatement)?.getName(ParseMode.BrighterScript), + range: node.tokens.type?.range + }); + }, + //track enum declarations (.bs and .d.bs only) + EnumStatement: (node) => { + const annotations = node.annotations ?? []; + this.prefixableDeclarations.push({ + name: node.tokens.name.text, + nameOffset: this.positionToOffset(node.tokens.name.range.start), + hasNamespace: !!node.namespaceName, + //Use annotation start position if available, otherwise use class keyword + startOffset: this.positionToOffset( + (annotations?.length > 0 ? annotations[0] : node.tokens.enum).range.start + ), + endOffset: this.positionToOffset(node.tokens.endEnum.range.end) + }); + }, + //track enum declarations (.bs and .d.bs only) + ConstStatement: (node) => { + const annotations = node.annotations ?? []; + this.prefixableDeclarations.push({ + name: node.tokens.name.text, + nameOffset: this.positionToOffset(node.tokens.name.range.start), + hasNamespace: !!node.findAncestor(isNamespaceStatement), + //Use annotation start position if available, otherwise use class keyword + startOffset: this.positionToOffset( + (annotations?.length > 0 ? annotations[0] : node.tokens.const).range.start + ), + endOffset: this.positionToOffset(node.value.range!.end ?? node.tokens.equals.range.end) + }); + }, + //track enum declarations (.bs and .d.bs only) + InterfaceStatement: (node) => { + const annotations = node.annotations ?? []; + this.prefixableDeclarations.push({ + name: node.tokens.name.text, + nameOffset: this.positionToOffset(node.tokens.name.range.start), + hasNamespace: !!node.findAncestor(isNamespaceStatement), + //Use annotation start position if available, otherwise use class keyword + startOffset: this.positionToOffset( + (annotations?.length > 0 ? annotations[0] : node.tokens.interface).range.start + ), + endOffset: this.positionToOffset(node.tokens.endInterface.range.end) + }); }, FunctionExpression: (func) => { const namespaceName = func.namespaceName?.getName(ParseMode.BrighterScript); //any parameters containing custom types for (const param of func.parameters) { - if (isCustomType(param.type)) { - this.addClassRef( - param.type.name, - namespaceName, - param.typeToken!.range - ); - } + this.tryAddPrefixableRef({ + name: param.typeToken?.text, + containingNamespace: namespaceName, + range: param.typeToken?.range + }); } if (isCustomType(func.returnType)) { - this.addClassRef( - func.returnType.name, - namespaceName, - func.returnTypeToken!.range - ); + this.tryAddPrefixableRef({ + name: func.returnTypeToken?.text, + containingNamespace: namespaceName, + range: func.returnTypeToken?.range + }); } }, FunctionStatement: (func) => { @@ -353,7 +442,7 @@ export class File { hasNamespace: !!func.namespaceName, //Use annotation start position if available, otherwise use keyword startOffset: this.positionToOffset( - (annotations?.length > 0 ? annotations[0] : func.func.functionType)!.range.start + (annotations?.length > 0 ? annotations[0] : func.func.functionType)!.range.start as Position ), endOffset: this.positionToOffset(func.func.end.range.end) }); @@ -646,6 +735,7 @@ export class File { }); } } + } export interface Edit { diff --git a/src/prefixer/ModuleManager.spec.ts b/src/prefixer/ModuleManager.spec.ts index 01e41d1..c907afb 100644 --- a/src/prefixer/ModuleManager.spec.ts +++ b/src/prefixer/ModuleManager.spec.ts @@ -1690,7 +1690,7 @@ describe('ModuleManager', () => { }); }); - it('wraps top-level functions and classes with a namespace', async () => { + it('wraps top-level declarations with a namespace', async () => { await testProcess({ 'logger:source/lib.d.bs': [ trim` @@ -1698,6 +1698,13 @@ describe('ModuleManager', () => { end function class Person end class + enum Direction + up = "up" + end enum + const PI = 3.14 + interface Movie + uri as string + end interface `, trim` namespace logger @@ -1708,6 +1715,138 @@ describe('ModuleManager', () => { class Person end class end namespace + namespace logger + enum Direction + up = "up" + end enum + end namespace + namespace logger + const PI = 3.14 + end namespace + namespace logger + interface Movie + uri as string + end interface + end namespace + ` + ] + }); + }); + + it('prefixes enums, classes, and interfaces in function param types', async () => { + await testProcess({ + 'logger:source/lib.d.bs': [ + trim` + function move(direction as Direction, vid as Video, mov as Movie) + end function + enum Direction + up + end enum + interface Video + uri as string + end interface + class Movie + uri as string + end class + `, + trim` + namespace logger + function move(direction as logger.Direction, vid as logger.Video, mov as logger.Movie) + end function + end namespace + namespace logger + enum Direction + up + end enum + end namespace + namespace logger + interface Video + uri as string + end interface + end namespace + namespace logger + class Movie + uri as string + end class + end namespace + ` + ] + }); + }); + + it('prefixes enums, classes, and interfaces in interface members', async () => { + await testProcess({ + 'logger:source/lib.d.bs': [ + trim` + interface Video + whichWay as Direction + thingToPlay as Video + parent as Movie + end interface + enum Direction + up + end enum + class Movie + uri as string + end class + `, + trim` + namespace logger + interface Video + whichWay as logger.Direction + thingToPlay as logger.Video + parent as logger.Movie + end interface + end namespace + namespace logger + enum Direction + up + end enum + end namespace + namespace logger + class Movie + uri as string + end class + end namespace + ` + ] + }); + }); + + it('prefixes enums, classes, and interfaces in class members', async () => { + await testProcess({ + 'logger:source/lib.d.bs': [ + trim` + class Movie + whichWay as Direction + thingToPlay as Video + parent as Movie + end class + enum Direction + up + end enum + interface Video + uri as string + end interface + `, + trim` + namespace logger + class Movie + whichWay as logger.Direction + thingToPlay as logger.Video + parent as logger.Movie + end class + end namespace + namespace logger + enum Direction + up + end enum + end namespace + namespace logger + interface Video + uri as string + end interface + end namespace ` ] }); diff --git a/src/prefixer/RopmModule.ts b/src/prefixer/RopmModule.ts index 200e8ae..d4effbc 100644 --- a/src/prefixer/RopmModule.ts +++ b/src/prefixer/RopmModule.ts @@ -384,16 +384,16 @@ export class RopmModule { } } - //wrap un-namespaced classes with prefix namespace - for (const cls of file.classDeclarations) { + //wrap un-namespaced classes, enums, namespaces, etc... with prefix namespace + for (const cls of file.prefixableDeclarations) { if (!cls.hasNamespace) { file.addEdit(cls.startOffset, cls.startOffset, `namespace ${brighterscriptPrefix}\n`); file.addEdit(cls.endOffset, cls.endOffset, `\nend namespace`); } } - //prefix d.bs class references - for (const ref of file.classReferences) { + //prefix d.bs custom references + for (const ref of file.prefixableReferences) { const baseNamespace = util.getBaseNamespace(ref.fullyQualifiedName); const alias = getAlias(baseNamespace);