Skip to content

Commit

Permalink
fix(rosetta): didCompile evaluates to true when compilation not att…
Browse files Browse the repository at this point in the history
…empted (#3149)

When `rosetta:extract` is run, the property `didCompile` evalutes to `true`
even when compilation was not attempted. This causes lots of confusion.
The correct way to handle this is to return `undefined` when compilation is
not attempted. Then, (I assume) `didCompile` will not show up in the tablet
file at all when compilation is not asked for (via `--compile` or `strict`.

---

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
kaizencc committed Nov 9, 2021
1 parent 62b3e9b commit 7ad9e0a
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
10 changes: 8 additions & 2 deletions packages/jsii-rosetta/lib/translate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export class SnippetTranslator {
public readonly compileDiagnostics: ts.Diagnostic[] = [];
private readonly visibleSpans: Spans;
private readonly compilation!: CompilationResult;
private readonly tryCompile: boolean;

public constructor(snippet: TypeScriptSnippet, private readonly options: SnippetTranslatorOptions = {}) {
const compiler = options.compiler ?? new TypeScriptCompiler();
Expand All @@ -170,7 +171,9 @@ export class SnippetTranslator {
this.visibleSpans = Spans.visibleSpansFromSource(source);

// This makes it about 5x slower, so only do it on demand
if (options.includeCompilerDiagnostics || snippet.strict) {
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
this.tryCompile = (options.includeCompilerDiagnostics || snippet.strict) ?? false;
if (this.tryCompile) {
const program = this.compilation.program;
const diagnostics = [
...neverThrowing(program.getGlobalDiagnostics)(),
Expand Down Expand Up @@ -208,8 +211,11 @@ export class SnippetTranslator {
}
}

/**
* Returns a boolean if compilation was attempted, and undefined if it was not.
*/
public get didSuccessfullyCompile() {
return this.compileDiagnostics.length === 0;
return this.tryCompile ? this.compileDiagnostics.length === 0 : undefined;
}

public renderUsing(visitor: AstHandler<any>) {
Expand Down
32 changes: 32 additions & 0 deletions packages/jsii-rosetta/test/translate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,35 @@ test('Snippets from different locations have different keys', () => {

expect(snippetKey(snippet1)).not.toEqual(snippetKey(snippet2));
});

test('didSuccessfullyCompile is true when compilation is attempted', () => {
const visibleSource = 'console.log("banana");';

const snippet: TypeScriptSnippet = {
visibleSource,
location: { api: { api: 'type', fqn: 'my.class' } },
};

// WHEN
const subject = new SnippetTranslator(snippet, {
includeCompilerDiagnostics: true,
});
subject.renderUsing(new PythonVisitor());

expect(subject.didSuccessfullyCompile).toBeTruthy();
});

test('didSuccessfullyCompile is undefined when compilation is not attempted', () => {
const visibleSource = 'console.log("banana");';

const snippet: TypeScriptSnippet = {
visibleSource,
location: { api: { api: 'type', fqn: 'my.class' } },
};

// WHEN
const subject = new SnippetTranslator(snippet);
subject.renderUsing(new PythonVisitor());

expect(subject.didSuccessfullyCompile).toBeUndefined();
});

0 comments on commit 7ad9e0a

Please sign in to comment.