From b009d07c2ae34248d1e7beea3c66121b8deef957 Mon Sep 17 00:00:00 2001 From: kaizen3031593 <36202692+kaizen3031593@users.noreply.github.com> Date: Tue, 16 Nov 2021 14:01:23 -0500 Subject: [PATCH 01/10] fix(rosetta): types from submodules not recognized properly (#3174) Rosetta used to store a guessed version of the type fqn when running `rosetta extract`. These guessed fqns are correct for v1 but break down on v2, since they do not properly account for namespaces. This PR correctly determines the fqn of a type by computing the symbolId, loading the relevant assembly, and matching the symbolId with the actual fqn. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- .../@scope/jsii-calc-lib/test/assembly.jsii | 6 +- packages/jsii-calc/test/assembly.jsii | 32 +++---- packages/jsii-rosetta/lib/jsii/assemblies.ts | 93 +++++++++++++++++++ packages/jsii-rosetta/lib/jsii/jsii-utils.ts | 87 +++++++++++++++++ .../lib/languages/record-references.ts | 62 +------------ packages/jsii-rosetta/lib/tablets/key.ts | 2 + packages/jsii-rosetta/package.json | 5 +- .../test/record-references.test.ts | 43 ++++++--- packages/jsii/lib/assembler.ts | 2 +- packages/jsii/lib/index.ts | 1 + packages/jsii/lib/symbol-id.ts | 86 +++++++++++++++++ .../lib/transforms/deprecation-warnings.ts | 2 +- packages/jsii/lib/utils.ts | 85 ----------------- packages/jsii/test/symbol-identifiers.test.ts | 25 ++++- 14 files changed, 350 insertions(+), 181 deletions(-) create mode 100644 packages/jsii/lib/symbol-id.ts diff --git a/packages/@scope/jsii-calc-lib/test/assembly.jsii b/packages/@scope/jsii-calc-lib/test/assembly.jsii index 687f2a5ec6..981dcd29f1 100644 --- a/packages/@scope/jsii-calc-lib/test/assembly.jsii +++ b/packages/@scope/jsii-calc-lib/test/assembly.jsii @@ -811,7 +811,7 @@ } } ], - "symbolId": "lib/submodule/index:NestedClass" + "symbolId": "lib/submodule/index:NestingClass.NestedClass" }, "@scope/jsii-calc-lib.submodule.NestingClass.NestedStruct": { "assembly": "@scope/jsii-calc-lib", @@ -846,7 +846,7 @@ } } ], - "symbolId": "lib/submodule/index:NestedStruct" + "symbolId": "lib/submodule/index:NestingClass.NestedStruct" }, "@scope/jsii-calc-lib.submodule.ReflectableEntry": { "assembly": "@scope/jsii-calc-lib", @@ -948,5 +948,5 @@ } }, "version": "0.0.0", - "fingerprint": "BXEo4aMVmNYUV0dd8hK2zOYLM8iFbpxQbUzGyGagFu8=" + "fingerprint": "LN1bs46m2O4aR6AhHvi6UDh/f90EoUT3n5apMPzr9+c=" } diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index 8b57e4e827..4a90175111 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -4082,7 +4082,7 @@ } } ], - "symbolId": "lib/compliance:Base" + "symbolId": "lib/compliance:DerivedClassHasNoProperties.Base" }, "jsii-calc.DerivedClassHasNoProperties.Derived": { "assembly": "jsii-calc", @@ -4103,7 +4103,7 @@ }, "name": "Derived", "namespace": "DerivedClassHasNoProperties", - "symbolId": "lib/compliance:Derived" + "symbolId": "lib/compliance:DerivedClassHasNoProperties.Derived" }, "jsii-calc.DerivedStruct": { "assembly": "jsii-calc", @@ -7562,7 +7562,7 @@ } } ], - "symbolId": "lib/compliance:Foo" + "symbolId": "lib/compliance:InterfaceInNamespaceIncludesClasses.Foo" }, "jsii-calc.InterfaceInNamespaceIncludesClasses.Hello": { "assembly": "jsii-calc", @@ -7595,7 +7595,7 @@ } } ], - "symbolId": "lib/compliance:Hello" + "symbolId": "lib/compliance:InterfaceInNamespaceIncludesClasses.Hello" }, "jsii-calc.InterfaceInNamespaceOnlyInterface.Hello": { "assembly": "jsii-calc", @@ -7628,7 +7628,7 @@ } } ], - "symbolId": "lib/compliance:Hello" + "symbolId": "lib/compliance:InterfaceInNamespaceOnlyInterface.Hello" }, "jsii-calc.InterfacesMaker": { "assembly": "jsii-calc", @@ -9004,7 +9004,7 @@ } } ], - "symbolId": "lib/compliance:PropBooleanValue" + "symbolId": "lib/compliance:LevelOne.PropBooleanValue" }, "jsii-calc.LevelOne.PropProperty": { "assembly": "jsii-calc", @@ -9037,7 +9037,7 @@ } } ], - "symbolId": "lib/compliance:PropProperty" + "symbolId": "lib/compliance:LevelOne.PropProperty" }, "jsii-calc.LevelOneProps": { "assembly": "jsii-calc", @@ -11141,7 +11141,7 @@ } } ], - "symbolId": "lib/compliance:ClassWithSelf" + "symbolId": "lib/compliance:PythonSelf.ClassWithSelf" }, "jsii-calc.PythonSelf.ClassWithSelfKwarg": { "assembly": "jsii-calc", @@ -11189,7 +11189,7 @@ } } ], - "symbolId": "lib/compliance:ClassWithSelfKwarg" + "symbolId": "lib/compliance:PythonSelf.ClassWithSelfKwarg" }, "jsii-calc.PythonSelf.IInterfaceWithSelf": { "assembly": "jsii-calc", @@ -11230,7 +11230,7 @@ ], "name": "IInterfaceWithSelf", "namespace": "PythonSelf", - "symbolId": "lib/compliance:IInterfaceWithSelf" + "symbolId": "lib/compliance:PythonSelf.IInterfaceWithSelf" }, "jsii-calc.PythonSelf.StructWithSelf": { "assembly": "jsii-calc", @@ -11263,7 +11263,7 @@ } } ], - "symbolId": "lib/compliance:StructWithSelf" + "symbolId": "lib/compliance:PythonSelf.StructWithSelf" }, "jsii-calc.ReferenceEnumFromScopedPackage": { "assembly": "jsii-calc", @@ -14863,7 +14863,7 @@ } } ], - "symbolId": "lib/calculator:CompositeOperation" + "symbolId": "lib/calculator:composition.CompositeOperation" }, "jsii-calc.composition.CompositeOperation.CompositionStringStyle": { "assembly": "jsii-calc", @@ -14895,7 +14895,7 @@ ], "name": "CompositionStringStyle", "namespace": "composition.CompositeOperation", - "symbolId": "lib/calculator:CompositionStringStyle" + "symbolId": "lib/calculator:composition.CompositeOperation.CompositionStringStyle" }, "jsii-calc.module2530.MyClass": { "assembly": "jsii-calc", @@ -16673,7 +16673,7 @@ } } ], - "symbolId": "lib/submodule/nested_submodule:Namespaced" + "symbolId": "lib/submodule/nested_submodule:nested_submodule.Namespaced" }, "jsii-calc.submodule.nested_submodule.deeplyNested.INamespaced": { "assembly": "jsii-calc", @@ -16705,7 +16705,7 @@ } } ], - "symbolId": "lib/submodule/nested_submodule:INamespaced" + "symbolId": "lib/submodule/nested_submodule:nested_submodule.deeplyNested.INamespaced" }, "jsii-calc.submodule.param.SpecialParameter": { "assembly": "jsii-calc", @@ -16779,5 +16779,5 @@ } }, "version": "3.20.120", - "fingerprint": "2ST30e+9gb6XYfIMHyGjwObar3fI+/BV0Ex1hIad+2s=" + "fingerprint": "XZczlgiEPAQC/n86Dqa40vixNH4txnPoyuF1Q+jR6I0=" } diff --git a/packages/jsii-rosetta/lib/jsii/assemblies.ts b/packages/jsii-rosetta/lib/jsii/assemblies.ts index 4cd977d2eb..0f020a21ad 100644 --- a/packages/jsii-rosetta/lib/jsii/assemblies.ts +++ b/packages/jsii-rosetta/lib/jsii/assemblies.ts @@ -13,6 +13,7 @@ import { ApiLocation, } from '../snippet'; import { enforcesStrictMode } from '../strict'; +import { mkDict } from '../util'; // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports const sortJson = require('sort-json'); @@ -22,6 +23,29 @@ export interface LoadedAssembly { directory: string; } +export function loadAssembliesSync( + assemblyLocations: readonly string[], + validateAssemblies: boolean, +): readonly LoadedAssembly[] { + return assemblyLocations.map(loadAssemblySync); + + function loadAssemblySync(location: string): LoadedAssembly { + const stat = fs.statSync(location); + if (stat.isDirectory()) { + return loadAssemblySync(path.join(location, '.jsii')); + } + return { + assembly: loadAssemblyFromFileSync(location, validateAssemblies), + directory: path.dirname(location), + }; + } +} + +function loadAssemblyFromFileSync(filename: string, validate: boolean): spec.Assembly { + const contents = fs.readJSONSync(filename, { encoding: 'utf-8' }); + return validate ? spec.validateAssembly(contents) : (contents as spec.Assembly); +} + /** * Load assemblies by filename or directory */ @@ -164,3 +188,72 @@ function _fingerprint(assembly: spec.Assembly): spec.Assembly { const fingerprint = crypto.createHash('sha256').update(JSON.stringify(assembly)).digest('base64'); return { ...assembly, fingerprint }; } + +export interface TypeLookupAssembly { + readonly assembly: spec.Assembly; + readonly assemblyFile: string; + readonly symbolIdMap: Record; +} + +const MAX_ASM_CACHE = 3; +const ASM_CACHE: TypeLookupAssembly[] = []; + +/** + * Recursively searches for a .jsii file in the directory. + * When file is found, checks cache to see if we already + * stored the assembly in memory. If not, we synchronously + * load the assembly into memory. + */ +export function findTypeLookupAssembly(directory: string): TypeLookupAssembly | undefined { + const pjLocation = findPackageJsonLocation(path.resolve(directory)); + if (!pjLocation) { + return undefined; + } + + const assemblyFile = path.join(path.dirname(pjLocation), '.jsii'); + + const fromCache = ASM_CACHE.find((c) => c.assemblyFile === assemblyFile); + if (fromCache) { + return fromCache; + } + + if (!fs.existsSync(assemblyFile)) { + return undefined; + } + + const loaded = loadLookupAssembly(assemblyFile); + while (ASM_CACHE.length >= MAX_ASM_CACHE) { + ASM_CACHE.pop(); + } + ASM_CACHE.unshift(loaded); + return loaded; +} + +function loadLookupAssembly(assemblyFile: string): TypeLookupAssembly { + const assembly: spec.Assembly = fs.readJSONSync(assemblyFile, { encoding: 'utf-8' }); + const symbolIdMap = mkDict( + Object.values(assembly.types ?? {}).map((type) => [type.symbolId ?? '', type.fqn] as const), + ); + + return { + assembly, + assemblyFile, + symbolIdMap, + }; +} + +function findPackageJsonLocation(currentPath: string): string | undefined { + // eslint-disable-next-line no-constant-condition + while (true) { + const candidate = path.join(currentPath, 'package.json'); + if (fs.existsSync(candidate)) { + return candidate; + } + + const parentPath = path.resolve(currentPath, '..'); + if (parentPath === currentPath) { + return undefined; + } + currentPath = parentPath; + } +} diff --git a/packages/jsii-rosetta/lib/jsii/jsii-utils.ts b/packages/jsii-rosetta/lib/jsii/jsii-utils.ts index 8be4bd7100..d9a5d9b23b 100644 --- a/packages/jsii-rosetta/lib/jsii/jsii-utils.ts +++ b/packages/jsii-rosetta/lib/jsii/jsii-utils.ts @@ -1,7 +1,9 @@ +import { symbolIdentifier } from 'jsii'; import * as ts from 'typescript'; import { AstRenderer } from '../renderer'; import { typeContainsUndefined } from '../typescript/types'; +import { findTypeLookupAssembly } from './assemblies'; import { findPackageJson } from './packages'; export function isNamedLikeStruct(name: string) { @@ -95,3 +97,88 @@ export function refersToJsiiSymbol(symbol: ts.Symbol): boolean { const pj = findPackageJson(declaringFile.fileName); return !!(pj && pj.jsii); } + +/** + * Returns the jsii FQN for a TypeScript (class or type) symbol + * + * TypeScript only knows the symbol NAME plus the FILE the symbol is defined + * in. We need to extract two things: + * + * 1. The package name (extracted from the nearest `package.json`) + * 2. The submodule name (...?? don't know how to get this yet) + * 3. Any containing type names or namespace names. + */ +export function jsiiFqnFromSymbol(typeChecker: ts.TypeChecker, sym: ts.Symbol): string | undefined { + const decl: ts.Node | undefined = sym.declarations?.[0]; + if (!decl || !isDeclaration(decl)) { + return undefined; + } + + const declSym = getSymbolFromDeclaration(decl, typeChecker); + if (!declSym) { + return undefined; + } + + const fileName = decl.getSourceFile().fileName; + if (hasAnyFlag(declSym.flags, ts.SymbolFlags.Method | ts.SymbolFlags.Property | ts.SymbolFlags.EnumMember)) { + return fqnFromMemberSymbol(typeChecker, sym, fileName); + } + return fqnFromTypeSymbol(typeChecker, sym, fileName); +} + +/** + * Look up the jsii fqn for a given type symbol + */ +function fqnFromTypeSymbol(typeChecker: ts.TypeChecker, typeSymbol: ts.Symbol, fileName: string): string | undefined { + const symbolId = symbolIdentifier(typeChecker, typeSymbol); + if (!symbolId) { + return undefined; + } + + const assembly = findTypeLookupAssembly(fileName); + return assembly?.symbolIdMap[symbolId]; +} + +function fqnFromMemberSymbol( + typeChecker: ts.TypeChecker, + memberSymbol: ts.Symbol, + fileName: string, +): string | undefined { + const declParent = memberSymbol.declarations?.[0]?.parent; + if (!declParent || !isDeclaration(declParent)) { + return undefined; + } + + const declParentSym = getSymbolFromDeclaration(declParent, typeChecker); + if (!declParentSym) { + return undefined; + } + + const result = fqnFromTypeSymbol(typeChecker, declParentSym, fileName); + return result ? `${result}#${memberSymbol.name}` : undefined; +} + +function isDeclaration(x: ts.Node): x is ts.Declaration { + return ( + ts.isClassDeclaration(x) || + ts.isNamespaceExportDeclaration(x) || + ts.isNamespaceExport(x) || + ts.isModuleDeclaration(x) || + ts.isEnumDeclaration(x) || + ts.isEnumMember(x) || + ts.isInterfaceDeclaration(x) || + ts.isMethodDeclaration(x) || + ts.isMethodSignature(x) || + ts.isPropertyDeclaration(x) || + ts.isPropertySignature(x) + ); +} + +function getSymbolFromDeclaration(decl: ts.Node, typeChecker: ts.TypeChecker): ts.Symbol | undefined { + if (!isDeclaration(decl)) { + return undefined; + } + + const name = ts.getNameOfDeclaration(decl); + return name ? typeChecker.getSymbolAtLocation(name) : undefined; +} diff --git a/packages/jsii-rosetta/lib/languages/record-references.ts b/packages/jsii-rosetta/lib/languages/record-references.ts index 22f1d01f93..db1027f14f 100644 --- a/packages/jsii-rosetta/lib/languages/record-references.ts +++ b/packages/jsii-rosetta/lib/languages/record-references.ts @@ -1,7 +1,6 @@ import * as ts from 'typescript'; -import { hasAnyFlag } from '../jsii/jsii-utils'; -import { findPackageJson } from '../jsii/packages'; +import { jsiiFqnFromSymbol } from '../jsii/jsii-utils'; import { TargetLanguage } from '../languages/target-language'; import { OTree, NO_SYNTAX } from '../o-tree'; import { AstRenderer } from '../renderer'; @@ -17,6 +16,8 @@ type RecordReferencesRenderer = AstRenderer; * A visitor that collects all types referenced in a particular piece of sample code */ export class RecordReferencesVisitor extends DefaultVisitor { + public static readonly VERSION = '2'; + public readonly language = TargetLanguage.PYTHON; // Doesn't matter, but we need it to use the visitor infra :( public readonly defaultContext = {}; private readonly references = new Set(); @@ -125,7 +126,6 @@ export class RecordReferencesVisitor extends DefaultVisitor { assembly = await TestJsiiModule.fromSource( - ` - export class ClassA { - public someMethod() { - } - } - export class ClassB { - public argumentMethod(args: BeeArgs) { - Array.isArray(args); - } - } + { + 'index.ts': ` + export class ClassA { + public someMethod() { + } + } + export class ClassB { + public argumentMethod(args: BeeArgs) { + Array.isArray(args); + } + } + + export interface BeeArgs { readonly value: string; readonly nested?: NestedType; } - export interface BeeArgs { readonly value: string; readonly nested?: NestedType; } + export interface NestedType { readonly x: number; } - export interface NestedType { readonly x: number; } - `, + export * as submod from './submodule'; + `, + + 'submodule.ts': ` + export class SubmoduleClass { + } + `, + }, { name: 'my_assembly', jsii: DUMMY_ASSEMBLY_TARGETS, @@ -78,3 +87,11 @@ test('detect nested types of parameter used in method calls', () => { `); expect(translator.fqnsReferenced()).toContain('my_assembly.NestedType'); }); + +test('detect types in submodules', () => { + const translator = assembly.successfullyCompile(` + import { submod as subby } from 'my_assembly'; + const b = new subby.SubmoduleClass(); + `); + expect(translator.fqnsReferenced()).toContain('my_assembly.submod.SubmoduleClass'); +}); diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index 675653ad4e..257d3d2012 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -21,12 +21,12 @@ import * as literate from './literate'; import * as bindings from './node-bindings'; import { ProjectInfo } from './project-info'; import { isReservedName } from './reserved-words'; +import { symbolIdentifier } from './symbol-id'; import { DeprecatedRemover } from './transforms/deprecated-remover'; import { DeprecationWarningsInjector } from './transforms/deprecation-warnings'; import { RuntimeTypeInfoInjector } from './transforms/runtime-info'; import { TsCommentReplacer } from './transforms/ts-comment-replacer'; import { combinedTransformers } from './transforms/utils'; -import { symbolIdentifier } from './utils'; import { Validator } from './validator'; import { SHORT_VERSION, VERSION } from './version'; import { enabledWarnings } from './warnings'; diff --git a/packages/jsii/lib/index.ts b/packages/jsii/lib/index.ts index 08d6d4f268..8f1fff9ecf 100644 --- a/packages/jsii/lib/index.ts +++ b/packages/jsii/lib/index.ts @@ -1,2 +1,3 @@ export * from './jsii-diagnostic'; +export * from './symbol-id'; export * from './helpers'; diff --git a/packages/jsii/lib/symbol-id.ts b/packages/jsii/lib/symbol-id.ts new file mode 100644 index 0000000000..c58deb827a --- /dev/null +++ b/packages/jsii/lib/symbol-id.ts @@ -0,0 +1,86 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as ts from 'typescript'; + +export function symbolIdentifier( + typeChecker: ts.TypeChecker, + sym: ts.Symbol, +): string | undefined { + const inFileNameParts: string[] = []; + + let decl: ts.Node | undefined = sym.declarations?.[0]; + while (decl && !ts.isSourceFile(decl)) { + if ( + ts.isClassDeclaration(decl) || + ts.isNamespaceExportDeclaration(decl) || + ts.isNamespaceExport(decl) || + ts.isModuleDeclaration(decl) || + ts.isEnumDeclaration(decl) || + ts.isEnumMember(decl) || + ts.isInterfaceDeclaration(decl) || + ts.isMethodDeclaration(decl) || + ts.isMethodSignature(decl) || + ts.isPropertyDeclaration(decl) || + ts.isPropertySignature(decl) + ) { + const name = ts.getNameOfDeclaration(decl); + const declSym = name ? typeChecker.getSymbolAtLocation(name) : undefined; + if (declSym) { + inFileNameParts.unshift(declSym.name); + } + } + decl = decl.parent; + } + if (!decl) { + return undefined; + } + const namespace = assemblyRelativeSourceFile(decl.getSourceFile().fileName); + + if (!namespace) { + return undefined; + } + + return `${namespace}:${inFileNameParts.join('.')}`; +} + +function assemblyRelativeSourceFile(sourceFileName: string) { + const packageJsonLocation = findPackageJsonLocation( + path.dirname(sourceFileName), + ); + + if (!packageJsonLocation) { + return undefined; + } + + const packageJson = JSON.parse( + fs.readFileSync(packageJsonLocation).toString(), + ); + + const sourcePath = removePrefix( + packageJson.jsii?.outdir ?? '', + path.relative(path.dirname(packageJsonLocation), sourceFileName), + ); + + return sourcePath.replace(/(\.d)?\.ts$/, ''); + + function findPackageJsonLocation(currentPath: string): string | undefined { + const candidate = path.join(currentPath, 'package.json'); + if (fs.existsSync(candidate)) { + return candidate; + } + const parentPath = path.resolve(currentPath, '..'); + return parentPath !== currentPath + ? findPackageJsonLocation(parentPath) + : undefined; + } + + function removePrefix(prefix: string, filePath: string) { + const prefixParts = prefix.split(/[/\\]/g); + const pathParts = filePath.split(/[/\\]/g); + let i = 0; + while (prefixParts[i] === pathParts[i]) { + i++; + } + return pathParts.slice(i).join('/'); + } +} diff --git a/packages/jsii/lib/transforms/deprecation-warnings.ts b/packages/jsii/lib/transforms/deprecation-warnings.ts index 253419d433..41c7c41318 100644 --- a/packages/jsii/lib/transforms/deprecation-warnings.ts +++ b/packages/jsii/lib/transforms/deprecation-warnings.ts @@ -6,7 +6,7 @@ import { EmitHint, Statement } from 'typescript'; import * as ts from 'typescript/lib/tsserverlibrary'; import { ProjectInfo } from '../project-info'; -import { symbolIdentifier } from '../utils'; +import { symbolIdentifier } from '../symbol-id'; const FILE_NAME = '.warnings.jsii.js'; const WARNING_FUNCTION_NAME = 'print'; diff --git a/packages/jsii/lib/utils.ts b/packages/jsii/lib/utils.ts index e90e6b9a05..f413bdbf27 100644 --- a/packages/jsii/lib/utils.ts +++ b/packages/jsii/lib/utils.ts @@ -1,6 +1,4 @@ -import * as fs from 'fs'; import * as log4js from 'log4js'; -import * as path from 'path'; import * as ts from 'typescript'; import { JsiiDiagnostic } from './jsii-diagnostic'; @@ -165,86 +163,3 @@ export function parseRepository(value: string): { url: string } { throw new Error(`Unknown host service: ${host}`); } } - -export function symbolIdentifier( - typeChecker: ts.TypeChecker, - sym: ts.Symbol, -): string | undefined { - const inFileNameParts: string[] = []; - - let decl: ts.Node | undefined = sym.declarations[0]; - while (decl && !ts.isSourceFile(decl)) { - if ( - ts.isClassDeclaration(decl) || - ts.isNamespaceExportDeclaration(decl) || - ts.isNamespaceExport(decl) || - ts.isEnumDeclaration(decl) || - ts.isEnumMember(decl) || - ts.isInterfaceDeclaration(decl) || - ts.isMethodDeclaration(decl) || - ts.isMethodSignature(decl) || - ts.isPropertyDeclaration(decl) || - ts.isPropertySignature(decl) - ) { - const name = ts.getNameOfDeclaration(decl); - const declSym = name ? typeChecker.getSymbolAtLocation(name) : undefined; - if (declSym) { - inFileNameParts.unshift(declSym.name); - } - } - decl = decl.parent; - } - if (!decl) { - return undefined; - } - - const namespace = getNamespace(decl.getSourceFile().fileName); - - if (!namespace) { - return undefined; - } - - return `${namespace}:${inFileNameParts.join('.')}`; -} - -export function getNamespace(sourceFileName: string) { - const packageJsonLocation = findPackageJsonLocation( - path.dirname(sourceFileName), - ); - - if (!packageJsonLocation) { - return undefined; - } - - const packageJson = JSON.parse( - fs.readFileSync(packageJsonLocation).toString(), - ); - - const sourcePath = removePrefix( - packageJson.jsii?.outdir ?? '', - path.relative(path.dirname(packageJsonLocation), sourceFileName), - ); - - return sourcePath.replace(/(\.d)?\.ts$/, ''); - - function findPackageJsonLocation(currentPath: string): string | undefined { - const candidate = path.join(currentPath, 'package.json'); - if (fs.existsSync(candidate)) { - return candidate; - } - const parentPath = path.resolve(currentPath, '..'); - return parentPath !== currentPath - ? findPackageJsonLocation(parentPath) - : undefined; - } - - function removePrefix(prefix: string, filePath: string) { - const prefixParts = prefix.split(/[/\\]/g); - const pathParts = filePath.split(/[/\\]/g); - let i = 0; - while (prefixParts[i] === pathParts[i]) { - i++; - } - return pathParts.slice(i).join('/'); - } -} diff --git a/packages/jsii/test/symbol-identifiers.test.ts b/packages/jsii/test/symbol-identifiers.test.ts index 60ef5015c6..6c221e3b57 100644 --- a/packages/jsii/test/symbol-identifiers.test.ts +++ b/packages/jsii/test/symbol-identifiers.test.ts @@ -7,7 +7,7 @@ test('Symbol map is generated', async () => { export * from './some/nested/file'; export class Foo { public bar(){} - } + } `, 'some/nested/file.ts': ` export interface Bar { @@ -28,3 +28,26 @@ test('Symbol map is generated', async () => { expect(types['testpkg.Bar'].symbolId).toEqual('some/nested/file:Bar'); expect(types['testpkg.Baz'].symbolId).toEqual('some/nested/file:Baz'); }); + +test('Module declarations are included in symbolId', async () => { + const result = await compileJsiiForTest( + { + 'index.ts': ` + export class Foo { + constructor() { + } + } + export namespace Foo { + export class Bar { + public baz() {} + } + } + `, + }, + undefined /* callback */, + { stripDeprecated: true }, + ); + + const types = result.assembly.types ?? {}; + expect(types['testpkg.Foo.Bar'].symbolId).toEqual('index:Foo.Bar'); +}); From ace0b83a0c951052349b102c3af34e92cae76767 Mon Sep 17 00:00:00 2001 From: kaizen3031593 <36202692+kaizen3031593@users.noreply.github.com> Date: Wed, 17 Nov 2021 03:41:53 -0500 Subject: [PATCH 02/10] fix(pacmak): dotnet code docs loses indentation (#3180) Currently code examples in dotnet lose their indentation styles because we call `trim()` on each line in the code example. We want that behavior for other types of xml docs but not for code. This PR makes code a special case that calls `trimRight()` on each line, preserving the indentation on the left. The only change in the snapshots is translating this example: ```ts docs const x = 12 + 44; const s1 = "string"; const s2 = "string \nwith new newlines"; // see https://github.com/aws/jsii/issues/2569 const s3 = `string with new lines`; ``` to this: ``` int x = 12 + 44; string s1 = "string"; string s2 = @"string with new newlines"; // see https://github.com/aws/jsii/issues/2569 string s3 = @"string with new lines"; ``` --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- .../jsii-pacmak/lib/targets/dotnet/dotnetdocgenerator.ts | 6 ++++-- .../generated-code/__snapshots__/target-dotnet.test.ts.snap | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/jsii-pacmak/lib/targets/dotnet/dotnetdocgenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/dotnetdocgenerator.ts index 4b336c235c..e10ba5b582 100644 --- a/packages/jsii-pacmak/lib/targets/dotnet/dotnetdocgenerator.ts +++ b/packages/jsii-pacmak/lib/targets/dotnet/dotnetdocgenerator.ts @@ -205,8 +205,10 @@ export class DotNetDocGenerator { xml.att(name, value); } const xmlstring = xml.end({ allowEmpty: true, pretty: false }); - - for (const line of xmlstring.split('\n').map((x) => x.trim())) { + const trimLeft = tag !== 'code'; + for (const line of xmlstring + .split('\n') + .map((x) => (trimLeft ? x.trim() : x.trimRight()))) { this.code.line(`/// ${line}`); } } diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap index 812245c6e1..4f5d46a03d 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap @@ -6235,8 +6235,8 @@ namespace Amazon.JSII.Tests.CalculatorNamespace /// string s2 = @"string /// with new newlines"; // see https://github.com/aws/jsii/issues/2569 /// string s3 = @"string - /// with - /// new lines"; + /// with + /// new lines"; /// [JsiiClass(nativeType: typeof(Amazon.JSII.Tests.CalculatorNamespace.DocumentedClass), fullyQualifiedName: "jsii-calc.DocumentedClass")] public class DocumentedClass : DeputyBase From 92a7d5e3b10dc7c881ef70c12730f1f0cdcf9b63 Mon Sep 17 00:00:00 2001 From: kaizen3031593 <36202692+kaizen3031593@users.noreply.github.com> Date: Thu, 18 Nov 2021 03:29:44 -0500 Subject: [PATCH 03/10] fix(rosetta): diagnostics not showing (#3182) One-liner to make sure that we slice `diags` correctly. Tested locally on my own machine to ensure that it works. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- packages/jsii-rosetta/lib/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jsii-rosetta/lib/util.ts b/packages/jsii-rosetta/lib/util.ts index 9ae8f9db6a..4d131ad254 100644 --- a/packages/jsii-rosetta/lib/util.ts +++ b/packages/jsii-rosetta/lib/util.ts @@ -15,7 +15,7 @@ export function printDiagnostics(diags: readonly RosettaDiagnostic[], stream: No // Don't print too much, at some point it just clogs up the log const maxDiags = 50; - for (const diag of diags.slice(maxDiags)) { + for (const diag of diags.slice(0, maxDiags)) { stream.write(diag.formattedMessage); } From 0f5f349d2f6d7d916c59e4f13e0c5195cc0f80c9 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Thu, 18 Nov 2021 14:33:32 +0100 Subject: [PATCH 04/10] fix: C# NamespaceDoc emitted to wrong location (#3183) The namespace determination for the `NamespaceDoc` class generated for submodules was incorrect, as the `jsiiNs` passed to the type resovler was absolute, instead of being the required assembly-relative form. Additionally, removed the `readme` and `locationInSource` property from submodule descriptors included in the jsii assembly under `dependencyClosure`, as those are not necessary and needlessly bloat the assembly document. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- packages/@jsii/spec/lib/assembly.ts | 18 +++- .../jsii-calc-lib/lib/submodule/README.md | 3 + .../@scope/jsii-calc-lib/test/assembly.jsii | 5 +- packages/jsii-calc/test/assembly.jsii | 6 +- .../lib/targets/dotnet/dotnetgenerator.ts | 6 +- .../__snapshots__/target-dotnet.test.ts.snap | 100 ++++++++++-------- .../__snapshots__/target-java.test.ts.snap | 11 ++ .../__snapshots__/target-python.test.ts.snap | 5 + packages/jsii/lib/assembler.ts | 31 +++++- 9 files changed, 129 insertions(+), 56 deletions(-) create mode 100644 packages/@scope/jsii-calc-lib/lib/submodule/README.md diff --git a/packages/@jsii/spec/lib/assembly.ts b/packages/@jsii/spec/lib/assembly.ts index 07d483a515..a60a88d807 100644 --- a/packages/@jsii/spec/lib/assembly.ts +++ b/packages/@jsii/spec/lib/assembly.ts @@ -3,7 +3,10 @@ export const SPEC_FILE_NAME = '.jsii'; /** * A JSII assembly specification. */ -export interface Assembly extends AssemblyConfiguration, Documentable { +export interface Assembly + extends AssemblyConfiguration, + Documentable, + ReadMeContainer { /** * The version of the spec schema */ @@ -120,7 +123,7 @@ export interface Assembly extends AssemblyConfiguration, Documentable { * * @default none */ - dependencyClosure?: { [assembly: string]: AssemblyConfiguration }; + dependencyClosure?: { [assembly: string]: DependencyConfiguration }; /** * List if bundled dependencies (these are not expected to be jsii @@ -157,6 +160,10 @@ export interface AssemblyConfiguration extends Targetable { submodules?: { [fqn: string]: Submodule }; } +export interface DependencyConfiguration extends Targetable { + submodules?: { [fqn: string]: Targetable }; +} + /** * A targetable module-like thing * @@ -170,7 +177,12 @@ export interface Targetable { * @default none */ targets?: AssemblyTargets; +} +/** + * Elements that can contain a `readme` property. + */ +export interface ReadMeContainer { /** * The readme document for this module (if any). * @@ -192,7 +204,7 @@ export interface ReadMe { * The difference between a top-level module (the assembly) and a submodule is * that the submodule is annotated with its location in the repository. */ -export type Submodule = SourceLocatable & Targetable; +export type Submodule = ReadMeContainer & SourceLocatable & Targetable; /** * Versions of the JSII Assembly Specification. diff --git a/packages/@scope/jsii-calc-lib/lib/submodule/README.md b/packages/@scope/jsii-calc-lib/lib/submodule/README.md new file mode 100644 index 0000000000..b388cd9eb8 --- /dev/null +++ b/packages/@scope/jsii-calc-lib/lib/submodule/README.md @@ -0,0 +1,3 @@ +# Submodule Readme + +This is a submodule readme. diff --git a/packages/@scope/jsii-calc-lib/test/assembly.jsii b/packages/@scope/jsii-calc-lib/test/assembly.jsii index 981dcd29f1..131d74d634 100644 --- a/packages/@scope/jsii-calc-lib/test/assembly.jsii +++ b/packages/@scope/jsii-calc-lib/test/assembly.jsii @@ -95,6 +95,9 @@ "filename": "lib/index.ts", "line": 130 }, + "readme": { + "markdown": "# Submodule Readme\n\nThis is a submodule readme.\n" + }, "targets": { "dotnet": { "namespace": "Amazon.JSII.Tests.CustomSubmoduleName" @@ -948,5 +951,5 @@ } }, "version": "0.0.0", - "fingerprint": "LN1bs46m2O4aR6AhHvi6UDh/f90EoUT3n5apMPzr9+c=" + "fingerprint": "fCLOsQQLslSg+W8rn7XDZ1RSenH5QeaoTna+j7o+URc=" } diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index 4a90175111..597752aca0 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -95,10 +95,6 @@ "@scope/jsii-calc-lib": { "submodules": { "@scope/jsii-calc-lib.submodule": { - "locationInModule": { - "filename": "lib/index.ts", - "line": 130 - }, "targets": { "dotnet": { "namespace": "Amazon.JSII.Tests.CustomSubmoduleName" @@ -16779,5 +16775,5 @@ } }, "version": "3.20.120", - "fingerprint": "XZczlgiEPAQC/n86Dqa40vixNH4txnPoyuF1Q+jR6I0=" + "fingerprint": "WpcHNV2L+v3B7Ou+8xsRMzye4rf1U+SURQA97A3JT6g=" } diff --git a/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts index b6dd9bacce..fa15aa74ed 100644 --- a/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts +++ b/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts @@ -159,7 +159,9 @@ export class DotNetGenerator extends Generator { const dotnetNs = this.typeresolver.resolveNamespace( this.assembly, this.assembly.name, - jsiiNs, + // Strip the `${assmName}.` prefix here, as the "assembly-relative" NS + // is expected by `this.typeResolver.resovleNamespace`. + jsiiNs.substr(this.assembly.name.length + 1), ); this.emitNamespaceDocs(dotnetNs, jsiiNs, submodule); } @@ -1158,7 +1160,7 @@ export class DotNetGenerator extends Generator { private emitNamespaceDocs( namespace: string, jsiiFqn: string, - docSource: spec.Targetable, + docSource: spec.Targetable & spec.ReadMeContainer, ) { if (!docSource.readme) { return; diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap index 4f5d46a03d..48f7cdd035 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap @@ -995,6 +995,7 @@ exports[`Generated code for "@scope/jsii-calc-lib": / 1`] = ` ┃ ┗━ πŸ“ CustomSubmoduleName ┃ ┣━ πŸ“„ IReflectable.cs ┃ ┣━ πŸ“„ IReflectableEntry.cs + ┃ ┣━ πŸ“„ NamespaceDoc.cs ┃ ┣━ πŸ“„ NestingClass.cs ┃ ┣━ πŸ“„ ReflectableEntry.cs ┃ ┗━ πŸ“„ Reflector.cs @@ -2249,6 +2250,24 @@ namespace Amazon.JSII.Tests.CustomSubmoduleName `; +exports[`Generated code for "@scope/jsii-calc-lib": /dotnet/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId/Amazon/JSII/Tests/CustomSubmoduleName/NamespaceDoc.cs 1`] = ` +#pragma warning disable CS0672,CS0809,CS1591 + +namespace Amazon.JSII.Tests.CustomSubmoduleName +{ + /// + ///

Submodule Readme

+ /// + /// This is a submodule readme. + ///
+ [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + public class NamespaceDoc + { + } +} + +`; + exports[`Generated code for "@scope/jsii-calc-lib": /dotnet/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId/Amazon/JSII/Tests/CustomSubmoduleName/NestingClass.cs 1`] = ` using Amazon.JSII.Runtime.Deputy; @@ -2894,11 +2913,6 @@ exports[`Generated code for "jsii-calc": / 1`] = ` ┃ ┣━ πŸ“„ Jsii487Derived.cs ┃ ┣━ πŸ“„ Jsii496Derived.cs ┃ ┣━ πŸ“„ JsiiAgent.cs - ┃ ┣━ πŸ“ JsiiCalc - ┃ ┃ ┗━ πŸ“ Submodule - ┃ ┃ ┣━ πŸ“ Isolated - ┃ ┃ ┃ ┗━ πŸ“„ NamespaceDoc.cs - ┃ ┃ ┗━ πŸ“„ NamespaceDoc.cs ┃ ┣━ πŸ“„ JSObjectLiteralForInterface.cs ┃ ┣━ πŸ“„ JSObjectLiteralToNative.cs ┃ ┣━ πŸ“„ JSObjectLiteralToNativeClass.cs @@ -3035,8 +3049,10 @@ exports[`Generated code for "jsii-calc": / 1`] = ` ┃ ┃ ┣━ πŸ“„ Default.cs ┃ ┃ ┣━ πŸ“„ IDefault.cs ┃ ┃ ┣━ πŸ“ Isolated - ┃ ┃ ┃ ┗━ πŸ“„ Kwargs.cs + ┃ ┃ ┃ ┣━ πŸ“„ Kwargs.cs + ┃ ┃ ┃ ┗━ πŸ“„ NamespaceDoc.cs ┃ ┃ ┣━ πŸ“„ MyClass.cs + ┃ ┃ ┣━ πŸ“„ NamespaceDoc.cs ┃ ┃ ┣━ πŸ“ NestedSubmodule ┃ ┃ ┃ ┣━ πŸ“ DeeplyNested ┃ ┃ ┃ ┃ ┗━ πŸ“„ INamespaced.cs @@ -12097,42 +12113,6 @@ namespace Amazon.JSII.Tests.CalculatorNamespace `; -exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/JsiiCalc/Submodule/Isolated/NamespaceDoc.cs 1`] = ` -#pragma warning disable CS0672,CS0809,CS1591 - -namespace Amazon.JSII.Tests.CalculatorNamespace.JsiiCalc.Submodule.Isolated -{ - /// - ///

Read you, read me

- /// - /// This is the readme of the jsii-calc.submodule.isolated module. - ///
- [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] - public class NamespaceDoc - { - } -} - -`; - -exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/JsiiCalc/Submodule/NamespaceDoc.cs 1`] = ` -#pragma warning disable CS0672,CS0809,CS1591 - -namespace Amazon.JSII.Tests.CalculatorNamespace.JsiiCalc.Submodule -{ - /// - ///

