Require interfaces to use abstract classes

captbaritone · Jul 20, 2024 · 61e8e28 · 61e8e28
1 parent e79ff86
commit 61e8e28
Show file tree

Hide file tree

Showing 13 changed files with 296 additions and 570 deletions.
diff --git a/src/Errors.ts b/src/Errors.ts
@@ -64,6 +64,10 @@ export function invalidInterfaceTagUsage() {
   return `\`@${INTERFACE_TAG}\` can only be used on interface or abstract class declarations. e.g. \`interface MyInterface {}\` or \`abstract class MyInterface {}\``;
 }
 
+export function interfaceClassNotAbstract() {
+  return `Expected \`@${INTERFACE_TAG}\` class to be abstract. \`@${INTERFACE_TAG}\` can only be used on interface or abstract class declarations. e.g. \`interface MyInterface {}\` or \`abstract class MyInterface {}\``;
+}
+
 export function invalidEnumTagUsage() {
   return `\`@${ENUM_TAG}\` can only be used on enum declarations or TypeScript unions. e.g. \`enum MyEnum {}\` or \`type MyEnum = "foo" | "bar"\``;
 }

diff --git a/src/Extractor.ts b/src/Extractor.ts
@@ -1127,6 +1127,12 @@ class Extractor {
   }
 
   interfaceClassDeclaration(node: ts.ClassDeclaration, tag: ts.JSDocTag) {
+    const isAbstract = node.modifiers?.some((modifier) => {
+      return modifier.kind === ts.SyntaxKind.AbstractKeyword;
+    });
+    if (!isAbstract) {
+      return this.report(node, E.interfaceClassNotAbstract());
+    }
     if (node.name == null) {
       return this.report(node, E.typeTagOnUnnamedClass());
     }

diff --git a/src/TypeContext.ts b/src/TypeContext.ts
@@ -219,39 +219,18 @@ export class TypeContext {
 
   // Given the name of a class or interface, return all the parent classes and
   // interfaces.
-  getAllParentsForName(name: ts.Identifier): Set<NameDefinition> {
+  getAllParentClassesForName(name: ts.Identifier): Set<NameDefinition> {
     const symbol = this.checker.getSymbolAtLocation(name);
     if (symbol == null) {
       return new Set();
     }
-    return this.getAllParents(symbol);
+    return this.getAllParentClasses(symbol);
   }
 
   /*
-   * Walk the inheritance chain and collect all the parent classes and
-   * interfaces.
-   *
-   * NOTE! Recursion order here is important and part of our documented
-   * behavior. We do an ordered breadth-first traversal to ensure that
-   * if a class implements multiple interfaces, and those interfaces each
-   * implement the same field, we'll inherit the field implementation from
-   * the first interface in the list.
-   *
-   * Normally JavaScript/TypeScript avoid this issue by implementing only
-   * _single_ inheritance, but because we allow inheriting fields from
-   * interfaces, and TypeScript allows classes (and interfaces!) to
-   * implement/extend multiple interfaces, we must handle this multi-inheritance
-   * issue.
-   *
-   * The approach taken here is modeled after Python's MRO (Method Resolution
-   * Order) algorithm.
-   *
-   * https://docs.python.org/3/howto/mro.html
-   *
-   * TODO: Actually read that paper and see if that's the approach we want to
-   * take then implement it.
+   * Walk the inheritance chain and collect all the parent classes.
    */
-  getAllParents(
+  getAllParentClasses(
     symbol: ts.Symbol,
     parents: Set<NameDefinition> = new Set(),
   ): Set<NameDefinition> {
@@ -260,11 +239,11 @@ export class TypeContext {
     }
 
     for (const declaration of symbol.declarations) {
-      const heritage = getHeritage(declaration);
-      if (heritage == null) {
+      const extendsClauses = getClassExtendClauses(declaration);
+      if (extendsClauses == null) {
         continue;
       }
-      for (const heritageClause of heritage) {
+      for (const heritageClause of extendsClauses) {
         for (const type of heritageClause.types) {
           const typeSymbol = this.checker.getSymbolAtLocation(type.expression);
           if (typeSymbol == null || typeSymbol.declarations == null) {
@@ -277,22 +256,25 @@ export class TypeContext {
             }
           }
           // Recurse to find the parents of the parent.
-          this.getAllParents(typeSymbol, parents);
+          this.getAllParentClasses(typeSymbol, parents);
         }
       }
     }
     return parents;
   }
 }
 
-function getHeritage(
+function getClassExtendClauses(
   declaration: ts.Declaration,
-): ts.NodeArray<ts.HeritageClause> | null {
-  if (
-    ts.isClassDeclaration(declaration) ||
-    ts.isInterfaceDeclaration(declaration)
-  ) {
-    return declaration.heritageClauses ?? null;
+): ts.HeritageClause[] | null {
+  if (ts.isClassDeclaration(declaration)) {
+    const { heritageClauses } = declaration;
+    if (heritageClauses == null) {
+      return null;
+    }
+    return heritageClauses.filter(
+      (clause) => clause.token === ts.SyntaxKind.ExtendsKeyword,
+    );
   }
   return null;
 }
diff --git a/src/tests/fixtures/extend_interface/addStringFieldToInterface.ts.expected b/src/tests/fixtures/extend_interface/addStringFieldToInterface.ts.expected
@@ -32,84 +32,43 @@ class Admin implements IPerson {
 -----------------
 OUTPUT
 -----------------
--- SDL --
-interface IPerson {
-  greeting: String @metadata(argCount: 1, exportName: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/addStringFieldToInterface.ts")
-  hello: String @metadata
-}
+src/tests/fixtures/extend_interface/addStringFieldToInterface.ts:9:1 - error: Interface field IPerson.greeting expected but Admin does not provide it.
 
-type Admin implements IPerson {
-  greeting: String @metadata(argCount: 1, exportName: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/addStringFieldToInterface.ts")
-  hello: String @metadata
-}
+ 9 export function greeting(person: IPerson): string {
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+10   return `Hello ${person.name}!`;
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+11 }
+   ~
 
-type User implements IPerson {
-  greeting: String @metadata(argCount: 1, exportName: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/addStringFieldToInterface.ts")
-  hello: String @metadata
-}
--- TypeScript --
-import { greeting as adminGreetingResolver } from "./addStringFieldToInterface";
-import { greeting as userGreetingResolver } from "./addStringFieldToInterface";
-import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType } from "graphql";
-export function getSchema(): GraphQLSchema {
-    const IPersonType: GraphQLInterfaceType = new GraphQLInterfaceType({
-        name: "IPerson",
-        fields() {
-            return {
-                greeting: {
-                    name: "greeting",
-                    type: GraphQLString
-                },
-                hello: {
-                    name: "hello",
-                    type: GraphQLString
-                }
-            };
-        }
-    });
-    const AdminType: GraphQLObjectType = new GraphQLObjectType({
-        name: "Admin",
-        fields() {
-            return {
-                greeting: {
-                    name: "greeting",
-                    type: GraphQLString,
-                    resolve(source) {
-                        return adminGreetingResolver(source);
-                    }
-                },
-                hello: {
-                    name: "hello",
-                    type: GraphQLString
-                }
-            };
-        },
-        interfaces() {
-            return [IPersonType];
-        }
-    });
-    const UserType: GraphQLObjectType = new GraphQLObjectType({
-        name: "User",
-        fields() {
-            return {
-                greeting: {
-                    name: "greeting",
-                    type: GraphQLString,
-                    resolve(source) {
-                        return userGreetingResolver(source);
-                    }
-                },
-                hello: {
-                    name: "hello",
-                    type: GraphQLString
-                }
-            };
-        },
-        interfaces() {
-            return [IPersonType];
-        }
-    });
-    return new GraphQLSchema({
-        types: [IPersonType, AdminType, UserType]
-    });
-}
+  src/tests/fixtures/extend_interface/addStringFieldToInterface.ts:22:1
+     22 class Admin implements IPerson {
+        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+     23   __typename: "Admin";
+        ~~~~~~~~~~~~~~~~~~~~~~
+    ... 
+     26   hello: string;
+        ~~~~~~~~~~~~~~~~
+     27 }
+        ~
+    Related location
+src/tests/fixtures/extend_interface/addStringFieldToInterface.ts:9:1 - error: Interface field IPerson.greeting expected but User does not provide it.
+
+ 9 export function greeting(person: IPerson): string {
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+10   return `Hello ${person.name}!`;
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+11 }
+   ~
+
+  src/tests/fixtures/extend_interface/addStringFieldToInterface.ts:14:1
+     14 class User implements IPerson {
+        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+     15   __typename: "User";
+        ~~~~~~~~~~~~~~~~~~~~~
+    ... 
+     18   hello: string;
+        ~~~~~~~~~~~~~~~~
+     19 }
+        ~
+    Related location