Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(typescript-estree): allow writing to deprecated node properties #6670

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 16 additions & 9 deletions packages/typescript-estree/src/convert.ts
Original file line number Diff line number Diff line change
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,
});
},
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
});

return node as Properties & Record<AliasKey, Properties[ValueKey]>;
Expand Down
48 changes: 38 additions & 10 deletions packages/typescript-estree/tests/lib/convert.test.ts
Original file line number Diff line number Diff line change
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');
});
});
});