Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rosetta): classes are not correctly identified if package uses an outDir #3225

Merged
merged 13 commits into from
Dec 7, 2021
12 changes: 8 additions & 4 deletions packages/jsii-rosetta/lib/jsii/jsii-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,16 @@ export function lookupJsiiSymbol(typeChecker: ts.TypeChecker, sym: ts.Symbol): J
// 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);
const tscRootDir = sourceAssembly?.assembly.metadata?.tscRootDir;
return fmap(
sourceAssembly,
(asm) =>
({
fqn:
fmap(symbolIdentifier(typeChecker, sym), (symbolId) => sourceAssembly?.symbolIdMap[symbolId]) ??
sourceAssembly?.assembly.name,
fmap(
symbolIdentifier(typeChecker, sym, false, tscRootDir),
(symbolId) => sourceAssembly?.symbolIdMap[symbolId],
) ?? sourceAssembly?.assembly.name,
sourceAssembly: asm,
symbolType: 'module',
} as JsiiSymbol),
Expand Down Expand Up @@ -187,12 +190,13 @@ function lookupTypeSymbol(
typeSymbol: ts.Symbol,
fileName: string,
): JsiiSymbol | undefined {
const symbolId = symbolIdentifier(typeChecker, typeSymbol);
const sourceAssembly = findTypeLookupAssembly(fileName);
const tscRootDir = sourceAssembly?.assembly.metadata?.tscRootDir;
const symbolId = symbolIdentifier(typeChecker, typeSymbol, false, tscRootDir);
if (!symbolId) {
return undefined;
}

const sourceAssembly = findTypeLookupAssembly(fileName);
return fmap(sourceAssembly?.symbolIdMap[symbolId], (fqn) => ({ fqn, sourceAssembly, symbolType: 'type' }));
}

Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/languages/record-references.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class RecordReferencesVisitor extends DefaultVisitor<RecordReferencesCont
(node.type && renderer.typeOfType(node.type)) ||
(node.initializer && renderer.typeOfExpression(node.initializer));

this.recordSymbol(type?.symbol, renderer);
this.recordSymbol(type.symbol, renderer);
}

return super.variableDeclaration(node, renderer);
Expand Down
72 changes: 72 additions & 0 deletions packages/jsii-rosetta/test/commands/extract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,45 @@ test('do not ignore example strings', async () => {
}
});

describe('can find fqns via symbolId when ', () => {
test('there is an outDir', async () => {
const outDir = 'jsii-outDir';
const otherAssembly = await createAssemblyWithDirectories(undefined, outDir);
try {
const outputFile = path.join(otherAssembly.moduleDirectory, 'test.tabl.json');
await extract.extractSnippets([otherAssembly.moduleDirectory], {
outputFile,
...defaultExtractOptions,
});

const tablet = await LanguageTablet.fromFile(outputFile);
const tr = tablet.tryGetSnippet(tablet.snippetKeys[0]);
expect(tr?.fqnsReferenced()).toEqual(['my_assembly.ClassA']);
} finally {
await otherAssembly.cleanup();
}
});

test('there is an outDir and rootDir', async () => {
const outDir = 'jsii-outDir';
const rootDir = '.';
const otherAssembly = await createAssemblyWithDirectories(rootDir, outDir);
try {
const outputFile = path.join(otherAssembly.moduleDirectory, 'test.tabl.json');
await extract.extractSnippets([otherAssembly.moduleDirectory], {
outputFile,
...defaultExtractOptions,
});

const tablet = await LanguageTablet.fromFile(outputFile);
const tr = tablet.tryGetSnippet(tablet.snippetKeys[0]);
expect(tr?.fqnsReferenced()).toEqual(['my_assembly.ClassA']);
} finally {
await otherAssembly.cleanup();
}
});
});

