From 5d2392b7713cb6dfea6092c4ac3ee45360a5d28a Mon Sep 17 00:00:00 2001
From: Ben Chaimberg <chaimber@amazon.com>
Date: Tue, 29 Jun 2021 08:37:02 -0700
Subject: [PATCH] fix(rosetta): extract does not respect strict metadata entry
 (#2863)

The `jsii.rosetta.strict` assembly metadata entry was not being respected by the `extract` command due to two bugs:
1. Fixturizing (the process by which snippets are enriched by a separate fixture) did not transfer the "strict" flag from the original snippet to the fixturized snippet. Fix: transfer the strict flag to the fixturized snippet.
2. Translation of snippets to other languages using the `worker_threads` node module involves sending compiler diagnostics across the wire. These diagnostics should be annotated with a "strict brand" that marks them as failing compilation and allows further steps to fail if such a diagnostic exist. Because the brand was a [Symbol](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol), it is not correctly serialized. Fix: make the brand a string so it can be properly serialized.

fixes #2861

---

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/lib/fixtures.ts       |  3 +-
 packages/jsii-rosetta/lib/util.ts           |  4 +-
 packages/jsii-rosetta/test/fixtures.test.ts | 19 ++++++
 packages/jsii-rosetta/test/util.test.ts     | 65 +++++++++++++++++++++
 4 files changed, 87 insertions(+), 4 deletions(-)
 create mode 100644 packages/jsii-rosetta/test/fixtures.test.ts
 create mode 100644 packages/jsii-rosetta/test/util.test.ts

diff --git a/packages/jsii-rosetta/lib/fixtures.ts b/packages/jsii-rosetta/lib/fixtures.ts
index b12750361d..81f3d09979 100644
--- a/packages/jsii-rosetta/lib/fixtures.ts
+++ b/packages/jsii-rosetta/lib/fixtures.ts
@@ -39,9 +39,8 @@ export function fixturize(snippet: TypeScriptSnippet): TypeScriptSnippet {
   }
 
   return {
-    visibleSource: snippet.visibleSource,
+    ...snippet,
     completeSource: source,
-    where: snippet.where,
     parameters,
   };
 }
diff --git a/packages/jsii-rosetta/lib/util.ts b/packages/jsii-rosetta/lib/util.ts
index b2e326ae01..888cc672bb 100644
--- a/packages/jsii-rosetta/lib/util.ts
+++ b/packages/jsii-rosetta/lib/util.ts
@@ -38,7 +38,7 @@ export function printDiagnostic(
   stream.write(message);
 }
 
-const StrictBrand = Symbol('strict');
+export const StrictBrand = 'jsii.strict';
 interface MaybeStrictDiagnostic {
   readonly [StrictBrand]?: boolean;
 }
@@ -46,7 +46,7 @@ interface MaybeStrictDiagnostic {
 export function annotateStrictDiagnostic(diag: ts.Diagnostic) {
   Object.defineProperty(diag, StrictBrand, {
     configurable: false,
-    enumerable: false,
+    enumerable: true,
     value: true,
     writable: false,
   });
diff --git a/packages/jsii-rosetta/test/fixtures.test.ts b/packages/jsii-rosetta/test/fixtures.test.ts
new file mode 100644
index 0000000000..92e9f9ff7c
--- /dev/null
+++ b/packages/jsii-rosetta/test/fixtures.test.ts
@@ -0,0 +1,19 @@
+import { fixturize } from '../lib/fixtures';
+import { SnippetParameters } from '../lib/snippet';
+
+describe('fixturize', () => {
+  test('snippet retains properties', () => {
+    const snippet = {
+      visibleSource: 'visibleSource',
+      where: 'where',
+      parameters: {
+        [SnippetParameters.$PROJECT_DIRECTORY]: 'directory',
+        [SnippetParameters.NO_FIXTURE]: '',
+        key: 'value',
+      },
+      strict: true,
+    };
+
+    expect(fixturize(snippet)).toEqual(expect.objectContaining(snippet));
+  });
+});
diff --git a/packages/jsii-rosetta/test/util.test.ts b/packages/jsii-rosetta/test/util.test.ts
new file mode 100644
index 0000000000..c565782502
--- /dev/null
+++ b/packages/jsii-rosetta/test/util.test.ts
@@ -0,0 +1,65 @@
+import * as ts from 'typescript';
+
+import {
+  StrictBrand,
+  annotateStrictDiagnostic,
+  isErrorDiagnostic,
+} from '../lib/util';
+
+describe(annotateStrictDiagnostic, () => {
+  const diagnostic = {
+    category: ts.DiagnosticCategory.Error,
+    code: 999,
+    messageText: 'messageText',
+    file: undefined,
+    start: undefined,
+    length: undefined,
+  };
+
+  test('adds strict property', () => {
+    annotateStrictDiagnostic(diagnostic);
+
+    expect(diagnostic).toHaveProperty([StrictBrand]);
+  });
+});
+
+describe(isErrorDiagnostic, () => {
+  const warningDiagnostic = makeDiagnostic(ts.DiagnosticCategory.Warning);
+  const errorDiagnostic = makeDiagnostic(ts.DiagnosticCategory.Error);
+  const strictErrorDiagnostic = {
+    ...makeDiagnostic(ts.DiagnosticCategory.Error),
+    [StrictBrand]: true,
+  };
+  const diagnostics = [
+    warningDiagnostic,
+    errorDiagnostic,
+    strictErrorDiagnostic,
+  ];
+
+  test('returns all error diagnostics if onlyStrict is false', () => {
+    const onlyStrict = false;
+
+    expect(
+      diagnostics.filter((diag) => isErrorDiagnostic(diag, { onlyStrict })),
+    ).toStrictEqual([errorDiagnostic, strictErrorDiagnostic]);
+  });
+
+  test('returns only strict error diagnostics if onlyStrict is true', () => {
+    const onlyStrict = true;
+
+    expect(
+      diagnostics.filter((diag) => isErrorDiagnostic(diag, { onlyStrict })),
+    ).toStrictEqual([strictErrorDiagnostic]);
+  });
+});
+
+function makeDiagnostic(category: ts.DiagnosticCategory): ts.Diagnostic {
+  return {
+    category: category,
+    code: 999,
+    messageText: 'messageText',
+    file: undefined,
+    start: undefined,
+    length: undefined,
+  };
+}