new rule: no-object-spread-of-iterable

baselines/packages/mimir/api/packlist.txt

@@ -10,6 +10,8 @@ src/formatters/json.js.map
 src/formatters/stylish.d.ts
 src/formatters/stylish.js
 src/formatters/stylish.js.map
+src/iteration.js
+src/iteration.js.map
 src/restricted-property.js
 src/restricted-property.js.map
 src/rules/async-function-assignability.d.ts
@@ -57,6 +59,9 @@ src/rules/no-misused-generics.js.map
 src/rules/no-nan-compare.d.ts
 src/rules/no-nan-compare.js
 src/rules/no-nan-compare.js.map
+src/rules/no-object-spread-of-iterable.d.ts
+src/rules/no-object-spread-of-iterable.js
+src/rules/no-object-spread-of-iterable.js.map
 src/rules/no-octal-escape.d.ts
 src/rules/no-octal-escape.js
 src/rules/no-octal-escape.js.map

baselines/packages/mimir/api/src/rules/no-object-spread-of-iterable.d.ts

@@ -0,0 +1,4 @@
+import { TypedRule } from '@fimbul/ymir';
+export declare class Rule extends TypedRule {
+    apply(): void;
+}

baselines/packages/mimir/test/no-object-spread-of-iterable/default/test.tsx.lint

@@ -0,0 +1,32 @@
+export {};
+
+[...[]];
+[...[1]];
+[...new Set()];
+[...new Map()];
+
+declare function doStuff(...args: any[]): void;
+doStuff(...[]);
+
+declare const arrayLike: ArrayLike<number>;
+({...arrayLike});
+let {...a} = [];
+({...a} = []);
+let b: Array<number>;
+({...b} = [1]);
+
+({...[]});
+     ~~    [error no-object-spread-of-iterable: Spreading an Iterable type into an object is most likely a mistake. Did you intend to use array spread instead?]
+({...[1]});
+     ~~~    [error no-object-spread-of-iterable: Spreading an Iterable type into an object is most likely a mistake. Did you intend to use array spread instead?]
+({...new Set()});
+     ~~~~~~~~~    [error no-object-spread-of-iterable: Spreading an Iterable type into an object is most likely a mistake. Did you intend to use array spread instead?]
+({...new Map()});
+     ~~~~~~~~~    [error no-object-spread-of-iterable: Spreading an Iterable type into an object is most likely a mistake. Did you intend to use array spread instead?]
+
+({a: 1, ...[1] as const});
+({..."..."});
+     ~~~~~    [error no-object-spread-of-iterable: Spreading an Iterable type into an object is most likely a mistake. Did you intend to use array spread instead?]
+
+<span {...new Map()}></span>;
+          ~~~~~~~~~           [error no-object-spread-of-iterable: Spreading an Iterable type into an object is most likely a mistake. Did you intend to use array spread instead?]

baselines/packages/mimir/test/prefer-for-of/es3/types.ts.lint

@@ -213,3 +213,16 @@ declare let privateIterable: PrivateIterable;
 for (let i = 0; i < privateIterable.length; ++i) {
     privateIterable[i]
 }
+
+declare let record: Record<string, number>;
+for (let i = 0; i < record.length; ++i) {
+    record[i];
+}
+
+{
+    interface Array extends ArrayLike {}
+    const customArrayType = null! as Array;
+    for (let i = 0; i < customArrayType.length; ++i) {
+        customArrayType[i];
+    }
+}

baselines/packages/mimir/test/prefer-for-of/es5-iteration/types.ts.lint

@@ -223,3 +223,16 @@ declare let privateIterable: PrivateIterable;
 for (let i = 0; i < privateIterable.length; ++i) {
     privateIterable[i]
 }
+
+declare let record: Record<string, number>;
+for (let i = 0; i < record.length; ++i) {
+    record[i];
+}
+
+{
+    interface Array extends ArrayLike {}
+    const customArrayType = null! as Array;
+    for (let i = 0; i < customArrayType.length; ++i) {
+        customArrayType[i];
+    }
+}

baselines/packages/mimir/test/prefer-for-of/es5/types.ts.lint

@@ -214,3 +214,16 @@ declare let privateIterable: PrivateIterable;
 for (let i = 0; i < privateIterable.length; ++i) {
     privateIterable[i]
 }
