diff --git a/.changeset/pretty-pigs-push.md b/.changeset/pretty-pigs-push.md new file mode 100644 index 00000000..392b7bcd --- /dev/null +++ b/.changeset/pretty-pigs-push.md @@ -0,0 +1,7 @@ +--- +"@effect/language-service": patch +--- + +Add Effect v4 support for the `runEffectInsideEffect` diagnostic so it suggests and fixes `Effect.run*With` usage based on `Effect.services`. + +Update the generated metadata, schema, README entry, and v4 harness examples/snapshots to document and verify the new behavior. diff --git a/README.md b/README.md index b66110a2..f8bbf651 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ Some diagnostics are off by default or have a default severity of suggestion, bu leakingRequirements💡Detects implementation services leaked in service methods✓✓ multipleEffectProvide⚠️🔧Warns against chaining Effect.provide calls which can cause service lifecycle issues✓✓ returnEffectInGen💡🔧Warns when returning an Effect in a generator causes nested Effect<Effect<...>>✓✓ - runEffectInsideEffect💡🔧Suggests using Runtime methods instead of Effect.run* inside Effect contexts✓ + runEffectInsideEffect💡🔧Suggests using Runtime or Effect.run*With methods instead of Effect.run* inside Effect contexts✓✓ schemaSyncInEffect💡Suggests using Effect-based Schema methods instead of sync methods inside Effect generators✓ scopeInLayerEffect⚠️🔧Suggests using Layer.scoped instead of Layer.effect when Scope is in requirements✓ strictEffectProvide➖Warns when using Effect.provide with layers outside of application entry points✓✓ diff --git a/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect.ts.codefixes b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect.ts.codefixes new file mode 100644 index 00000000..3e8c38c5 --- /dev/null +++ b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect.ts.codefixes @@ -0,0 +1,5 @@ +runEffectInsideEffect_skipNextLine from 404 to 418 +runEffectInsideEffect_skipFile from 404 to 418 +runEffectInsideEffect_fix from 702 to 719 +runEffectInsideEffect_skipNextLine from 702 to 719 +runEffectInsideEffect_skipFile from 702 to 719 \ No newline at end of file diff --git a/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect.ts.output b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect.ts.output new file mode 100644 index 00000000..457697e7 --- /dev/null +++ b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect.ts.output @@ -0,0 +1,6 @@ +Effect.runSync +14:20 - 14:34 | 2 | Using Effect.runSync inside an Effect is not recommended. Effects inside generators can usually just be yielded. effect(runEffectInsideEffect) + +Effect.runPromise +22:4 - 22:21 | 2 | Using Effect.runPromise inside an Effect is not recommended. The same services should generally be used instead to run child effects. +Consider extracting the current services by using for example Effect.services and then use Effect.runPromiseWith with the extracted services instead. effect(runEffectInsideEffect) \ No newline at end of file diff --git a/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect.ts.runEffectInsideEffect_fix.from702to719.output b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect.ts.runEffectInsideEffect_fix.from702to719.output new file mode 100644 index 00000000..2c5c17a2 --- /dev/null +++ b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect.ts.runEffectInsideEffect_fix.from702to719.output @@ -0,0 +1,28 @@ +// code fix runEffectInsideEffect_fix output for range 702 - 719 +import { Data, Effect } from "effect" + +class DebuggerError extends Data.TaggedError("DebuggerError")<{ + cause: unknown +}> {} + +export const program = Effect.gen(function*() { +const effectServices = yield* Effect.services() + + const response = yield* Effect.tryPromise({ + try: () => fetch("http://localhost:9229"), + catch: (e) => new DebuggerError({ cause: e }) + }) + const data = yield* Effect.promise(() => response.json()) + + const websocket = Effect.runSync(Effect.sync(() => new WebSocket(data.url))) + // ^- do not runSync in here + + websocket.onmessage = (event) => { + const check = Effect.tryPromise({ + try: () => fetch(event.data as string), + catch: (e) => new DebuggerError({ cause: e }) + }) + Effect.runPromiseWith(effectServices)(check) + // ^- no runPromise, use the current services instead + } +}) diff --git a/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect_preview.ts.codefixes b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect_preview.ts.codefixes new file mode 100644 index 00000000..c1e3db57 --- /dev/null +++ b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect_preview.ts.codefixes @@ -0,0 +1,3 @@ +runEffectInsideEffect_fix from 183 to 197 +runEffectInsideEffect_skipNextLine from 183 to 197 +runEffectInsideEffect_skipFile from 183 to 197 \ No newline at end of file diff --git a/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect_preview.ts.output b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect_preview.ts.output new file mode 100644 index 00000000..39cfd908 --- /dev/null +++ b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect_preview.ts.output @@ -0,0 +1,3 @@ +Effect.runSync +6:20 - 6:34 | 0 | Using Effect.runSync inside an Effect is not recommended. The same services should generally be used instead to run child effects. +Consider extracting the current services by using for example Effect.services and then use Effect.runSyncWith with the extracted services instead. effect(runEffectInsideEffect) \ No newline at end of file diff --git a/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect_preview.ts.runEffectInsideEffect_fix.from183to197.output b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect_preview.ts.runEffectInsideEffect_fix.from183to197.output new file mode 100644 index 00000000..8b1b1dc3 --- /dev/null +++ b/packages/harness-effect-v4/__snapshots__/diagnostics/runEffectInsideEffect_preview.ts.runEffectInsideEffect_fix.from183to197.output @@ -0,0 +1,11 @@ +// code fix runEffectInsideEffect_fix output for range 183 - 197 +// @effect-diagnostics *:off +// @effect-diagnostics runEffectInsideEffect:warning +import { Effect } from "effect" + +export const preview = Effect.gen(function*() { +const effectServices = yield* Effect.services() + + const run = () => Effect.runSyncWith(effectServices)(Effect.succeed(1)) + return run() +}) diff --git a/packages/harness-effect-v4/examples/diagnostics/runEffectInsideEffect.ts b/packages/harness-effect-v4/examples/diagnostics/runEffectInsideEffect.ts new file mode 100644 index 00000000..2d316e32 --- /dev/null +++ b/packages/harness-effect-v4/examples/diagnostics/runEffectInsideEffect.ts @@ -0,0 +1,25 @@ +import { Data, Effect } from "effect" + +class DebuggerError extends Data.TaggedError("DebuggerError")<{ + cause: unknown +}> {} + +export const program = Effect.gen(function*() { + const response = yield* Effect.tryPromise({ + try: () => fetch("http://localhost:9229"), + catch: (e) => new DebuggerError({ cause: e }) + }) + const data = yield* Effect.promise(() => response.json()) + + const websocket = Effect.runSync(Effect.sync(() => new WebSocket(data.url))) + // ^- do not runSync in here + + websocket.onmessage = (event) => { + const check = Effect.tryPromise({ + try: () => fetch(event.data as string), + catch: (e) => new DebuggerError({ cause: e }) + }) + Effect.runPromise(check) + // ^- no runPromise, use the current services instead + } +}) diff --git a/packages/harness-effect-v4/examples/diagnostics/runEffectInsideEffect_preview.ts b/packages/harness-effect-v4/examples/diagnostics/runEffectInsideEffect_preview.ts new file mode 100644 index 00000000..2ede0764 --- /dev/null +++ b/packages/harness-effect-v4/examples/diagnostics/runEffectInsideEffect_preview.ts @@ -0,0 +1,8 @@ +// @effect-diagnostics *:off +// @effect-diagnostics runEffectInsideEffect:warning +import { Effect } from "effect" + +export const preview = Effect.gen(function*() { + const run = () => Effect.runSync(Effect.succeed(1)) + return run() +}) diff --git a/packages/language-service/src/diagnostics/runEffectInsideEffect.ts b/packages/language-service/src/diagnostics/runEffectInsideEffect.ts index 03ee4e50..aba9cb62 100644 --- a/packages/language-service/src/diagnostics/runEffectInsideEffect.ts +++ b/packages/language-service/src/diagnostics/runEffectInsideEffect.ts @@ -10,16 +10,16 @@ import * as TypeScriptUtils from "../core/TypeScriptUtils.js" export const runEffectInsideEffect = LSP.createDiagnostic({ name: "runEffectInsideEffect", code: 32, - description: "Suggests using Runtime methods instead of Effect.run* inside Effect contexts", + description: "Suggests using Runtime or Effect.run*With methods instead of Effect.run* inside Effect contexts", group: "antipattern", severity: "suggestion", fixable: true, - supportedEffect: ["v3"], + supportedEffect: ["v3", "v4"], apply: Nano.fn("runEffectInsideEffect.apply")(function*(sourceFile, report) { const ts = yield* Nano.service(TypeScriptApi.TypeScriptApi) const typeParser = yield* Nano.service(TypeParser.TypeParser) const tsUtils = yield* Nano.service(TypeScriptUtils.TypeScriptUtils) - if (typeParser.supportedEffect() === "v4") return + const supportedEffect = typeParser.supportedEffect() const parseEffectMethod = (node: ts.Node, methodName: string) => pipe( @@ -65,12 +65,13 @@ export const runEffectInsideEffect = LSP.createDiagnostic({ if (scopeNode && scopeNode !== effectGen.generatorFunction) { const fixAddRuntime = Nano.gen(function*() { const changeTracker = yield* Nano.service(TypeScriptApi.ChangeTracker) + const effectModuleIdentifier = + tsUtils.findImportedModuleIdentifierByPackageAndNameOrBarrel(sourceFile, "effect", "Effect") || "Effect" const runtimeModuleIdentifier = tsUtils.findImportedModuleIdentifierByPackageAndNameOrBarrel(sourceFile, "effect", "Runtime") || "Runtime" - const effectModuleIdentifier = - tsUtils.findImportedModuleIdentifierByPackageAndNameOrBarrel(sourceFile, "effect", "Effect") || "Effect" let runtimeIdentifier: string | undefined = undefined + let servicesIdentifier: string | undefined = undefined for (const statement of effectGen.generatorFunction.body.statements) { if (ts.isVariableStatement(statement) && statement.declarationList.declarations.length === 1) { const declaration = statement.declarationList.declarations[0] @@ -87,11 +88,46 @@ export const runEffectInsideEffect = LSP.createDiagnostic({ if (Option.isSome(maybeEffectRuntime) && ts.isIdentifier(declaration.name)) { runtimeIdentifier = ts.idText(declaration.name) } + const maybeEffectServices = yield* pipe( + typeParser.isNodeReferenceToEffectModuleApi("services")(yieldedExpression.expression), + Nano.option + ) + if (Option.isSome(maybeEffectServices) && ts.isIdentifier(declaration.name)) { + servicesIdentifier = ts.idText(declaration.name) + } } } } } - if (!runtimeIdentifier) { + if (supportedEffect === "v4" && !servicesIdentifier) { + changeTracker.insertNodeAt( + sourceFile, + effectGen.body.statements[0].pos, + ts.factory.createVariableStatement( + undefined, + ts.factory.createVariableDeclarationList([ts.factory.createVariableDeclaration( + "effectServices", + undefined, + undefined, + ts.factory.createYieldExpression( + ts.factory.createToken(ts.SyntaxKind.AsteriskToken), + ts.factory.createCallExpression( + ts.factory.createPropertyAccessExpression( + ts.factory.createIdentifier(effectModuleIdentifier), + "services" + ), + [ts.factory.createKeywordTypeNode(ts.SyntaxKind.NeverKeyword)], + [] + ) + ) + )], ts.NodeFlags.Const) + ), + { + prefix: "\n", + suffix: "\n" + } + ) + } else if (supportedEffect === "v3" && !runtimeIdentifier) { changeTracker.insertNodeAt( sourceFile, effectGen.body.statements[0].pos, @@ -127,17 +163,29 @@ export const runEffectInsideEffect = LSP.createDiagnostic({ changeTracker.insertText( sourceFile, node.arguments[0].pos, - `${runtimeModuleIdentifier}.${isEffectRunCall.value.methodName}(${runtimeIdentifier || "effectRuntime"}, ` + supportedEffect === "v4" + ? `${effectModuleIdentifier}.${isEffectRunCall.value.methodName}With(${ + servicesIdentifier || "effectServices" + })(` + : `${runtimeModuleIdentifier}.${isEffectRunCall.value.methodName}(${ + runtimeIdentifier || "effectRuntime" + }, ` ) }) + const v4MethodName = `${isEffectRunCall.value.methodName}With` + const messageText = supportedEffect === "v4" + ? `Using ${nodeText} inside an Effect is not recommended. The same services should generally be used instead to run child effects.\nConsider extracting the current services by using for example Effect.services and then use Effect.${v4MethodName} with the extracted services instead.` + : `Using ${nodeText} inside an Effect is not recommended. The same runtime should generally be used instead to run child effects.\nConsider extracting the Runtime by using for example Effect.runtime and then use Runtime.${isEffectRunCall.value.methodName} with the extracted runtime instead.` + report({ location: node.expression, - messageText: - `Using ${nodeText} inside an Effect is not recommended. The same runtime should generally be used instead to run child effects.\nConsider extracting the Runtime by using for example Effect.runtime and then use Runtime.${isEffectRunCall.value.methodName} with the extracted runtime instead.`, + messageText, fixes: [{ fixName: "runEffectInsideEffect_fix", - description: "Use a runtime to run the Effect", + description: supportedEffect === "v4" + ? "Use the current services to run the Effect" + : "Use a runtime to run the Effect", apply: fixAddRuntime }] }) diff --git a/packages/language-service/src/metadata.json b/packages/language-service/src/metadata.json index 279e149d..7c0b8543 100644 --- a/packages/language-service/src/metadata.json +++ b/packages/language-service/src/metadata.json @@ -631,11 +631,12 @@ { "name": "runEffectInsideEffect", "group": "antipattern", - "description": "Suggests using Runtime methods instead of Effect.run* inside Effect contexts", + "description": "Suggests using Runtime or Effect.run*With methods instead of Effect.run* inside Effect contexts", "defaultSeverity": "suggestion", "fixable": true, "supportedEffect": [ - "v3" + "v3", + "v4" ], "preview": { "sourceText": "import { Effect } from \"effect\"\n\nexport const preview = Effect.gen(function*() {\n const run = () => Effect.runSync(Effect.succeed(1))\n return run()\n})\n", @@ -643,7 +644,7 @@ { "start": 101, "end": 115, - "text": "Using Effect.runSync inside an Effect is not recommended. The same runtime should generally be used instead to run child effects.\nConsider extracting the Runtime by using for example Effect.runtime and then use Runtime.runSync with the extracted runtime instead. effect(runEffectInsideEffect)" + "text": "Using Effect.runSync inside an Effect is not recommended. The same services should generally be used instead to run child effects.\nConsider extracting the current services by using for example Effect.services and then use Effect.runSyncWith with the extracted services instead. effect(runEffectInsideEffect)" } ] } diff --git a/schema.json b/schema.json index e12d6cc3..e62767d5 100644 --- a/schema.json +++ b/schema.json @@ -2757,7 +2757,7 @@ "suggestion" ], "default": "suggestion", - "description": "Suggests using Runtime methods instead of Effect.run* inside Effect contexts Default severity: suggestion." + "description": "Suggests using Runtime or Effect.run*With methods instead of Effect.run* inside Effect contexts Default severity: suggestion." }, "schemaStructWithTag": { "type": "string",