Skip to content

Rule proposal: prevent array, iterable and function spreads into objects #748

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

Closed
ckknight opened this issue Jul 23, 2019 · 5 comments · Fixed by #10551
Closed

Rule proposal: prevent array, iterable and function spreads into objects #748

ckknight opened this issue Jul 23, 2019 · 5 comments · Fixed by #10551
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new plugin rule New rule request for eslint-plugin locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@ckknight
Copy link

ckknight commented Jul 23, 2019

Repro

// Array spread
const squares = [1, 4, 9, 16];
const cubes = [1, 8, 27];
const bothArray = { ...squares, ...cubes };
// rule would error here, as spreading an array causes surprising results
bothArray === {0: 1, 1: 8, 2: 27, 3: 16}
// TS: typeof bothArray === (typeof Array<number>) ❌

// Iterable spread
const map = new Map([[1,2], [3,4]]);
const set = new Set([5,6,7,8]);
const bothIterable = { ...map, ...set };
// rule would error here as spreading an iterable does nothing
bothIterable === {}
// TS: typeof allFunc === (typeof Set<number> & typeof Map<number, number>) ❌

// Function spread
function funcDecl() {}
const funcExpr = function () {};
const arrowFunc = () => {};
const allFunc = { ...funcDecl, ...funcExpr, ...arrowFunc };
// rule would error here as spreading a function does nothing
allFunc === {};
// TS: typeof allFunc === ({}) ✅

// Object with call-signature
interface FuncWithProps {
  property?: string;
  (): number;
}
const funcWithProps: FuncWithProps = () => 1;
funcWithProps.property = 'foo';
const spreadFuncWithProps = {...funcWithProps};
// rule would NOT error here as custom function props are spread as expected
spreadFuncWithProps === {property: 'foo'};
// TS: typeof allFunc === (FuncWithProps) ✅

TypeScript does not have an understanding of how iterable object spreads work, and borks the types.
This leads to unsoundness and broken code.

TypeScript does understand function object spreads, but they are noops, so they shouldn't be allowed.

Additional Information
Detecting this should be a matter of detecting whether the type of each expression being spread within an object implements Iterable<T>, defined within lib.es2015.iterable.d.ts.

The converse rule of preventing objects being spread within an array seems unnecessary/out-of-scope, given that the TypeScript type system is able to determine that each expression spread within an array is iterable.

@ckknight ckknight added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jul 23, 2019
@bradzacher bradzacher added enhancement: new plugin rule New rule request for eslint-plugin and removed triage Waiting for team members to take a look labels Jul 23, 2019
@bradzacher
Copy link
Member

bradzacher commented Jul 23, 2019

I hadn't ever thought about this sort of thing.

Just running some expanded examples using builtins:

const map = new Map([['a', 1], ['b', 2]])
const set = new Set([1, 2])
const arr = [1,2]
const obj = {a: 1, b: 2}
const str = "asdf" as string;
const num = 1 as number;
const bool = true as boolean;
const fn = () => {};
class Clazz { static staticProp = 1; instanceProp = 2; }
const classInstance = new Clazz();

const mapArr = [...map] // === [['a', 1], ['b', 2]]
const mapObj = {...map} // === {}

const setArr = [...set] // === [1, 2]
const setObj = {...set} // === {}

const arrArr = [...arr] // === [1, 2]
const arrObj = {...arr} // === {0: 1, 1: 2}

const objArr = [...obj] // compile time error: Type '{ a: number; b: number; }' must have a '[Symbol.iterator]()' method that returns an iterator.
                        // runtime error     : Uncaught TypeError: object is not iterable (cannot read property Symbol(Symbol.iterator))
const objObj = {...obj} // === {a: 1, b: 2}

const strArr = [...str]; // === ['a', 's', 'd', 'f']
const strObj = {...str}; // === {0: 'a', 1: 'b', 2: 'd', 3: 'f'}
                         // compile time error: Spread types may only be created from object types.
                         
const numArr = [...num]; // compile time error: Type 'number' must have a '[Symbol.iterator]()' method that returns an iterator.
                         // runtime error     : Uncaught TypeError: 1 is not iterable
const numObj = {...num}; // === {}
                         // compile time error: Spread types may only be created from object types.
                         
const boolArr = [...bool]; // compile time error: Type 'boolean' must have a '[Symbol.iterator]()' method that returns an iterator.
                           // runtime error     : Uncaught TypeError: true is not iterable
const boolObj = {...bool}; // === {}
                           // compile time error: Spread types may only be created from object types.

const fnArr = [...fn]; // compile time error: Type '() => void' must have a '[Symbol.iterator]()' method that returns an iterator.
                       // runtime error     : Uncaught TypeError: fn is not iterable
const fnObj = {...fn}; // === {}
                       // generally undesirable though

const classDeclArr = [...Clazz]; // compile time error: Type 'typeof Clazz' must have a '[Symbol.iterator]()' method that returns an iterator.
                                 // runtime error     : Uncaught TypeError: Clazz is not iterable
const classDeclObj = {...Clazz}; // === {staticProp: 1}

const classInstanceArr = [...classInstance]; // compile time error: Type 'Clazz' must have a '[Symbol.iterator]()' method that returns an iterator.
                                             // runtime error     : Uncaught TypeError: classInstance is not iterable
const classInstanceObj = {...classInstance}; // === {instanceProp: 2}

typescript repl

@bradzacher bradzacher changed the title Rule proposal: prevent array/iterable spread into objects Rule proposal: prevent array, iterable and function spreads into objects May 9, 2021
@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
@stefanpl
Copy link

stefanpl commented Dec 6, 2022

I'd ❤️ to see this. Came here because, once more, I spent time debugging when I spread a function without calling it.

@OliverJAsh
Copy link
Contributor

This should cover functions. I imagine it wouldn't be too hard to extend this to cover arrays and iterables. WDYT?

import { ESLintUtils, TSESLint, TSESTree } from '@typescript-eslint/utils';

import { ruleCreator } from '../utils';

export const noSpreadFunction = ruleCreator({
  defaultOptions: [],
  meta: {
    docs: {
      description: '',
      recommended: false,
    },
    messages: {
      forbidden: 'Spreading a function is almost always a mistake.',
    },
    schema: [],
    type: 'problem',
  },
  create: (context): TSESLint.RuleListener => {
    const listener = (node: TSESTree.SpreadElement): void => {
      const svc = ESLintUtils.getParserServices(context);
      const tc = svc.program.getTypeChecker();

      const tsNode = svc.esTreeNodeToTSNodeMap.get(node.argument);
      const type = tc.getTypeAtLocation(tsNode);

      if (type.getCallSignatures().length > 0) {
        context.report({
          node,
          messageId: 'forbidden',
        });
      }
    };
    return {
      SpreadElement: listener,
    };
  },
});

@lcharlois-neotys
Copy link

Hello,

Is there any chance to see this rule added to the plugin?
Do you still accept PR for this one?

@JoshuaKGoldberg
Copy link
Member

We are a community run project. The volunteer maintainers spend most of their time triaging issues and reviewing PRs. This means that most issues will not progress unless a member of the community steps up and champions it.

If this issue is important to you — consider being that champion.
If not — please use the subscribe function and wait patiently for someone else to implement this.

StyleShit added a commit to StyleShit/typescript-eslint that referenced this issue Feb 19, 2024
@github-actions github-actions bot added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Jan 20, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new plugin rule New rule request for eslint-plugin locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
6 participants