Additional notice for misused yield instead of yield* (#60)

.changeset/shaggy-pandas-dream.md

@@ -0,0 +1,5 @@
+---
+"@effect/language-service": patch
+---
+
+Additional notice for misused yield instead of yield\* in generators

examples/diagnostics/missingStarInYieldEffectGen.ts

@@ -9,6 +9,12 @@ const missingStarInYield = Effect.gen(function*(){
     yield Effect.succeed(1)
 })
 
+// @ts-expect-error
+const missingStarInMultipleYield = Effect.gen(function*(){
+    yield Effect.succeed(1)
+    yield Effect.succeed(2)
+})
+
 const missingStarInInnerYield = Effect.gen(function*(){
     // @ts-expect-error
     yield* Effect.gen(function*(){

src/diagnostics/missingStarInYieldEffectGen.ts

@@ -9,49 +9,56 @@ export const missingStarInYieldEffectGen = createDiagnostic({
   apply: (ts, program) => (sourceFile) => {
     const typeChecker = program.getTypeChecker()
     const effectDiagnostics: Array<ApplicableDiagnosticDefinition> = []
+    const brokenGenerators = new Set<ts.Node>()
+    const brokenYields = new Set<ts.Node>()
 
-    const visitWhileInGenerator = (node: ts.Node) => {
+    const visit = (functionStarNode: ts.Node | undefined) => (node: ts.Node) => {
       // error if yield is not followed by *
       if (
-        ts.isYieldExpression(node) && node.expression && node.asteriskToken === undefined
+        functionStarNode && ts.isYieldExpression(node) && node.expression &&
+        node.asteriskToken === undefined
       ) {
         const type = typeChecker.getTypeAtLocation(node.expression)
         const effect = TypeParser.effectType(ts, typeChecker)(type, node.expression)
         if (Option.isSome(effect)) {
-          effectDiagnostics.push({
-            node,
-            category: ts.DiagnosticCategory.Error,
-            messageText:
-              `When yielding Effects inside Effect.gen, you should use yield* instead of yield.`
-          })
+          brokenGenerators.add(functionStarNode)
+          brokenYields.add(node)
         }
       }
       // continue if we hit another effect gen
       const effectGen = TypeParser.effectGen(ts, typeChecker)(node)
       if (Option.isSome(effectGen)) {
-        ts.forEachChild(effectGen.value.body, visitWhileInGenerator)
+        ts.forEachChild(effectGen.value.body, visit(effectGen.value.functionStar))
       } // stop when we hit a generator function
       else if (
         (ts.isFunctionExpression(node) || ts.isMethodDeclaration(node)) &&
         node.asteriskToken !== undefined
       ) {
-        ts.forEachChild(node, visit)
+        // continue with new parent function star node
+        ts.forEachChild(node, visit(undefined))
       } else {
-        // any other node
-        ts.forEachChild(node, visitWhileInGenerator)
+        // continue with current parent function star node
+        ts.forEachChild(node, visit(functionStarNode))
       }
     }
+    ts.forEachChild(sourceFile, visit(undefined))
 
-    const visit = (node: ts.Node) => {
-      const effectGen = TypeParser.effectGen(ts, typeChecker)(node)
-      if (Option.isSome(effectGen)) {
-        ts.forEachChild(effectGen.value.body, visitWhileInGenerator)
-      } else {
-        ts.forEachChild(node, visit)
-      }
-    }
-
-    ts.forEachChild(sourceFile, visit)
+    // emit diagnostics
+    brokenGenerators.forEach((node) =>
+      effectDiagnostics.push({
+        node,
+        category: ts.DiagnosticCategory.Error,
+        messageText: `Seems like you used yield instead of yield* inside this Effect.gen.`
+      })
+    )
+    brokenYields.forEach((node) =>
+      effectDiagnostics.push({
+        node,
+        category: ts.DiagnosticCategory.Error,
+        messageText:
+          `When yielding Effects inside Effect.gen, you should use yield* instead of yield.`
+      })
+    )
 
     return effectDiagnostics
   }

src/utils/TypeParser.ts

@@ -118,7 +118,10 @@ export function effectGen(ts: TypeScriptApi, typeChecker: ts.TypeChecker) {
       if (propertyAccess.name.text !== "gen") return yield* Option.none()
       // check Effect module
       return yield* importedEffectModule(ts, typeChecker)(propertyAccess.expression).pipe(
-        Option.map(() => ({ body: generatorFunction.body }))
+        Option.map(() => ({
+          body: generatorFunction.body,
+          functionStar: generatorFunction.getFirstToken()
+        }))
       )
     })
 }

test/__snapshots__/diagnostics.test.ts.snap

@@ -84,9 +84,24 @@ effectWithErrors
 `;
 
 exports[`missingStarInYieldEffectGen.ts > missingStarInYieldEffectGen.ts 1`] = `
-"yield Effect.succeed(1)
+"function
+8:38 - 8:46 | Seems like you used yield instead of yield* inside this Effect.gen.
+
+yield Effect.succeed(1)
 9:4 - 9:27 | When yielding Effects inside Effect.gen, you should use yield* instead of yield.
 
+function
+13:46 - 13:54 | Seems like you used yield instead of yield* inside this Effect.gen.
+
+yield Effect.succeed(1)
+14:4 - 14:27 | When yielding Effects inside Effect.gen, you should use yield* instead of yield.
+
+yield Effect.succeed(2)
+15:4 - 15:27 | When yielding Effects inside Effect.gen, you should use yield* instead of yield.
+
+function
+20:22 - 20:30 | Seems like you used yield instead of yield* inside this Effect.gen.
+
 yield Effect.succeed(1)
-15:8 - 15:31 | When yielding Effects inside Effect.gen, you should use yield* instead of yield."
+21:8 - 21:31 | When yielding Effects inside Effect.gen, you should use yield* instead of yield."
 `;

test/diagnostics.test.ts

@@ -35,6 +35,9 @@ function testDiagnosticOnExample(diagnostic: DiagnosticDefinition, fileName: str
       return
     }
 
+    // sort by start position
+    canApply.sort((a, b) => a.node.getStart(sourceFile) - b.node.getStart(sourceFile))
+
     // create human readable messages
     const humanMessages = canApply.map((error) => {
       const start = ts.getLineAndCharacterOfPosition(sourceFile, error.node.getStart(sourceFile))