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

prefer-object-spread should ignore use with functions #3055

Closed
davidgoli opened this issue Jul 19, 2017 · 4 comments
Closed

prefer-object-spread should ignore use with functions #3055

davidgoli opened this issue Jul 19, 2017 · 4 comments

Comments

@davidgoli
Copy link
Contributor

Bug Report

  • TSLint version: 5.4.3
  • TypeScript version: 2.4.1
  • Running TSLint via: (pick one) CLI

TypeScript code being linted

Object.assign(
  () => {},
  {
    foo() {},
  }
)

with tslint.json configuration:

{
  "extends": ["tslint:latest", "tslint-config-prettier"],
  "rules": {
    "array-type": [true, "array"],
    "arrow-parens": [true, "ban-single-arg-parens"],
    "arrow-return-shorthand": [true, "multiline"],
    "await-promise": true,
    "curly": false,
    "interface-name": [true, "never-prefix"],
    "match-default-export-name": true,
    "max-classes-per-file": [true, 3],
    "max-line-length": [true, 120],
    "member-access": false,
    "no-consecutive-blank-lines": false,
    "no-duplicate-super": true,
    "no-invalid-template-strings": true,
    "no-misused-new": true,
    "no-sparse-arrays": true,
    "no-trailing-whitespace": false,
    "no-unnecessary-callback-wrapper": true,
    "no-unsafe-finally": true,
    "no-unused-expression": [true, "allow-fast-null-checks"],
    "object-literal-key-quotes": [true, "as-needed"],
    "object-literal-sort-keys": false,
    "ordered-imports": true,
    "prefer-const": true,
    "quotemark": false,
    "semicolon": false,
    "trailing-comma": false,
    "variable-name": [true, "ban-keywords", "check-format", "allow-leading-underscore", "allow-pascal-case"]
  }
}

Actual behavior

Result:
ERROR: /Users/David/dev/hello-epics/power-up/src/app/cache.ts[12, 10]: 'Object.assign' returns the first argument. Prefer object spread if you want a new object.

With prettier fix:

  {...() => {},
      foo() {},
  }

Expected behavior

prefer-object-spread should prefer Object.assign when the first argument is a function, as object rest spread on a function is somewhat nonsensical & probably not desired behavior.

@ajafff
Copy link
Contributor

ajafff commented Jul 22, 2017

I don't know if we want to add special handling for functions (and classes and everything that has a call or contruct signature ...)
For the time being you can use the following workaround:

let fn = () => {};
Object.assign(fn, {
    foo() {}
});

@adidahiya
Copy link
Contributor

The special handling could all be syntactic (done without the type checker), right? You'd just need to check for function expressions and class expressions as the first argument. I think we could take a PR for this.

@davidgoli
Copy link
Contributor Author

I'll take a look at getting a PR together, thanks!

@davidgoli
Copy link
Contributor Author

Closing, thanks for accepting my PR!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants