renameShorthandProperties has confusing name

[OliverJAsh]

• VSCode Version: 1.43.2
• OS Version: macOS 10.15.4

Steps to Reproduce:

1. Try to understand the renameShorthandProperties setting

I have been trying to get my head around the renameShorthandProperties setting. After reading through microsoft/TypeScript#29238 and running some tests, it seems that this setting may have been named or configured incorrectly. For this reason, I am suggesting some changes.

As far as I can tell, the setting name represents what the setting will do when disabled—not what it will do when it's enabled (the default).

Example from microsoft/TypeScript#29238 (comment):

const x = {
	y: 'value'
//  ^- rename at key
}
const { y } = x;
//      ^- rename at declaration
console.log(y);
//          ^- rename at use

What happens if I rename at use (with the default setting)?

The setting name ("rename shorthand properties") and its default value of true would seem to suggest that an alias should not be introduced, and the destructured property (aka "shorthand property") should also be renamed:

const x = {
    s: 'value',
};
const { s } = x;
console.log(s);

However, the opposite will happen: the destructured property will be aliased, not renamed:

const x = {
    y: 'value',
};
const { y: s } = x;
console.log(s);

Am I right in thinking this is potentially confusing, or do I misunderstand something?

If my thinking is correct, I believe the setting should be renamed (to avoid confusion), or the current name can stay but the default should be flipped from true to false, to reflect what the setting is actually doing. Note however that this would be a breaking change.

Does this issue occur when all extensions are disabled?: Yes

Related:

• b452299
• Option to disable aliased JS/TS destructured property renaming TypeScript#29238

[mjbvz]

Yes the general idea of what this setting does is captured in the description:

Enable/disable introducing aliases for object shorthand properties during renames

PRs welcome if you have a suggestion for a different name

[OliverJAsh]

I don't think we need to rename the setting, but rather I think the boolean should be flipped, so that the name matches the behaviour.

If my thinking is correct, I believe the setting should be renamed (to avoid confusion), or the current name can stay but the default should be flipped from true to false, to reflect what the setting is actually doing. Note however that this would be a breaking change.

[mjbvz]

We can't flip the value of an existing setting because that will flip the behavior for everyone who has changed the setting, but we can flip (or modify) the setting name name if you have thoughts on that

[OliverJAsh]

What is currently "renameShorthandProperties": true could be mapped to:

• "doNotRenameShorthandProperties": true
• "safeRenames": true
• "useAliasesForRenames": true

[mjbvz]

Renamed to useAliasesForRenames

For verification:

• Make sure the new setting works (useAliasesForRenames)
• Make sure the old setting still works if you don't use the new setting
• Make sure the old setting is marked as deprecated