Skip to content

Commit

Permalink
fix: deprecated symbol stripping does not strip out heritage clause w…
Browse files Browse the repository at this point in the history
…ith imported type (#2783)

When a non-deprecated class or interface inherits a deprecated class or
interface, the `--strip-deprecated` option does not strip out the
heritage clause.

This particularly affects cases where the inherited class or interface
is non-local, i.e., imported from another file.

---

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
Niranjan Jayakar authored Apr 13, 2021
1 parent 0648d0b commit e87d879
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 11 deletions.
6 changes: 5 additions & 1 deletion packages/jsii/lib/transforms/deprecated-remover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,11 @@ class Transformation {
const symbol = typeChecker.getSymbolAtLocation(
ts.getNameOfDeclaration(node as ts.Declaration) ?? node,
);
return symbol && typeChecker.getFullyQualifiedName(symbol);
// This symbol ☝️ does not contain enough information in some cases - when
// an imported type is part of a heritage clause - to produce the fqn.
// Round tripping this to its type and back to a symbol seems to fix this.
const type = symbol && typeChecker.getDeclaredTypeOfSymbol(symbol);
return type?.symbol && typeChecker.getFullyQualifiedName(type.symbol);
}

private static typeReference(
Expand Down
56 changes: 46 additions & 10 deletions packages/jsii/test/deprecated-remover.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { compileJsiiForTest } from '../lib';
import { compileJsiiForTest, HelperCompilationResult } from '../lib';

const DEPRECATED = '/** @deprecated stripped */';

Expand Down Expand Up @@ -114,15 +114,7 @@ test('produces correct output', async () => {
},
}
`);
expect(
Object.entries(result.files)
.filter(([name]) => name.endsWith('.d.ts'))
.map(([name, content]) => {
const separator = '/'.repeat(name.length + 8);
return `${separator}\n/// ${name} ///\n${content}${separator}\n`;
})
.join('\n\n'),
).toMatchInlineSnapshot(`
expect(declFilesSnapshot(result)).toMatchInlineSnapshot(`
"//////////////////
/// index.d.ts ///
import './deprecated';
Expand Down Expand Up @@ -159,3 +151,47 @@ test('produces correct output', async () => {
"
`);
});

test('cross-file deprecated heritage', async () => {
const result = await compileJsiiForTest(
{
'index.ts': `
import { IDeprecated } from './deprecated';
export * from './deprecated';
export interface INotDeprecated extends IDeprecated {}
`,
'deprecated.ts': `
${DEPRECATED}
export interface IDeprecated {}
`,
},
undefined /* callback */,
{ stripDeprecated: true },
);

expect(declFilesSnapshot(result)).toMatchInlineSnapshot(`
"//////////////////
/// index.d.ts ///
import './deprecated';
import './deprecated';
export interface INotDeprecated {
}
//////////////////
///////////////////////
/// deprecated.d.ts ///
///////////////////////
"
`);
});

function declFilesSnapshot(result: HelperCompilationResult) {
return Object.entries(result.files)
.filter(([name]) => name.endsWith('.d.ts'))
.map(([name, content]) => {
const separator = '/'.repeat(name.length + 8);
return `${separator}\n/// ${name} ///\n${content}${separator}\n`;
})
.join('\n\n');
}

0 comments on commit e87d879

Please sign in to comment.