Skip to content

Commit

Permalink
Add lint rule to prefer map((): T => {}) over map<T>(() => {}) (#…
Browse files Browse the repository at this point in the history
…6995)

**User-Facing Changes**
None

**Description**

TypeScript currently widens inferred types of function expressions
(microsoft/TypeScript#241). This results in
the following [sad
situation](https://www.typescriptlang.org/play?#code/MYewdgzgLgBAhjAvDA2gRgDQwExYMwC6A3AFAlQCeADgKYwAqSMA3iTOzAB4BcMYArgFsARjQBOpAL5k4AOkFwqACiUBLAJS9GiAHwsSAegMwAegH42HMTSj8xYfRyddeqjJecwKvAEQgQVBCqND5YMEYw4mIgYjCAvBuAsjseMNKS6qQkcgpUADz0OioaSHqsEebJ1rb2jp4uMG7JTt4wfgFBIWERYCCRYtGxgHwbgCi7yanpZEA):

```ts
const a = [1, 2, 3];

type T = {
    x: number;
}

a.map((i): T => {
    return {
        x: i,
        y: "oopsie",  // error 👍
    }
});

a.map<T>((i) => {
    return {
        x: i,
        y: "oopsie",  // no error 🤔
    }
});
```

This PR adds a lint rule to automatically replace `map<T>((i) => ...`
with `map((i): T => ...`.

Depends on https://github.com/foxglove/studio/pull/6989 to merge first.

Resolves FG-5336
  • Loading branch information
jtbandes authored and pezy committed Oct 25, 2023
1 parent 431bae4 commit e7d36f3
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 3 deletions.
2 changes: 2 additions & 0 deletions packages/eslint-plugin-studio/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module.exports = {
"link-target": require("./link-target"),
"lodash-ramda-imports": require("./lodash-ramda-imports"),
"ramda-usage": require("./ramda-usage"),
"no-map-type-argument": require("./no-map-type-argument"),
},

configs: {
Expand All @@ -16,6 +17,7 @@ module.exports = {
"@foxglove/studio/link-target": "error",
"@foxglove/studio/lodash-ramda-imports": "error",
"@foxglove/studio/ramda-usage": "error",
"@foxglove/studio/no-map-type-argument": "error",
},
},
},
Expand Down
67 changes: 67 additions & 0 deletions packages/eslint-plugin-studio/no-map-type-argument.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/

const { ESLintUtils } = require("@typescript-eslint/utils");

/**
* @type {import("eslint").Rule.RuleModule}
*/
module.exports = {
meta: {
type: "problem",
fixable: "code",
messages: {
preferReturnTypeAnnotation: `Annotate the function return type explicitly instead of passing generic arguments to Array#map, to avoid return type widening (https://github.com/microsoft/TypeScript/issues/241)`,
},
},
create: (context) => {
return {
[`CallExpression[arguments.length>=1][typeArguments.params.length=1][arguments.0.type=ArrowFunctionExpression]:not([arguments.0.returnType]) > MemberExpression.callee[property.name="map"]`]:
(/** @type {import("estree").MemberExpression} */ node) => {
/** @type {import("estree").CallExpression} */
const callExpr = node.parent;

const { esTreeNodeToTSNodeMap, program } = ESLintUtils.getParserServices(context);
const sourceCode = context.getSourceCode();
const checker = program.getTypeChecker();
const objectTsNode = esTreeNodeToTSNodeMap.get(node.object);
const objectType = checker.getTypeAtLocation(objectTsNode);
if (!checker.isArrayType(objectType) && !checker.isTupleType(objectType)) {
return;
}

const arrowToken = sourceCode.getTokenBefore(
callExpr.arguments[0].body,
(token) => token.type === "Punctuator" && token.value === "=>",
);
if (!arrowToken) {
return;
}
const maybeCloseParenToken = sourceCode.getTokenBefore(arrowToken);
const closeParenToken =
maybeCloseParenToken.type === "Punctuator" && maybeCloseParenToken.value === ")"
? maybeCloseParenToken
: undefined;

context.report({
node: callExpr.typeArguments,
messageId: "preferReturnTypeAnnotation",
*fix(fixer) {
const returnType = sourceCode.getText(callExpr.typeArguments.params[0]);
yield fixer.remove(callExpr.typeArguments);
if (closeParenToken) {
yield fixer.insertTextAfter(closeParenToken, `: ${returnType}`);
} else {
yield fixer.insertTextBefore(callExpr.arguments[0], "(");
yield fixer.insertTextAfter(
callExpr.arguments[0].params[callExpr.arguments[0].params.length - 1],
`): ${returnType}`,
);
}
},
});
},
};
},
};
48 changes: 48 additions & 0 deletions packages/eslint-plugin-studio/no-map-type-argument.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/

import { RuleTester } from "@typescript-eslint/rule-tester";
import { TSESLint } from "@typescript-eslint/utils";
import path from "path";

// eslint-disable-next-line @typescript-eslint/no-var-requires
const rule = require("./no-map-type-argument") as TSESLint.RuleModule<"preferReturnTypeAnnotation">;

const ruleTester = new RuleTester({
parser: "@typescript-eslint/parser",
parserOptions: {
ecmaVersion: 2020,
tsconfigRootDir: path.join(__dirname, "fixture"),
project: "tsconfig.json",
},
});

ruleTester.run("no-map-type-argument", rule, {
valid: [
/* ts */ `
[1, 2].map((x) => x + 1);
[1, 2].map((x): number => x + 1);
[1, 2].map<number>((x): number => x + 1);
[1, 2].map<number, string>((x) => x + 1);
({ x: 1 }).map<number>((x) => x + 1);
`,
],

invalid: [
{
code: /* ts */ `
[1, 2].map<number>(x => x + 1);
[1, 2].map<number>((x) => x + 1);
`,
errors: [
{ messageId: "preferReturnTypeAnnotation", line: 2 },
{ messageId: "preferReturnTypeAnnotation", line: 3 },
],
output: /* ts */ `
[1, 2].map((x): number => x + 1);
[1, 2].map((x): number => x + 1);
`,
},
],
});
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ function PanelExtensionAdapter(
if (!isMounted()) {
return;
}
const subscribePayloads = topics.map<SubscribePayload>((item) => {
const subscribePayloads = topics.map((item): SubscribePayload => {
if (typeof item === "string") {
// For backwards compatability with the topic-string-array api `subscribe(["/topic"])`
// results in a topic subscription with full preloading
Expand All @@ -407,7 +407,7 @@ function PanelExtensionAdapter(
});

// ExtensionPanel-Facing subscription type
const localSubs = topics.map<Subscription>((item) => {
const localSubs = topics.map((item): Subscription => {
if (typeof item === "string") {
return { topic: item, preload: true };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ function initRenderStateBuilder(): BuildRenderStateFn {
if (sortedTopics !== prevSortedTopics || prevMessageConverters !== messageConverters) {
shouldRender = true;

const topics = sortedTopics.map<Topic>((topic) => {
const topics = sortedTopics.map((topic): Topic => {
const newTopic: Topic = {
name: topic.name,
datatype: topic.schemaName ?? "",
Expand Down

0 comments on commit e7d36f3

Please sign in to comment.