feat(rosetta): hoist imports above fixtures (#2211)

When `import` statements are used in a code example, they must be
hoisted at the top of the fixture (if any fixture is used), as they
otherwise may occur at locations where such statements are illegal (they
are only accepted at the top-level scope).

Additionaly, added some (hidden) comments to highlight where the hoisted
imports begin and end, as well as where the snipped code begins and ends
in an attempt to make the "complete source code" a little easier to
navigate, especially in the context of larger fixtures.



---

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/README.md

@@ -135,7 +135,9 @@ class MyClass {
 ```
 
 The example will be inserted at the location marked as `/// here` and
-will have access to `module`, `obj` and `this`.
+will have access to `module`, `obj` and `this`. Any `import` statements found
+in the example will automatically be hoisted at the top of the fixture, where
+they are guaranteed to be syntactically valid.
 
 The default file loaded as a fixture is called `rosetta/default.ts-fixture`
 in the package directory (if it exists).

packages/jsii-rosetta/lib/fixtures.ts

@@ -1,5 +1,11 @@
 import * as fs from 'fs-extra';
 import * as path from 'path';
+import {
+  createSourceFile,
+  ScriptKind,
+  ScriptTarget,
+  SyntaxKind,
+} from 'typescript';
 
 import { TypeScriptSnippet, SnippetParameters } from './snippet';
 
@@ -75,10 +81,71 @@ function loadAndSubFixture(
     encoding: 'utf-8',
   });
 
-  const subRegex = /\/\/\/ here/i;
+  const subRegex = /[/]{3}[ \t]*here[ \t]*$/im;
   if (!subRegex.test(fixtureContents)) {
     throw new Error(`Fixture does not contain '/// here': ${fixtureFileName}`);
   }
 
-  return fixtureContents.replace(subRegex, `/// !show\n${source}\n/// !hide`);
+  const { imports, statements } = sidelineImports(source);
+  const show = '/// !show';
+  const hide = '/// !hide';
+
+  const result = fixtureContents.replace(
+    subRegex,
+    [
+      '// Code snippet begins after !show marker below',
+      show,
+      statements,
+      hide,
+      '// Code snippet ended before !hide marker above',
+    ].join('\n'),
+  );
+
+  return imports
+    ? [
+        '// Hoisted imports begin after !show marker below',
+        show,
+        imports,
+        hide,
+        '// Hoisted imports ended before !hide marker above',
+        result,
+      ].join('\n')
+    : result;
+}
+
+/**
+ * When embedding code fragments in a fixture, "import" statements must be
+ * hoisted up to the top of the resulting document, as TypeScript only allows
+ * those to be present in the top-level context of an ESM.
+ *
+ * @param source a block of TypeScript source
+ *
+ * @returns an object containing the import statements on one end, and the rest
+ *          on the other hand.
+ */
+function sidelineImports(
+  source: string,
+): { imports: string; statements: string } {
+  let imports = '';
+  let statements = '';
+
+  const sourceFile = createSourceFile(
+    'index.ts',
+    source,
+    ScriptTarget.Latest,
+    true,
+    ScriptKind.TS,
+  );
+  for (const statement of sourceFile.statements) {
+    switch (statement.kind) {
+      case SyntaxKind.ImportDeclaration:
+      case SyntaxKind.ImportEqualsDeclaration:
+        imports += statement.getFullText(sourceFile);
+        break;
+      default:
+        statements += statement.getFullText(sourceFile);
+    }
+  }
+
+  return { imports, statements };
 }

packages/jsii-rosetta/lib/renderer.ts

