From 844391b72e35bcb1efa525774afd62425cb03336 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 18 Mar 2023 18:43:39 -0400 Subject: [PATCH 1/6] fix(typescript-estree): allow writing to deprecated node properties --- packages/typescript-estree/src/convert.ts | 40 ++++++++++++++--------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index 4610d41f665..4783e954564 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -3319,23 +3319,33 @@ export class Converter { aliasKey: AliasKey, valueKey: ValueKey, ): Properties & Record { - let errored = false; + if (this.options.suppressDeprecatedPropertyWarnings) { + (node as any)[aliasKey] = node[valueKey]; + return node as Properties & Record; + } + + let warned = false; Object.defineProperty(node, aliasKey, { - 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; - } - } - return node[valueKey]; - }, + configurable: true, + get(): Properties[typeof valueKey] { + if (!warned) { + // 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.`, + ); + warned = true; + } + + return node[valueKey]; + }, + set(value): void { + Object.defineProperty(node, aliasKey, { + enumerable: true, + writable: true, + value, + }); + }, }); return node as Properties & Record; From 15e724aff33cba5c0006c21a14c77d4c419cac14 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 19 Mar 2023 09:34:19 -0400 Subject: [PATCH 2/6] Always Object.defineProperty --- packages/typescript-estree/src/convert.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index 4783e954564..91191d97100 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -3319,16 +3319,15 @@ export class Converter { aliasKey: AliasKey, valueKey: ValueKey, ): Properties & Record { - if (this.options.suppressDeprecatedPropertyWarnings) { - (node as any)[aliasKey] = node[valueKey]; - return node as Properties & Record; - } - let warned = false; Object.defineProperty(node, aliasKey, { configurable: true, get(): Properties[typeof valueKey] { + if (this.options.suppressDeprecatedPropertyWarnings) { + (node as any)[aliasKey] = node[valueKey]; + } + if (!warned) { // eslint-disable-next-line no-console console.warn( From 4aeb3aac5464e7c7529df8037aa3dc3b1d461827 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 19 Mar 2023 10:55:25 -0400 Subject: [PATCH 3/6] Correct getter assignment --- packages/typescript-estree/src/convert.ts | 26 +++++++++++------------ 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index 91191d97100..59d96ed4a55 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -3323,21 +3323,19 @@ export class Converter { Object.defineProperty(node, aliasKey, { configurable: true, - get(): Properties[typeof valueKey] { - if (this.options.suppressDeprecatedPropertyWarnings) { - (node as any)[aliasKey] = node[valueKey]; - } - - if (!warned) { - // 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.`, - ); - warned = true; - } + get: this.options.suppressDeprecatedPropertyWarnings + ? (): Properties[typeof valueKey] => node[valueKey] + : (): Properties[typeof valueKey] => { + if (!warned) { + // 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.`, + ); + warned = true; + } - return node[valueKey]; - }, + return node[valueKey]; + }, set(value): void { Object.defineProperty(node, aliasKey, { enumerable: true, From b7c9677f95543740b55a749618c17f0add49bca3 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 19 Mar 2023 11:36:48 -0400 Subject: [PATCH 4/6] Add unit tests for enumeration and writing --- .../tests/lib/convert.test.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/typescript-estree/tests/lib/convert.test.ts b/packages/typescript-estree/tests/lib/convert.test.ts index 7d48204cf8f..ea5d8fd9080 100644 --- a/packages/typescript-estree/tests/lib/convert.test.ts +++ b/packages/typescript-estree/tests/lib/convert.test.ts @@ -268,7 +268,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) @@ -324,5 +324,22 @@ describe('convert', () => { expect(warn).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'); + }); }); }); From fbbb419cfa1c3abc22d2adaf35540eb50e02829e Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 20 Mar 2023 08:14:55 -0400 Subject: [PATCH 5/6] process.emitWarning --- packages/typescript-estree/src/convert.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index 59d96ed4a55..ac39b0b487b 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -3327,9 +3327,9 @@ export class Converter { ? (): Properties[typeof valueKey] => node[valueKey] : (): Properties[typeof valueKey] => { if (!warned) { - // eslint-disable-next-line no-console - console.warn( + 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; } From 71b51c9d479519123f4955164992879d5a7a15cb Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 20 Mar 2023 08:18:52 -0400 Subject: [PATCH 6/6] Updated convert tests too --- .../tests/lib/convert.test.ts | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/packages/typescript-estree/tests/lib/convert.test.ts b/packages/typescript-estree/tests/lib/convert.test.ts index ea5d8fd9080..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', @@ -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,7 @@ 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', () => {