+
+declare let record: Record<string, number>;
+for (let i = 0; i < record.length; ++i) {
+    record[i];
+}
+
+{
+    interface Array extends ArrayLike {}
+    const customArrayType = null! as Array;
+    for (let i = 0; i < customArrayType.length; ++i) {
+        customArrayType[i];
+    }
+}

baselines/packages/mimir/test/prefer-for-of/es6/types.ts.lint

@@ -223,3 +223,16 @@ declare let privateIterable: PrivateIterable;
 for (let i = 0; i < privateIterable.length; ++i) {
     privateIterable[i]
 }
+
+declare let record: Record<string, number>;
+for (let i = 0; i < record.length; ++i) {
+    record[i];
+}
+
+{
+    interface Array extends ArrayLike {}
+    const customArrayType = null! as Array;
+    for (let i = 0; i < customArrayType.length; ++i) {
+        customArrayType[i];
+    }
+}

packages/mimir/README.md

@@ -41,6 +41,7 @@ Rule | Description | Difference to TSLint rule / Why you should use it
 [`no-invalid-assertion`](docs/no-invalid-assertion.md) | :mag: Disallows asserting a literal type to a different literal type of the same widened type, e.g. `'foo' as 'bar'`.| TSLint has no similar rule.
 [`no-misused-generics`](docs/no-misused-generics.md) | Detects generic type parameters that cannot be inferred from the functions parameters. It also detects generics that don't enforce any constraint between types. | There's no similar TSLint rule.
 [`no-nan-compare`](docs/no-nan-compare.md) | Disallows comparing with `NaN`, use `isNaN(number)` or `Number.isNaN(number)` instead. | Performance!
+[`no-object-spread-of-iterable`](docs/no-object-spread-of-iterable.md) | :mag: Disallows spreading iterable types into an object. |
 [`no-octal-escape`](docs/no-octal-escape.md) | :wrench: Disallows octal escape sequences in strings and template strings. | No such rule in TSLint.
 [`no-restricted-property-access`](docs/no-restricted-property-access.md) | :mag: Disallows accessing properties via computed name that would not be accessible using a static name. | TSLint has no similar rule.
 [`no-return-await`](docs/no-return-await.md) | :wrench: Disallows unnecesary `return await foo;` when you can simply `return foo;` | The same as TSLint's rule. I wrote both, but this one is faster.

packages/mimir/docs/no-object-spread-of-iterable.md

@@ -0,0 +1,52 @@
+# no-object-spread-of-iterable
+
+:mag: requires type information
+
+Disallows spreading iterable types into an object.
+
+## Rationale
+
+Spreading an iterable type like `Array` via object spread gives an object with the same numeric properties as the original array, but it no longer behaves like an array. There's no `length` property and all methods are not available. Unfortunately TypeScript cannot correctly represent that. So it tells you that object spread of an array gives you an array.
+
+Most of the time this is a typo, as you need to use array spread to copy an array (square brackets instead of curly brackets): `[...arr]` instead of `{...arr}`.
+
+Another possible mistake is a naive attempt to copy complex types like `Map` or `Set` via object spread. This gives you an empty object at runtime, but once again TypeScript doesn't warn you about that.
+
+## Examples
+
+:thumbsdown: Examples of incorrect code
+
+```ts
+declare const arr: Array<number>;
+declare const set: Set<number>;
+declare const map: Map<number>;
+
+console.log({...arr}.reverse()); // will throw at runtime, because 'reverse' does not exist on the resulting object
+console.log({...set}); // this is not how you copy Set
+console.log({...map}); // this is not how you copy Map
+```
+
+:thumbsup: Examples of correct code
+
+```ts
+declare const arr: Array<number>;
+declare const set: Set<number>;
+declare const map: Map<number>;
+
+console.log([...arr].reverse()); // works as expected with square brackets
+console.log(new Set(set)); // this is how you copy Set
+console.log([...set]); // creates a new Array from the values in the Set
+console.log(new Map(map)); // this is not how you copy Map
+console.log([...map]); // creates a new Array with tuples representing the Map's entries
+```
+
+## Further Reading
+
+* MDN: [Spread Syntax](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax)
+
+## Related Rules
+
+* [`no-duplicate-spread-property`](no-duplicate-spread-property.md)
+* [`no-useless-spread`](no-useless-spread.md)
+* [`prefer-object-spread`](prefer-object-spread.md)
+

