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.

Sign up for GitHub

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): attempts transliteraiton to unsupported languages #3478

Merged
merged 2 commits into from
Apr 26, 2022
Merged

fix(rosetta): attempts transliteraiton to unsupported languages #3478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0a50d2
fix(rosetta): attempts transliteraiton to unsupported languages
RomainMuller Apr 11, 2022
41158cf
Merge branch 'main' into rmuller/translate-missing-skip-unsupported
RomainMuller Apr 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/jsii-rosetta/lib/commands/transliterate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { readJson, writeJson } from 'fs-extra';
import { resolve } from 'path';

import { TargetLanguage } from '../languages';
import { targetName } from '../languages/target-language';
import { debug } from '../logging';
import { RosettaTabletReader, UnknownSnippetMode } from '../rosetta-reader';
import { typeScriptSnippetFromVisibleSource, ApiLocation } from '../snippet';
Expand Down Expand Up @@ -99,6 +100,12 @@ export async function transliterateAssembly(
const now = new Date().getTime();
// eslint-disable-next-line no-await-in-loop
const result = await loadAssembly();

if (result.targets?.[targetName(language)] == null) {
// This language is not supported by the assembly, so we skip it...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if we need some explicit output to tell callers that a language is unsupported?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah but we'd want that ONLY if the language was specifically requested...?

continue;
}

if (result.readme?.markdown) {
result.readme.markdown = rosetta.translateSnippetsInMarkdown(
{ api: 'moduleReadme', moduleFqn: result.name },
Expand Down
31 changes: 31 additions & 0 deletions packages/jsii-rosetta/lib/languages/target-language.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,37 @@
import * as assert from 'assert';

export enum TargetLanguage {
PYTHON = 'python',
CSHARP = 'csharp',
JAVA = 'java',
GO = 'go',
}

const VALID_TARGET_LANGUAGES = new Set(Object.values(TargetLanguage));

export function targetName(language: TargetLanguage.PYTHON): 'python';
export function targetName(language: TargetLanguage.CSHARP): 'dotnet';
export function targetName(language: TargetLanguage.JAVA): 'java';
export function targetName(language: TargetLanguage.GO): 'go';
export function targetName(language: TargetLanguage): 'python' | 'dotnet' | 'java' | 'go';
/**
* @param language a possible value for `TargetLanguage`.
*
* @returns the name of the target configuration block for the given language.
*/
export function targetName(language: TargetLanguage): 'python' | 'dotnet' | 'java' | 'go' {
// The TypeScript compiler should guarantee the below `switch` statement covers all possible
// values of the TargetLanguage enum, but we add an assert here for clarity of intent.
assert(VALID_TARGET_LANGUAGES.has(language), `Invalid/unexpected target language identifier: ${language}`);

switch (language) {
case TargetLanguage.PYTHON:
return 'python';
case TargetLanguage.CSHARP:
return 'dotnet';
case TargetLanguage.JAVA:
return 'java';
case TargetLanguage.GO:
return 'go';
}
}
209 changes: 166 additions & 43 deletions packages/jsii-rosetta/test/commands/transliterate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,18 @@ import * as path from 'path';

import { extractSnippets } from '../../lib/commands/extract';
import { transliterateAssembly } from '../../lib/commands/transliterate';
import { TargetLanguage } from '../../lib/languages/target-language';
import { TargetLanguage, targetName } from '../../lib/languages/target-language';
import { TabletSchema } from '../../lib/tablets/schema';
import { withTemporaryDirectory, TestJsiiModule, DUMMY_JSII_CONFIG } from '../testutil';

jest.setTimeout(60_000);

// A targets configuration block with ALL targets enabled (although with phony configuration).
const targets = Object.values(TargetLanguage).reduce((tgt, lang) => {
tgt[targetName(lang)] = { phony: true };
return tgt;
}, {} as Record<string, unknown>);

test('single assembly, all languages', () =>
withTemporaryDirectory(async (tmpDir) => {
// GIVEN
Expand Down Expand Up @@ -90,9 +96,13 @@ export class ClassName implements IInterface {
}
}`,
});
fs.writeJsonSync(path.join(tmpDir, SPEC_FILE_NAME), compilationResult.assembly, {
spaces: 2,
});
fs.writeJsonSync(
path.join(tmpDir, SPEC_FILE_NAME),
{ ...compilationResult.assembly, targets },
{
spaces: 2,
},
);
for (const [file, content] of Object.entries(compilationResult.files)) {
fs.writeFileSync(path.resolve(tmpDir, file), content, 'utf-8');
}
Expand All @@ -115,6 +125,7 @@ export class ClassName implements IInterface {
{
fingerprint: expect.any(String),
jsiiVersion: expect.any(String),
targets: expect.any(Object),
},
`
Object {
Expand Down Expand Up @@ -153,11 +164,7 @@ export class ClassName implements IInterface {
"url": "https://github.com/aws/jsii.git",
},
"schema": "jsii/0.10.0",
"targets": Object {
"js": Object {
"npm": "testpkg",
},
},
"targets": Any<Object>,
"types": Object {
"testpkg.ClassName": Object {
"assembly": "testpkg",
Expand Down Expand Up @@ -373,6 +380,7 @@ export class ClassName implements IInterface {
{
fingerprint: expect.any(String),
jsiiVersion: expect.any(String),
targets: expect.any(Object),
},
`
Object {
Expand Down Expand Up @@ -411,11 +419,7 @@ export class ClassName implements IInterface {
"url": "https://github.com/aws/jsii.git",
},
"schema": "jsii/0.10.0",
"targets": Object {
"js": Object {
"npm": "testpkg",
},
},
"targets": Any<Object>,
"types": Object {
"testpkg.ClassName": Object {
"assembly": "testpkg",
Expand Down Expand Up @@ -631,6 +635,7 @@ export class ClassName implements IInterface {
{
fingerprint: expect.any(String),
jsiiVersion: expect.any(String),
targets: expect.any(Object),
},
`
Object {
Expand Down Expand Up @@ -669,11 +674,7 @@ export class ClassName implements IInterface {
"url": "https://github.com/aws/jsii.git",
},
"schema": "jsii/0.10.0",
"targets": Object {
"js": Object {
"npm": "testpkg",
},
},
"targets": Any<Object>,
"types": Object {
"testpkg.ClassName": Object {
"assembly": "testpkg",
Expand Down Expand Up @@ -933,9 +934,13 @@ import { SampleClass } from './index';
new SampleClass('omitted-literate');
`,
});
fs.writeJsonSync(path.join(tmpDir, SPEC_FILE_NAME), compilationResult.assembly, {
spaces: 2,
});
fs.writeJsonSync(
path.join(tmpDir, SPEC_FILE_NAME),
{ ...compilationResult.assembly, targets },
{
spaces: 2,
},
);
for (const [file, content] of Object.entries(compilationResult.files)) {
if (file.startsWith('omit-')) {
continue;
Expand All @@ -955,6 +960,7 @@ new SampleClass('omitted-literate');
{
fingerprint: expect.any(String),
jsiiVersion: expect.any(String),
targets: expect.any(Object),
},
`
Object {
Expand Down Expand Up @@ -1008,11 +1014,7 @@ new SampleClass('omitted-literate');
"url": "https://github.com/aws/jsii.git",
},
"schema": "jsii/0.10.0",
"targets": Object {
"js": Object {
"npm": "testpkg",
},
},
"targets": Any<Object>,
"types": Object {
"testpkg.SampleClass": Object {
"assembly": "testpkg",
Expand Down Expand Up @@ -1053,6 +1055,7 @@ new SampleClass('omitted-literate');
{
fingerprint: expect.any(String),
jsiiVersion: expect.any(String),
targets: expect.any(Object),
},
`
Object {
Expand Down Expand Up @@ -1106,11 +1109,7 @@ new SampleClass('omitted-literate');
"url": "https://github.com/aws/jsii.git",
},
"schema": "jsii/0.10.0",
"targets": Object {
"js": Object {
"npm": "testpkg",
},
},
"targets": Any<Object>,
"types": Object {
"testpkg.SampleClass": Object {
"assembly": "testpkg",
Expand Down Expand Up @@ -1151,6 +1150,7 @@ new SampleClass('omitted-literate');
{
fingerprint: expect.any(String),
jsiiVersion: expect.any(String),
targets: expect.any(Object),
},
`
Object {
Expand Down Expand Up @@ -1204,11 +1204,7 @@ new SampleClass('omitted-literate');
"url": "https://github.com/aws/jsii.git",
},
"schema": "jsii/0.10.0",
"targets": Object {
"js": Object {
"npm": "testpkg",
},
},
"targets": Any<Object>,
"types": Object {
"testpkg.SampleClass": Object {
"assembly": "testpkg",
Expand Down Expand Up @@ -1322,9 +1318,13 @@ export class ClassName implements IInterface {
}
}`,
});
fs.writeJsonSync(path.join(tmpDir, SPEC_FILE_NAME), compilationResult.assembly, {
spaces: 2,
});
fs.writeJsonSync(
path.join(tmpDir, SPEC_FILE_NAME),
{ ...compilationResult.assembly, targets },
{
spaces: 2,
},
);
for (const [file, content] of Object.entries(compilationResult.files)) {
fs.writeFileSync(path.resolve(tmpDir, file), content, 'utf-8');
}
Expand Down Expand Up @@ -1471,9 +1471,14 @@ export class ClassName implements IInterface {
}
}`,
});
fs.writeJsonSync(path.join(tmpDir, SPEC_FILE_NAME), compilationResult.assembly, {
spaces: 2,
});

fs.writeJsonSync(
path.join(tmpDir, SPEC_FILE_NAME),
{ ...compilationResult.assembly, targets },
{
spaces: 2,
},
);
for (const [file, content] of Object.entries(compilationResult.files)) {
fs.writeFileSync(path.resolve(tmpDir, file), content, 'utf-8');
}
Expand All @@ -1500,3 +1505,121 @@ export class ClassName implements IInterface {
expect(fs.statSync(path.join(outdir, `${SPEC_FILE_NAME}.${lang}`)).isFile()).toBe(true);
});
}));

test('will not attempt to produce output for an unsupported language', async () =>
withTemporaryDirectory(async (tmpDir) => {
// GIVEN
const compilationResult = jsii.compileJsiiForTest({
'README.md': `
# README
\`\`\`ts
const object: IInterface = new ClassName('this', 1337, { foo: 'bar' });
object.property = EnumType.OPTION_A;
object.methodCall();

ClassName.staticMethod(EnumType.OPTION_B);
\`\`\`
`,
'index.ts': `
/**
* @example new ClassName('this', 1337, { property: EnumType.OPTION_B });
*/
export enum EnumType {
/**
* @example new ClassName('this', 1337, { property: EnumType.OPTION_A });
*/
OPTION_A = 1,

/**
* @example new ClassName('this', 1337, { property: EnumType.OPTION_B });
*/
OPTION_B = 2,
}

export interface IInterface {
/**
* A property value.
*
* @example
* iface.property = EnumType.OPTION_B;
*/
property: EnumType;

/**
* An instance method call.
*
* @example
* iface.methodCall();
*/
methodCall(): void;
}

export interface ClassNameProps {
readonly property?: EnumType;
readonly foo?: string;
}

export class ClassName implements IInterface {
/**
* A static method. It can be invoked easily.
*
* @example ClassName.staticMethod();
*/
public static staticMethod(_enm?: EnumType): void {
// ...
}

public property: EnumType;

/**
* Create a new instance of ClassName.
*
* @example new ClassName('this', 1337, { property: EnumType.OPTION_B });
*/
public constructor(_this: string, _elite: number, props: ClassNameProps) {
this.property = props.property ?? EnumType.OPTION_A;
}

public methodCall(): void {
// ...
}
}`,
});

fs.writeJsonSync(
path.join(tmpDir, SPEC_FILE_NAME),
{ ...compilationResult.assembly, targets: { ...targets, [targetName(TargetLanguage.GO)]: undefined } },
{
spaces: 2,
},
);
for (const [file, content] of Object.entries(compilationResult.files)) {
fs.writeFileSync(path.resolve(tmpDir, file), content, 'utf-8');
}
fs.mkdirSync(path.resolve(tmpDir, 'rosetta'));
fs.writeFileSync(
path.resolve(tmpDir, 'rosetta', 'default.ts-fixture'),
`import { EnumType, IInterface, ClassName } from '.';\ndeclare const iface: IInterface\n/// here`,
'utf-8',
);

// WHEN
// create outdir
const outdir = path.resolve(tmpDir, 'out');
fs.mkdirSync(outdir);

await expect(
transliterateAssembly([tmpDir], Object.values(TargetLanguage), {
strict: true,
outdir,
}),
).resolves.not.toThrow();

Object.values(TargetLanguage).forEach((lang) => {
if (lang === TargetLanguage.GO) {
expect(fs.pathExistsSync(path.join(outdir, `${SPEC_FILE_NAME}.${lang}`))).toBe(false);
} else {
expect(fs.statSync(path.join(outdir, `${SPEC_FILE_NAME}.${lang}`)).isFile()).toBe(true);
}
});
}));