Skip to content

Commit

Permalink
fix(typescript-estree): allow writing to deprecated node properties (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
JoshuaKGoldberg committed Mar 20, 2023
1 parent 28a64b5 commit 6652ebe
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 19 deletions.
25 changes: 16 additions & 9 deletions packages/typescript-estree/src/convert.ts
Expand Up @@ -3319,23 +3319,30 @@ export class Converter {
aliasKey: AliasKey,
valueKey: ValueKey,
): Properties & Record<AliasKey, Properties[ValueKey]> {
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<AliasKey, Properties[ValueKey]>;
Expand Down
48 changes: 38 additions & 10 deletions packages/typescript-estree/tests/lib/convert.test.ts
Expand Up @@ -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',
Expand Down Expand Up @@ -268,7 +272,7 @@ describe('convert', () => {

describe('suppressDeprecatedPropertyWarnings', () => {
const getEsCallExpression = (
converterOptions: ConverterOptions,
converterOptions?: ConverterOptions,
): TSESTree.CallExpression => {
const ast = convertCode(`callee<T>();`);
const tsCallExpression = (ast.statements[0] as ts.ExpressionStatement)
Expand All @@ -285,22 +289,27 @@ 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,
});

// 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,
});
Expand All @@ -310,19 +319,38 @@ 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,
});

// 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');
});
});
});

0 comments on commit 6652ebe

Please sign in to comment.