Skip to content

Commit

Permalink
fix(rosetta): stop skipping example values (#3128)
Browse files Browse the repository at this point in the history
In CDK, we use the `@example` tag a lot to indicate example values
of some property, instead of code examples. For example, a `bucketArn`
property might have as annotation `@example arn:aws:s3:....`.

Rosetta would try to compile those, and fail. To get around this, we would skip
values that *looked* like they weren't source in `rosetta extract`.

This doesn't combine well with pacmak live-translation. We are either forced to
try and compile them again during pacmak (adding compile time), or fail
pacmak as soon as we encounter these. And all of this is to work around
abuse of the `@example` tag for something it's not intended for.

So stop treating these examples specially. They will just fail compilation
unless rewritten. This will force us to deal with them properly in the
CDK source code.



---

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 5, 2021
1 parent ab60275 commit ee0620a
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 16 deletions.
17 changes: 1 addition & 16 deletions packages/jsii-rosetta/lib/jsii/assemblies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export function allSnippetSources(assembly: spec.Assembly): AssemblySnippetSourc
location,
});
}
if (docs.example && exampleLooksLikeSource(docs.example)) {
if (docs.example) {
ret.push({
type: 'example',
source: docs.example,
Expand Down Expand Up @@ -142,19 +142,6 @@ export function allTypeScriptSnippets(assemblies: readonly LoadedAssembly[], loo
return ret;
}

/**
* See if the given source text looks like a code sample
*
* Many @examples for properties are examples of values (ARNs, formatted strings)
* not code samples, which should not be translated
*
* If the value contains whitespace (newline, space) then we'll assume it's a code
* sample.
*/
function exampleLooksLikeSource(text: string) {
return !!WHITESPACE.exec(text.trim());
}

/**
* Replaces the file where the original assembly file *should* be found with a new assembly file.
* Recalculates the fingerprint of the assembly to avoid tampering detection.
Expand All @@ -177,5 +164,3 @@ function _fingerprint(assembly: spec.Assembly): spec.Assembly {
const fingerprint = crypto.createHash('sha256').update(JSON.stringify(assembly)).digest('base64');
return { ...assembly, fingerprint };
}

const WHITESPACE = new RegExp('\\s');
36 changes: 36 additions & 0 deletions packages/jsii-rosetta/test/commands/extract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,39 @@ describe('with cache file', () => {
}
});
});

test('do not ignore example strings', async () => {
// Create an assembly in a temp directory
const otherAssembly = await AssemblyFixture.fromSource(
{
'index.ts': `
export class ClassA {
/**
* Some method
* @example x
*/
public someMethod() {
}
}
`,
},
{
name: 'my_assembly',
jsii: DUMMY_ASSEMBLY_TARGETS,
},
);
try {
const outputFile = path.join(otherAssembly.directory, 'test.tabl.json');
await extract.extractSnippets([otherAssembly.directory], {
outputFile,
...defaultExtractOptions,
});

const tablet = await LanguageTablet.fromFile(outputFile);
expect(tablet.count).toEqual(1);
const tr = tablet.tryGetSnippet(tablet.snippetKeys[0]);
expect(tr?.originalSource.source).toEqual('x');
} finally {
await assembly.cleanup();
}
});

0 comments on commit ee0620a

Please sign in to comment.