test('extract and infuse in one command', async () => {
const outputFile = path.join(assembly.moduleDirectory, 'test.tabl.json');
await extract.extractAndInfuse([assembly.moduleDirectory], {
Expand Down Expand Up @@ -222,3 +261,36 @@ class MockTranslator extends RosettaTranslator {
this.translateAll = translatorFn;
}
}

async function createAssemblyWithDirectories(rootDir?: string, outDir?: string) {
return TestJsiiModule.fromSource(
{
'index.ts': `
export class ClassA {
/**
* Some method
*
* @example
* import * as ass from 'my_assembly';
* new ass.ClassA.someMethod();
*/
public someMethod() {
}
}
`,
'README.md': '',
},
{
name: 'my_assembly',
main: `${outDir}/index.js`,
types: `${outDir}/index.d.ts`,
jsii: {
...DUMMY_JSII_CONFIG,
tsc: {
outDir,
rootDir,
},
},
},
);
}
4 changes: 3 additions & 1 deletion packages/jsii-rosetta/test/testutil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export type MultipleSources = { [key: string]: string; 'index.ts': string };
export class TestJsiiModule {
public static async fromSource(
source: string | MultipleSources,
packageInfo: Partial<PackageInfo> & { name: string },
packageInfo: Partial<PackageInfo> & { name: string; main?: string; types?: string },
) {
const { assembly, files } = await compileJsiiForTest(source, (pi) => {
Object.assign(pi, packageInfo);
Expand All @@ -41,6 +41,8 @@ export class TestJsiiModule {
await fs.writeJSON(path.join(modDir, '.jsii'), assembly);
await fs.writeJSON(path.join(modDir, 'package.json'), {
name: packageInfo.name,
main: packageInfo.main,
types: packageInfo.types,
jsii: packageInfo.jsii,
});
for (const [fileName, fileContents] of Object.entries(files)) {
Expand Down
12 changes: 8 additions & 4 deletions packages/jsii/lib/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export class Assembler implements Emitter {
private readonly warningsInjector?: DeprecationWarningsInjector;

private readonly mainFile: string;
private readonly tscRootDir?: string;

private _diagnostics = new Array<JsiiDiagnostic>();
private _deferred = new Array<DeferredRecord>();
Expand Down Expand Up @@ -113,10 +114,10 @@ export class Assembler implements Emitter {

// rootDir may be set explicitly or not. If not, inferRootDir replicates
// tsc's behavior of using the longest prefix of all built source files.
const tscRootDir =
this.tscRootDir =
program.getCompilerOptions().rootDir ?? inferRootDir(program);
if (tscRootDir != null) {
mainFile = path.join(tscRootDir, mainFile);
if (this.tscRootDir != null) {
mainFile = path.join(this.tscRootDir, mainFile);
}
}

Expand Down Expand Up @@ -253,7 +254,10 @@ export class Assembler implements Emitter {
types: this._types,
submodules: noEmptyDict(toSubmoduleDeclarations(this.mySubmodules())),
targets: this.projectInfo.targets,
metadata: this.projectInfo.metadata,
metadata: {
...this.projectInfo.metadata,
tscRootDir: this.tscRootDir,
},
docs,
readme,
jsiiVersion,
Expand Down
8 changes: 6 additions & 2 deletions packages/jsii/lib/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,12 @@ export async function compileJsiiForTest(
const files: Record<string, string> = {};

for (const filename of Object.keys(source)) {
const jsFile = filename.replace(/\.ts$/, '.js');
const dtsFile = filename.replace(/\.ts$/, '.d.ts');
let jsFile = filename.replace(/\.ts$/, '.js');
let dtsFile = filename.replace(/\.ts$/, '.d.ts');
if (projectInfo.tsc?.outDir && filename !== 'README.md') {
jsFile = path.join(projectInfo.tsc.outDir, jsFile);
dtsFile = path.join(projectInfo.tsc.outDir, dtsFile);
}

// eslint-disable-next-line no-await-in-loop
files[jsFile] = await fs.readFile(jsFile, { encoding: 'utf-8' });
Expand Down
32 changes: 29 additions & 3 deletions packages/jsii/lib/symbol-id.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import * as ts from 'typescript';
export function symbolIdentifier(
typeChecker: ts.TypeChecker,
sym: ts.Symbol,
valueAsIs = true,
kaizencc marked this conversation as resolved.
Show resolved Hide resolved
tscRootDir?: string,
): string | undefined {
// If this symbol happens to be an alias, resolve it first
while ((sym.flags & ts.SymbolFlags.Alias) !== 0) {
Expand Down Expand Up @@ -39,7 +41,11 @@ export function symbolIdentifier(
if (!decl) {
return undefined;
}
const namespace = assemblyRelativeSourceFile(decl.getSourceFile().fileName);
const namespace = assemblyRelativeSourceFile(
decl.getSourceFile().fileName,
valueAsIs,
tscRootDir,
);

if (!namespace) {
return undefined;
Expand All @@ -48,7 +54,11 @@ export function symbolIdentifier(
return `${namespace}:${inFileNameParts.join('.')}`;
}

function assemblyRelativeSourceFile(sourceFileName: string) {
function assemblyRelativeSourceFile(
sourceFileName: string,
valueAsIs: boolean,
tscRootDir?: string,
) {
const packageJsonLocation = findPackageJsonLocation(
path.dirname(sourceFileName),
);
Expand All @@ -61,11 +71,27 @@ function assemblyRelativeSourceFile(sourceFileName: string) {
fs.readFileSync(packageJsonLocation).toString(),
);

const sourcePath = removePrefix(
let sourcePath = removePrefix(
packageJson.jsii?.outdir ?? '',
path.relative(path.dirname(packageJsonLocation), sourceFileName),
);

// Modify the namespace if we allow changes and if the outDir exists.
// We should only allow changes if we call the function from rosetta.
if (!valueAsIs && packageJson.jsii?.tsc?.outDir) {
const paths = path.normalize(sourcePath).split(path.sep);
const pathDir = paths.shift();
kaizencc marked this conversation as resolved.
Show resolved Hide resolved
const outDir = path.normalize(packageJson.jsii.tsc.outDir);
// Theoretically we should always find tscRootDir if valueAsIs is false.
const rootDir =
packageJson.jsii.tsc.rootDir ??
(tscRootDir !== undefined ? path.normalize(tscRootDir) : undefined);
// If we find our outDir, replace it with rootDir
if (outDir === pathDir && rootDir) {
sourcePath =
rootDir === '.' ? paths.join('/') : `${rootDir}/${paths.join('/')}`;
}
}
return sourcePath.replace(/(\.d)?\.ts$/, '');

function findPackageJsonLocation(currentPath: string): string | undefined {
Expand Down
47 changes: 46 additions & 1 deletion packages/jsii/test/compiler.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { mkdtemp, remove, writeFile, readFile, readJson } from 'fs-extra';
import {
ensureDir,
mkdtemp,
remove,
writeFile,
readFile,
readJson,
} from 'fs-extra';
import { tmpdir } from 'os';
import { join } from 'path';

Expand Down Expand Up @@ -40,6 +47,7 @@ describe(Compiler, () => {
).toEqual(expectedTypeScriptConfig());
});
});

test('"watch" mode', async () => {
// This can be a little slow, allowing 15 seconds maximum here (default is 5 seconds)
jest.setTimeout(15_000);
Expand Down Expand Up @@ -103,6 +111,43 @@ describe(Compiler, () => {
await remove(sourceDir);
}
});

test('rootDir is added to assembly', async () => {
const outDir = 'jsii-outdir';
const rootDir = 'jsii-rootdir';
const sourceDir = await mkdtemp(join(tmpdir(), 'jsii-tmpdir'));
await ensureDir(join(sourceDir, rootDir));

try {
await writeFile(
join(sourceDir, rootDir, 'index.ts'),
'export class MarkerA {}',
);
// Intentionally using lower case name - it should be case-insensitive
await writeFile(join(sourceDir, rootDir, 'readme.md'), '# Test Package');

const compiler = new Compiler({
projectInfo: {
..._makeProjectInfo(sourceDir, join(outDir, 'index.d.ts')),
tsc: {
outDir,
rootDir,
},
},
failOnWarnings: true,
projectReferences: false,
});

await compiler.emit();

const assembly = await readJson(join(sourceDir, '.jsii'), 'utf-8');
expect(assembly.metadata).toEqual({
kaizencc marked this conversation as resolved.
Show resolved Hide resolved
tscRootDir: rootDir,
});
} finally {
await remove(sourceDir);
}
});
});

function _makeProjectInfo(sourceDir: string, types: string): ProjectInfo {
Expand Down