diff --git a/.changeset/extends-native-error-diagnostic.md b/.changeset/extends-native-error-diagnostic.md new file mode 100644 index 00000000..9e6686f8 --- /dev/null +++ b/.changeset/extends-native-error-diagnostic.md @@ -0,0 +1,7 @@ +--- +"@effect/language-service": minor +--- + +Add the `extendsNativeError` diagnostic to warn when classes directly extend the native `Error` constructor, including common local aliases such as `const E = Error`. + +This helps steer users toward tagged errors that preserve stronger typing in the Effect failure channel. diff --git a/README.md b/README.md index bcf4dd1e..0d95c257 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,7 @@ And you're done! You'll now be able to use a set of refactors and diagnostics th - Warn when using catch functions (`catchAll`, `catch`, `catchIf`, `catchSome`, `catchTag`, `catchTags`) on effects that never fail - Warn when catch callbacks in `Effect.tryPromise`, `Effect.tryMap`, or `Effect.tryMapPromise` return `unknown` or `any` types - Warn when catch callbacks in `Effect.tryPromise`, `Effect.try`, `Effect.tryMap`, or `Effect.tryMapPromise` return the global `Error` type instead of typed errors +- Warn when classes directly extend the native `Error` class, including through local aliases, recommending tagged errors instead - Warn when using `Effect.runSync`, `Effect.runPromise`, `Effect.runFork`, or `Effect.runCallback` inside an Effect - Warn when using `Schema.decodeSync`, `Schema.decodeUnknownSync`, `Schema.encodeSync`, or `Schema.encodeUnknownSync` inside Effect generators, suggesting Effect-based alternatives - Suggest using Effect Schema for JSON operations instead of `JSON.parse`/`JSON.stringify` inside Effect contexts diff --git a/packages/harness-effect-v3/__snapshots__/completions.test.ts.snap b/packages/harness-effect-v3/__snapshots__/completions.test.ts.snap index 26e6cd66..b2147575 100644 --- a/packages/harness-effect-v3/__snapshots__/completions.test.ts.snap +++ b/packages/harness-effect-v3/__snapshots__/completions.test.ts.snap @@ -248,7 +248,7 @@ exports[`Completion effectDataClasses > effectDataClasses_directImportTaggedErro exports[`Completion effectDiagnosticsComment > effectDiagnosticsComment.ts at 2:5 1`] = ` [ { - "insertText": "@effect-diagnostics \${1|anyUnknownInErrorContext,catchAllToMapError,catchUnfailableEffect,classSelfMismatch,deterministicKeys,duplicatePackage,effectFnIife,effectFnOpportunity,effectGenUsesAdapter,effectInFailure,effectInVoidSuccess,effectMapVoid,effectSucceedWithVoid,floatingEffect,genericEffectServices,globalErrorInEffectCatch,globalErrorInEffectFailure,importFromBarrel,instanceOfSchema,layerMergeAllWithDependencies,leakingRequirements,missedPipeableOpportunity,missingEffectContext,missingEffectError,missingEffectServiceDependency,missingLayerContext,missingReturnYieldStar,missingStarInYieldEffectGen,multipleEffectProvide,nonObjectEffectServiceType,outdatedApi,outdatedEffectCodegen,overriddenSchemaConstructor,preferSchemaOverJson,redundantSchemaTagIdentifier,returnEffectInGen,runEffectInsideEffect,schemaStructWithTag,schemaSyncInEffect,schemaUnionOfLiterals,scopeInLayerEffect,strictBooleanExpressions,strictEffectProvide,tryCatchInEffectGen,unknownInEffectCatch,unnecessaryEffectGen,unnecessaryFailYieldableError,unnecessaryPipe,unnecessaryPipeChain,unsupportedServiceAccessors|}:\${2|off,warning,error,message,suggestion|}$0", + "insertText": "@effect-diagnostics \${1|anyUnknownInErrorContext,catchAllToMapError,catchUnfailableEffect,classSelfMismatch,deterministicKeys,duplicatePackage,effectFnIife,effectFnOpportunity,effectGenUsesAdapter,effectInFailure,effectInVoidSuccess,effectMapVoid,effectSucceedWithVoid,extendsNativeError,floatingEffect,genericEffectServices,globalErrorInEffectCatch,globalErrorInEffectFailure,importFromBarrel,instanceOfSchema,layerMergeAllWithDependencies,leakingRequirements,missedPipeableOpportunity,missingEffectContext,missingEffectError,missingEffectServiceDependency,missingLayerContext,missingReturnYieldStar,missingStarInYieldEffectGen,multipleEffectProvide,nonObjectEffectServiceType,outdatedApi,outdatedEffectCodegen,overriddenSchemaConstructor,preferSchemaOverJson,redundantSchemaTagIdentifier,returnEffectInGen,runEffectInsideEffect,schemaStructWithTag,schemaSyncInEffect,schemaUnionOfLiterals,scopeInLayerEffect,strictBooleanExpressions,strictEffectProvide,tryCatchInEffectGen,unknownInEffectCatch,unnecessaryEffectGen,unnecessaryFailYieldableError,unnecessaryPipe,unnecessaryPipeChain,unsupportedServiceAccessors|}:\${2|off,warning,error,message,suggestion|}$0", "isSnippet": true, "kind": "string", "name": "@effect-diagnostics", @@ -259,7 +259,7 @@ exports[`Completion effectDiagnosticsComment > effectDiagnosticsComment.ts at 2: "sortText": "11", }, { - "insertText": "@effect-diagnostics-next-line \${1|anyUnknownInErrorContext,catchAllToMapError,catchUnfailableEffect,classSelfMismatch,deterministicKeys,duplicatePackage,effectFnIife,effectFnOpportunity,effectGenUsesAdapter,effectInFailure,effectInVoidSuccess,effectMapVoid,effectSucceedWithVoid,floatingEffect,genericEffectServices,globalErrorInEffectCatch,globalErrorInEffectFailure,importFromBarrel,instanceOfSchema,layerMergeAllWithDependencies,leakingRequirements,missedPipeableOpportunity,missingEffectContext,missingEffectError,missingEffectServiceDependency,missingLayerContext,missingReturnYieldStar,missingStarInYieldEffectGen,multipleEffectProvide,nonObjectEffectServiceType,outdatedApi,outdatedEffectCodegen,overriddenSchemaConstructor,preferSchemaOverJson,redundantSchemaTagIdentifier,returnEffectInGen,runEffectInsideEffect,schemaStructWithTag,schemaSyncInEffect,schemaUnionOfLiterals,scopeInLayerEffect,strictBooleanExpressions,strictEffectProvide,tryCatchInEffectGen,unknownInEffectCatch,unnecessaryEffectGen,unnecessaryFailYieldableError,unnecessaryPipe,unnecessaryPipeChain,unsupportedServiceAccessors|}:\${2|off,warning,error,message,suggestion|}$0", + "insertText": "@effect-diagnostics-next-line \${1|anyUnknownInErrorContext,catchAllToMapError,catchUnfailableEffect,classSelfMismatch,deterministicKeys,duplicatePackage,effectFnIife,effectFnOpportunity,effectGenUsesAdapter,effectInFailure,effectInVoidSuccess,effectMapVoid,effectSucceedWithVoid,extendsNativeError,floatingEffect,genericEffectServices,globalErrorInEffectCatch,globalErrorInEffectFailure,importFromBarrel,instanceOfSchema,layerMergeAllWithDependencies,leakingRequirements,missedPipeableOpportunity,missingEffectContext,missingEffectError,missingEffectServiceDependency,missingLayerContext,missingReturnYieldStar,missingStarInYieldEffectGen,multipleEffectProvide,nonObjectEffectServiceType,outdatedApi,outdatedEffectCodegen,overriddenSchemaConstructor,preferSchemaOverJson,redundantSchemaTagIdentifier,returnEffectInGen,runEffectInsideEffect,schemaStructWithTag,schemaSyncInEffect,schemaUnionOfLiterals,scopeInLayerEffect,strictBooleanExpressions,strictEffectProvide,tryCatchInEffectGen,unknownInEffectCatch,unnecessaryEffectGen,unnecessaryFailYieldableError,unnecessaryPipe,unnecessaryPipeChain,unsupportedServiceAccessors|}:\${2|off,warning,error,message,suggestion|}$0", "isSnippet": true, "kind": "string", "name": "@effect-diagnostics-next-line", diff --git a/packages/harness-effect-v3/__snapshots__/diagnostics/extendsNativeError.ts.codefixes b/packages/harness-effect-v3/__snapshots__/diagnostics/extendsNativeError.ts.codefixes new file mode 100644 index 00000000..17a12b4a --- /dev/null +++ b/packages/harness-effect-v3/__snapshots__/diagnostics/extendsNativeError.ts.codefixes @@ -0,0 +1,6 @@ +extendsNativeError_skipNextLine from 90 to 97 +extendsNativeError_skipFile from 90 to 97 +extendsNativeError_skipNextLine from 160 to 168 +extendsNativeError_skipFile from 160 to 168 +extendsNativeError_skipNextLine from 244 to 248 +extendsNativeError_skipFile from 244 to 248 \ No newline at end of file diff --git a/packages/harness-effect-v3/__snapshots__/diagnostics/extendsNativeError.ts.output b/packages/harness-effect-v3/__snapshots__/diagnostics/extendsNativeError.ts.output new file mode 100644 index 00000000..fb96d330 --- /dev/null +++ b/packages/harness-effect-v3/__snapshots__/diagnostics/extendsNativeError.ts.output @@ -0,0 +1,8 @@ +MyError +4:6 - 4:13 | 0 | Avoid extending the native 'Error' class directly. Consider using a tagged error (e.g. Data.TaggedError) to maintain type safety in the Effect failure channel. effect(extendsNativeError) + +MyError2 +8:6 - 8:14 | 0 | Avoid extending the native 'Error' class directly. Consider using a tagged error (e.g. Data.TaggedError) to maintain type safety in the Effect failure channel. effect(extendsNativeError) + +Base +11:6 - 11:10 | 0 | Avoid extending the native 'Error' class directly. Consider using a tagged error (e.g. Data.TaggedError) to maintain type safety in the Effect failure channel. effect(extendsNativeError) \ No newline at end of file diff --git a/packages/harness-effect-v3/examples/diagnostics/extendsNativeError.ts b/packages/harness-effect-v3/examples/diagnostics/extendsNativeError.ts new file mode 100644 index 00000000..52b1937b --- /dev/null +++ b/packages/harness-effect-v3/examples/diagnostics/extendsNativeError.ts @@ -0,0 +1,16 @@ +// @effect-diagnostics extendsNativeError:warning + +// Flagged: direct extends Error +class MyError extends Error {} + +// Flagged: via alias +const E = Error +class MyError2 extends E {} + +// Not flagged: indirect (extends a subclass of Error) +class Base extends Error {} +class MyError3 extends Base {} + +// Not flagged: unrelated class +class Bar {} +class Foo extends Bar {} diff --git a/packages/harness-effect-v4/__snapshots__/completions.test.ts.snap b/packages/harness-effect-v4/__snapshots__/completions.test.ts.snap index 9d193051..e4224a7c 100644 --- a/packages/harness-effect-v4/__snapshots__/completions.test.ts.snap +++ b/packages/harness-effect-v4/__snapshots__/completions.test.ts.snap @@ -143,7 +143,7 @@ exports[`Completion effectDataClasses > effectDataClasses.ts at 4:35 1`] = ` exports[`Completion effectDiagnosticsComment > effectDiagnosticsComment.ts at 2:5 1`] = ` [ { - "insertText": "@effect-diagnostics \${1|anyUnknownInErrorContext,catchAllToMapError,catchUnfailableEffect,classSelfMismatch,deterministicKeys,duplicatePackage,effectFnIife,effectFnOpportunity,effectGenUsesAdapter,effectInFailure,effectInVoidSuccess,effectMapVoid,effectSucceedWithVoid,floatingEffect,genericEffectServices,globalErrorInEffectCatch,globalErrorInEffectFailure,importFromBarrel,instanceOfSchema,layerMergeAllWithDependencies,leakingRequirements,missedPipeableOpportunity,missingEffectContext,missingEffectError,missingEffectServiceDependency,missingLayerContext,missingReturnYieldStar,missingStarInYieldEffectGen,multipleEffectProvide,nonObjectEffectServiceType,outdatedApi,outdatedEffectCodegen,overriddenSchemaConstructor,preferSchemaOverJson,redundantSchemaTagIdentifier,returnEffectInGen,runEffectInsideEffect,schemaStructWithTag,schemaSyncInEffect,schemaUnionOfLiterals,scopeInLayerEffect,strictBooleanExpressions,strictEffectProvide,tryCatchInEffectGen,unknownInEffectCatch,unnecessaryEffectGen,unnecessaryFailYieldableError,unnecessaryPipe,unnecessaryPipeChain,unsupportedServiceAccessors|}:\${2|off,warning,error,message,suggestion|}$0", + "insertText": "@effect-diagnostics \${1|anyUnknownInErrorContext,catchAllToMapError,catchUnfailableEffect,classSelfMismatch,deterministicKeys,duplicatePackage,effectFnIife,effectFnOpportunity,effectGenUsesAdapter,effectInFailure,effectInVoidSuccess,effectMapVoid,effectSucceedWithVoid,extendsNativeError,floatingEffect,genericEffectServices,globalErrorInEffectCatch,globalErrorInEffectFailure,importFromBarrel,instanceOfSchema,layerMergeAllWithDependencies,leakingRequirements,missedPipeableOpportunity,missingEffectContext,missingEffectError,missingEffectServiceDependency,missingLayerContext,missingReturnYieldStar,missingStarInYieldEffectGen,multipleEffectProvide,nonObjectEffectServiceType,outdatedApi,outdatedEffectCodegen,overriddenSchemaConstructor,preferSchemaOverJson,redundantSchemaTagIdentifier,returnEffectInGen,runEffectInsideEffect,schemaStructWithTag,schemaSyncInEffect,schemaUnionOfLiterals,scopeInLayerEffect,strictBooleanExpressions,strictEffectProvide,tryCatchInEffectGen,unknownInEffectCatch,unnecessaryEffectGen,unnecessaryFailYieldableError,unnecessaryPipe,unnecessaryPipeChain,unsupportedServiceAccessors|}:\${2|off,warning,error,message,suggestion|}$0", "isSnippet": true, "kind": "string", "name": "@effect-diagnostics", @@ -154,7 +154,7 @@ exports[`Completion effectDiagnosticsComment > effectDiagnosticsComment.ts at 2: "sortText": "11", }, { - "insertText": "@effect-diagnostics-next-line \${1|anyUnknownInErrorContext,catchAllToMapError,catchUnfailableEffect,classSelfMismatch,deterministicKeys,duplicatePackage,effectFnIife,effectFnOpportunity,effectGenUsesAdapter,effectInFailure,effectInVoidSuccess,effectMapVoid,effectSucceedWithVoid,floatingEffect,genericEffectServices,globalErrorInEffectCatch,globalErrorInEffectFailure,importFromBarrel,instanceOfSchema,layerMergeAllWithDependencies,leakingRequirements,missedPipeableOpportunity,missingEffectContext,missingEffectError,missingEffectServiceDependency,missingLayerContext,missingReturnYieldStar,missingStarInYieldEffectGen,multipleEffectProvide,nonObjectEffectServiceType,outdatedApi,outdatedEffectCodegen,overriddenSchemaConstructor,preferSchemaOverJson,redundantSchemaTagIdentifier,returnEffectInGen,runEffectInsideEffect,schemaStructWithTag,schemaSyncInEffect,schemaUnionOfLiterals,scopeInLayerEffect,strictBooleanExpressions,strictEffectProvide,tryCatchInEffectGen,unknownInEffectCatch,unnecessaryEffectGen,unnecessaryFailYieldableError,unnecessaryPipe,unnecessaryPipeChain,unsupportedServiceAccessors|}:\${2|off,warning,error,message,suggestion|}$0", + "insertText": "@effect-diagnostics-next-line \${1|anyUnknownInErrorContext,catchAllToMapError,catchUnfailableEffect,classSelfMismatch,deterministicKeys,duplicatePackage,effectFnIife,effectFnOpportunity,effectGenUsesAdapter,effectInFailure,effectInVoidSuccess,effectMapVoid,effectSucceedWithVoid,extendsNativeError,floatingEffect,genericEffectServices,globalErrorInEffectCatch,globalErrorInEffectFailure,importFromBarrel,instanceOfSchema,layerMergeAllWithDependencies,leakingRequirements,missedPipeableOpportunity,missingEffectContext,missingEffectError,missingEffectServiceDependency,missingLayerContext,missingReturnYieldStar,missingStarInYieldEffectGen,multipleEffectProvide,nonObjectEffectServiceType,outdatedApi,outdatedEffectCodegen,overriddenSchemaConstructor,preferSchemaOverJson,redundantSchemaTagIdentifier,returnEffectInGen,runEffectInsideEffect,schemaStructWithTag,schemaSyncInEffect,schemaUnionOfLiterals,scopeInLayerEffect,strictBooleanExpressions,strictEffectProvide,tryCatchInEffectGen,unknownInEffectCatch,unnecessaryEffectGen,unnecessaryFailYieldableError,unnecessaryPipe,unnecessaryPipeChain,unsupportedServiceAccessors|}:\${2|off,warning,error,message,suggestion|}$0", "isSnippet": true, "kind": "string", "name": "@effect-diagnostics-next-line", diff --git a/packages/harness-effect-v4/__snapshots__/diagnostics/extendsNativeError.ts.codefixes b/packages/harness-effect-v4/__snapshots__/diagnostics/extendsNativeError.ts.codefixes new file mode 100644 index 00000000..17a12b4a --- /dev/null +++ b/packages/harness-effect-v4/__snapshots__/diagnostics/extendsNativeError.ts.codefixes @@ -0,0 +1,6 @@ +extendsNativeError_skipNextLine from 90 to 97 +extendsNativeError_skipFile from 90 to 97 +extendsNativeError_skipNextLine from 160 to 168 +extendsNativeError_skipFile from 160 to 168 +extendsNativeError_skipNextLine from 244 to 248 +extendsNativeError_skipFile from 244 to 248 \ No newline at end of file diff --git a/packages/harness-effect-v4/__snapshots__/diagnostics/extendsNativeError.ts.output b/packages/harness-effect-v4/__snapshots__/diagnostics/extendsNativeError.ts.output new file mode 100644 index 00000000..fb96d330 --- /dev/null +++ b/packages/harness-effect-v4/__snapshots__/diagnostics/extendsNativeError.ts.output @@ -0,0 +1,8 @@ +MyError +4:6 - 4:13 | 0 | Avoid extending the native 'Error' class directly. Consider using a tagged error (e.g. Data.TaggedError) to maintain type safety in the Effect failure channel. effect(extendsNativeError) + +MyError2 +8:6 - 8:14 | 0 | Avoid extending the native 'Error' class directly. Consider using a tagged error (e.g. Data.TaggedError) to maintain type safety in the Effect failure channel. effect(extendsNativeError) + +Base +11:6 - 11:10 | 0 | Avoid extending the native 'Error' class directly. Consider using a tagged error (e.g. Data.TaggedError) to maintain type safety in the Effect failure channel. effect(extendsNativeError) \ No newline at end of file diff --git a/packages/harness-effect-v4/examples/diagnostics/extendsNativeError.ts b/packages/harness-effect-v4/examples/diagnostics/extendsNativeError.ts new file mode 100644 index 00000000..52b1937b --- /dev/null +++ b/packages/harness-effect-v4/examples/diagnostics/extendsNativeError.ts @@ -0,0 +1,16 @@ +// @effect-diagnostics extendsNativeError:warning + +// Flagged: direct extends Error +class MyError extends Error {} + +// Flagged: via alias +const E = Error +class MyError2 extends E {} + +// Not flagged: indirect (extends a subclass of Error) +class Base extends Error {} +class MyError3 extends Base {} + +// Not flagged: unrelated class +class Bar {} +class Foo extends Bar {} diff --git a/packages/language-service/src/diagnostics.ts b/packages/language-service/src/diagnostics.ts index 298cabb0..1f1cc51c 100644 --- a/packages/language-service/src/diagnostics.ts +++ b/packages/language-service/src/diagnostics.ts @@ -11,6 +11,7 @@ import { effectInFailure } from "./diagnostics/effectInFailure.js" import { effectInVoidSuccess } from "./diagnostics/effectInVoidSuccess.js" import { effectMapVoid } from "./diagnostics/effectMapVoid.js" import { effectSucceedWithVoid } from "./diagnostics/effectSucceedWithVoid.js" +import { extendsNativeError } from "./diagnostics/extendsNativeError.js" import { floatingEffect } from "./diagnostics/floatingEffect.js" import { genericEffectServices } from "./diagnostics/genericEffectServices.js" import { globalErrorInEffectCatch } from "./diagnostics/globalErrorInEffectCatch.js" @@ -99,5 +100,6 @@ export const diagnostics = [ effectFnOpportunity, redundantSchemaTagIdentifier, schemaSyncInEffect, - preferSchemaOverJson + preferSchemaOverJson, + extendsNativeError ] diff --git a/packages/language-service/src/diagnostics/extendsNativeError.ts b/packages/language-service/src/diagnostics/extendsNativeError.ts new file mode 100644 index 00000000..4817297a --- /dev/null +++ b/packages/language-service/src/diagnostics/extendsNativeError.ts @@ -0,0 +1,64 @@ +import type ts from "typescript" +import * as LSP from "../core/LSP.js" +import * as Nano from "../core/Nano.js" +import * as TypeCheckerApi from "../core/TypeCheckerApi.js" +import * as TypeScriptApi from "../core/TypeScriptApi.js" + +export const extendsNativeError = LSP.createDiagnostic({ + name: "extendsNativeError", + code: 50, + description: "Warns when a class directly extends the native Error class", + severity: "off", + apply: Nano.fn("extendsNativeError.apply")(function*(sourceFile, report) { + const ts = yield* Nano.service(TypeScriptApi.TypeScriptApi) + const typeChecker = yield* Nano.service(TypeCheckerApi.TypeCheckerApi) + + const errorSymbol = typeChecker.resolveName("Error", undefined, ts.SymbolFlags.Type, false) + if (!errorSymbol) return + + const nodeToVisit: Array = [] + const appendNodeToVisit = (node: ts.Node) => { + nodeToVisit.push(node) + return undefined + } + ts.forEachChild(sourceFile, appendNodeToVisit) + + while (nodeToVisit.length > 0) { + const node = nodeToVisit.shift()! + + if (ts.isClassDeclaration(node) && node.heritageClauses) { + for (const clause of node.heritageClauses) { + if (clause.token === ts.SyntaxKind.ExtendsKeyword && clause.types.length > 0) { + const typeExpression = clause.types[0].expression + const exprSymbol = typeChecker.getSymbolAtLocation(typeExpression) + // Resolve import aliases to their target symbol + const resolvedSymbol = exprSymbol && (exprSymbol.flags & ts.SymbolFlags.Alias) + ? typeChecker.getAliasedSymbol(exprSymbol) + : exprSymbol + // Direct symbol match or variable alias (e.g. const E = Error) + const isNativeError = resolvedSymbol === errorSymbol || (() => { + if (!resolvedSymbol || resolvedSymbol === errorSymbol) return false + const exprType = typeChecker.getTypeAtLocation(typeExpression) + const constructSignatures = exprType.getConstructSignatures() + if (constructSignatures.length > 0) { + const instanceType = typeChecker.getReturnTypeOfSignature(constructSignatures[0]) + return instanceType.getSymbol() === errorSymbol + } + return false + })() + if (isNativeError) { + report({ + location: node.name ?? typeExpression, + messageText: + "Avoid extending the native 'Error' class directly. Consider using a tagged error (e.g. Data.TaggedError) to maintain type safety in the Effect failure channel.", + fixes: [] + }) + } + } + } + } + + ts.forEachChild(node, appendNodeToVisit) + } + }) +})