Read you, read me

- /// - /// This is the readme of the jsii-calc.submodule module. - ///
- [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] - public class NamespaceDoc - { - } -} - -`; - exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/JsonFormatter.cs 1`] = ` using Amazon.JSII.Runtime.Deputy; @@ -16945,6 +16925,24 @@ namespace Amazon.JSII.Tests.CalculatorNamespace.Submodule.Isolated `; +exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/Submodule/Isolated/NamespaceDoc.cs 1`] = ` +#pragma warning disable CS0672,CS0809,CS1591 + +namespace Amazon.JSII.Tests.CalculatorNamespace.Submodule.Isolated +{ + /// + ///

Read you, read me

+ /// + /// This is the readme of the jsii-calc.submodule.isolated module. + ///
+ [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + public class NamespaceDoc + { + } +} + +`; + exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/Submodule/MyClass.cs 1`] = ` using Amazon.JSII.Runtime.Deputy; @@ -17015,6 +17013,24 @@ namespace Amazon.JSII.Tests.CalculatorNamespace.Submodule `; +exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/Submodule/NamespaceDoc.cs 1`] = ` +#pragma warning disable CS0672,CS0809,CS1591 + +namespace Amazon.JSII.Tests.CalculatorNamespace.Submodule +{ + /// + ///

