From 6652ebea3e338f05a377f6f124d20520a840b1d5 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 20 Mar 2023 08:30:11 -0400 Subject: [PATCH] fix(typescript-estree): allow writing to deprecated node properties (#6670) * fix(typescript-estree): allow writing to deprecated node properties * Always Object.defineProperty * Correct getter assignment * Add unit tests for enumeration and writing * process.emitWarning * Updated convert tests too --- packages/typescript-estree/src/convert.ts | 25 ++++++---- .../tests/lib/convert.test.ts | 48 +++++++++++++++---- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index 4610d41f665..ac39b0b487b 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -3319,23 +3319,30 @@ export class Converter { aliasKey: AliasKey, valueKey: ValueKey, ): Properties & Record { - let errored = false; + let warned = false; Object.defineProperty(node, aliasKey, { + configurable: true, get: this.options.suppressDeprecatedPropertyWarnings ? (): Properties[typeof valueKey] => node[valueKey] : (): Properties[typeof valueKey] => { - if (!this.options.suppressDeprecatedPropertyWarnings) { - if (!errored) { - // eslint-disable-next-line no-console - console.warn( - `The '${aliasKey}' property is deprecated on ${node.type} nodes. Use '${valueKey}' instead. See https://typescript-eslint.io/linting/troubleshooting#the-key-property-is-deprecated-on-type-nodes-use-key-instead-warnings.`, - ); - errored = true; - } + if (!warned) { + process.emitWarning( + `The '${aliasKey}' property is deprecated on ${node.type} nodes. Use '${valueKey}' instead. See https://typescript-eslint.io/linting/troubleshooting#the-key-property-is-deprecated-on-type-nodes-use-key-instead-warnings.`, + 'DeprecationWarning', + ); + warned = true; } + return node[valueKey]; }, + set(value): void { + Object.defineProperty(node, aliasKey, { + enumerable: true, + writable: true, + value, + }); + }, }); return node as Properties & Record; diff --git a/packages/typescript-estree/tests/lib/convert.test.ts b/packages/typescript-estree/tests/lib/convert.test.ts index 7d48204cf8f..3e7991683e8 100644 --- a/packages/typescript-estree/tests/lib/convert.test.ts +++ b/packages/typescript-estree/tests/lib/convert.test.ts @@ -7,6 +7,10 @@ import type { ConverterOptions } from '../../src/convert'; import { Converter } from '../../src/convert'; describe('convert', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + function convertCode(code: string): ts.SourceFile { return ts.createSourceFile( 'text.ts', @@ -268,7 +272,7 @@ describe('convert', () => { describe('suppressDeprecatedPropertyWarnings', () => { const getEsCallExpression = ( - converterOptions: ConverterOptions, + converterOptions?: ConverterOptions, ): TSESTree.CallExpression => { const ast = convertCode(`callee();`); const tsCallExpression = (ast.statements[0] as ts.ExpressionStatement) @@ -285,8 +289,10 @@ describe('convert', () => { return maps.tsNodeToESTreeNodeMap.get(tsCallExpression); }; - it('logs on a deprecated property access when suppressDeprecatedPropertyWarnings is false', () => { - const warn = jest.spyOn(console, 'warn').mockImplementation(); + it('warns on a deprecated property access when suppressDeprecatedPropertyWarnings is false', () => { + const emitWarning = jest + .spyOn(process, 'emitWarning') + .mockImplementation(); const esCallExpression = getEsCallExpression({ suppressDeprecatedPropertyWarnings: false, }); @@ -294,13 +300,16 @@ describe('convert', () => { // eslint-disable-next-line deprecation/deprecation esCallExpression.typeParameters; - expect(warn).toHaveBeenCalledWith( + expect(emitWarning).toHaveBeenCalledWith( `The 'typeParameters' property is deprecated on CallExpression nodes. Use 'typeArguments' instead. See https://typescript-eslint.io/linting/troubleshooting#the-key-property-is-deprecated-on-type-nodes-use-key-instead-warnings.`, + 'DeprecationWarning', ); }); - it('does not log on a subsequent deprecated property access when suppressDeprecatedPropertyWarnings is false', () => { - const warn = jest.spyOn(console, 'warn').mockImplementation(); + it('does not warn on a subsequent deprecated property access when suppressDeprecatedPropertyWarnings is false', () => { + const emitWarning = jest + .spyOn(process, 'emitWarning') + .mockImplementation(); const esCallExpression = getEsCallExpression({ suppressDeprecatedPropertyWarnings: false, }); @@ -310,11 +319,13 @@ describe('convert', () => { esCallExpression.typeParameters; /* eslint-enable deprecation/deprecation */ - expect(warn).toHaveBeenCalledTimes(1); + expect(emitWarning).toHaveBeenCalledTimes(1); }); - it('does not log on a deprecated property access when suppressDeprecatedPropertyWarnings is true', () => { - const warn = jest.spyOn(console, 'warn').mockImplementation(); + it('does not warn on a deprecated property access when suppressDeprecatedPropertyWarnings is true', () => { + const emitWarning = jest + .spyOn(process, 'emitWarning') + .mockImplementation(); const esCallExpression = getEsCallExpression({ suppressDeprecatedPropertyWarnings: true, }); @@ -322,7 +333,24 @@ describe('convert', () => { // eslint-disable-next-line deprecation/deprecation esCallExpression.typeParameters; - expect(warn).not.toHaveBeenCalled(); + expect(emitWarning).not.toHaveBeenCalled(); + }); + + it('does not allow enumeration of deprecated properties', () => { + const esCallExpression = getEsCallExpression(); + + expect(Object.keys(esCallExpression)).not.toContain('typeParameters'); + }); + + it('allows writing to the deprecated property as a new enumerable value', () => { + const esCallExpression = getEsCallExpression(); + + // eslint-disable-next-line deprecation/deprecation + esCallExpression.typeParameters = undefined; + + // eslint-disable-next-line deprecation/deprecation + expect(esCallExpression.typeParameters).toBeUndefined(); + expect(Object.keys(esCallExpression)).toContain('typeParameters'); }); }); });