@@ -415,38 +415,40 @@ export class AstRenderer<C> {
 
     const precede: OTree[] = [];
     for (const range of leadingRanges) {
+      let trivia: OTree | undefined = undefined;
       switch (range.type) {
         case 'other':
-          precede.push(
-            new OTree(
-              [
-                repeatNewlines(
-                  this.sourceFile.text.substring(range.pos, range.end),
-                ),
-              ],
-              [],
-              {
-                renderOnce: `ws-${range.pos}`,
-              },
-            ),
+          trivia = new OTree(
+            [
+              repeatNewlines(
+                this.sourceFile.text.substring(range.pos, range.end),
+              ),
+            ],
+            [],
+            {
+              renderOnce: `ws-${range.pos}`,
+            },
           );
           break;
         case 'linecomment':
         case 'blockcomment':
-          precede.push(
-            this.handler.commentRange(
-              commentSyntaxFromCommentRange(
-                commentRangeFromTextRange(range),
-                this,
-              ),
+          trivia = this.handler.commentRange(
+            commentSyntaxFromCommentRange(
+              commentRangeFromTextRange(range),
               this,
             ),
+            this,
           );
           break;
 
         case 'directive':
           break;
       }
+      if (trivia != null) {
+        // Set spans on comments to make sure their visibility is toggled correctly.
+        trivia.setSpan(range.pos, range.end);
+        precede.push(trivia);
+      }
     }
 
     // FIXME: No trailing comments for now, they're too tricky

packages/jsii-rosetta/lib/snippet.ts

@@ -66,16 +66,16 @@ export function completeSource(snippet: TypeScriptSnippet) {
 function parametersFromSourceDirectives(
   source: string,
 ): [string, Record<string, string>] {
-  const [firstLine, rest] = source.split('\n', 2);
+  const [firstLine, ...rest] = source.split('\n');
   // Also extract parameters from an initial line starting with '/// ' (getting rid of that line).
-  const m = /\/\/\/(.*)$/.exec(firstLine);
+  const m = /[/]{3}(.*)$/.exec(firstLine);
   if (m) {
     const paramClauses = m[1]
       .trim()
       .split(' ')
       .map((s) => s.trim())
       .filter((s) => s !== '');
-    return [rest, parseKeyValueList(paramClauses)];
+    return [rest.join('\n'), parseKeyValueList(paramClauses)];
   }
 
   return [source, {}];

packages/jsii-rosetta/lib/typescript/ast-utils.ts

@@ -14,7 +14,7 @@ export function calculateVisibleSpans(source: string): Span[] {
 }
 
 export function calculateMarkedSpans(source: string): MarkedSpan[] {
-  const regEx = /\/\/\/ (.*)(\r?\n)?$/gm;
+  const regEx = /[/]{3}[ \t]*(!(?:show|hide))[ \t]*$/gm;
 
   const ret = new Array<MarkedSpan>();
   let match;
@@ -56,7 +56,7 @@ export function stripCommentMarkers(comment: string, multiline: boolean) {
       .replace(/^[ \t]*\*[ \t]?/gm, ''); // Strip "* " from start of line
   }
   // The text *must* start with '//'
-  return comment.replace(/^\/\/[ \t]?/gm, '');
+  return comment.replace(/^[/]{2}[ \t]?/gm, '');
 }
 
 export function stringFromLiteral(expr: ts.Expression) {

packages/jsii-rosetta/test/jsii/assemblies.test.ts

@@ -4,6 +4,7 @@ import * as path from 'path';
 
 import { allTypeScriptSnippets } from '../../lib/jsii/assemblies';
 import { SnippetParameters } from '../../lib/snippet';
+import { fakeAssembly } from './fake-assembly';
 
 test('Extract snippet from README', () => {
   const snippets = Array.from(
@@ -81,11 +82,20 @@ test('Snippet can include fixture', () => {
   );
 
   expect(snippets[0].visibleSource).toEqual('someExample();');
-  expect(snippets[0].completeSource).toEqual(
-    ['// This is a fixture', '/// !show', 'someExample();', '/// !hide'].join(
-      '\n',
-    ),
-  );
+  expect(snippets[0].completeSource).toMatchInlineSnapshot(`
+    "// This is a fixture
+
+    // This is a wrapper so that \`import\` statements are invalid if included in
+    // the code example that'll be inlined at the \`here\` marker.
+    (function () {
+      // Code snippet begins after !show marker below
+    /// !show
+    someExample();
+    /// !hide
+    // Code snippet ended before !hide marker above
+    })()
+    "
+  `);
 });
 
 test('Use fixture from example', () => {
@@ -110,9 +120,73 @@ test('Use fixture from example', () => {
     ]),
   );
 
+  expect(snippets[0].completeSource).toMatchInlineSnapshot(`
+    "// This is a fixture
+
+    // This is a wrapper so that \`import\` statements are invalid if included in
+    // the code example that'll be inlined at the \`here\` marker.
+    (function () {
+      // Code snippet begins after !show marker below
+    /// !show
+    someExample();
+    /// !hide
+    // Code snippet ended before !hide marker above
+    })()
+    "
+  `);
   expect(snippets[0].visibleSource).toEqual('someExample();');
-  expect(snippets[0].completeSource).toEqual(
-    ['// This is a fixture', '/// !show', 'someExample();', '/// !hide'].join(
+});
+
+test('Fixture allows use of import statements', () => {
+  const snippets = Array.from(
+    allTypeScriptSnippets([
+      {
+        assembly: fakeAssembly({
+          types: {
+            'asm.MyType': {
+              kind: spec.TypeKind.Class,
+              assembly: 'asm',
+              fqn: 'asm.MyType',
+              name: 'MyType',
+              docs: {
+                example: [
+                  '/// fixture=explicit',
+                  'import { exit } from "process";',
+                  'someExample();',
+                  'exit(0);',
+                ].join('\n'),
+              },
+            },
+          },
+        }),
+        directory: path.join(__dirname, 'fixtures'),
+      },
+    ]),
+  );
+
+  expect(snippets[0].completeSource).toMatchInlineSnapshot(`
+    "// Hoisted imports begin after !show marker below
+    /// !show
+    import { exit } from \\"process\\";
+    /// !hide
+    // Hoisted imports ended before !hide marker above
+    // This is a fixture
+
+    // This is a wrapper so that \`import\` statements are invalid if included in
+    // the code example that'll be inlined at the \`here\` marker.
+    (function () {
+      // Code snippet begins after !show marker below
+    /// !show
+
+    someExample();
+    exit(0);
+    /// !hide
+    // Code snippet ended before !hide marker above
+    })()
+    "
+  `);
+  expect(snippets[0].visibleSource).toEqual(
+    ['import { exit } from "process";', 'someExample();', 'exit(0);'].join(
       '\n',
     ),
   );
@@ -152,21 +226,3 @@ test('Backwards compatibility with literate integ tests', () => {
     mockfs.restore();
   }
 });
-
-export function fakeAssembly(parts: Partial<spec.Assembly>): spec.Assembly {
-  return Object.assign(
-    {
-      schema: spec.SchemaVersion.LATEST,
-      name: '',
-      description: '',
-      homepage: '',
-      repository: { directory: '', type: '', url: '' },
-      author: { email: '', name: '', organization: false, roles: [], url: '' },
-      fingerprint: '',
-      version: '',
-      jsiiVersion: '',
-      license: '',
-    },
-    parts,
-  );
-}

packages/jsii-rosetta/test/jsii/fake-assembly.ts

@@ -0,0 +1,17 @@
+import { Assembly, SchemaVersion } from '@jsii/spec';
+
+export function fakeAssembly(parts: Partial<Assembly>): Assembly {
+  return {
+    schema: SchemaVersion.LATEST,
+    name: 'test-assembly',
+    description: 'A fake assembly used for tests',
+    homepage: '',
+    repository: { directory: '', type: '', url: '' },
+    author: { email: '', name: '', organization: false, roles: [], url: '' },
+    fingerprint: '<NONE>',
+    version: '0.0.0-use.local',
+    jsiiVersion: '0.0.0-use.local',
+    license: 'UNLICENSED',
+    ...parts,
+  };
+}

packages/jsii-rosetta/test/jsii/fixtures/rosetta/explicit.ts-fixture

@@ -1,2 +1,7 @@
 // This is a fixture
-/// here
+
+// This is a wrapper so that `import` statements are invalid if included in
+// the code example that'll be inlined at the `here` marker.
+(function () {
+  /// here
+})()

packages/jsii-rosetta/test/rosetta.test.ts

@@ -8,7 +8,7 @@ import {
   DEFAULT_TABLET_NAME,
 } from '../lib';
 import { TargetLanguage } from '../lib/languages';
-import { fakeAssembly } from './jsii/assemblies.test';
+import { fakeAssembly } from './jsii/fake-assembly';
 
 const SAMPLE_CODE: TypeScriptSnippet = {
   visibleSource: 'callThisFunction();',