feat(rules): add mapMapErrorToMapBoth diagnostic

.changeset/map-maperror-to-mapboth.md

@@ -0,0 +1,5 @@
+---
+"@effect/tsgo": minor
+---
+
+Add `mapMapErrorToMapBoth` diagnostic (`TS377132`) to suggest using `Effect.mapBoth` instead of directly adjacent `Effect.map` and `Effect.mapError`.

README.md

@@ -140,6 +140,7 @@ Some diagnostics are off by default or have a default severity of suggestion, bu
     <tr><td><a href="https://github.com/Effect-TS/tsgo/blob/main/docs/rules/effect-succeed-with-void.md"><code>effectSucceedWithVoid</code></a></td><td>Suggests using Effect.void instead of Effect.succeed(undefined) or Effect.succeed(void 0)</td></tr>
     <tr><td><a href="https://github.com/Effect-TS/tsgo/blob/main/docs/rules/flat-map-conditional-to-filter-or-fail.md"><code>flatMapConditionalToFilterOrFail</code></a></td><td>Suggests Effect.filterOrFail or Effect.filterOrElse when Effect.flatMap conditionally passes its input through with Effect.succeed</td></tr>
     <tr><td><a href="https://github.com/Effect-TS/tsgo/blob/main/docs/rules/flat-map-to-map.md"><code>flatMapToMap</code></a></td><td>Suggests using Effect.map instead of Effect.flatMap when the callback only wraps its result with Effect.succeed</td></tr>
+    <tr><td><a href="https://github.com/Effect-TS/tsgo/blob/main/docs/rules/map-map-error-to-map-both.md"><code>mapMapErrorToMapBoth</code></a></td><td>Suggests using Effect.mapBoth instead of directly adjacent Effect.map and Effect.mapError</td></tr>
     <tr><td><a href="https://github.com/Effect-TS/tsgo/blob/main/docs/rules/map-some-to-as-some.md"><code>mapSomeToAsSome</code></a></td><td>Suggests using Effect.asSome instead of Effect.map when the mapper only wraps the success value with Option.some</td></tr>
     <tr><td><a href="https://github.com/Effect-TS/tsgo/blob/main/docs/rules/match-effect-to-map-both.md"><code>matchEffectToMapBoth</code></a></td><td>Suggests Effect.mapBoth when Effect.matchEffect only transforms the failure and success channels</td></tr>
     <tr><td><a href="https://github.com/Effect-TS/tsgo/blob/main/docs/rules/match-effect-to-match.md"><code>matchEffectToMatch</code></a></td><td>Suggests Effect.match or Effect.matchCause when both Effect.matchEffect handlers only return Effect.succeed</td></tr>

_packages/tsgo/src/metadata.json

@@ -1961,6 +1961,30 @@
         ]
       }
     },