Read you, read me

+ /// + /// This is the readme of the jsii-calc.submodule module. + ///
+ [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + public class NamespaceDoc + { + } +} + +`; + exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/Submodule/NestedSubmodule/DeeplyNested/INamespaced.cs 1`] = ` using Amazon.JSII.Runtime.Deputy; diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap index cef9f9ae79..313cb7c0e0 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap @@ -1418,6 +1418,7 @@ exports[`Generated code for "@scope/jsii-calc-lib": / 1`] = ` ┃ ┣━ πŸ“ custom_submodule_name ┃ ┃ ┣━ πŸ“„ IReflectable.java ┃ ┃ ┣━ πŸ“„ NestingClass.java + ┃ ┃ ┣━ πŸ“„ package-info.java ┃ ┃ ┣━ πŸ“„ ReflectableEntry.java ┃ ┃ ┗━ πŸ“„ Reflector.java ┃ ┗━ πŸ“ lib @@ -2249,6 +2250,16 @@ public class Reflector extends software.amazon.jsii.JsiiObject { `; +exports[`Generated code for "@scope/jsii-calc-lib": /java/src/main/java/software/amazon/jsii/tests/calculator/custom_submodule_name/package-info.java 1`] = ` +/** + *

Submodule Readme

+ *

+ * This is a submodule readme. + */ +package software.amazon.jsii.tests.calculator.custom_submodule_name; + +`; + exports[`Generated code for "@scope/jsii-calc-lib": /java/src/main/java/software/amazon/jsii/tests/calculator/lib/$Module.java 1`] = ` package software.amazon.jsii.tests.calculator.lib; diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap index 6485f5a72f..7f1c6c9856 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap @@ -1863,6 +1863,11 @@ publication.publish() exports[`Generated code for "@scope/jsii-calc-lib": /python/src/scope/jsii_calc_lib/_jsii/jsii-calc-lib@0.0.0.jsii.tgz 1`] = `python/src/scope/jsii_calc_lib/_jsii/jsii-calc-lib@0.0.0.jsii.tgz is a tarball`; exports[`Generated code for "@scope/jsii-calc-lib": /python/src/scope/jsii_calc_lib/custom_submodule_name/__init__.py 1`] = ` +''' +# Submodule Readme + +This is a submodule readme. +''' import abc import builtins import datetime diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index 257d3d2012..200a3b1b50 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -3100,19 +3100,44 @@ function noEmptyDict( } function toDependencyClosure(assemblies: readonly spec.Assembly[]): { - [name: string]: spec.AssemblyConfiguration; + [name: string]: spec.DependencyConfiguration; } { - const result: { [name: string]: spec.AssemblyTargets } = {}; + const result: { [name: string]: spec.DependencyConfiguration } = {}; for (const assembly of assemblies) { if (!assembly.targets) { continue; } result[assembly.name] = { - submodules: assembly.submodules, + submodules: cleanUp(assembly.submodules), targets: assembly.targets, }; } return result; + + /** + * Removes unneeded fields from the entries part of the `dependencyClosure` + * property. Fields such as `readme` are not necessary and can bloat up the + * assembly object. + * + * This removes the `readme` and `locationInModule` fields from the submodule + * descriptios if present. + * + * @param submodules the submodules list to clean up. + * + * @returns the cleaned up submodules list. + */ + function cleanUp( + submodules: spec.Assembly['submodules'], + ): spec.DependencyConfiguration['submodules'] { + if (submodules == null) { + return submodules; + } + const result: spec.DependencyConfiguration['submodules'] = {}; + for (const [fqn, { targets }] of Object.entries(submodules)) { + result[fqn] = { targets }; + } + return result; + } } function toSubmoduleDeclarations( From b0afe51a8cb36a9ebdd39bd19a842285c58ee2c1 Mon Sep 17 00:00:00 2001 From: Sebastian Korfmann Date: Thu, 18 Nov 2021 15:51:01 +0100 Subject: [PATCH 05/10] fix(pacmak): Generate Relative Module Imports in Python (#3181) The goal of this PR is, to find a way for generated Python code to work with namespaced modules regardless of the way it is consumed. While the current implementation of submodule imports seem to work fine when distributed as packages, it's breaking apart when the generated code is imported relatively as part of a single code base. That's what we do in cdktf and the reason why we have locked jsii to `1.37.0` at the moment. The general idea is, to generate relative imports rather than fqn imports in Python. This could be a potential way to address https://github.com/aws/jsii/issues/3078 ```python # Before import jsii_calc.submodule import jsii_calc.submodule.nested_submodule import jsii_calc.submodule.nested_submodule.deeply_nested # After from . import submodule from .submodule import nested_submodule from .submodule.nested_submodule import deeply_nested ``` Besides from a few snapshots being changed, it seems like the tests are still working with this change. However, I'm not sure about the general implications. So, this entire PR is really meant to be a conversation starter. I have confirmed that an import like `import . from some_aws_namespace` would work for us in cdktf. ## References - https://github.com/aws/jsii/issues/3078 - https://github.com/aws/jsii/pull/3049 - https://github.com/hashicorp/terraform-cdk/issues/1152 - https://github.com/hashicorp/terraform-cdk/pull/1288 --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- packages/jsii-pacmak/lib/targets/python.ts | 22 ++++++- .../lib/targets/python/type-name.ts | 2 +- .../__snapshots__/target-python.test.ts.snap | 66 +++++++++---------- 3 files changed, 55 insertions(+), 35 deletions(-) diff --git a/packages/jsii-pacmak/lib/targets/python.ts b/packages/jsii-pacmak/lib/targets/python.ts index b8c6a84763..dcd4807b91 100644 --- a/packages/jsii-pacmak/lib/targets/python.ts +++ b/packages/jsii-pacmak/lib/targets/python.ts @@ -25,6 +25,7 @@ import { PythonImports, mergePythonImports, toPackageName, + toPythonFqn, } from './python/type-name'; import { die, toPythonIdentifier } from './python/util'; import { toPythonVersionRange, toReleaseVersion } from './version-utils'; @@ -1673,7 +1674,26 @@ class PythonModule implements PythonType { for (const module of this.modules.sort((l, r) => l.pythonName.localeCompare(r.pythonName), )) { - code.line(`import ${module.pythonName}`); + // Rather than generating an absolute import like + // "import jsii_calc.submodule.nested_submodule.deeply_nested" + // this builds a relative import like + // "from .submodule.nested_submodule import deeply_nested" + // This enables distributing python packages and using the + // generated modules in the same codebase. + const assemblyName = toPythonFqn( + module.assembly.name, + module.assembly, + ).pythonFqn; + + const submodule = module.pythonName + .replace(`${assemblyName}.`, '') + .split('.'); + + const submodulePath = submodule + .slice(0, submodule.length - 1) + .join('.'); + const submoduleName = submodule[submodule.length - 1]; + code.line(`from .${submodulePath} import ${submoduleName}`); } } } diff --git a/packages/jsii-pacmak/lib/targets/python/type-name.ts b/packages/jsii-pacmak/lib/targets/python/type-name.ts index f015d5e9c2..f9410ef746 100644 --- a/packages/jsii-pacmak/lib/targets/python/type-name.ts +++ b/packages/jsii-pacmak/lib/targets/python/type-name.ts @@ -373,7 +373,7 @@ class UserType implements TypeName { } } -function toPythonFqn(fqn: string, rootAssm: Assembly) { +export function toPythonFqn(fqn: string, rootAssm: Assembly) { const { assemblyName, packageName, tail } = getPackageName(fqn, rootAssm); const fqnParts: string[] = [packageName]; diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap index 7f1c6c9856..c75ddb6185 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap @@ -1830,7 +1830,7 @@ __all__ = [ publication.publish() # Loading modules to ensure their types are registered with the jsii runtime library -import scope.jsii_calc_lib.custom_submodule_name +from . import custom_submodule_name `; @@ -10200,38 +10200,38 @@ __all__ = [ publication.publish() # Loading modules to ensure their types are registered with the jsii runtime library -import jsii_calc.cdk16625 -import jsii_calc.cdk16625.donotimport -import jsii_calc.composition -import jsii_calc.derived_class_has_no_properties -import jsii_calc.interface_in_namespace_includes_classes -import jsii_calc.interface_in_namespace_only_interface -import jsii_calc.module2530 -import jsii_calc.module2617 -import jsii_calc.module2647 -import jsii_calc.module2689 -import jsii_calc.module2689.methods -import jsii_calc.module2689.props -import jsii_calc.module2689.retval -import jsii_calc.module2689.structs -import jsii_calc.module2692 -import jsii_calc.module2692.submodule1 -import jsii_calc.module2692.submodule2 -import jsii_calc.module2700 -import jsii_calc.module2702 -import jsii_calc.nodirect -import jsii_calc.nodirect.sub1 -import jsii_calc.nodirect.sub2 -import jsii_calc.onlystatic -import jsii_calc.python_self -import jsii_calc.submodule -import jsii_calc.submodule.back_references -import jsii_calc.submodule.child -import jsii_calc.submodule.isolated -import jsii_calc.submodule.nested_submodule -import jsii_calc.submodule.nested_submodule.deeply_nested -import jsii_calc.submodule.param -import jsii_calc.submodule.returnsparam +from . import cdk16625 +from .cdk16625 import donotimport +from . import composition +from . import derived_class_has_no_properties +from . import interface_in_namespace_includes_classes +from . import interface_in_namespace_only_interface +from . import module2530 +from . import module2617 +from . import module2647 +from . import module2689 +from .module2689 import methods +from .module2689 import props +from .module2689 import retval +from .module2689 import structs +from . import module2692 +from .module2692 import submodule1 +from .module2692 import submodule2 +from . import module2700 +from . import module2702 +from . import nodirect +from .nodirect import sub1 +from .nodirect import sub2 +from . import onlystatic +from . import python_self +from . import submodule +from .submodule import back_references +from .submodule import child +from .submodule import isolated +from .submodule import nested_submodule +from .submodule.nested_submodule import deeply_nested +from .submodule import param +from .submodule import returnsparam `; From 3d90ae699fdcf6730d9b5559c55e78ec5f9c1260 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Thu, 18 Nov 2021 16:13:09 +0000 Subject: [PATCH 06/10] fix(jsii): require statement for the warning file is generated when it's not used (#3184) When `add-deprecation-warnings` is `true` but no calls were injected in a given file, the `require` statement for `.warnings.jsii.js` should not be generated. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- .../lib/transforms/deprecation-warnings.ts | 14 ++++++++++++- .../jsii/test/deprecation-warnings.test.ts | 20 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/jsii/lib/transforms/deprecation-warnings.ts b/packages/jsii/lib/transforms/deprecation-warnings.ts index 41c7c41318..10331757b8 100644 --- a/packages/jsii/lib/transforms/deprecation-warnings.ts +++ b/packages/jsii/lib/transforms/deprecation-warnings.ts @@ -333,6 +333,8 @@ module.exports.DeprecationError = DeprecationError; } class Transformer { + private warningCallsWereInjected = false; + public constructor( private readonly typeChecker: ts.TypeChecker, private readonly context: ts.TransformationContext, @@ -342,9 +344,11 @@ class Transformer { ) {} public transform(node: T): T { + this.warningCallsWereInjected = false; + const result = this.visitEachChild(node); - if (ts.isSourceFile(result)) { + if (ts.isSourceFile(result) && this.warningCallsWereInjected) { const importDir = path.relative( path.dirname(result.fileName), this.projectRoot, @@ -368,6 +372,8 @@ class Transformer { private visitor(node: T): ts.VisitResult { if (ts.isMethodDeclaration(node) && node.body != null) { const statements = this.getStatementsForDeclaration(node); + this.warningCallsWereInjected = + this.warningCallsWereInjected || statements.length > 0; return ts.updateMethod( node, node.decorators, @@ -385,6 +391,8 @@ class Transformer { ) as any; } else if (ts.isGetAccessorDeclaration(node) && node.body != null) { const statements = this.getStatementsForDeclaration(node); + this.warningCallsWereInjected = + this.warningCallsWereInjected || statements.length > 0; return ts.updateGetAccessor( node, node.decorators, @@ -399,6 +407,8 @@ class Transformer { ) as any; } else if (ts.isSetAccessorDeclaration(node) && node.body != null) { const statements = this.getStatementsForDeclaration(node); + this.warningCallsWereInjected = + this.warningCallsWereInjected || statements.length > 0; return ts.updateSetAccessor( node, node.decorators, @@ -412,6 +422,8 @@ class Transformer { ) as any; } else if (ts.isConstructorDeclaration(node) && node.body != null) { const statements = this.getStatementsForDeclaration(node); + this.warningCallsWereInjected = + this.warningCallsWereInjected || statements.length > 0; return ts.updateConstructor( node, node.decorators, diff --git a/packages/jsii/test/deprecation-warnings.test.ts b/packages/jsii/test/deprecation-warnings.test.ts index ded6bfc61b..44fc829e6d 100644 --- a/packages/jsii/test/deprecation-warnings.test.ts +++ b/packages/jsii/test/deprecation-warnings.test.ts @@ -388,6 +388,26 @@ describe('Call injections', () => { ); }); + test('does not generate a require statement when no calls were injected', async () => { + const result = await compileJsiiForTest( + { + 'index.ts': `export * from './some/folder/handler'`, + 'some/folder/handler.ts': ` + export function handler(event: any) { return event; } + `, + }, + undefined /* callback */, + { addDeprecationWarnings: true }, + ); + + const expectedPath = ['..', '..', '.warnings.jsii.js'].join('/'); + + const content = jsFile(result, 'some/folder/handler'); + expect(content).not.toContain( + `const jsiiDeprecationWarnings = require("${expectedPath}")`, + ); + }); + test('deprecated methods', async () => { const result = await compileJsiiForTest( ` From 5c7d148104f6cfb54573a73a76efb288dd2f346b Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 18 Nov 2021 19:05:50 +0100 Subject: [PATCH 07/10] fix(rosetta): Rosetta is not submodule-aware (#3176) Rosetta was not properly aware of submodules and namespaces, at all. This manifested in two different ways: - Imports of (Python) modules from submodules were not translated properly - Synthetically generated struct constructors were not namespaced with a proper import. The translation of a call involving a struct: ```ts import { aws_ec2 as ec2 } from 'aws-cdk'; const construct = new ec2.MyConstruct('some', 'values', { arg: { ... } }); ``` Used to be translated into this: ```py from aws_cdk import aws_ec2 as ec2 construct = ec2.MyConstruct('some', 'values', arg=DeeperStruct(...) ) ``` Notice that `DeeperStruct` is missing the module prefix that would allow this code to work. We trace what jsii FQNs every imported symbol refers to, and see if we can reuse an import that already exists to retrieve our struct. If not, we'll generate a new import at the top of the example. To properly track what is being imported in an example (if it's a class or a module, or potentially a submodule) we needed to be able to identify submodules in TypeScript source, so we've also attached a `symbolId` to `submodules` in the jsii assembly. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- packages/@jsii/spec/lib/assembly.ts | 35 +- .../@scope/jsii-calc-lib/test/assembly.jsii | 3 +- packages/jsii-calc/test/assembly.jsii | 98 ++-- .../__snapshots__/target-java.test.ts.snap | 4 +- packages/jsii-rosetta/lib/jsii/assemblies.ts | 52 +- packages/jsii-rosetta/lib/jsii/jsii-types.ts | 16 +- packages/jsii-rosetta/lib/jsii/jsii-utils.ts | 196 +++++--- packages/jsii-rosetta/lib/jsii/packages.ts | 9 +- packages/jsii-rosetta/lib/languages/csharp.ts | 129 +++-- .../jsii-rosetta/lib/languages/default.ts | 7 +- packages/jsii-rosetta/lib/languages/java.ts | 154 +++--- packages/jsii-rosetta/lib/languages/python.ts | 158 +++++- .../lib/languages/record-references.ts | 8 +- .../jsii-rosetta/lib/typescript/imports.ts | 57 ++- packages/jsii-rosetta/lib/typescript/types.ts | 9 +- packages/jsii-rosetta/lib/util.ts | 85 +++- packages/jsii-rosetta/package.json | 3 +- .../test/commands/extract.test.ts | 6 +- .../jsii-rosetta/test/commands/infuse.test.ts | 4 +- .../test/commands/transliterate.test.ts | 14 +- .../jsii-rosetta/test/jsii-imports.test.ts | 459 ++++++++++++++++++ .../jsii-rosetta/test/jsii/assemblies.test.ts | 4 +- .../test/record-references.test.ts | 4 +- .../test/rosetta-translator.test.ts | 2 + .../jsii-rosetta/test/syntax-counter.test.ts | 4 +- packages/jsii-rosetta/test/testutil.ts | 56 ++- .../jsii-rosetta/test/translations.test.ts | 19 +- packages/jsii/lib/assembler.ts | 8 + packages/jsii/lib/symbol-id.ts | 5 + packages/jsii/test/symbol-identifiers.test.ts | 41 ++ 30 files changed, 1318 insertions(+), 331 deletions(-) create mode 100644 packages/jsii-rosetta/test/jsii-imports.test.ts diff --git a/packages/@jsii/spec/lib/assembly.ts b/packages/@jsii/spec/lib/assembly.ts index a60a88d807..7fc1a165d7 100644 --- a/packages/@jsii/spec/lib/assembly.ts +++ b/packages/@jsii/spec/lib/assembly.ts @@ -204,7 +204,10 @@ export interface ReadMe { * The difference between a top-level module (the assembly) and a submodule is * that the submodule is annotated with its location in the repository. */ -export type Submodule = ReadMeContainer & SourceLocatable & Targetable; +export type Submodule = ReadMeContainer & + SourceLocatable & + Targetable & + TypeScriptLocatable; /** * Versions of the JSII Assembly Specification. @@ -440,6 +443,26 @@ export interface SourceLocatable { locationInModule?: SourceLocation; } +/** + * Indicates that a jsii entity's origin can be traced to TypeScript code + * + * This is interface is not the same as `SourceLocatable`. SourceLocatable + * identifies lines in source files in a source repository (in a `.ts` file, + * with respect to a git root). + * + * On the other hand, `TypeScriptLocatable` identifies a symbol name inside a + * potentially distributed TypeScript file (in either a `.d.ts` or `.ts` + * file, with respect to the package root). + */ +export interface TypeScriptLocatable { + /** + * Unique string representation of the corresponding Typescript symbol + * + * Used to map from TypeScript code back into the assembly. + */ + symbolId?: string; +} + /** * Kinds of collections. */ @@ -779,7 +802,10 @@ export type Type = TypeBase & (ClassType | EnumType | InterfaceType); /** * Common attributes of a type definition. */ -export interface TypeBase extends Documentable, SourceLocatable { +export interface TypeBase + extends Documentable, + SourceLocatable, + TypeScriptLocatable { /** * The fully qualified name of the type (``..``) * @@ -823,11 +849,6 @@ export interface TypeBase extends Documentable, SourceLocatable { * The kind of the type. */ kind: TypeKind; - - /** - * Unique string representation of the corresponding Typescript symbol - */ - symbolId?: string; } /** diff --git a/packages/@scope/jsii-calc-lib/test/assembly.jsii b/packages/@scope/jsii-calc-lib/test/assembly.jsii index 131d74d634..b932b84ed4 100644 --- a/packages/@scope/jsii-calc-lib/test/assembly.jsii +++ b/packages/@scope/jsii-calc-lib/test/assembly.jsii @@ -98,6 +98,7 @@ "readme": { "markdown": "# Submodule Readme\n\nThis is a submodule readme.\n" }, + "symbolId": "lib/submodule/index:", "targets": { "dotnet": { "namespace": "Amazon.JSII.Tests.CustomSubmoduleName" @@ -951,5 +952,5 @@ } }, "version": "0.0.0", - "fingerprint": "fCLOsQQLslSg+W8rn7XDZ1RSenH5QeaoTna+j7o+URc=" + "fingerprint": "BJ528BPiptJdqvxyoh4Ax2l2Knaz2KcuNYBYhVpyTw8=" } diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index 597752aca0..5d53eb9849 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -181,145 +181,169 @@ "locationInModule": { "filename": "lib/compliance.ts", "line": 325 - } + }, + "symbolId": "lib/compliance:DerivedClassHasNoProperties" }, "jsii-calc.InterfaceInNamespaceIncludesClasses": { "locationInModule": { "filename": "lib/compliance.ts", "line": 1206 - } + }, + "symbolId": "lib/compliance:InterfaceInNamespaceIncludesClasses" }, "jsii-calc.InterfaceInNamespaceOnlyInterface": { "locationInModule": { "filename": "lib/compliance.ts", "line": 1199 - } + }, + "symbolId": "lib/compliance:InterfaceInNamespaceOnlyInterface" }, "jsii-calc.PythonSelf": { "locationInModule": { "filename": "lib/compliance.ts", "line": 1090 - } + }, + "symbolId": "lib/compliance:PythonSelf" }, "jsii-calc.cdk16625": { "locationInModule": { "filename": "lib/index.ts", "line": 23 - } + }, + "symbolId": "lib/cdk16625/index:" }, "jsii-calc.cdk16625.donotimport": { "locationInModule": { "filename": "lib/cdk16625/index.ts", "line": 6 - } + }, + "symbolId": "lib/cdk16625/donotimport/index:" }, "jsii-calc.composition": { "locationInModule": { "filename": "lib/calculator.ts", "line": 143 - } + }, + "symbolId": "lib/calculator:composition" }, "jsii-calc.module2530": { "locationInModule": { "filename": "lib/index.ts", "line": 20 - } + }, + "symbolId": "lib/module2530/index:" }, "jsii-calc.module2617": { "locationInModule": { "filename": "lib/index.ts", "line": 16 - } + }, + "symbolId": "lib/module2617/index:" }, "jsii-calc.module2647": { "locationInModule": { "filename": "lib/index.ts", "line": 15 - } + }, + "symbolId": "lib/module2647/index:" }, "jsii-calc.module2689": { "locationInModule": { "filename": "lib/index.ts", "line": 17 - } + }, + "symbolId": "lib/module2689/index:" }, "jsii-calc.module2689.methods": { "locationInModule": { "filename": "lib/module2689/index.ts", "line": 8 - } + }, + "symbolId": "lib/module2689/methods/index:" }, "jsii-calc.module2689.props": { "locationInModule": { "filename": "lib/module2689/index.ts", "line": 9 - } + }, + "symbolId": "lib/module2689/props/index:" }, "jsii-calc.module2689.retval": { "locationInModule": { "filename": "lib/module2689/index.ts", "line": 10 - } + }, + "symbolId": "lib/module2689/retval/index:" }, "jsii-calc.module2689.structs": { "locationInModule": { "filename": "lib/module2689/index.ts", "line": 7 - } + }, + "symbolId": "lib/module2689/structs/index:" }, "jsii-calc.module2692": { "locationInModule": { "filename": "lib/index.ts", "line": 19 - } + }, + "symbolId": "lib/module2692/index:" }, "jsii-calc.module2692.submodule1": { "locationInModule": { "filename": "lib/module2692/index.ts", "line": 1 - } + }, + "symbolId": "lib/module2692/submodule1/index:" }, "jsii-calc.module2692.submodule2": { "locationInModule": { "filename": "lib/module2692/index.ts", "line": 2 - } + }, + "symbolId": "lib/module2692/submodule2/index:" }, "jsii-calc.module2700": { "locationInModule": { "filename": "lib/index.ts", "line": 21 - } + }, + "symbolId": "lib/module2700/index:" }, "jsii-calc.module2702": { "locationInModule": { "filename": "lib/index.ts", "line": 18 - } + }, + "symbolId": "lib/module2702/index:" }, "jsii-calc.nodirect": { "locationInModule": { "filename": "lib/index.ts", "line": 14 - } + }, + "symbolId": "lib/no-direct-types/index:" }, "jsii-calc.nodirect.sub1": { "locationInModule": { "filename": "lib/no-direct-types/index.ts", "line": 3 - } + }, + "symbolId": "lib/no-direct-types/sub1/index:" }, "jsii-calc.nodirect.sub2": { "locationInModule": { "filename": "lib/no-direct-types/index.ts", "line": 4 - } + }, + "symbolId": "lib/no-direct-types/sub2/index:" }, "jsii-calc.onlystatic": { "locationInModule": { "filename": "lib/index.ts", "line": 13 - } + }, + "symbolId": "lib/only-static/index:" }, "jsii-calc.submodule": { "locationInModule": { @@ -328,19 +352,22 @@ }, "readme": { "markdown": "Read you, read me\n=================\n\nThis is the readme of the `jsii-calc.submodule` module.\n" - } + }, + "symbolId": "lib/submodule/index:" }, "jsii-calc.submodule.back_references": { "locationInModule": { "filename": "lib/submodule/index.ts", "line": 7 - } + }, + "symbolId": "lib/submodule/refers-to-parent/index:" }, "jsii-calc.submodule.child": { "locationInModule": { "filename": "lib/submodule/index.ts", "line": 1 - } + }, + "symbolId": "lib/submodule/child/index:" }, "jsii-calc.submodule.isolated": { "locationInModule": { @@ -349,31 +376,36 @@ }, "readme": { "markdown": "Read you, read me\n=================\n\nThis is the readme of the `jsii-calc.submodule.isolated` module.\n" - } + }, + "symbolId": "lib/submodule/isolated:" }, "jsii-calc.submodule.nested_submodule": { "locationInModule": { "filename": "lib/submodule/nested_submodule.ts", "line": 4 - } + }, + "symbolId": "lib/submodule/nested_submodule:nested_submodule" }, "jsii-calc.submodule.nested_submodule.deeplyNested": { "locationInModule": { "filename": "lib/submodule/nested_submodule.ts", "line": 6 - } + }, + "symbolId": "lib/submodule/nested_submodule:nested_submodule.deeplyNested" }, "jsii-calc.submodule.param": { "locationInModule": { "filename": "lib/submodule/index.ts", "line": 3 - } + }, + "symbolId": "lib/submodule/param/index:" }, "jsii-calc.submodule.returnsparam": { "locationInModule": { "filename": "lib/submodule/index.ts", "line": 4 - } + }, + "symbolId": "lib/submodule/returns-param/index:" } }, "targets": { @@ -16775,5 +16807,5 @@ } }, "version": "3.20.120", - "fingerprint": "WpcHNV2L+v3B7Ou+8xsRMzye4rf1U+SURQA97A3JT6g=" + "fingerprint": "wBGhJE4ZhvVb8R2qk5FWg49mt+ZKqW60TjlMMtT1Klo=" } diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap index 313cb7c0e0..ad5c61affa 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap @@ -5626,7 +5626,7 @@ package software.amazon.jsii.tests.calculator; * Calculator calculator = new Calculator(); * calculator.add(5); * calculator.mul(3); - * System.out.println(calculator.expression.getValue()); + * System.out.println(calculator.getExpression().getValue()); * *

* I will repeat this example again, but in an @example tag. @@ -5637,7 +5637,7 @@ package software.amazon.jsii.tests.calculator; * Calculator calculator = new Calculator(); * calculator.add(5); * calculator.mul(3); - * System.out.println(calculator.expression.getValue()); + * System.out.println(calculator.getExpression().getValue()); * */ @javax.annotation.Generated(value = "jsii-pacmak") diff --git a/packages/jsii-rosetta/lib/jsii/assemblies.ts b/packages/jsii-rosetta/lib/jsii/assemblies.ts index 0f020a21ad..07b00993b0 100644 --- a/packages/jsii-rosetta/lib/jsii/assemblies.ts +++ b/packages/jsii-rosetta/lib/jsii/assemblies.ts @@ -13,7 +13,7 @@ import { ApiLocation, } from '../snippet'; import { enforcesStrictMode } from '../strict'; -import { mkDict } from '../util'; +import { mkDict, sortBy } from '../util'; // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports const sortJson = require('sort-json'); @@ -190,8 +190,9 @@ function _fingerprint(assembly: spec.Assembly): spec.Assembly { } export interface TypeLookupAssembly { + readonly packageJson: any; readonly assembly: spec.Assembly; - readonly assemblyFile: string; + readonly directory: string; readonly symbolIdMap: Record; } @@ -204,24 +205,23 @@ const ASM_CACHE: TypeLookupAssembly[] = []; * stored the assembly in memory. If not, we synchronously * load the assembly into memory. */ -export function findTypeLookupAssembly(directory: string): TypeLookupAssembly | undefined { - const pjLocation = findPackageJsonLocation(path.resolve(directory)); +export function findTypeLookupAssembly(startingDirectory: string): TypeLookupAssembly | undefined { + const pjLocation = findPackageJsonLocation(path.resolve(startingDirectory)); if (!pjLocation) { return undefined; } + const directory = path.dirname(pjLocation); - const assemblyFile = path.join(path.dirname(pjLocation), '.jsii'); - - const fromCache = ASM_CACHE.find((c) => c.assemblyFile === assemblyFile); + const fromCache = ASM_CACHE.find((c) => c.directory === directory); if (fromCache) { return fromCache; } - if (!fs.existsSync(assemblyFile)) { + const loaded = loadLookupAssembly(directory); + if (!loaded) { return undefined; } - const loaded = loadLookupAssembly(assemblyFile); while (ASM_CACHE.length >= MAX_ASM_CACHE) { ASM_CACHE.pop(); } @@ -229,15 +229,23 @@ export function findTypeLookupAssembly(directory: string): TypeLookupAssembly | return loaded; } -function loadLookupAssembly(assemblyFile: string): TypeLookupAssembly { +function loadLookupAssembly(directory: string): TypeLookupAssembly | undefined { + const assemblyFile = path.join(directory, '.jsii'); + if (!fs.pathExistsSync(assemblyFile)) { + return undefined; + } + + const packageJson = fs.readJSONSync(path.join(directory, 'package.json'), { encoding: 'utf-8' }); const assembly: spec.Assembly = fs.readJSONSync(assemblyFile, { encoding: 'utf-8' }); - const symbolIdMap = mkDict( - Object.values(assembly.types ?? {}).map((type) => [type.symbolId ?? '', type.fqn] as const), - ); + const symbolIdMap = mkDict([ + ...Object.values(assembly.types ?? {}).map((type) => [type.symbolId ?? '', type.fqn] as const), + ...Object.entries(assembly.submodules ?? {}).map(([fqn, mod]) => [mod.symbolId ?? '', fqn] as const), + ]); return { + packageJson, assembly, - assemblyFile, + directory, symbolIdMap, }; } @@ -257,3 +265,19 @@ function findPackageJsonLocation(currentPath: string): string | undefined { currentPath = parentPath; } } + +/** + * Find the jsii [sub]module that contains the given FQN + * + * @returns `undefined` if the type is a member of the assembly root. + */ +export function findContainingSubmodule(assembly: spec.Assembly, fqn: string): string | undefined { + const submoduleNames = Object.keys(assembly.submodules ?? {}); + sortBy(submoduleNames, (s) => [-s.length]); // Longest first + for (const s of submoduleNames) { + if (fqn.startsWith(`${s}.`)) { + return s; + } + } + return undefined; +} diff --git a/packages/jsii-rosetta/lib/jsii/jsii-types.ts b/packages/jsii-rosetta/lib/jsii/jsii-types.ts index a3345cc590..36302bdb7d 100644 --- a/packages/jsii-rosetta/lib/jsii/jsii-types.ts +++ b/packages/jsii-rosetta/lib/jsii/jsii-types.ts @@ -1,7 +1,7 @@ import * as ts from 'typescript'; import { inferredTypeOfExpression, BuiltInType, builtInTypeName, mapElementType } from '../typescript/types'; -import { hasAnyFlag, analyzeStructType } from './jsii-utils'; +import { hasAnyFlag, analyzeStructType, JsiiSymbol } from './jsii-utils'; // eslint-disable-next-line prettier/prettier export type JsiiType = @@ -69,11 +69,11 @@ export function determineJsiiType(typeChecker: ts.TypeChecker, type: ts.Type): J return { kind: 'unknown' }; } -export type ObjectLiteralAnalysis = - | { readonly kind: 'struct'; readonly type: ts.Type } - | { readonly kind: 'local-struct'; readonly type: ts.Type } - | { readonly kind: 'map' } - | { readonly kind: 'unknown' }; +export type ObjectLiteralAnalysis = ObjectLiteralStruct | { readonly kind: 'map' } | { readonly kind: 'unknown' }; + +export type ObjectLiteralStruct = + | { readonly kind: 'struct'; readonly type: ts.Type; readonly jsiiSym: JsiiSymbol } + | { readonly kind: 'local-struct'; readonly type: ts.Type }; export function analyzeObjectLiteral( typeChecker: ts.TypeChecker, @@ -100,9 +100,9 @@ export function analyzeObjectLiteral( // If the type is a union between a struct and something else, return the first possible struct const structCandidates = type.isUnion() ? type.types : [type]; for (const candidate of structCandidates) { - const structType = analyzeStructType(candidate); + const structType = analyzeStructType(typeChecker, candidate); if (structType) { - return { kind: structType, type: candidate }; + return structType; } } diff --git a/packages/jsii-rosetta/lib/jsii/jsii-utils.ts b/packages/jsii-rosetta/lib/jsii/jsii-utils.ts index d9a5d9b23b..e6f530ee8e 100644 --- a/packages/jsii-rosetta/lib/jsii/jsii-utils.ts +++ b/packages/jsii-rosetta/lib/jsii/jsii-utils.ts @@ -3,15 +3,16 @@ import * as ts from 'typescript'; import { AstRenderer } from '../renderer'; import { typeContainsUndefined } from '../typescript/types'; -import { findTypeLookupAssembly } from './assemblies'; -import { findPackageJson } from './packages'; +import { fmap } from '../util'; +import { findTypeLookupAssembly, TypeLookupAssembly } from './assemblies'; +import { ObjectLiteralStruct } from './jsii-types'; export function isNamedLikeStruct(name: string) { // Start with an I and another uppercase character return !/^I[A-Z]/.test(name); } -export function analyzeStructType(type: ts.Type): 'struct' | 'local-struct' | false { +export function analyzeStructType(typeChecker: ts.TypeChecker, type: ts.Type): ObjectLiteralStruct | false { if ( !type.isClassOrInterface() || !hasAllFlags(type.objectFlags, ts.ObjectFlags.Interface) || @@ -20,11 +21,12 @@ export function analyzeStructType(type: ts.Type): 'struct' | 'local-struct' | fa return false; } - if (refersToJsiiSymbol(type.symbol)) { - return 'struct'; + const jsiiSym = lookupJsiiSymbol(typeChecker, type.symbol); + if (jsiiSym) { + return { kind: 'struct', type, jsiiSym }; } - return 'local-struct'; + return { kind: 'local-struct', type }; } export function hasAllFlags(flags: A, test: A) { @@ -70,32 +72,32 @@ export function structPropertyAcceptsUndefined(prop: StructProperty): boolean { } /** - * Whether or not the given call expression seems to refer to a jsii symbol - * - * If it does, we treat it differently than if it's a class or symbol defined - * in the same example source. - * - * To do this, we look for whether it's defined in a directory that's compiled - * for jsii and has a jsii assembly. - * - * FIXME: Look up the actual symbol identifier when we finally have those. - * - * For tests, we also treat symbols in a file that has the string '/// fake-from-jsii' - * as coming from jsii. + * A TypeScript symbol resolved to its jsii type */ -export function refersToJsiiSymbol(symbol: ts.Symbol): boolean { - const declaration = symbol.declarations[0]; - if (!declaration) { - return false; - } +export interface JsiiSymbol { + /** + * FQN of the symbol + * + * Is either the FQN of a type (for a type). For a membr, the FQN looks like: + * 'type.fqn#memberName'. + */ + readonly fqn: string; - const declaringFile = declaration.getSourceFile(); - if (/^\/\/\/ fake-from-jsii/m.test(declaringFile.getFullText())) { - return true; - } + /** + * What kind of symbol this is + */ + readonly symbolType: 'module' | 'type' | 'member'; - const pj = findPackageJson(declaringFile.fileName); - return !!(pj && pj.jsii); + /** + * Assembly where the type was found + * + * Might be undefined if the type was FAKE from jsii (for tests) + */ + readonly sourceAssembly?: TypeLookupAssembly; +} + +export function lookupJsiiSymbolFromNode(typeChecker: ts.TypeChecker, node: ts.Node): JsiiSymbol | undefined { + return fmap(typeChecker.getSymbolAtLocation(node), (s) => lookupJsiiSymbol(typeChecker, s)); } /** @@ -107,13 +109,48 @@ export function refersToJsiiSymbol(symbol: ts.Symbol): boolean { * 1. The package name (extracted from the nearest `package.json`) * 2. The submodule name (...?? don't know how to get this yet) * 3. Any containing type names or namespace names. + * + * For tests, we also treat symbols in a file that has the string '/// fake-from-jsii' + * as coming from jsii. */ -export function jsiiFqnFromSymbol(typeChecker: ts.TypeChecker, sym: ts.Symbol): string | undefined { +export function lookupJsiiSymbol(typeChecker: ts.TypeChecker, sym: ts.Symbol): JsiiSymbol | undefined { + // Resolve alias, if it is one. This comes into play if the symbol refers to a module, + // we need to resolve the alias to find the ACTUAL module. + if (hasAnyFlag(sym.flags, ts.SymbolFlags.Alias)) { + sym = typeChecker.getAliasedSymbol(sym); + } + const decl: ts.Node | undefined = sym.declarations?.[0]; - if (!decl || !isDeclaration(decl)) { + if (!decl) { return undefined; } + if (ts.isSourceFile(decl)) { + // This is a module. + // FIXME: for now assume this is the assembly root. Handle the case where it isn't later. + const sourceAssembly = findTypeLookupAssembly(decl.fileName); + return fmap( + sourceAssembly, + (asm) => + ({ + fqn: + fmap(symbolIdentifier(typeChecker, sym), (symbolId) => sourceAssembly?.symbolIdMap[symbolId]) ?? + sourceAssembly?.assembly.name, + sourceAssembly: asm, + symbolType: 'module', + } as JsiiSymbol), + ); + } + + if (!isDeclaration(decl)) { + return undefined; + } + + const declaringFile = decl.getSourceFile(); + if (/^\/\/\/ fake-from-jsii/m.test(declaringFile.getFullText())) { + return { fqn: `fake_jsii.${sym.name}`, symbolType: 'type' }; + } + const declSym = getSymbolFromDeclaration(decl, typeChecker); if (!declSym) { return undefined; @@ -121,29 +158,49 @@ export function jsiiFqnFromSymbol(typeChecker: ts.TypeChecker, sym: ts.Symbol): const fileName = decl.getSourceFile().fileName; if (hasAnyFlag(declSym.flags, ts.SymbolFlags.Method | ts.SymbolFlags.Property | ts.SymbolFlags.EnumMember)) { - return fqnFromMemberSymbol(typeChecker, sym, fileName); + return lookupMemberSymbol(typeChecker, sym, fileName); } - return fqnFromTypeSymbol(typeChecker, sym, fileName); + return lookupTypeSymbol(typeChecker, sym, fileName); +} + +function isDeclaration(x: ts.Node): x is ts.Declaration { + return ( + ts.isClassDeclaration(x) || + ts.isNamespaceExportDeclaration(x) || + ts.isNamespaceExport(x) || + ts.isModuleDeclaration(x) || + ts.isEnumDeclaration(x) || + ts.isEnumMember(x) || + ts.isInterfaceDeclaration(x) || + ts.isMethodDeclaration(x) || + ts.isMethodSignature(x) || + ts.isPropertyDeclaration(x) || + ts.isPropertySignature(x) + ); } /** * Look up the jsii fqn for a given type symbol */ -function fqnFromTypeSymbol(typeChecker: ts.TypeChecker, typeSymbol: ts.Symbol, fileName: string): string | undefined { +function lookupTypeSymbol( + typeChecker: ts.TypeChecker, + typeSymbol: ts.Symbol, + fileName: string, +): JsiiSymbol | undefined { const symbolId = symbolIdentifier(typeChecker, typeSymbol); if (!symbolId) { return undefined; } - const assembly = findTypeLookupAssembly(fileName); - return assembly?.symbolIdMap[symbolId]; + const sourceAssembly = findTypeLookupAssembly(fileName); + return fmap(sourceAssembly?.symbolIdMap[symbolId], (fqn) => ({ fqn, sourceAssembly, symbolType: 'type' })); } -function fqnFromMemberSymbol( +function lookupMemberSymbol( typeChecker: ts.TypeChecker, memberSymbol: ts.Symbol, fileName: string, -): string | undefined { +): JsiiSymbol | undefined { const declParent = memberSymbol.declarations?.[0]?.parent; if (!declParent || !isDeclaration(declParent)) { return undefined; @@ -154,24 +211,28 @@ function fqnFromMemberSymbol( return undefined; } - const result = fqnFromTypeSymbol(typeChecker, declParentSym, fileName); - return result ? `${result}#${memberSymbol.name}` : undefined; + const result = lookupTypeSymbol(typeChecker, declParentSym, fileName); + return fmap(result, (result) => ({ ...result, fqn: `${result.fqn}#${memberSymbol.name}`, symbolType: 'member' })); } -function isDeclaration(x: ts.Node): x is ts.Declaration { - return ( - ts.isClassDeclaration(x) || - ts.isNamespaceExportDeclaration(x) || - ts.isNamespaceExport(x) || - ts.isModuleDeclaration(x) || - ts.isEnumDeclaration(x) || - ts.isEnumMember(x) || - ts.isInterfaceDeclaration(x) || - ts.isMethodDeclaration(x) || - ts.isMethodSignature(x) || - ts.isPropertyDeclaration(x) || - ts.isPropertySignature(x) - ); +/** + * If the given type is an enum literal, resolve to the enum type + */ +export function resolveEnumLiteral(typeChecker: ts.TypeChecker, type: ts.Type) { + if (!hasAnyFlag(type.flags, ts.TypeFlags.EnumLiteral)) { + return type; + } + + const parentDeclaration = type.symbol.declarations?.[0]?.parent; + return fmap(parentDeclaration, typeChecker.getTypeAtLocation) ?? type; +} + +export function resolvedSymbolAtLocation(typeChecker: ts.TypeChecker, node: ts.Node) { + let symbol = typeChecker.getSymbolAtLocation(node); + while (symbol && hasAnyFlag(symbol.flags, ts.SymbolFlags.Alias)) { + symbol = typeChecker.getAliasedSymbol(symbol); + } + return symbol; } function getSymbolFromDeclaration(decl: ts.Node, typeChecker: ts.TypeChecker): ts.Symbol | undefined { @@ -182,3 +243,30 @@ function getSymbolFromDeclaration(decl: ts.Node, typeChecker: ts.TypeChecker): t const name = ts.getNameOfDeclaration(decl); return name ? typeChecker.getSymbolAtLocation(name) : undefined; } + +export function parentSymbol(sym: JsiiSymbol): JsiiSymbol | undefined { + const parts = sym.fqn.split('.'); + if (parts.length === 1) { + return undefined; + } + + return { + fqn: parts.slice(0, -1).join('.'), + symbolType: 'module', // Might not be true, but probably good enough + sourceAssembly: sym.sourceAssembly, + }; +} + +/** + * Get the last part of a dot-separated string + */ +export function simpleName(x: string) { + return x.split('.').slice(-1)[0]; +} + +/** + * Get all parts except the last of a dot-separated string + */ +export function namespaceName(x: string) { + return x.split('.').slice(0, -1).join('.'); +} diff --git a/packages/jsii-rosetta/lib/jsii/packages.ts b/packages/jsii-rosetta/lib/jsii/packages.ts index eb07c5475f..9aa5e2ec61 100644 --- a/packages/jsii-rosetta/lib/jsii/packages.ts +++ b/packages/jsii-rosetta/lib/jsii/packages.ts @@ -1,3 +1,4 @@ +import * as spec from '@jsii/spec'; import * as fs from 'fs'; import * as path from 'path'; @@ -40,11 +41,9 @@ export function findPackageJson(fileName: string) { } } -export function jsiiTargetParam(packageName: string, field: string) { - const pkgJson = resolvePackage(packageName); - - const path = ['jsii', 'targets', ...field.split('.')]; - let r = pkgJson; +export function jsiiTargetParameter(target: spec.Targetable, field: string) { + const path = field.split('.'); + let r: any = target.targets; while (path.length > 0 && typeof r === 'object' && r !== null) { r = r[path.splice(0, 1)[0]]; } diff --git a/packages/jsii-rosetta/lib/languages/csharp.ts b/packages/jsii-rosetta/lib/languages/csharp.ts index 813e3c1843..b31daf9d39 100644 --- a/packages/jsii-rosetta/lib/languages/csharp.ts +++ b/packages/jsii-rosetta/lib/languages/csharp.ts @@ -1,7 +1,8 @@ import * as ts from 'typescript'; -import { determineJsiiType, JsiiType } from '../jsii/jsii-types'; -import { jsiiTargetParam } from '../jsii/packages'; +import { determineJsiiType, JsiiType, ObjectLiteralStruct } from '../jsii/jsii-types'; +import { JsiiSymbol, simpleName, namespaceName } from '../jsii/jsii-utils'; +import { jsiiTargetParameter } from '../jsii/packages'; import { OTree, NO_SYNTAX } from '../o-tree'; import { AstRenderer, nimpl } from '../renderer'; import { @@ -21,7 +22,7 @@ import { inferMapElementType, determineReturnType, } from '../typescript/types'; -import { flat, partition, setExtend } from '../util'; +import { flat, fmap } from '../util'; import { DefaultVisitor } from './default'; import { TargetLanguage } from './target-language'; @@ -96,14 +97,21 @@ export class CSharpVisitor extends DefaultVisitor { * * If these are encountered in the LHS of a property access, they will be dropped. */ - private readonly importedModuleAliases = new Set(); + private readonly dropPropertyAccesses = new Set(); /** - * Elements imported into current namespace + * Already imported modules so we don't emit duplicate imports + */ + private readonly alreadyImportedNamespaces = new Set(); + + /** + * A map to undo import renames + * + * We will always reference the original name in the translation. * - * All namespace elements that can be imported need to be uppercased. + * Maps a local-name to a C# name. */ - private readonly importedModuleSymbols = new Set(); + private readonly renamedSymbols = new Map(); public mergeContext(old: CSharpLanguageContext, update: Partial): CSharpLanguageContext { return Object.assign({}, old, update); @@ -128,28 +136,42 @@ export class CSharpVisitor extends DefaultVisitor { } public importStatement(importStatement: ImportStatement, context: CSharpRenderer): OTree { - const namespace = this.lookupModuleNamespace(importStatement.packageName); + const guessedNamespace = guessDotnetNamespace(importStatement.packageName); + const namespace = fmap(importStatement.moduleSymbol, findDotnetName) ?? guessedNamespace; + if (importStatement.imports.import === 'full') { - this.importedModuleAliases.add(importStatement.imports.alias); + this.dropPropertyAccesses.add(importStatement.imports.alias); + this.alreadyImportedNamespaces.add(namespace); return new OTree([`using ${namespace};`], [], { canBreakLine: true }); } if (importStatement.imports.import === 'selective') { - const statements = []; - const [withoutAlias, withAlias] = partition(importStatement.imports.elements, (im) => im.alias === undefined); - - // If there's at least one import without an alias, emit a namespace import. - if (withoutAlias) { - statements.push(`using ${namespace};`); - setExtend( - this.importedModuleSymbols, - withoutAlias.map((w) => w.sourceName), - ); - } - - // For every aliased import, emit an aliasing 'using' statement - for (const aliasedImport of withAlias) { - statements.push(`using ${ucFirst(aliasedImport.alias!)} = ${namespace}.${ucFirst(aliasedImport.sourceName)};`); - this.importedModuleSymbols.add(aliasedImport.alias!); + const statements = new Array(); + + for (const el of importStatement.imports.elements) { + const dotnetNs = fmap(el.importedSymbol, findDotnetName) ?? `${guessedNamespace}.${ucFirst(el.sourceName)}`; + + // If this is an alias, we only honor it if it's NOT for sure a module + // (could be an alias import of a class or enum). + if (el.alias && el.importedSymbol?.symbolType !== 'module') { + this.renamedSymbols.set(el.alias, simpleName(dotnetNs)); + statements.push(`using ${ucFirst(el.alias)} = ${dotnetNs};`); + continue; + } + + // If we are importing a module directly, drop the occurrences of that + // identifier further down (turn `mod.MyClass` into `MyClass`). + if (el.importedSymbol?.symbolType === 'module') { + this.dropPropertyAccesses.add(el.alias ?? el.sourceName); + } + + // Output an import statement for the containing namespace + const importableNamespace = el.importedSymbol?.symbolType === 'module' ? dotnetNs : namespaceName(dotnetNs); + if (this.alreadyImportedNamespaces.has(importableNamespace)) { + continue; + } + + this.alreadyImportedNamespaces.add(importableNamespace); + statements.push(`using ${importableNamespace};`); } return new OTree([], statements, { canBreakLine: true, separator: '\n' }); @@ -316,7 +338,7 @@ export class CSharpVisitor extends DefaultVisitor { // Suppress the LHS of the dot operator if it's "this." (not necessary in C#) // or if it's an imported module reference (C# has namespace-wide imports). const objectExpression = - lhs === 'this' || this.importedModuleAliases.has(lhs) + lhs === 'this' || this.dropPropertyAccesses.has(lhs) ? [] : [renderer.updateContext({ propertyOrMethod: false }).convert(node.expression), '.']; @@ -426,11 +448,10 @@ export class CSharpVisitor extends DefaultVisitor { public knownStructObjectLiteralExpression( node: ts.ObjectLiteralExpression, - structType: ts.Type, - _definedInExample: boolean, + structType: ObjectLiteralStruct, renderer: CSharpRenderer, ): OTree { - return new OTree(['new ', structType.symbol.name, ' { '], renderer.convertAll(node.properties), { + return new OTree(['new ', structType.type.symbol.name, ' { '], renderer.convertAll(node.properties), { suffix: renderer.mirrorNewlineBefore(node.properties[0], '}', ' '), separator: ', ', indent: 4, @@ -600,21 +621,6 @@ export class CSharpVisitor extends DefaultVisitor { }); } - protected lookupModuleNamespace(ref: string) { - // Get the .NET namespace from the referenced package (if available) - const resolvedNamespace = jsiiTargetParam(ref, 'dotnet.namespace'); - - // Return that or some default-derived module name representation - return ( - resolvedNamespace || - ref - .split(/[^a-zA-Z0-9]+/g) - .filter((s) => s !== '') - .map(ucFirst) - .join('.') - ); - } - private renderTypeNode(typeNode: ts.TypeNode | undefined, questionMark: boolean, renderer: CSharpRenderer): string { if (!typeNode) { return 'void'; @@ -683,3 +689,38 @@ export class CSharpVisitor extends DefaultVisitor { function ucFirst(x: string) { return x.substr(0, 1).toUpperCase() + x.substr(1); } + +/** + * Find the Java name of a module or type + */ +function findDotnetName(jsiiSymbol: JsiiSymbol): string | undefined { + if (!jsiiSymbol.sourceAssembly?.assembly) { + // Don't have accurate info, just guess + return jsiiSymbol.symbolType !== 'module' ? simpleName(jsiiSymbol.fqn) : guessDotnetNamespace(jsiiSymbol.fqn); + } + + const asm = jsiiSymbol.sourceAssembly?.assembly; + return recurse(jsiiSymbol.fqn); + + function recurse(fqn: string): string { + if (fqn === asm.name) { + return jsiiTargetParameter(asm, 'dotnet.namespace') ?? guessDotnetNamespace(fqn); + } + if (asm.submodules?.[fqn]) { + const modName = jsiiTargetParameter(asm.submodules[fqn], 'dotnet.namespace'); + if (modName) { + return modName; + } + } + + return `${recurse(namespaceName(fqn))}.${simpleName(jsiiSymbol.fqn)}`; + } +} + +function guessDotnetNamespace(ref: string) { + return ref + .split(/[^a-zA-Z0-9]+/g) + .filter((s) => s !== '') + .map(ucFirst) + .join('.'); +} diff --git a/packages/jsii-rosetta/lib/languages/default.ts b/packages/jsii-rosetta/lib/languages/default.ts index 319c26a997..dc6a83fd2f 100644 --- a/packages/jsii-rosetta/lib/languages/default.ts +++ b/packages/jsii-rosetta/lib/languages/default.ts @@ -1,6 +1,6 @@ import * as ts from 'typescript'; -import { analyzeObjectLiteral } from '../jsii/jsii-types'; +import { analyzeObjectLiteral, ObjectLiteralStruct } from '../jsii/jsii-types'; import { isNamedLikeStruct } from '../jsii/jsii-utils'; import { OTree, NO_SYNTAX } from '../o-tree'; import { AstRenderer, AstHandler, nimpl, CommentSyntax } from '../renderer'; @@ -160,7 +160,7 @@ export abstract class DefaultVisitor implements AstHandler { return this.unknownTypeObjectLiteralExpression(node, context); case 'struct': case 'local-struct': - return this.knownStructObjectLiteralExpression(node, lit.type, lit.kind === 'local-struct', context); + return this.knownStructObjectLiteralExpression(node, lit, context); case 'map': return this.keyValueObjectLiteralExpression(node, context); } @@ -172,8 +172,7 @@ export abstract class DefaultVisitor implements AstHandler { public knownStructObjectLiteralExpression( node: ts.ObjectLiteralExpression, - _structType: ts.Type, - _definedInExample: boolean, + _structType: ObjectLiteralStruct, context: AstRenderer, ): OTree { return this.notImplemented(node, context); diff --git a/packages/jsii-rosetta/lib/languages/java.ts b/packages/jsii-rosetta/lib/languages/java.ts index 22ff74db03..97b4488980 100644 --- a/packages/jsii-rosetta/lib/languages/java.ts +++ b/packages/jsii-rosetta/lib/languages/java.ts @@ -1,23 +1,27 @@ import * as ts from 'typescript'; -import { determineJsiiType, JsiiType, analyzeObjectLiteral } from '../jsii/jsii-types'; -import { jsiiTargetParam } from '../jsii/packages'; +import { determineJsiiType, JsiiType, analyzeObjectLiteral, ObjectLiteralStruct } from '../jsii/jsii-types'; +import { JsiiSymbol, simpleName, namespaceName } from '../jsii/jsii-utils'; +import { jsiiTargetParameter } from '../jsii/packages'; import { TargetLanguage } from '../languages/target-language'; import { OTree, NO_SYNTAX } from '../o-tree'; import { AstRenderer } from '../renderer'; import { isReadOnly, matchAst, nodeOfType, quoteStringLiteral, visibility } from '../typescript/ast-utils'; import { ImportStatement } from '../typescript/imports'; import { isEnumAccess, isStaticReadonlyAccess, determineReturnType } from '../typescript/types'; +import { fmap, setExtend } from '../util'; import { DefaultVisitor } from './default'; interface JavaContext { /** * Whether to ignore the left-hand part of a property access expression. - * Used to strip out TypeScript namespace prefixes from 'extends' and 'new' clauses. + * + * Used to strip out TypeScript namespace prefixes from 'extends' and 'new' clauses, + * EVEN if the source doesn't compile. * * @default false */ - readonly ignorePropertyPrefix?: boolean; + readonly discardPropertyAccess?: boolean; /** * Whether a property access ('sth.b') should be substituted by a getter ('sth.getB()'). @@ -101,6 +105,13 @@ export class JavaVisitor extends DefaultVisitor { */ public static readonly VERSION = '1'; + /** + * Aliases for modules + * + * If these are encountered in the LHS of a property access, they will be dropped. + */ + private readonly dropPropertyAccesses = new Set(); + public readonly language = TargetLanguage.JAVA; public readonly defaultContext = {}; @@ -109,15 +120,27 @@ export class JavaVisitor extends DefaultVisitor { } public importStatement(importStatement: ImportStatement): OTree { - const namespace = this.lookupModuleNamespace(importStatement.packageName); + const guessedNamespace = guessJavaNamespaceName(importStatement.packageName); + if (importStatement.imports.import === 'full') { + this.dropPropertyAccesses.add(importStatement.imports.alias); + const namespace = fmap(importStatement.moduleSymbol, findJavaName) ?? guessedNamespace; + return new OTree([`import ${namespace}.*;`], [], { canBreakLine: true }); } - return new OTree( - [], - importStatement.imports.elements.map((importEl) => `import ${namespace}.${importEl.sourceName};`), - { canBreakLine: true, separator: '\n' }, - ); + + const imports = importStatement.imports.elements.map((e) => { + const fqn = fmap(e.importedSymbol, findJavaName) ?? `${guessedNamespace}.${e.sourceName}`; + + return e.importedSymbol?.symbolType === 'module' ? `import ${fqn}.*;` : `import ${fqn};`; + }); + + const localNames = importStatement.imports.elements + .filter((el) => el.importedSymbol?.symbolType === 'module') + .map((el) => el.alias ?? el.sourceName); + setExtend(this.dropPropertyAccesses, localNames); + + return new OTree([], imports, { canBreakLine: true, separator: '\n' }); } public classDeclaration(node: ts.ClassDeclaration, renderer: JavaRenderer): OTree { @@ -141,7 +164,7 @@ export class JavaVisitor extends DefaultVisitor { 'public ', 'interface ', renderer.convert(node.name), - ...this.typeHeritage(node, renderer.updateContext({ ignorePropertyPrefix: true })), + ...this.typeHeritage(node, renderer.updateContext({ discardPropertyAccess: true })), ' {', ], renderer @@ -445,7 +468,7 @@ export class JavaVisitor extends DefaultVisitor { const className = renderer .updateContext({ - ignorePropertyPrefix: true, + discardPropertyAccess: true, convertPropertyToGetter: false, }) .convert(node.expression); @@ -487,8 +510,7 @@ export class JavaVisitor extends DefaultVisitor { public knownStructObjectLiteralExpression( node: ts.ObjectLiteralExpression, - structType: ts.Type, - definedInExample: boolean, + structType: ObjectLiteralStruct, renderer: JavaRenderer, ): OTree { // Special case: we're rendering an object literal, but the containing constructor @@ -500,10 +522,10 @@ export class JavaVisitor extends DefaultVisitor { // Jsii-generated classes have builders, classes we generated in the course of // this example do not. - const hasBuilder = !definedInExample; + const hasBuilder = structType.kind !== 'local-struct'; return new OTree( - hasBuilder ? [structType.symbol.name, '.builder()'] : ['new ', structType.symbol.name, '()'], + hasBuilder ? [structType.type.symbol.name, '.builder()'] : ['new ', structType.type.symbol.name, '()'], [ ...renderer.convertAll(node.properties), new OTree([renderer.mirrorNewlineBefore(node.properties[0])], [hasBuilder ? '.build()' : '']), @@ -530,34 +552,30 @@ export class JavaVisitor extends DefaultVisitor { const rightHandSide = renderer.convert(node.name); let parts: Array; - if (renderer.currentContext.ignorePropertyPrefix) { - // ignore al prefixes when resolving properties - // only used for type names, in things like - // 'MyClass extends cdk.Construct' - // and 'new' expressions + const leftHandSide = renderer.textOf(node.expression); + + // Suppress the LHS of the dot operator if it matches an alias for a module import. + if (this.dropPropertyAccesses.has(leftHandSide) || renderer.currentContext.discardPropertyAccess) { parts = [rightHandSide]; + } else if (leftHandSide === 'this') { + // for 'this', assume this is a field, and access it directly + parts = ['this', '.', rightHandSide]; } else { - const leftHandSide = renderer.textOf(node.expression); - if (leftHandSide === 'this') { - // for 'this', assume this is a field, and access it directly - parts = ['this', '.', rightHandSide]; - } else { - let convertToGetter = renderer.currentContext.convertPropertyToGetter !== false; - - // See if we're not accessing an enum member or public static readonly property (const). - if (isEnumAccess(renderer.typeChecker, node)) { - convertToGetter = false; - } - if (isStaticReadonlyAccess(renderer.typeChecker, node)) { - convertToGetter = false; - } - - // add a 'get' prefix to the property name, and change the access to a method call, if required - const renderedRightHandSide = convertToGetter ? `get${capitalize(node.name.text)}()` : rightHandSide; - - // strip any trailing ! from the left-hand side, as they're not meaningful in Java - parts = [stripTrailingBang(leftHandSide), '.', renderedRightHandSide]; + let convertToGetter = renderer.currentContext.convertPropertyToGetter !== false; + + // See if we're not accessing an enum member or public static readonly property (const). + if (isEnumAccess(renderer.typeChecker, node)) { + convertToGetter = false; + } + if (isStaticReadonlyAccess(renderer.typeChecker, node)) { + convertToGetter = false; } + + // add a 'get' prefix to the property name, and change the access to a method call, if required + const renderedRightHandSide = convertToGetter ? `get${capitalize(node.name.text)}()` : rightHandSide; + + // strip any trailing ! from the left-hand side, as they're not meaningful in Java + parts = [renderer.convert(node.expression), '.', renderedRightHandSide]; } return new OTree(parts); @@ -634,27 +652,13 @@ export class JavaVisitor extends DefaultVisitor { ); } - private lookupModuleNamespace(packageName: string): string { - // get the Java package name from the referenced package (if available) - const resolvedNamespace = jsiiTargetParam(packageName, 'java.package'); - - // return that or some default-derived module name representation - return ( - resolvedNamespace || - packageName - .split(/[^a-zA-Z0-9]+/g) - .filter((s) => s !== '') - .join('.') - ); - } - private renderClassDeclaration(node: ts.ClassDeclaration | ts.InterfaceDeclaration, renderer: JavaRenderer) { return new OTree( [ 'public ', 'class ', renderer.convert(node.name), - ...this.typeHeritage(node, renderer.updateContext({ ignorePropertyPrefix: true })), + ...this.typeHeritage(node, renderer.updateContext({ discardPropertyAccess: true })), ' {', ], renderer.updateContext({ insideTypeDeclaration: { typeName: node.name } }).convertAll(node.members), @@ -825,10 +829,6 @@ export class JavaVisitor extends DefaultVisitor { } } -function stripTrailingBang(str: string): string { - return str.replace(/!+$/, ''); -} - function capitalize(str: string): string { return str.charAt(0).toUpperCase() + str.slice(1); } @@ -836,3 +836,37 @@ function capitalize(str: string): string { function lastElement(strings: string[]): string { return strings[strings.length - 1]; } + +/** + * Find the Java name of a module or type + */ +function findJavaName(jsiiSymbol: JsiiSymbol): string | undefined { + if (!jsiiSymbol.sourceAssembly?.assembly) { + // Don't have accurate info, just guess + return jsiiSymbol.symbolType !== 'module' ? simpleName(jsiiSymbol.fqn) : guessJavaNamespaceName(jsiiSymbol.fqn); + } + + const asm = jsiiSymbol.sourceAssembly?.assembly; + return recurse(jsiiSymbol.fqn); + + function recurse(fqn: string): string { + if (fqn === asm.name) { + return jsiiTargetParameter(asm, 'java.package') ?? guessJavaNamespaceName(fqn); + } + if (asm.submodules?.[fqn]) { + const modName = jsiiTargetParameter(asm.submodules[fqn], 'java.package'); + if (modName) { + return modName; + } + } + + return `${recurse(namespaceName(fqn))}.${simpleName(jsiiSymbol.fqn)}`; + } +} + +function guessJavaNamespaceName(packageName: string) { + return packageName + .split(/[^a-zA-Z0-9]+/g) + .filter((s) => s !== '') + .join('.'); +} diff --git a/packages/jsii-rosetta/lib/languages/python.ts b/packages/jsii-rosetta/lib/languages/python.ts index 08e90692ef..73b0c434dc 100644 --- a/packages/jsii-rosetta/lib/languages/python.ts +++ b/packages/jsii-rosetta/lib/languages/python.ts @@ -1,13 +1,16 @@ import * as ts from 'typescript'; -import { determineJsiiType, JsiiType } from '../jsii/jsii-types'; +import { determineJsiiType, JsiiType, ObjectLiteralStruct } from '../jsii/jsii-types'; import { propertiesOfStruct, StructProperty, structPropertyAcceptsUndefined, analyzeStructType, + JsiiSymbol, + simpleName, + namespaceName, } from '../jsii/jsii-utils'; -import { jsiiTargetParam } from '../jsii/packages'; +import { jsiiTargetParameter } from '../jsii/packages'; import { TargetLanguage } from '../languages/target-language'; import { NO_SYNTAX, OTree, renderTree } from '../o-tree'; import { AstRenderer, nimpl, CommentSyntax } from '../renderer'; @@ -20,7 +23,7 @@ import { } from '../typescript/ast-utils'; import { ImportStatement } from '../typescript/imports'; import { parameterAcceptsUndefined } from '../typescript/types'; -import { startsWithUppercase, flat } from '../util'; +import { startsWithUppercase, flat, sortBy, groupBy, fmap } from '../util'; import { DefaultVisitor } from './default'; interface StructVar { @@ -87,6 +90,11 @@ export interface PythonVisitorOptions { disclaimer?: string; } +interface ImportedModule { + readonly importedFqn: string; + readonly importName: string; +} + export class PythonVisitor extends DefaultVisitor { /** * Translation version @@ -99,6 +107,16 @@ export class PythonVisitor extends DefaultVisitor { public readonly language = TargetLanguage.PYTHON; public readonly defaultContext = {}; + /** + * Keep track of module imports we've seen, so that if we need to render a type we can pick from these modules + */ + private readonly imports = new Array(); + + /** + * Synthetic imports that need to be added as a final step + */ + private readonly syntheticImportsToAdd = new Array(); + protected statementTerminator = ''; public constructor(private readonly options: PythonVisitorOptions = {}) { @@ -125,26 +143,52 @@ export class PythonVisitor extends DefaultVisitor { } public sourceFile(node: ts.SourceFile, context: PythonVisitorContext): OTree { - const rendered = super.sourceFile(node, context); + let rendered = super.sourceFile(node, context); + + // Add synthetic imports + if (this.syntheticImportsToAdd.length > 0) { + rendered = new OTree([...this.renderSyntheticImports(), rendered]); + } + if (this.options.disclaimer) { - return new OTree([`# ${this.options.disclaimer}\n`, rendered]); + rendered = new OTree([`# ${this.options.disclaimer}\n`, rendered]); } return rendered; } public importStatement(node: ImportStatement, context: PythonVisitorContext): OTree { - const moduleName = this.convertModuleReference(node.packageName); if (node.imports.import === 'full') { + const moduleName = fmap(node.moduleSymbol, findPythonName) ?? guessPythonPackageName(node.packageName); + + this.addImport({ + importedFqn: node.moduleSymbol?.fqn ?? node.packageName, + importName: node.imports.alias, + }); + return new OTree([`import ${moduleName} as ${mangleIdentifier(node.imports.alias)}`], [], { canBreakLine: true, }); } if (node.imports.import === 'selective') { - const imports = node.imports.elements.map((im) => - im.alias - ? `${mangleIdentifier(im.sourceName)} as ${mangleIdentifier(im.alias)}` - : mangleIdentifier(im.sourceName), - ); + for (const im of node.imports.elements) { + if (im.importedSymbol) { + this.addImport({ + importName: im.alias ? im.alias : im.sourceName, + importedFqn: im.importedSymbol.fqn, + }); + } + } + + const imports = node.imports.elements.map((im) => { + const localName = im.alias ?? im.sourceName; + const originalName = fmap(fmap(im.importedSymbol, findPythonName), simpleName) ?? im.sourceName; + + return localName === originalName + ? mangleIdentifier(originalName) + : `${mangleIdentifier(originalName)} as ${mangleIdentifier(localName)}`; + }); + + const moduleName = fmap(node.moduleSymbol, findPythonName) ?? guessPythonPackageName(node.packageName); return new OTree([`from ${moduleName} import ${imports.join(', ')}`], [], { canBreakLine: true, @@ -293,7 +337,11 @@ export class PythonVisitor extends DefaultVisitor { public parameterDeclaration(node: ts.ParameterDeclaration, context: PythonVisitorContext): OTree { const type = node.type && context.typeOfType(node.type); - if (context.currentContext.tailPositionParameter && type && analyzeStructType(type) !== false) { + if ( + context.currentContext.tailPositionParameter && + type && + analyzeStructType(context.typeChecker, type) !== false + ) { // Return the parameter that we exploded so that we can use this information // while translating the body. if (context.currentContext.returnExplodedParameter) { @@ -351,15 +399,18 @@ export class PythonVisitor extends DefaultVisitor { public knownStructObjectLiteralExpression( node: ts.ObjectLiteralExpression, - structType: ts.Type, - _definedInExample: boolean, + structType: ObjectLiteralStruct, context: PythonVisitorContext, ): OTree { if (context.currentContext.tailPositionArgument) { // We know it's a struct we can DEFINITELY inline the args for return this.renderObjectLiteralExpression('', '', true, node, context); } - return this.renderObjectLiteralExpression(`${structType.symbol.name}(`, ')', true, node, context); + + const structName = + structType.kind === 'struct' ? this.importedNameForType(structType.jsiiSym) : structType.type.symbol.name; + + return this.renderObjectLiteralExpression(`${structName}(`, ')', true, node, context); } public keyValueObjectLiteralExpression(node: ts.ObjectLiteralExpression, context: PythonVisitorContext): OTree { @@ -590,15 +641,6 @@ export class PythonVisitor extends DefaultVisitor { return NO_SYNTAX; } - protected convertModuleReference(ref: string) { - // Get the Python target name from the referenced package (if available) - const resolvedPackage = jsiiTargetParam(ref, 'python.module'); - - // Return that or some default-derived module name representation - - return resolvedPackage || ref.replace(/^@/, '').replace(/\//g, '.').replace(/-/g, '_'); - } - /** * Convert parameters * @@ -700,6 +742,40 @@ export class PythonVisitor extends DefaultVisitor { } } } + + private addImport(x: ImportedModule) { + this.imports.push(x); + // Sort in reverse order of FQN length + sortBy(this.imports, (i) => [-i.importedFqn.length]); + } + + /** + * Find the import for the FQNs submodule, and return it and the rest of the name + */ + private importedNameForType(jsiiSym: JsiiSymbol) { + // Look for an existing import that contains this symbol + for (const imp of this.imports) { + if (jsiiSym.fqn.startsWith(`${imp.importedFqn}.`)) { + const remainder = jsiiSym.fqn.substring(imp.importedFqn.length + 1); + return `${imp.importName}.${remainder}`; + } + } + + // Otherwise look up the Python name of this symbol (but not for fake imports from tests) + const pythonName = findPythonName(jsiiSym); + if (!jsiiSym.fqn.startsWith('fake_jsii.') && pythonName) { + this.syntheticImportsToAdd.push(pythonName); + } + return simpleName(jsiiSym.fqn); + } + + private renderSyntheticImports(): string[] { + const grouped = groupBy(this.syntheticImportsToAdd, namespaceName); + return Object.entries(grouped).map(([namespaceFqn, fqns]) => { + const simpleNames = fqns.map(simpleName); + return `from ${namespaceFqn} import ${simpleNames.join(', ')}\n`; + }); + } } function mangleIdentifier(originalIdentifier: string) { @@ -729,3 +805,37 @@ const IDENTIFIER_KEYWORDS: string[] = ['lambda']; function last(xs: readonly A[]): A { return xs[xs.length - 1]; } + +/** + * Find the Python name of a module or type + */ +function findPythonName(jsiiSymbol: JsiiSymbol): string | undefined { + if (!jsiiSymbol.sourceAssembly?.assembly) { + // Don't have accurate info, just guess + return jsiiSymbol.symbolType !== 'module' ? simpleName(jsiiSymbol.fqn) : guessPythonPackageName(jsiiSymbol.fqn); + } + + const asm = jsiiSymbol.sourceAssembly?.assembly; + return recurse(jsiiSymbol.fqn); + + function recurse(fqn: string): string { + if (fqn === asm.name) { + return jsiiTargetParameter(asm, 'python.module') ?? guessPythonPackageName(fqn); + } + if (asm.submodules?.[fqn]) { + const modName = jsiiTargetParameter(asm.submodules[fqn], 'python.module'); + if (modName) { + return modName; + } + } + + return `${recurse(namespaceName(fqn))}.${simpleName(jsiiSymbol.fqn)}`; + } +} + +/** + * Pythonify an assembly name and hope it is correct + */ +function guessPythonPackageName(ref: string) { + return ref.replace(/^@/, '').replace(/\//g, '.').replace(/-/g, '_'); +} diff --git a/packages/jsii-rosetta/lib/languages/record-references.ts b/packages/jsii-rosetta/lib/languages/record-references.ts index db1027f14f..3b4b049a10 100644 --- a/packages/jsii-rosetta/lib/languages/record-references.ts +++ b/packages/jsii-rosetta/lib/languages/record-references.ts @@ -1,6 +1,6 @@ import * as ts from 'typescript'; -import { jsiiFqnFromSymbol } from '../jsii/jsii-utils'; +import { lookupJsiiSymbol } from '../jsii/jsii-utils'; import { TargetLanguage } from '../languages/target-language'; import { OTree, NO_SYNTAX } from '../o-tree'; import { AstRenderer } from '../renderer'; @@ -126,11 +126,11 @@ export class RecordReferencesVisitor extends DefaultVisitor): ImportStatement { @@ -32,6 +45,7 @@ export function analyzeImportEquals(node: ts.ImportEqualsDeclaration, context: A return { node, packageName: moduleName, + moduleSymbol: lookupJsiiSymbolFromNode(context.typeChecker, node.name), imports: { import: 'full', alias: context.textOf(node.name) }, }; } @@ -51,6 +65,7 @@ export function analyzeImportDeclaration(node: ts.ImportDeclaration, context: As return { node, packageName, + moduleSymbol: lookupJsiiSymbolFromNode(context.typeChecker, starBindings.namespace.name), imports: { import: 'full', alias: context.textOf(starBindings.namespace.name), @@ -72,17 +87,22 @@ export function analyzeImportDeclaration(node: ts.ImportDeclaration, context: As const elements: ImportBinding[] = []; if (namedBindings) { elements.push( - ...namedBindings.specifiers.map((spec) => + ...namedBindings.specifiers.map((spec) => { // regular import { name }, renamed import { propertyName, name } - spec.propertyName - ? { - sourceName: context.textOf(spec.propertyName), - alias: spec.name ? context.textOf(spec.name) : '???', - } - : { - sourceName: spec.name ? context.textOf(spec.name) : '???', - }, - ), + if (spec.propertyName) { + // Renamed import + return { + sourceName: context.textOf(spec.propertyName), + alias: context.textOf(spec.name), + importedSymbol: lookupJsiiSymbolFromNode(context.typeChecker, spec.propertyName), + } as ImportBinding; + } + + return { + sourceName: context.textOf(spec.name), + importedSymbol: lookupJsiiSymbolFromNode(context.typeChecker, spec.name), + }; + }), ); } @@ -90,5 +110,6 @@ export function analyzeImportDeclaration(node: ts.ImportDeclaration, context: As node, packageName, imports: { import: 'selective', elements }, + moduleSymbol: fmap(elements?.[0]?.importedSymbol, parentSymbol), }; } diff --git a/packages/jsii-rosetta/lib/typescript/types.ts b/packages/jsii-rosetta/lib/typescript/types.ts index 4ce2916221..006a3515f8 100644 --- a/packages/jsii-rosetta/lib/typescript/types.ts +++ b/packages/jsii-rosetta/lib/typescript/types.ts @@ -1,6 +1,6 @@ import * as ts from 'typescript'; -import { hasAllFlags, hasAnyFlag } from '../jsii/jsii-utils'; +import { hasAllFlags, hasAnyFlag, resolveEnumLiteral, resolvedSymbolAtLocation } from '../jsii/jsii-utils'; /** * Return the first non-undefined type from a union @@ -149,7 +149,8 @@ export function arrayElementType(type: ts.Type): ts.Type | undefined { } export function typeOfExpression(typeChecker: ts.TypeChecker, node: ts.Expression) { - return typeChecker.getContextualType(node) ?? typeChecker.getTypeAtLocation(node); + const t = typeChecker.getContextualType(node) ?? typeChecker.getTypeAtLocation(node); + return resolveEnumLiteral(typeChecker, t); } function isDefined(x: A): x is NonNullable { @@ -174,12 +175,12 @@ export function isNumber(x: any): x is number { } export function isEnumAccess(typeChecker: ts.TypeChecker, access: ts.PropertyAccessExpression) { - const symbol = typeChecker.getSymbolAtLocation(access.expression); + const symbol = resolvedSymbolAtLocation(typeChecker, access.expression); return symbol ? hasAnyFlag(symbol.flags, ts.SymbolFlags.Enum) : false; } export function isStaticReadonlyAccess(typeChecker: ts.TypeChecker, access: ts.PropertyAccessExpression) { - const symbol = typeChecker.getSymbolAtLocation(access); + const symbol = resolvedSymbolAtLocation(typeChecker, access); const decl = symbol?.getDeclarations(); if (decl && decl[0] && ts.isPropertyDeclaration(decl[0])) { const flags = ts.getCombinedModifierFlags(decl[0]); diff --git a/packages/jsii-rosetta/lib/util.ts b/packages/jsii-rosetta/lib/util.ts index 4d131ad254..979acfb36a 100644 --- a/packages/jsii-rosetta/lib/util.ts +++ b/packages/jsii-rosetta/lib/util.ts @@ -100,8 +100,30 @@ export function mkDict(xs: Array): Record< return ret; } -export function fmap(value: NonNullable, fn: (x: A) => B): B; -export function fmap(value: undefined, fn: (x: A) => B): undefined; +/** + * Apply a function to a value, as long as it's not `undefined` + * + * This is a companion helper to TypeScript's nice `??` and `?.` nullish + * operators. Those operators are helpful if you're calling methods: + * + * object?.method() <- returns 'undefined' if 'object' is nullish + * + * But are no help when you want to use free functions: + * + * func(object) <- but what if 'object' is nullish and func + * expects it not to be? + * + * Yes you can write `object ? func(object) : undefined` but the trailing + * `: undefined` clutters your code. Instead, you write: + * + * fmap(object, func) + * + * The name `fmap` is taken from Haskell: it's a "Functor-map" (although + * only for the `Maybe` Functor). + */ +export function fmap(value: NonNullable, fn: (x: NonNullable) => B): B; +export function fmap(value: undefined, fn: (x: NonNullable) => B): undefined; +export function fmap(value: A | undefined, fn: (x: A) => B): B | undefined; export function fmap(value: A, fn: (x: A) => B): B | undefined { if (value === undefined) { return undefined; @@ -116,3 +138,62 @@ export function mapValues(xs: Record, fn: (x: A) => B): Record< } return ret; } + +/** + * Sort an array by a key function. + * + * Instead of having to write your own comparators for your types any time you + * want to sort, you supply a function that maps a value to a compound sort key + * consisting of numbers or strings. The sorting will happen by that sort key + * instead. + */ +export function sortBy(xs: A[], keyFn: (x: A) => Array) { + return xs.sort((a, b) => { + const aKey = keyFn(a); + const bKey = keyFn(b); + + for (let i = 0; i < Math.min(aKey.length, bKey.length); i++) { + // Compare aKey[i] to bKey[i] + const av = aKey[i]; + const bv = bKey[i]; + + if (av === bv) { + continue; + } + + if (typeof av !== typeof bv) { + throw new Error(`Type of sort key ${JSON.stringify(aKey)} not same as ${JSON.stringify(bKey)}`); + } + + if (typeof av === 'number' && typeof bv === 'number') { + return av - bv; + } + + if (typeof av === 'string' && typeof bv === 'string') { + return av.localeCompare(bv); + } + } + + return aKey.length - bKey.length; + }); +} + +/** + * Group elements by a key + * + * Supply a function that maps each element to a key string. + * + * Returns a map of the key to the list of elements that map to that key. + */ +export function groupBy(xs: A[], keyFn: (x: A) => string): Record { + const ret: Record = {}; + for (const x of xs) { + const key = keyFn(x); + if (ret[key]) { + ret[key].push(x); + } else { + ret[key] = [x]; + } + } + return ret; +} diff --git a/packages/jsii-rosetta/package.json b/packages/jsii-rosetta/package.json index 87c8561a5f..d406679d12 100644 --- a/packages/jsii-rosetta/package.json +++ b/packages/jsii-rosetta/package.json @@ -24,8 +24,7 @@ "@types/workerpool": "^6.1.0", "eslint": "^7.32.0", "jest": "^27.2.4", - "jsii": "^0.0.0", - "jsii-build-tools": "^0.0.0", + "jsii-build-tools": "0.0.0", "memory-streams": "^0.1.3", "mock-fs": "^5.1.1", "prettier": "^2.4.1" diff --git a/packages/jsii-rosetta/test/commands/extract.test.ts b/packages/jsii-rosetta/test/commands/extract.test.ts index 0969adb715..3de081d7ed 100644 --- a/packages/jsii-rosetta/test/commands/extract.test.ts +++ b/packages/jsii-rosetta/test/commands/extract.test.ts @@ -3,7 +3,7 @@ import * as path from 'path'; import { LanguageTablet, RosettaTranslator, RosettaTranslatorOptions } from '../../lib'; import * as extract from '../../lib/commands/extract'; import { TARGET_LANGUAGES } from '../../lib/languages'; -import { TestJsiiModule, DUMMY_ASSEMBLY_TARGETS } from '../testutil'; +import { TestJsiiModule, DUMMY_JSII_CONFIG } from '../testutil'; const DUMMY_README = ` Here is an example of how to use ClassA: @@ -35,7 +35,7 @@ beforeAll(async () => { }, { name: 'my_assembly', - jsii: DUMMY_ASSEMBLY_TARGETS, + jsii: DUMMY_JSII_CONFIG, }, ); }); @@ -115,7 +115,7 @@ test('do not ignore example strings', async () => { }, { name: 'my_assembly', - jsii: DUMMY_ASSEMBLY_TARGETS, + jsii: DUMMY_JSII_CONFIG, }, ); try { diff --git a/packages/jsii-rosetta/test/commands/infuse.test.ts b/packages/jsii-rosetta/test/commands/infuse.test.ts index 4c60103f6e..a6c4a11413 100644 --- a/packages/jsii-rosetta/test/commands/infuse.test.ts +++ b/packages/jsii-rosetta/test/commands/infuse.test.ts @@ -5,7 +5,7 @@ import { LanguageTablet } from '../../lib'; import { extractSnippets } from '../../lib/commands/extract'; import { infuse, DEFAULT_INFUSION_RESULTS_NAME } from '../../lib/commands/infuse'; import { loadAssemblies } from '../../lib/jsii/assemblies'; -import { TestJsiiModule, DUMMY_ASSEMBLY_TARGETS } from '../testutil'; +import { TestJsiiModule, DUMMY_JSII_CONFIG } from '../testutil'; const DUMMY_README = ` Here is an example of how to use ClassA: @@ -41,7 +41,7 @@ beforeEach(async () => { }, { name: 'my_assembly', - jsii: DUMMY_ASSEMBLY_TARGETS, + jsii: DUMMY_JSII_CONFIG, }, ); diff --git a/packages/jsii-rosetta/test/commands/transliterate.test.ts b/packages/jsii-rosetta/test/commands/transliterate.test.ts index 86fbac3a16..98863eecce 100644 --- a/packages/jsii-rosetta/test/commands/transliterate.test.ts +++ b/packages/jsii-rosetta/test/commands/transliterate.test.ts @@ -425,10 +425,10 @@ export class ClassName implements IInterface { // See https://github.com/aws/jsii/issues/826 for more information. IInterface object = new ClassName(\\"this\\", 1337, new ClassNameProps().foo(\\"bar\\")); - object.getProperty() = EnumType.getOPTION_A(); + object.getProperty() = EnumType.OPTION_A; object.methodCall(); - ClassName.staticMethod(EnumType.getOPTION_B()); + ClassName.staticMethod(EnumType.OPTION_B); \`\`\`", }, "repository": Object { @@ -450,7 +450,7 @@ export class ClassName implements IInterface { "example": "// This example was automatically transliterated. // See https://github.com/aws/jsii/issues/826 for more information. - new ClassName(\\"this\\", 1337, new ClassNameProps().property(EnumType.getOPTION_B()));", + new ClassName(\\"this\\", 1337, new ClassNameProps().property(EnumType.OPTION_B));", "summary": "Create a new instance of ClassName.", }, "locationInModule": Object { @@ -589,7 +589,7 @@ export class ClassName implements IInterface { "example": "// This example was automatically transliterated. // See https://github.com/aws/jsii/issues/826 for more information. - new ClassName(\\"this\\", 1337, new ClassNameProps().property(EnumType.getOPTION_B()));", + new ClassName(\\"this\\", 1337, new ClassNameProps().property(EnumType.OPTION_B));", }, "fqn": "testpkg.EnumType", "kind": "enum", @@ -603,7 +603,7 @@ export class ClassName implements IInterface { "example": "// This example was automatically transliterated. // See https://github.com/aws/jsii/issues/826 for more information. - new ClassName(\\"this\\", 1337, new ClassNameProps().property(EnumType.getOPTION_A()));", + new ClassName(\\"this\\", 1337, new ClassNameProps().property(EnumType.OPTION_A));", }, "name": "OPTION_A", }, @@ -612,7 +612,7 @@ export class ClassName implements IInterface { "example": "// This example was automatically transliterated. // See https://github.com/aws/jsii/issues/826 for more information. - new ClassName(\\"this\\", 1337, new ClassNameProps().property(EnumType.getOPTION_B()));", + new ClassName(\\"this\\", 1337, new ClassNameProps().property(EnumType.OPTION_B));", }, "name": "OPTION_B", }, @@ -653,7 +653,7 @@ export class ClassName implements IInterface { "example": "// This example was automatically transliterated. // See https://github.com/aws/jsii/issues/826 for more information. - iface.getProperty() = EnumType.getOPTION_B();", + iface.getProperty() = EnumType.OPTION_B;", "summary": "A property value.", }, "locationInModule": Object { diff --git a/packages/jsii-rosetta/test/jsii-imports.test.ts b/packages/jsii-rosetta/test/jsii-imports.test.ts new file mode 100644 index 0000000000..67aec7f362 --- /dev/null +++ b/packages/jsii-rosetta/test/jsii-imports.test.ts @@ -0,0 +1,459 @@ +// Test translation of imports with actual jsii assemblies +// +// - For Python, there is a lot of variation in what imports get translated to (mirroring +// the style in TypeScript, occasionally adding extra imports as required). +// - For other languages, we'll mostly translate the same thing. + +import { TargetLanguage, TranslatedSnippet } from '../lib'; +import { MultipleSources, TestJsiiModule, DUMMY_JSII_CONFIG } from './testutil'; + +describe('no submodule', () => { + describe('top-level struct', () => { + let module: TestJsiiModule; + beforeAll(async () => { + module = await makeJsiiModule({ withModule: false, nestedStruct: false }); + }); + + afterAll(() => module.cleanup()); + + describe('package import', () => { + let trans: TranslatedSnippet; + beforeAll(() => { + trans = module.translateHere(` + import * as masm from 'my_assembly'; + const obj = new masm.MyClass('value', { + myStruct: { + value: 'v', + }, + }); + `); + }); + + test('to Python', () => { + expectTranslation(trans, TargetLanguage.PYTHON, [ + 'import example_test_demo as masm', + 'obj = masm.MyClass("value",', + ' my_struct=masm.MyStruct(', + ' value="v"', + ' )', + ')', + ]); + }); + + test('to Java', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.JAVA, [ + 'import example.test.demo.*;', + ...DEFAULT_JAVA_CODE, + ]); + }); + + test('to C#', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.CSHARP, [ + 'using Example.Test.Demo;', + ...DEFAULT_CSHARP_CODE, + ]); + }); + }); + + describe('class import', () => { + let trans: TranslatedSnippet; + beforeAll(() => { + trans = module.translateHere( + `import { MyClass } from 'my_assembly'; + const obj = new MyClass('value', { + myStruct: { + value: 'v', + }, + }); + `, + ); + }); + + test('to Python', () => { + expectTranslation(trans, TargetLanguage.PYTHON, [ + 'from example_test_demo import MyStruct', + 'from example_test_demo import MyClass', + 'obj = MyClass("value",', + ' my_struct=MyStruct(', + ' value="v"', + ' )', + ')', + ]); + }); + + test('to Java', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.JAVA, [ + 'import example.test.demo.MyClass;', + ...DEFAULT_JAVA_CODE, + ]); + }); + + test('to C#', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.CSHARP, [ + 'using Example.Test.Demo;', + ...DEFAULT_CSHARP_CODE, + ]); + }); + }); + }); + + describe('nested struct', () => { + let module: TestJsiiModule; + beforeAll(async () => { + module = await makeJsiiModule({ withModule: false, nestedStruct: true }); + }); + afterAll(() => module.cleanup()); + + describe('package import', () => { + let trans: TranslatedSnippet; + beforeAll(() => { + trans = module.translateHere(` + import * as masm from 'my_assembly'; + const obj = new masm.MyClass('value', { + myStruct: { + value: 'v', + }, + }); + `); + }); + + test('to Python', () => { + expectTranslation(trans, TargetLanguage.PYTHON, [ + 'import example_test_demo as masm', + 'obj = masm.MyClass("value",', + ' my_struct=masm.MyClass.MyStruct(', + ' value="v"', + ' )', + ')', + ]); + }); + + test('to Java', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.JAVA, [ + 'import example.test.demo.*;', + ...DEFAULT_JAVA_CODE, + ]); + }); + + test('to C#', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.CSHARP, [ + 'using Example.Test.Demo;', + ...DEFAULT_CSHARP_CODE, + ]); + }); + }); + + describe('class import', () => { + let trans: TranslatedSnippet; + beforeAll(() => { + trans = module.translateHere(` + import { MyClass } from 'my_assembly'; + const obj = new MyClass('value', { + myStruct: { + value: 'v', + }, + }); + `); + }); + + test('to Python', () => { + expectTranslation(trans, TargetLanguage.PYTHON, [ + 'from example_test_demo import MyClass', + 'obj = MyClass("value",', + ' my_struct=MyClass.MyStruct(', + ' value="v"', + ' )', + ')', + ]); + }); + + test('to Java', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.JAVA, [ + 'import example.test.demo.MyClass;', + ...DEFAULT_JAVA_CODE, + ]); + }); + + test('to C#', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.CSHARP, [ + 'using Example.Test.Demo;', + ...DEFAULT_CSHARP_CODE, + ]); + }); + }); + }); + + describe('enum', () => { + let module: TestJsiiModule; + beforeAll(async () => { + module = await TestJsiiModule.fromSource( + { + 'index.ts': `export enum MyEnum { OPTION_A = 'a', OPTION_B = 'b' }`, + }, + { + name: 'my_assembly', + jsii: DUMMY_JSII_CONFIG, + }, + ); + }); + + afterAll(() => module.cleanup()); + + describe('package import', () => { + let trans: TranslatedSnippet; + beforeAll(() => { + trans = module.translateHere(` + import * as masm from 'my_assembly'; + const x = masm.MyEnum.OPTION_A; + `); + }); + + test('to Python', () => { + expectTranslation(trans, TargetLanguage.PYTHON, [ + 'import example_test_demo as masm', + 'x = masm.MyEnum.OPTION_A', + ]); + }); + + test('to Java', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.JAVA, [ + 'import example.test.demo.*;', + 'MyEnum x = MyEnum.OPTION_A;', + ]); + }); + + test('to C#', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.CSHARP, [ + 'using Example.Test.Demo;', + 'MyEnum x = MyEnum.OPTION_A;', + ]); + }); + }); + + describe('direct import', () => { + let trans: TranslatedSnippet; + beforeAll(() => { + trans = module.translateHere( + `import { MyEnum } from 'my_assembly'; + const x = MyEnum.OPTION_A; + `, + ); + }); + + test('to Python', () => { + expectTranslation(trans, TargetLanguage.PYTHON, [ + 'from example_test_demo import MyEnum', + 'x = MyEnum.OPTION_A', + ]); + }); + + test('to Java', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.JAVA, [ + 'import example.test.demo.MyEnum;', + 'MyEnum x = MyEnum.OPTION_A;', + ]); + }); + + test('to C#', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.CSHARP, [ + 'using Example.Test.Demo;', + 'MyEnum x = MyEnum.OPTION_A;', + ]); + }); + }); + }); +}); + +describe('with submodule', () => { + describe('top-level struct', () => { + let module: TestJsiiModule; + beforeAll(async () => { + module = await makeJsiiModule({ withModule: true, nestedStruct: false }); + }); + + afterAll(() => module.cleanup()); + + describe('namespace import', () => { + let trans: TranslatedSnippet; + beforeAll(() => { + trans = module.translateHere(` + import { submod as mod } from 'my_assembly'; + const obj = new mod.MyClass('value', { + myStruct: { + value: 'v', + }, + }); + `); + }); + + test('to Python', () => { + expectTranslation(trans, TargetLanguage.PYTHON, [ + 'from example_test_demo import boop as mod', + 'obj = mod.MyClass("value",', + ' my_struct=mod.MyStruct(', + ' value="v"', + ' )', + ')', + ]); + }); + + test('to Java', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.JAVA, [ + 'import example.test.demo.boop.*;', + ...DEFAULT_JAVA_CODE, + ]); + }); + + test('to C#', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.CSHARP, [ + 'using Example.Test.Demo.Boop;', + ...DEFAULT_CSHARP_CODE, + ]); + }); + }); + }); + + describe('nested struct', () => { + let module: TestJsiiModule; + beforeAll(async () => { + module = await makeJsiiModule({ withModule: true, nestedStruct: true }); + }); + + afterAll(() => module.cleanup()); + + describe('namespace import', () => { + let trans: TranslatedSnippet; + beforeAll(() => { + trans = module.translateHere(` + import { submod as mod } from 'my_assembly'; + const obj = new mod.MyClass('value', { + myStruct: { + value: 'v', + }, + }); + `); + }); + + test('to Python', () => { + expectTranslation(trans, TargetLanguage.PYTHON, [ + 'from example_test_demo import boop as mod', + 'obj = mod.MyClass("value",', + ' my_struct=mod.MyClass.MyStruct(', + ' value="v"', + ' )', + ')', + ]); + }); + + test('to Java', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.JAVA, [ + 'import example.test.demo.boop.*;', + ...DEFAULT_JAVA_CODE, + ]); + }); + + test('to C#', () => { + // eslint-disable-next-line prettier/prettier + expectTranslation(trans, TargetLanguage.CSHARP, [ + 'using Example.Test.Demo.Boop;', + ...DEFAULT_CSHARP_CODE, + ]); + }); + }); + }); +}); + +async function makeJsiiModule(options: { + readonly withModule: boolean; + readonly nestedStruct: boolean; +}): Promise { + const nsRef = options.nestedStruct ? 'MyClass.' : ''; + const nsDeclBegin = options.nestedStruct ? 'export namespace MyClass {\n' : ''; + const nsDeclEnd = options.nestedStruct ? '}' : ''; + + const payload = ` + export class MyClass { + constructor(value: string, props: ${nsRef}MyClassProps) { + Array.isArray(value); + Array.isArray(props); + } + } + + ${nsDeclBegin} + export interface MyClassProps { + readonly myStruct: MyStruct; + } + + export interface MyStruct { + readonly value: string; + } + ${nsDeclEnd} + `; + + const source: MultipleSources = options.withModule + ? { + 'index.ts': 'export * as submod from "./submodule/module";', + 'submodule/module.ts': payload, + 'submodule/.jsiirc.json': JSON.stringify({ + targets: { + python: { + module: 'example_test_demo.boop', + }, + java: { + package: 'example.test.demo.boop', + }, + dotnet: { + namespace: 'Example.Test.Demo.Boop', + }, + }, + }), + } + : { + 'index.ts': payload, + }; + + return TestJsiiModule.fromSource(source, { + name: 'my_assembly', + jsii: DUMMY_JSII_CONFIG, + }); +} + +// The implementation part of the Java code is always the same +const DEFAULT_JAVA_CODE = [ + 'MyClass obj = MyClass.Builder.create("value")', + ' .myStruct(MyStruct.builder()', + ' .value("v")', + ' .build())', + ' .build();', +]; + +// The implementation part of the CSharp code is always the same +const DEFAULT_CSHARP_CODE = [ + 'MyClass obj = new MyClass("value", new MyClassProps {', + ' MyStruct = new MyStruct {', + ' Value = "v"', + ' }', + '});', +]; + +/** + * Verify the Java output. All expected Java outputs look the same. + */ +function expectTranslation(trans: TranslatedSnippet, lang: TargetLanguage, expected: string[]) { + expect(trans.get(lang)?.source.split('\n')).toEqual(expected); +} diff --git a/packages/jsii-rosetta/test/jsii/assemblies.test.ts b/packages/jsii-rosetta/test/jsii/assemblies.test.ts index 76e1e772f8..97878d9a9f 100644 --- a/packages/jsii-rosetta/test/jsii/assemblies.test.ts +++ b/packages/jsii-rosetta/test/jsii/assemblies.test.ts @@ -5,7 +5,7 @@ import * as path from 'path'; import { allTypeScriptSnippets } from '../../lib/jsii/assemblies'; import { SnippetParameters } from '../../lib/snippet'; -import { TestJsiiModule, DUMMY_ASSEMBLY_TARGETS } from '../testutil'; +import { TestJsiiModule, DUMMY_JSII_CONFIG } from '../testutil'; import { fakeAssembly } from './fake-assembly'; test('Extract snippet from README', () => { @@ -248,7 +248,7 @@ test('rosetta fixture from submodule is preferred if it exists', async () => { }, { name: 'my_assembly', - jsii: DUMMY_ASSEMBLY_TARGETS, + jsii: DUMMY_JSII_CONFIG, }, ); try { diff --git a/packages/jsii-rosetta/test/record-references.test.ts b/packages/jsii-rosetta/test/record-references.test.ts index b4c8b7ed5b..72fd3d6bcc 100644 --- a/packages/jsii-rosetta/test/record-references.test.ts +++ b/packages/jsii-rosetta/test/record-references.test.ts @@ -1,4 +1,4 @@ -import { TestJsiiModule, DUMMY_ASSEMBLY_TARGETS } from './testutil'; +import { TestJsiiModule, DUMMY_JSII_CONFIG } from './testutil'; let assembly: TestJsiiModule; beforeAll(async () => { @@ -29,7 +29,7 @@ beforeAll(async () => { }, { name: 'my_assembly', - jsii: DUMMY_ASSEMBLY_TARGETS, + jsii: DUMMY_JSII_CONFIG, }, ); }); diff --git a/packages/jsii-rosetta/test/rosetta-translator.test.ts b/packages/jsii-rosetta/test/rosetta-translator.test.ts index 3397dc40d1..798990fb79 100644 --- a/packages/jsii-rosetta/test/rosetta-translator.test.ts +++ b/packages/jsii-rosetta/test/rosetta-translator.test.ts @@ -3,6 +3,8 @@ import { withTemporaryDirectory } from './testutil'; const location: SnippetLocation = { api: { api: 'file', fileName: 'test.ts' } }; +jest.setTimeout(60_000); + test('translator can translate', async () => { const translator = new RosettaTranslator({ includeCompilerDiagnostics: true, diff --git a/packages/jsii-rosetta/test/syntax-counter.test.ts b/packages/jsii-rosetta/test/syntax-counter.test.ts index 285f0d276d..2f3a424c26 100644 --- a/packages/jsii-rosetta/test/syntax-counter.test.ts +++ b/packages/jsii-rosetta/test/syntax-counter.test.ts @@ -1,4 +1,4 @@ -import { TestJsiiModule, DUMMY_ASSEMBLY_TARGETS } from './testutil'; +import { TestJsiiModule, DUMMY_JSII_CONFIG } from './testutil'; let assembly: TestJsiiModule; beforeAll(async () => { @@ -18,7 +18,7 @@ beforeAll(async () => { `, { name: 'my_assembly', - jsii: DUMMY_ASSEMBLY_TARGETS, + jsii: DUMMY_JSII_CONFIG, }, ); }); diff --git a/packages/jsii-rosetta/test/testutil.ts b/packages/jsii-rosetta/test/testutil.ts index ff39f98007..6708fc044e 100644 --- a/packages/jsii-rosetta/test/testutil.ts +++ b/packages/jsii-rosetta/test/testutil.ts @@ -5,11 +5,12 @@ import * as os from 'os'; import * as path from 'path'; import { - typeScriptSnippetFromSource, SnippetTranslator, SnippetParameters, rosettaDiagFromTypescript, SnippetLocation, + typeScriptSnippetFromCompleteSource, + Translator, } from '../lib'; export type MultipleSources = { [key: string]: string; 'index.ts': string }; @@ -43,6 +44,8 @@ export class TestJsiiModule { jsii: packageInfo.jsii, }); for (const [fileName, fileContents] of Object.entries(files)) { + // eslint-disable-next-line no-await-in-loop + await fs.ensureDir(path.dirname(path.join(modDir, fileName))); // eslint-disable-next-line no-await-in-loop await fs.writeFile(path.join(modDir, fileName), fileContents); } @@ -61,7 +64,7 @@ export class TestJsiiModule { */ public successfullyCompile(source: string) { const location = testSnippetLocation('testutil'); - const snippet = typeScriptSnippetFromSource(source, location, false, { + const snippet = typeScriptSnippetFromCompleteSource(source, location, false, { [SnippetParameters.$COMPILATION_DIRECTORY]: this.workspaceDirectory, }); const ret = new SnippetTranslator(snippet, { @@ -76,6 +79,23 @@ export class TestJsiiModule { return ret; } + public translateHere(source: string) { + const location = testSnippetLocation('testutil'); + const snip = typeScriptSnippetFromCompleteSource(source.trimLeft(), location, true, { + [SnippetParameters.$COMPILATION_DIRECTORY]: this.workspaceDirectory, + }); + + const trans = new Translator(true); + const ret = trans.translate(snip); + if (trans.diagnostics.length > 0) { + for (const diag of trans.diagnostics) { + console.error(diag.formattedMessage); + } + throw new Error('Compilation failures'); + } + return ret; + } + public async cleanup() { await fs.remove(this.moduleDirectory); } @@ -85,22 +105,24 @@ export function testSnippetLocation(fileName: string): SnippetLocation { return { api: { api: 'file', fileName }, field: { field: 'example' } }; } -export const DUMMY_ASSEMBLY_TARGETS = { - dotnet: { - namespace: 'Example.Test.Demo', - packageId: 'Example.Test.Demo', - }, - go: { moduleName: 'example.test/demo' }, - java: { - maven: { - groupId: 'example.test', - artifactId: 'demo', +export const DUMMY_JSII_CONFIG = { + targets: { + dotnet: { + namespace: 'Example.Test.Demo', + packageId: 'Example.Test.Demo', + }, + go: { moduleName: 'example.test/demo' }, + java: { + maven: { + groupId: 'example.test', + artifactId: 'demo', + }, + package: 'example.test.demo', + }, + python: { + distName: 'example-test.demo', + module: 'example_test_demo', }, - package: 'example.test.demo', - }, - python: { - distName: 'example-test.demo', - module: 'example_test_demo', }, }; diff --git a/packages/jsii-rosetta/test/translations.test.ts b/packages/jsii-rosetta/test/translations.test.ts index c37075ae60..86b45e5aca 100644 --- a/packages/jsii-rosetta/test/translations.test.ts +++ b/packages/jsii-rosetta/test/translations.test.ts @@ -1,10 +1,9 @@ import * as fs from 'fs-extra'; import * as path from 'path'; -import { JavaVisitor, PythonVisitor, SnippetTranslator } from '../lib'; -import { CSharpVisitor } from '../lib/languages/csharp'; +import { SnippetTranslator } from '../lib'; +import { TARGET_LANGUAGES, TargetLanguage, VisitorFactory } from '../lib/languages'; import { VisualizeAstVisitor } from '../lib/languages/visualize'; -import { AstHandler } from '../lib/renderer'; import { testSnippetLocation } from './testutil'; // This iterates through all subdirectories of this directory, @@ -28,24 +27,24 @@ interface SupportedLanguage { readonly extension: string; - readonly visitor: AstHandler; + readonly visitorFactory: VisitorFactory; } -const SUPPORTED_LANGUAGES = new Array( +export const SUPPORTED_LANGUAGES = new Array( { name: 'Python', extension: '.py', - visitor: new PythonVisitor(), + visitorFactory: TARGET_LANGUAGES[TargetLanguage.PYTHON], }, { name: 'Java', extension: '.java', - visitor: new JavaVisitor(), + visitorFactory: TARGET_LANGUAGES[TargetLanguage.JAVA], }, { name: 'C#', extension: '.cs', - visitor: new CSharpVisitor(), + visitorFactory: TARGET_LANGUAGES[TargetLanguage.CSHARP], }, ); @@ -78,7 +77,7 @@ for (const typeScriptTest of typeScriptTests) { translator = undefined as any; // Need this to properly release memory }); - for (const { name, extension, visitor } of SUPPORTED_LANGUAGES) { + for (const { name, extension, visitorFactory } of SUPPORTED_LANGUAGES) { const languageFile = replaceExtension(typeScriptTest, extension); // Use 'test.skip' if the file doesn't exist so that we can clearly see it's missing. @@ -87,7 +86,7 @@ for (const typeScriptTest of typeScriptTests) { testConstructor(`to ${name}`, () => { const expected = fs.readFileSync(languageFile, { encoding: 'utf-8' }); try { - const translation = translator.renderUsing(visitor); + const translation = translator.renderUsing(visitorFactory.createVisitor()); expect(stripEmptyLines(translation)).toEqual(stripEmptyLines(stripCommonWhitespace(expected))); } catch (e) { anyFailed = true; diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index 200a3b1b50..ad69225cbc 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -628,6 +628,7 @@ export class Assembler implements Emitter { this._submodules.set(symbol, { fqn, fqnResolutionPrefix, + symbolId: symbolIdentifier(this._typeChecker, symbol), locationInModule: this.declarationLocation(declaration), }); await this._addToSubmodule(symbol, symbol, packageRoot); @@ -707,6 +708,7 @@ export class Assembler implements Emitter { fqnResolutionPrefix, targets, readme, + symbolId: symbolIdentifier(this._typeChecker, symbol), locationInModule: this.declarationLocation(declaration), }); await this._addToSubmodule(symbol, sourceModule, packageRoot); @@ -2811,6 +2813,11 @@ interface SubmoduleSpec { */ readonly locationInModule: spec.SourceLocation; + /** + * Symbol identifier of the root of the root file that represents this submodule + */ + readonly symbolId?: string; + /** * Any customized configuration for the currentl submodule. */ @@ -3150,6 +3157,7 @@ function toSubmoduleDeclarations( locationInModule: submodule.locationInModule, targets: submodule.targets, readme: submodule.readme, + symbolId: submodule.symbolId, }; } diff --git a/packages/jsii/lib/symbol-id.ts b/packages/jsii/lib/symbol-id.ts index c58deb827a..2c6b189e49 100644 --- a/packages/jsii/lib/symbol-id.ts +++ b/packages/jsii/lib/symbol-id.ts @@ -6,6 +6,11 @@ export function symbolIdentifier( typeChecker: ts.TypeChecker, sym: ts.Symbol, ): string | undefined { + // If this symbol happens to be an alias, resolve it first + while ((sym.flags & ts.SymbolFlags.Alias) !== 0) { + sym = typeChecker.getAliasedSymbol(sym); + } + const inFileNameParts: string[] = []; let decl: ts.Node | undefined = sym.declarations?.[0]; diff --git a/packages/jsii/test/symbol-identifiers.test.ts b/packages/jsii/test/symbol-identifiers.test.ts index 6c221e3b57..995b467305 100644 --- a/packages/jsii/test/symbol-identifiers.test.ts +++ b/packages/jsii/test/symbol-identifiers.test.ts @@ -51,3 +51,44 @@ test('Module declarations are included in symbolId', async () => { const types = result.assembly.types ?? {}; expect(types['testpkg.Foo.Bar'].symbolId).toEqual('index:Foo.Bar'); }); + +test('Submodules also have symbol identifiers', async () => { + const result = await compileJsiiForTest( + { + 'index.ts': `export * as submod from './submodule';`, + 'submodule.ts': ` + export class Foo { + constructor() { + } + } + `, + }, + undefined /* callback */, + { stripDeprecated: true }, + ); + + expect(result.assembly.submodules?.['testpkg.submod']?.symbolId).toEqual( + 'submodule:', + ); +}); + +test('Submodules also have symbol identifiers', async () => { + const result = await compileJsiiForTest( + { + 'index.ts': ` + export namespace cookie { + export class Foo { + constructor() { + } + } + } + `, + }, + undefined /* callback */, + { stripDeprecated: true }, + ); + + expect(result.assembly.submodules?.['testpkg.cookie']?.symbolId).toEqual( + 'index:cookie', + ); +}); From bc6f3edc05682debcc66c2851c7cf0b7d2b786e3 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Thu, 18 Nov 2021 20:14:59 +0100 Subject: [PATCH 08/10] chore: record missing contributions (#3186) --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- .all-contributorsrc | 67 +++++++++++++++++++++++++++++++++++++++++++++ README.md | 59 ++++++++++++++++++++++----------------- 2 files changed, 101 insertions(+), 25 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index ebd5c45913..86223eafbf 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -1233,6 +1233,73 @@ "code", "bug" ] + }, + { + "login": "otaviomacedo", + "name": "Otavio Macedo", + "avatar_url": "https://avatars.githubusercontent.com/u/288203?v=4", + "profile": "https://otaviomacedo.github.io/", + "contributions": [ + "code", + "bug" + ] + }, + { + "login": "kaizen3031593", + "name": "kaizen3031593", + "avatar_url": "https://avatars.githubusercontent.com/u/36202692?v=4", + "profile": "https://github.com/kaizen3031593", + "contributions": [ + "code", + "bug" + ] + }, + { + "login": "madeline-k", + "name": "Madeline Kusters", + "avatar_url": "https://avatars.githubusercontent.com/u/80541297?v=4", + "profile": "https://github.com/madeline-k", + "contributions": [ + "code", + "bug" + ] + }, + { + "login": "comcalvi", + "name": "Calvin Combs", + "avatar_url": "https://avatars.githubusercontent.com/u/66279577?v=4", + "profile": "https://github.com/comcalvi", + "contributions": [ + "code", + "bug" + ] + }, + { + "login": "peterwoodworth", + "name": "Peter Woodworth", + "avatar_url": "https://avatars.githubusercontent.com/u/44349620?v=4", + "profile": "https://github.com/peterwoodworth", + "contributions": [ + "maintenance" + ] + }, + { + "login": "mergify", + "name": "mergify", + "avatar_url": "https://avatars.githubusercontent.com/u/18240476?v=4", + "profile": "https://github.com/mergify", + "contributions": [ + "maintenance" + ] + }, + { + "login": "oieduardorabelo", + "name": "Eduardo Rabelo", + "avatar_url": "https://avatars.githubusercontent.com/u/829902?v=4", + "profile": "https://eduardorabelo.me/", + "contributions": [ + "doc" + ] } ], "repoType": "github", diff --git a/README.md b/README.md index 53b9556834..7a1d76ddd4 100644 --- a/README.md +++ b/README.md @@ -83,132 +83,141 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
CaerusKaru

πŸ’» 🚧 +
Calvin Combs

πŸ’» πŸ›
Camilo BermΓΊdez

πŸ›
Campion Fellin

πŸ’»
Carter Van Deuren

πŸ›
Christophe Vico

πŸ›
Christopher Currie

πŸ’» πŸ€” -
Christopher Rybicki

πŸ“– +
Christopher Rybicki

πŸ“–
Cory Hall

πŸ›
Cristian Măgherușan-Stanciu

πŸ›
CyrusNajmabadi

πŸ› πŸ€”
Damian Silbergleith

πŸ’» πŸ›
Daniel Dinu

πŸ› πŸ’»
Daniel Schroeder

πŸ› πŸ’» πŸ“– πŸ€” 🚧 -
Dave Slotnick

πŸ› +
Dave Slotnick

πŸ›
Donald Stufft

πŸ› πŸ’» πŸ€” πŸ‘€
Dongie Agnir

πŸ’» πŸ‘€ +
Eduardo Rabelo

πŸ“–
Eduardo Sena S. Rosa

πŸ›
Elad Ben-Israel

πŸ› πŸ’» πŸ€” 🚧 πŸ‘€ πŸ“’
Eli Polonsky

πŸ› πŸ’» πŸ€” 🚧 πŸ‘€ -
Eric Z. Beard

πŸ“† -
Erik Karlsson

πŸ› +
Eric Z. Beard

πŸ“† +
Erik Karlsson

πŸ›
Eugene Kozlov

πŸ’»
Fabio Gentile

πŸ›
Florian Eitel

πŸ€”
Graham Lea

πŸ€” πŸ‘€
Hamza Assyad

πŸ› πŸ’» πŸ€” πŸ‘€ -
Hari Pachuveetil

πŸ“ πŸ“– -
Hsing-Hui Hsu

πŸ’» πŸ“– πŸ€” πŸ‘€ +
Hari Pachuveetil

πŸ“ πŸ“– +
Hsing-Hui Hsu

πŸ’» πŸ“– πŸ€” πŸ‘€
Ikko Ashimine

πŸ“–
James

πŸ› πŸ’»
James Kelley

πŸ›
James Mead

πŸ’»
James Siri

πŸ’» 🚧 -
Jason Del Ponte

πŸ€” πŸ‘€ -
Jason Fulghum

πŸ€” πŸ“† πŸ‘€ +
Jason Del Ponte

πŸ€” πŸ‘€ +
Jason Fulghum

πŸ€” πŸ“† πŸ‘€
Jerry Kindall

πŸ“– πŸ€”
Jimmy Gaussen

πŸ€”
Johannes Weber

πŸ“–
Jon Steinich

πŸ› πŸ€” πŸ’»
Joseph Lawson

πŸ‘€ -
Joseph Martin

πŸ› -
Junix

πŸ› +
Joseph Martin

πŸ› +
Junix

πŸ›
Justin Taylor

πŸ›
Kyle Thomson

πŸ’» πŸ‘€
Leandro Padua

πŸ›
Liang Zhou

πŸ› πŸ’» +
Madeline Kusters

πŸ’» πŸ› + +
Maja S Bratseth

πŸ›
Marcos Diez

πŸ›
Mark Nielsen

πŸ’» - -
Matthew Bonig

πŸ› πŸ“
Matthew Pirocchi

πŸ’» πŸ€” πŸ‘€
Michael Neil

🚧
Mike Lane

πŸ› + +
Mitch Garnaat

πŸ› πŸ’» πŸ€” πŸ‘€
Mitchell Valine

πŸ› πŸ’» πŸ€” 🚧 πŸ‘€
Mohamad Soufan

πŸ“– - -
Neta Nir

πŸ’» πŸ€” 🚧 πŸ‘€
Nick Lynch

πŸ› πŸ’» 🚧 πŸ‘€
Niranjan Jayakar

πŸ› πŸ’» πŸ€” 🚧 πŸ‘€
Noah Litov

πŸ’» 🚧 πŸ‘€ + + +
Otavio Macedo

πŸ’» πŸ›
PIDZ - Bart

πŸ€” +
Peter Woodworth

🚧
Petr Kacer

πŸ›
Petra Barus

πŸ’» - -
Philip Cali

πŸ€”
Quentin Loos

πŸ€” + +
Raphael

πŸ›
Richard H Boyd

πŸ›
Rico Huijbers

πŸ› πŸ’» πŸ€” 🚧 πŸ‘€
Romain Marcadier

πŸ› πŸ’» 🎨 πŸ€” 🚧 πŸ‘€ πŸ“
SADIK KUZU

πŸ‘€ - -
SK

πŸ€”
Sam Fink

πŸ’» πŸ‘€ + +
Sam Goodwin

πŸ‘€
Sebastian Korfmann

πŸ› πŸ’» πŸ€”
Shane Witbeck

πŸ€”
Shiv Lakshminarayan

πŸ’» 🚧 πŸ‘€
Somaya

πŸ’» πŸ€” 🚧 πŸ‘€ - -
The Gitter Badger

πŸ’» 🚧
Thomas Poignant

πŸ› + +
Thomas Steinbach

πŸ›
Thorsten Hoeger

πŸ’»
Tim Wagner

πŸ› πŸ€”
Tobias Lidskog

πŸ’»
Ty Coghlan

πŸ› - -
Tyler van Hensbergen

πŸ€”
Vlad Hrybok

πŸ› + +
Vladimir Shchur

πŸ›
Yan Zhulanow

πŸ’»
Yigong Liu

πŸ› πŸ€”
Zach Bienenfeld

πŸ›
ajnarang

πŸ€” - -
aniljava

πŸ’»
deccy-mcc

πŸ› + +
dependabot-preview[bot]

πŸ› 🚧
dependabot[bot]

🚧
dheffx

πŸ›
gregswdl

πŸ›
guyroberts21

πŸ“– +
kaizen3031593

πŸ’» πŸ› +
mattBrzezinski

πŸ“– -
mattBrzezinski

πŸ“– +
mergify

🚧
mergify[bot]

🚧
seiyashima42

πŸ› πŸ’» πŸ“–
sullis

πŸ’» From 07b6a6dc89da95010218a12fd770d61bffcd373a Mon Sep 17 00:00:00 2001 From: AWS CDK Automation <43080478+aws-cdk-automation@users.noreply.github.com> Date: Fri, 19 Nov 2021 02:27:58 +0530 Subject: [PATCH 09/10] chore(merge-back): 1.44.2 (#3188) See [CHANGELOG](https://github.com/aws/jsii/blob/merge-back/1.44.2/CHANGELOG.md) --- CHANGELOG.md | 7 +++++++ lerna.json | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cb3a108d9..74346e7a8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [1.44.2](https://github.com/aws/jsii/compare/v1.44.1...v1.44.2) (2021-11-18) + + +### Bug Fixes + +* **jsii:** require statement for the warning file is generated when it's not used ([#3184](https://github.com/aws/jsii/issues/3184)) ([8f53f89](https://github.com/aws/jsii/commit/8f53f897ebc03e3b3f9e5837fea988e7af592571)) + ## [1.44.1](https://github.com/aws/jsii/compare/v1.44.0...v1.44.1) (2021-11-16) * revert "fix: dependency submodules may not be discovered" ([#3170](https://github.com/aws/jsii/pull/3170)) ([0449dd9](https://github.com/aws/jsii/commit/0449dd92ce3297b065c171efafc28d1f877432cc)) diff --git a/lerna.json b/lerna.json index 56890a91fe..4333be0c9b 100644 --- a/lerna.json +++ b/lerna.json @@ -10,5 +10,5 @@ "rejectCycles": true } }, - "version": "1.44.1" + "version": "1.44.2" } From 1f0b34cdda0cab9d7e6e29a7e565af35634c6d93 Mon Sep 17 00:00:00 2001 From: AWS CDK Team Date: Thu, 18 Nov 2021 21:00:27 +0000 Subject: [PATCH 10/10] chore(release): 1.45.0 --- CHANGELOG.md | 13 +++++++++++++ lerna.json | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74346e7a8a..e48e50cbf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [1.45.0](https://github.com/aws/jsii/compare/v1.44.1...v1.45.0) (2021-11-18) + + +### Bug Fixes + +* C# NamespaceDoc emitted to wrong location ([#3183](https://github.com/aws/jsii/issues/3183)) ([0f5f349](https://github.com/aws/jsii/commit/0f5f349d2f6d7d916c59e4f13e0c5195cc0f80c9)) +* **jsii:** require statement for the warning file is generated when it's not used ([#3184](https://github.com/aws/jsii/issues/3184)) ([3d90ae6](https://github.com/aws/jsii/commit/3d90ae699fdcf6730d9b5559c55e78ec5f9c1260)) +* **pacmak:** dotnet code docs loses indentation ([#3180](https://github.com/aws/jsii/issues/3180)) ([ace0b83](https://github.com/aws/jsii/commit/ace0b83a0c951052349b102c3af34e92cae76767)) +* **pacmak:** Generate Relative Module Imports in Python ([#3181](https://github.com/aws/jsii/issues/3181)) ([b0afe51](https://github.com/aws/jsii/commit/b0afe51a8cb36a9ebdd39bd19a842285c58ee2c1)) +* **rosetta:** diagnostics not showing ([#3182](https://github.com/aws/jsii/issues/3182)) ([92a7d5e](https://github.com/aws/jsii/commit/92a7d5e3b10dc7c881ef70c12730f1f0cdcf9b63)) +* **rosetta:** Rosetta is not submodule-aware ([#3176](https://github.com/aws/jsii/issues/3176)) ([5c7d148](https://github.com/aws/jsii/commit/5c7d148104f6cfb54573a73a76efb288dd2f346b)) +* **rosetta:** types from submodules not recognized properly ([#3174](https://github.com/aws/jsii/issues/3174)) ([b009d07](https://github.com/aws/jsii/commit/b009d07c2ae34248d1e7beea3c66121b8deef957)) + ## [1.44.2](https://github.com/aws/jsii/compare/v1.44.1...v1.44.2) (2021-11-18) diff --git a/lerna.json b/lerna.json index 4333be0c9b..ab3198019d 100644 --- a/lerna.json +++ b/lerna.json @@ -10,5 +10,5 @@ "rejectCycles": true } }, - "version": "1.44.2" + "version": "1.45.0" }