Skip to content

Commit

Permalink
feat(pacmak): fail on untranslated snippets (#3127)
Browse files Browse the repository at this point in the history
Because of backwards compatibility reasons, pacmak will try to live-translate
example code it encounters. However, this can add unpredictably to the compile
times, especially while we're making sure that rosetta and pacmak agree on API
locations for all snippets that are being translated (which are now part of the
snippet keys).

Add an option to pacmak to fail if any snippets are encountered that are not
pre-translated in any of the tablets. This most likely signifies an error.

This will be enabled in cdklabs/cdk-ops#1777.



---

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
  • Loading branch information
rix0rrr authored Nov 8, 2021
1 parent bf769da commit 874e8e2
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 19 deletions.
17 changes: 16 additions & 1 deletion packages/jsii-pacmak/bin/jsii-pacmak.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env node
import '@jsii/check-node/run';

import { UnknownSnippetMode } from 'jsii-rosetta';
import * as yargs from 'yargs';

import { pacmak, configureLogging, TargetName } from '../lib';
Expand Down Expand Up @@ -93,9 +94,20 @@ import { VERSION_DESC } from '../lib/version';
})
.option('rosetta-translate-live', {
type: 'boolean',
desc: "Translate code samples on-the-fly if they can't be found in the samples tablet",
desc: "Translate code samples on-the-fly if they can't be found in the samples tablet (deprecated)",
default: true,
})
.option('rosetta-unknown-snippets', {
type: 'string',
requiresArg: true,
optional: true,
choices: [
UnknownSnippetMode.VERBATIM,
UnknownSnippetMode.TRANSLATE,
UnknownSnippetMode.FAIL,
],
desc: "What to do with code samples if they can't be found in the samples tablet",
})
.option('parallel', {
type: 'boolean',
desc: 'Generate all configured targets in parallel (disabling this might help if you encounter EMFILE errors)',
Expand Down Expand Up @@ -136,6 +148,9 @@ import { VERSION_DESC } from '../lib/version';
parallel: argv.parallel,
recurse: argv.recurse,
rosettaLiveConversion: argv['rosetta-translate-live'],
rosettaUnknownSnippets: argv['rosetta-unknown-snippets'] as
| UnknownSnippetMode
| undefined,
rosettaTablet: argv['rosetta-tablet'],
targets: argv.targets?.map((target) => target as TargetName),
updateNpmIgnoreFiles: argv.npmignore,
Expand Down
19 changes: 17 additions & 2 deletions packages/jsii-pacmak/lib/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TypeSystem } from 'jsii-reflect';
import { Rosetta } from 'jsii-rosetta';
import { Rosetta, UnknownSnippetMode } from 'jsii-rosetta';

import * as logging from './logging';
import { findJsiiModules, updateAllNpmIgnores } from './npm-modules';
Expand Down Expand Up @@ -31,10 +31,17 @@ export async function pacmak({
rosettaTablet,
targets = Object.values(TargetName),
timers = new Timers(),
rosettaUnknownSnippets = undefined,
updateNpmIgnoreFiles = false,
validateAssemblies = false,
}: PacmakOptions): Promise<void> {
const rosetta = new Rosetta({ liveConversion: rosettaLiveConversion });
const unknownSnippets =
rosettaUnknownSnippets ??
(rosettaLiveConversion
? UnknownSnippetMode.TRANSLATE
: UnknownSnippetMode.VERBATIM);

const rosetta = new Rosetta({ unknownSnippets });
if (rosettaTablet) {
await rosetta.loadTabletFromFile(rosettaTablet);
}
Expand Down Expand Up @@ -219,9 +226,17 @@ export interface PacmakOptions {
* already translated in the `rosettaTablet` file.
*
* @default false
* @deprecated Use `rosettaUnknownSnippets` instead.
*/
readonly rosettaLiveConversion?: boolean;

/**
* How rosetta should treat snippets that cannot be loaded from a translation tablet.
*
* @default - falls back to the default of `rosettaLiveConversion`.
*/
readonly rosettaUnknownSnippets?: UnknownSnippetMode;

/**
* A Rosetta tablet file where translations for code examples can be found.
*
Expand Down
4 changes: 2 additions & 2 deletions packages/jsii-rosetta/lib/commands/transliterate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { resolve } from 'path';
import { fixturize } from '../fixtures';
import { TargetLanguage } from '../languages';
import { debug } from '../logging';
import { Rosetta } from '../rosetta';
import { Rosetta, UnknownSnippetMode } from '../rosetta';
import { SnippetParameters, typeScriptSnippetFromSource, ApiLocation } from '../snippet';
import { Translation } from '../tablets/tablets';

Expand Down Expand Up @@ -53,7 +53,7 @@ export async function transliterateAssembly(
): Promise<void> {
const rosetta = new Rosetta({
includeCompilerDiagnostics: true,
liveConversion: true,
unknownSnippets: UnknownSnippetMode.TRANSLATE,
loose: options.loose,
targetLanguages,
});
Expand Down
66 changes: 55 additions & 11 deletions packages/jsii-rosetta/lib/rosetta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,35 @@ import {
ApiLocation,
typeScriptSnippetFromSource,
} from './snippet';
import { snippetKey } from './tablets/key';
import { DEFAULT_TABLET_NAME, LanguageTablet, Translation } from './tablets/tablets';
import { Translator } from './translate';
import { printDiagnostics } from './util';

export enum UnknownSnippetMode {
/**
* Return the snippet as given (untranslated)
*/
VERBATIM = 'verbatim',

/**
* Live-translate the snippet as best as we can
*/
TRANSLATE = 'translate',

/**
* Throw an error if this occurs
*/
FAIL = 'fail',
}

export interface RosettaOptions {
/**
* Whether or not to live-convert samples
*
* @default false
* @default UnknownSnippetMode.VERBATIM
*/
readonly liveConversion?: boolean;
readonly unknownSnippets?: UnknownSnippetMode;

/**
* Target languages to use for live conversion
Expand Down Expand Up @@ -75,9 +93,11 @@ export class Rosetta {
private readonly extractedSnippets = new Map<string, TypeScriptSnippet>();
private readonly translator: Translator;
private readonly loose: boolean;
private readonly unknownSnippets: UnknownSnippetMode;

public constructor(private readonly options: RosettaOptions = {}) {
this.loose = !!options.loose;
this.unknownSnippets = options.unknownSnippets ?? UnknownSnippetMode.VERBATIM;
this.translator = new Translator(options.includeCompilerDiagnostics ?? false);
}

Expand Down Expand Up @@ -115,7 +135,7 @@ export class Rosetta {
*
* Otherwise, if live conversion is enabled, the snippets in the assembly
* become available for live translation later. This is necessary because we probably
* need to fixture snippets for successful compilation, and the information
* need to fixturize snippets for successful compilation, and the information
* pacmak sends our way later on is not going to be enough to do that.
*/
public async addAssembly(assembly: spec.Assembly, assemblyDir: string) {
Expand All @@ -129,9 +149,11 @@ export class Rosetta {
}
}

if (this.options.liveConversion) {
// Inventarize the snippets from this assembly, but only if there's a chance
// we're going to need them.
if (this.unknownSnippets === UnknownSnippetMode.TRANSLATE) {
for (const tsnip of allTypeScriptSnippets([{ assembly, directory: assemblyDir }], this.loose)) {
this.extractedSnippets.set(tsnip.visibleSource, tsnip);
this.extractedSnippets.set(snippetKey(tsnip), tsnip);
}
}
}
Expand All @@ -147,19 +169,40 @@ export class Rosetta {
* will be based on the snippet key, which consists of a hash of the
* visible source and the API location.
* - Otherwise, translate the snippet as-is (without fixture information).
*
* This will do and store a full conversion of the given snippet, even if it only
* returns one language. Subsequent retrievals for the same snippet in other
* languages will reuse the translation from cache.
*
* If you are calling this for the side effect of adding translations to the live
* tablet, you only need to do that for one language.
*/
public translateSnippet(source: TypeScriptSnippet, targetLang: TargetLanguage): Translation | undefined {
// Look for it in loaded tablets
// Look for it in loaded tablets (or previous conversions)
for (const tab of this.allTablets) {
const ret = tab.lookup(source, targetLang);
if (ret !== undefined) {
return ret;
}
}

if (!this.options.liveConversion) {
return undefined;
if (this.unknownSnippets === UnknownSnippetMode.VERBATIM) {
return {
language: targetLang,
source: source.visibleSource,
};
}

if (this.unknownSnippets === UnknownSnippetMode.FAIL) {
const message = [
'The following snippet was not found in any of the loaded tablets:',
source.visibleSource,
`Location: ${JSON.stringify(source.location)}`,
`Language: ${targetLang}`,
].join('\n');
throw new Error(message);
}

if (this.options.targetLanguages && !this.options.targetLanguages.includes(targetLang)) {
throw new Error(
`Rosetta configured for live conversion to ${this.options.targetLanguages.join(
Expand All @@ -168,15 +211,16 @@ export class Rosetta {
);
}

// See if we're going to live-convert it with full source information
const extracted = this.extractedSnippets.get(source.visibleSource);
// See if we can find a fixturized version of this snippet. If so, use that do the live
// conversion.
const extracted = this.extractedSnippets.get(snippetKey(source));
if (extracted !== undefined) {
const snippet = this.translator.translate(extracted, this.options.targetLanguages);
this.liveTablet.addSnippet(snippet);
return snippet.get(targetLang);
}

// Try to live-convert it on the spot (we won't have "where" information or fixtures)
// Try to live-convert it as-is.
const snippet = this.translator.translate(source, this.options.targetLanguages);
this.liveTablet.addSnippet(snippet);
return snippet.get(targetLang);
Expand Down
33 changes: 30 additions & 3 deletions packages/jsii-rosetta/test/rosetta.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
TypeScriptSnippet,
DEFAULT_TABLET_NAME,
Translation,
UnknownSnippetMode,
} from '../lib';
import { TargetLanguage } from '../lib/languages';
import { fakeAssembly } from './jsii/fake-assembly';
Expand All @@ -23,7 +24,7 @@ describe('Rosetta object can do live translation', () => {
beforeEach(() => {
// GIVEN
rosetta = new Rosetta({
liveConversion: true,
unknownSnippets: UnknownSnippetMode.TRANSLATE,
targetLanguages: [TargetLanguage.PYTHON],
});

Expand Down Expand Up @@ -70,7 +71,7 @@ test('Can use preloaded tablet', () => {
test('Rosetta object can do live translation', () => {
// GIVEN
const rosetta = new Rosetta({
liveConversion: true,
unknownSnippets: UnknownSnippetMode.TRANSLATE,
targetLanguages: [TargetLanguage.PYTHON],
});

Expand All @@ -84,10 +85,36 @@ test('Rosetta object can do live translation', () => {
});
});

test('Rosetta object can fail on untranslated snippet', () => {
// GIVEN
const rosetta = new Rosetta({
unknownSnippets: UnknownSnippetMode.FAIL,
targetLanguages: [TargetLanguage.PYTHON],
});

// WHEN
expect(() => {
rosetta.translateSnippet(SAMPLE_CODE, TargetLanguage.PYTHON);
}).toThrow(/snippet was not found/);
});

test('Rosetta can give you an untranslated snippet back', () => {
// GIVEN
const rosetta = new Rosetta({
unknownSnippets: UnknownSnippetMode.VERBATIM,
targetLanguages: [TargetLanguage.PYTHON],
});

// WHEN
const translated = rosetta.translateSnippet(SAMPLE_CODE, TargetLanguage.PYTHON);

expect(translated?.source).toEqual('callThisFunction();');
});

test('Rosetta object can do translation and annotation of snippets in MarkDown', () => {
// GIVEN
const rosetta = new Rosetta({
liveConversion: true,
unknownSnippets: UnknownSnippetMode.TRANSLATE,
targetLanguages: [TargetLanguage.PYTHON],
});

Expand Down

0 comments on commit 874e8e2

Please sign in to comment.