+    {
+      "name": "mapMapErrorToMapBoth",
+      "group": "style",
+      "description": "Suggests using Effect.mapBoth instead of directly adjacent Effect.map and Effect.mapError",
+      "defaultSeverity": "suggestion",
+      "fixable": false,
+      "supportedEffect": [
+        "v3",
+        "v4"
+      ],
+      "codes": [
+        377132
+      ],
+      "preview": {
+        "sourceText": "import { Effect } from \"effect\"\n\ndeclare const fetchCount: Effect.Effect\u003cnumber, string\u003e\n\nexport const preview = fetchCount.pipe(\n  Effect.map((n) =\u003e n \u003e 0),\n  Effect.mapError((s) =\u003e s.length)\n)\n",
+        "diagnostics": [
+          {
+            "start": 160,
+            "end": 175,
+            "text": "`Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)"
+          }
+        ]
+      }
+    },
     {
       "name": "mapSomeToAsSome",
       "group": "style",

docs/rules/map-map-error-to-map-both.md

@@ -0,0 +1,68 @@
+<!-- This file is generated by internal/rules/rules_json_test.go. Do not edit it manually. -->
+
+# `mapMapErrorToMapBoth`
+
+Suggests using Effect.mapBoth instead of directly adjacent Effect.map and Effect.mapError
+
+| Property | Value |
+| --- | --- |
+| Category | Style |
+| Default severity | `suggestion` |
+| Fixable | No |
+| Effect versions | v3, v4 |
+| Diagnostic codes | `TS377132` |
+| Language Service name | `mapMapErrorToMapBoth` |
+| Oxlint name | `effecttsgo/map-map-error-to-map-both` |
+
+## Preview
+
+```ts
+import { Effect } from "effect"
+
+declare const fetchCount: Effect.Effect<number, string>
+
+export const preview = fetchCount.pipe(
+  Effect.map((n) => n > 0),
+  Effect.mapError((s) => s.length)
+/**
+  ^^^^^^^^^^^^^^^ effecttsgo(map-map-error-to-map-both): `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`.
+*/
+)
+```
+
+## Language Service Configuration
+
+See the [Language Service setup guide](../../README.md#installation) for installation instructions.
+
+```jsonc
+{
+  "$schema": "./node_modules/@effect/tsgo/schema.json",
+  "compilerOptions": {
+    "plugins": [
+      {
+        "name": "@effect/language-service",
+        "diagnosticSeverity": {
+          "mapMapErrorToMapBoth": "warning"
+        }
+      }
+    ]
+  }
+}
+```
+
+## Oxlint Configuration
+
+See the [Oxlint setup guide](../README.md#oxlint-setup) for installation and patching instructions.
+
+```json
+{
+  "$schema": "./node_modules/@effect/tsgo/oxlint-schema.json",
+  "options": {
+    "typeAware": true
+  },
+  "plugins": ["effecttsgo"],
+  "rules": {
+    "effecttsgo/map-map-error-to-map-both": "warn"
+  }
+}
+```

internal/diagnostics/effectDiagnosticMessages.json

@@ -506,5 +506,9 @@
     "This module reference imports `{0}`, which is obsolete in Effect v4. In Effect v4, Schema is provided directly by `Schema` from `effect` (or `effect/Schema`). effect(obsoleteSchemaImport)": {
         "category": "Warning",
         "code": 377128
+    },
+    "`Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)": {
+        "category": "Suggestion",
+        "code": 377132
     }
 }

internal/rules/map_map_error_to_map_both.go

@@ -0,0 +1,125 @@
+package rules
+
+import (
+	"github.com/effect-ts/tsgo/etscore"
+	"github.com/effect-ts/tsgo/internal/rule"
+	"github.com/effect-ts/tsgo/internal/typeparser"
+	"github.com/microsoft/TypeScript/tsc/shim/ast"
+	"github.com/microsoft/TypeScript/tsc/shim/checker"
+	"github.com/microsoft/TypeScript/tsc/shim/core"
+	tsdiag "github.com/microsoft/TypeScript/tsc/shim/diagnostics"
+	"github.com/microsoft/TypeScript/tsc/shim/scanner"
+)
+
+// MapMapErrorToMapBoth suggests using Effect.mapBoth instead of directly adjacent
+// Effect.map and Effect.mapError in piping flows or nested data-first calls.
+var MapMapErrorToMapBoth = rule.Rule{
+	Name:            "mapMapErrorToMapBoth",
+	Group:           "style",
+	Description:     "Suggests using Effect.mapBoth instead of directly adjacent Effect.map and Effect.mapError",
+	DefaultSeverity: etscore.SeveritySuggestion,
+	SupportedEffect: []string{"v3", "v4"},
+	Codes: []int32{
+		tsdiag.Effect_mapBoth_expresses_these_failure_and_success_transformations_more_directly_than_adjacent_Effect_map_and_Effect_mapError_effect_mapMapErrorToMapBoth.Code(),
+	},
+	Run: func(ctx *rule.Context) []*ast.Diagnostic {
+		matches := AnalyzeMapMapErrorToMapBoth(ctx.TypeParser, ctx.Checker, ctx.SourceFile)
+		diags := make([]*ast.Diagnostic, len(matches))
+		for i, match := range matches {
+			diags[i] = ctx.NewDiagnostic(
+				match.SourceFile,
+				match.Location,
+				tsdiag.Effect_mapBoth_expresses_these_failure_and_success_transformations_more_directly_than_adjacent_Effect_map_and_Effect_mapError_effect_mapMapErrorToMapBoth,
+				nil,
+			)
+		}
+		return diags
+	},
+}
+
+// MapMapErrorToMapBothMatch holds match details for diagnostic emission.
+type MapMapErrorToMapBothMatch struct {
+	SourceFile   *ast.SourceFile
+	Location     core.TextRange
+	Node         *ast.Node
+	FirstCallee  *ast.Node
+	SecondCallee *ast.Node
+}
+
+// AnalyzeMapMapErrorToMapBoth finds directly adjacent Effect.map and Effect.mapError
+// combinators in either order applied to an Effect receiver.
+func AnalyzeMapMapErrorToMapBoth(tp *typeparser.TypeParser, _ *checker.Checker, sf *ast.SourceFile) []MapMapErrorToMapBothMatch {
+	if tp == nil || sf == nil {
+		return nil
+	}
+
+	var matches []MapMapErrorToMapBothMatch
+
+	flows := tp.PipingFlows(sf, false)
+	for _, flow := range flows {
+		sequences := flow.FindTransformationSequences(
+			func(transformation *typeparser.PipingFlowTransformation) bool {
+				return len(transformation.Args) > 0 &&
+					(tp.IsNodeReferenceToEffectModuleApi(transformation.Callee, "map") ||
+						tp.IsNodeReferenceToEffectModuleApi(transformation.Callee, "mapError"))
+			},
+			func(transformation *typeparser.PipingFlowTransformation) bool {
+				return len(transformation.Args) > 0 &&
+					(tp.IsNodeReferenceToEffectModuleApi(transformation.Callee, "map") ||
+						tp.IsNodeReferenceToEffectModuleApi(transformation.Callee, "mapError"))
+			},
+		)
+
+		nextAllowedIndex := 0
+		for _, sequence := range sequences {
+			if sequence.Start < nextAllowedIndex {
+				continue
+			}
+
+			first := &flow.Transformations[sequence.Start]
+			second := &flow.Transformations[sequence.Start+1]
+
+			if first.Kind != second.Kind {
+				continue
+			}
+			if first.Kind != typeparser.TransformationKindPipe &&
+				first.Kind != typeparser.TransformationKindPipeable &&
+				first.Kind != typeparser.TransformationKindDataFirst {
+				continue
+			}
+
+		isFirstMap := tp.IsNodeReferenceToEffectModuleApi(first.Callee, "map")
+		isFirstMapError := tp.IsNodeReferenceToEffectModuleApi(first.Callee, "mapError")
+		isSecondMap := tp.IsNodeReferenceToEffectModuleApi(second.Callee, "map")
+		isSecondMapError := tp.IsNodeReferenceToEffectModuleApi(second.Callee, "mapError")
+		isMapPair := (isFirstMap && isSecondMapError) || (isFirstMapError && isSecondMap)
+
+		if !isMapPair {
+			continue
+		}
+
+			inputType := flow.TransformationInputType(sequence.Start)
+			if inputType == nil {
+				if sequence.Start == 0 && flow.Subject.Node != nil {
+					inputType = tp.GetTypeAtLocation(flow.Subject.Node)
+				} else if inputNode := flow.TransformationInputNode(sequence.Start); inputNode != nil {
+					inputType = tp.GetTypeAtLocation(inputNode)
+				}
+			}
+			if inputType == nil || !tp.IsEffectType(inputType) || tp.IsEffectSubtype(inputType) {
+				continue
+			}
+
+			nextAllowedIndex = sequence.Start + 2
+			matches = append(matches, MapMapErrorToMapBothMatch{
+				SourceFile:   sf,
+				Location:     scanner.GetErrorRangeForNode(sf, second.Callee),
+				Node:         second.Callee,
+				FirstCallee:  first.Callee,
+				SecondCallee: second.Callee,
+			})
+		}
+	}
+
+	return matches
+}

internal/rules/rules.go

@@ -119,4 +119,5 @@ var All = []rule.Rule{
 	AcquireReleaseDisposable,
 	RaceFirstWithSleepToTimeout,
 	RunOfExitToRunExit,
+	MapMapErrorToMapBoth,
 }

testdata/baselines/reference/effect-v3/mapMapErrorToMapBoth.errors.txt

@@ -0,0 +1,92 @@
+=== Metadata ===
+Effect version: 3.19.19
+
+/.src/mapMapErrorToMapBoth.ts(13,3): suggestion TS377132: `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)
+/.src/mapMapErrorToMapBoth.ts(19,3): suggestion TS377132: `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)
+/.src/mapMapErrorToMapBoth.ts(26,3): suggestion TS377132: `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)
+/.src/mapMapErrorToMapBoth.ts(30,37): suggestion TS377132: `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)
+/.src/mapMapErrorToMapBoth.ts(36,37): suggestion TS377132: `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)
+
+
+==== /.src/mapMapErrorToMapBoth.ts (5 errors) ====
+    // @effect-diagnostics mapMapErrorToMapBoth:suggestion
+    import { Data, Effect, pipe } from "effect"
+
+    class TaskError extends Data.TaggedError("TaskError")<{
+      readonly message: string
+    }> {}
+
+    declare const fetchCount: Effect.Effect<number, string>
+
+    // BAD: pipe adjacent map then mapError
+    export const badPipeMapThenMapError = fetchCount.pipe(
+      Effect.map((n) => n > 0),
+      Effect.mapError((message) => new TaskError({ message }))
+      ~~~~~~~~~~~~~~~
+!!! suggestion TS377132: `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)
+    )
+
+    // BAD: pipe adjacent mapError then map
+    export const badPipeMapErrorThenMap = fetchCount.pipe(
+      Effect.mapError((message) => new TaskError({ message })),
+      Effect.map((n) => n > 0)
+      ~~~~~~~~~~
+!!! suggestion TS377132: `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)
+    )
+
+    // BAD: function pipe style
+    export const badFunctionPipe = pipe(
+      fetchCount,
+      Effect.map((n) => n > 0),
+      Effect.mapError((message) => new TaskError({ message }))
+      ~~~~~~~~~~~~~~~
+!!! suggestion TS377132: `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)
+    )
+
+    // BAD: nested data-first map(mapError(eff, g), f)
+    export const badNestedMapMapError = Effect.map(
+                                        ~~~~~~~~~~
+!!! suggestion TS377132: `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)
+      Effect.mapError(fetchCount, (message) => new TaskError({ message })),
+      (n) => n > 0
+    )
+
+    // BAD: nested data-first mapError(map(eff, f), g)
+    export const badNestedMapErrorMap = Effect.mapError(
+                                        ~~~~~~~~~~~~~~~
+!!! suggestion TS377132: `Effect.mapBoth` expresses these failure and success transformations more directly than adjacent `Effect.map` and `Effect.mapError`. effect(mapMapErrorToMapBoth)
+      Effect.map(fetchCount, (n) => n > 0),
+      (message) => new TaskError({ message })
+    )
+
+    // GOOD: separated by other step (silent)
+    export const goodSeparated = fetchCount.pipe(
+      Effect.map((n) => n > 0),
+      Effect.tap((b) => Effect.log(String(b))),
+      Effect.mapError((message) => new TaskError({ message }))
+    )
+
+    // GOOD: already using mapBoth (silent)
+    export const goodMapBoth = fetchCount.pipe(
+      Effect.mapBoth({
+        onSuccess: (n) => n > 0,
+        onFailure: (message) => new TaskError({ message })
+      })
+    )
+
+    // GOOD: unrelated map functions (silent)
+    const unrelated = {
+      map: <A, B>(f: (a: A) => B) => <E, R>(self: Effect.Effect<A, E, R>) => Effect.map(self, f),
+      mapError: <E, E2>(f: (e: E) => E2) => <A, R>(self: Effect.Effect<A, E, R>) => Effect.mapError(self, f)
+    }
+
+    export const goodUnrelatedMap = fetchCount.pipe(
+      unrelated.map((n) => n > 0),
+      Effect.mapError((message) => new TaskError({ message }))
+    )
+
+    export const goodUnrelatedMapError = fetchCount.pipe(
+      Effect.map((n) => n > 0),
+      unrelated.mapError((message) => new TaskError({ message }))
+    )
+