packages/mimir/docs/no-useless-spread.md

@@ -69,4 +69,5 @@ Math.max(1, ...arr, 2);
 ## Related Rules
 
 * [`no-duplicate-spread-property`](no-duplicate-spread-property.md)
+* [`no-object-spread-of-iterable`](no-object-spread-of-iterable.md)
 * [`prefer-object-spread`](prefer-object-spread.md)

packages/mimir/docs/prefer-object-spread.md

@@ -56,4 +56,5 @@ function copy<T>(obj: T) {
 ## Related Rules
 
 * [`no-duplicate-spread-property`](no-duplciate-spread-property.md)
+* [`no-object-spread-of-iterable`](no-object-spread-of-iterable.md)
 * [`no-useless-spread`](no-useless-spread.md)

packages/mimir/recommended.yaml

@@ -17,6 +17,7 @@ rules:
   no-return-await: error
   no-misused-generics: error
   no-nan-compare: error
+  no-object-spread-of-iterable: error
   no-octal-escape: error
   no-unassigned-variable: error
   no-uninferred-type-parameter: error

packages/mimir/src/iteration.ts

@@ -0,0 +1,123 @@
+import {
+    getIteratorYieldResultFromIteratorResult,
+    getPropertyOfType,
+    isCompilerOptionEnabled,
+    isIntersectionType,
+    isModifierFlagSet,
+    isSymbolFlagSet,
+    isTypeFlagSet,
+    isTypeReference,
+    isUnionType,
+    removeOptionalityFromType,
+    someTypePart,
+    unionTypeParts,
+} from 'tsutils';
+import * as ts from 'typescript';
+import { tryGetBaseConstraintType, typesAreEqual } from './utils';
+
+function isIterationProtocolAvailable(compilerOptions: ts.CompilerOptions) {
+    return compilerOptions.target! >= ts.ScriptTarget.ES2015 || isCompilerOptionEnabled(compilerOptions, 'downlevelIteration');
+}
+
+export function isExpressionIterable(
+    node: ts.Expression,
+    checker: ts.TypeChecker,
+    compilerOptions: ts.CompilerOptions,
+    matchIndexSignature: boolean,
+): boolean {
+    const type = checker.getTypeAtLocation(node);
+    return isIterationProtocolAvailable(compilerOptions)
+        ? isIterable(checker.getApparentType(type), checker, node, matchIndexSignature)
+        : unionTypeParts(tryGetBaseConstraintType(type, checker)).every((t) => isArrayLike(t, compilerOptions));
+}
+
+function isArrayLike(type: ts.Type, compilerOptions: ts.CompilerOptions): boolean {
+    if (isTypeReference(type))
+        type = type.target;
+    if (type.getNumberIndexType() === undefined)
+        return false;
+    if (type.flags & ts.TypeFlags.StringLike)
+        return compilerOptions.target! >= ts.ScriptTarget.ES5; // iterating string is only possible starting from ES5
+    if (type.symbol !== undefined && /^(?:Readonly)?Array$/.test(<string>type.symbol.escapedName) &&
+        type.symbol.declarations?.some((node) => node.getSourceFile().hasNoDefaultLib))
+        return true;
+    if (isIntersectionType(type))
+        return type.types.some((t) => isArrayLike(t, compilerOptions));
+    return !!type.getBaseTypes()?.some((t) => isArrayLike(t, compilerOptions));
+}
+
+function isIterable(type: ts.Type, checker: ts.TypeChecker, node: ts.Node, matchIndexSignature: boolean): boolean {
+    const indexType = type.getNumberIndexType() || type.getStringIndexType();
+    if (indexType === undefined && matchIndexSignature)
+        return false;
+    const iteratorFn = getPropertyOfType(type, <ts.__String>'__@iterator');
+    if (!isPresentPublicAndRequired(iteratorFn))
+        return false;
+    return checkReturnTypeAndRequireZeroArity(checker.getTypeOfSymbolAtLocation(iteratorFn, node), (iterator) => {
+        const next = iterator.getProperty('next');
+        return isPresentPublicAndRequired(next) &&
+            checkReturnTypeAndRequireZeroArity(checker.getTypeOfSymbolAtLocation(next, node), (iteratorResult) => {
+                const done = iteratorResult.getProperty('done');
+                if (
+                    !isPresentAndPublic(done) ||
+                    someTypePart(
+                        removeOptionalityFromType(checker, checker.getTypeOfSymbolAtLocation(done, node)),
+                        isUnionType,
+                        (t) => !isTypeFlagSet(t, ts.TypeFlags.BooleanLike),
+                    )
+                )
+                    return false;
+                const value = getIteratorYieldResultFromIteratorResult(iteratorResult, node, checker).getProperty('value');
+                return isPresentAndPublic(value) &&
+                    (
+                        !matchIndexSignature ||
+                        typesAreEqual(checker.getTypeOfSymbolAtLocation(value, node), indexType!, checker)
+                    );
+            });
+
+    });
+}
+
+function checkReturnTypeAndRequireZeroArity(type: ts.Type, cb: (type: ts.Type) => boolean): boolean {
+    let zeroArity = false;
+    for (const signature of type.getCallSignatures()) {
+        if (!cb(signature.getReturnType()))
+            return false;
+        if (signatureHasArityZero(signature))
+            zeroArity = true;
+    }
+    return zeroArity;
+}
+
+function signatureHasArityZero(signature: ts.Signature): boolean {
+    if (signature.parameters.length === 0)
+        return true;
+    const decl = <ts.ParameterDeclaration | undefined>signature.parameters[0].declarations![0];
+    return decl !== undefined && isOptionalParameter(decl);
+}
+
+function isOptionalParameter(node: ts.ParameterDeclaration): boolean {
+    if (node.questionToken !== undefined || node.dotDotDotToken !== undefined)
+        return true;
+    if (node.flags & ts.NodeFlags.JavaScriptFile && ts.getJSDocParameterTags(node).some((tag) => tag.isBracketed))
+        return true;
+    if (node.initializer === undefined)
+        return false;
+    const parameters = node.parent!.parameters;
+    const nextIndex = parameters.indexOf(node) + 1;
+    if (nextIndex === parameters.length)
+        return true;
+    return isOptionalParameter(parameters[nextIndex]);
+}
+
+function isPresentPublicAndRequired(symbol: ts.Symbol | undefined): symbol is ts.Symbol {
+    return isPresentAndPublic(symbol) && !isSymbolFlagSet(symbol, ts.SymbolFlags.Optional);
+}
+
+function isPresentAndPublic(symbol: ts.Symbol | undefined): symbol is ts.Symbol {
+    return symbol !== undefined &&
+        (
+            symbol.declarations === undefined ||
+            symbol.declarations.every((d) => !isModifierFlagSet(d, ts.ModifierFlags.NonPublicAccessibilityModifier))
+        );
+}

packages/mimir/src/rules/no-object-spread-of-iterable.ts

@@ -0,0 +1,34 @@
+import { TypedRule, excludeDeclarationFiles } from '@fimbul/ymir';
+import * as ts from 'typescript';
+import {
+    WrappedAst,
+    getWrappedNodeAtPosition,
+    isReassignmentTarget,
+} from 'tsutils';
+import { isExpressionIterable } from '../iteration';
+
+@excludeDeclarationFiles
+export class Rule extends TypedRule {
+    public apply() {
+        const re = /\.{3}/g;
+        let wrappedAst: WrappedAst | undefined;
+        for (let match = re.exec(this.sourceFile.text); match !== null; match = re.exec(this.sourceFile.text)) {
+            const {node} = getWrappedNodeAtPosition(wrappedAst ??= this.context.getWrappedAst(), re.lastIndex)!;
+            if (node.pos !== re.lastIndex)
+                continue;
+            switch (node.parent!.kind) {
+                case ts.SyntaxKind.SpreadAssignment:
+                    if (isReassignmentTarget(<ts.ObjectLiteralExpression>node.parent.parent))
+                        continue;
+                    // falls through
+                case ts.SyntaxKind.JsxSpreadAttribute:
+                    this.checkObjectSpread(<ts.Expression>node);
+            }
+        }
+    }
+
+    private checkObjectSpread(node: ts.Expression) {
+        if (isExpressionIterable(node, this.checker, this.context.compilerOptions, false))
+            this.addFindingAtNode(node, 'Spreading an Iterable type into an object is most likely a mistake. Did you intend to use array spread instead?');
+    }
+}