Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

prefer-object-spread: ignore Object.assign({}, this) #3126

Merged

Conversation

aervin
Copy link
Contributor

@aervin aervin commented Aug 14, 2017

  • Addresses an existing issue: #3117
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

Allows for Object.assign(obj, this)

* object types."
*/
if (node.arguments[1] !== undefined) {
if (node.arguments[1].kind === ts.SyntaxKind.ThisKeyword) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's not limited to the second argument. We should exit if any parameter to Object.assign is this similar to the check !node.arguments.some(isSpreadElement) above

/**
* @url https://github.com/palantir/tslint/issues/3117
* Per TS2698, "spread types may only be created from
* object types."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also add a TODO to remove this when typescript get's support for spread types and a link to the PR (linked in the issue comments)

@@ -1,3 +1,5 @@
const form = Object.assign({}, this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test for const foo = Object.assign({}, bar, baz, this);

* object types."
*
*/
if (node.arguments.some((arg) => arg.kind === ts.SyntaxKind.ThisKeyword)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add this to the condition ending in line 62

@@ -1,3 +1,5 @@
const form = Object.assign({}, this);
const foo = Object.assign({}, bar, baz, this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs another test: foo = Object.assign(bar, this);

@ajafff ajafff merged commit 1197e15 into palantir:master Aug 15, 2017
HyphnKnight pushed a commit to HyphnKnight/tslint that referenced this pull request Apr 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants