fix missing QLDocs and refactor ServiceInterface

Author: Alvaro Muñoz

go/ql/lib/semmle/go/frameworks/Twirp.qll

@@ -1,6 +1,9 @@
+/** Provides models of commonly used functions and types in the twirp packages. */
+
 import go
 import semmle.go.security.RequestForgery
 
+/** Provides models of commonly used functions and types in the twirp packages. */
 module Twirp {
   /**
    * A *.pb.go file generated by Twirp.
@@ -47,17 +50,27 @@ module Twirp {
   /**
    * An interface type representing a Twirp service.
    */
-  class ServiceInterface extends NamedType {
+  class ServiceInterface extends InterfaceType {
+    NamedType serviceInterface;
+
     ServiceInterface() {
       exists(TypeEntity te |
-        te.getType() = this and
-        // To match an Interface type we need to use a NamedType whose getUnderlying type is an InterfaceType
-        this.getUnderlyingType() instanceof InterfaceType and
+        te.getType() = serviceInterface and
+        this instanceof InterfaceType and
+        serviceInterface.getUnderlyingType() = this and
         te.getDeclaration().getLocation().getFile() instanceof ServicesGeneratedFile
       )
     }
 
-    InterfaceType getInterfaceType() { result = this.getUnderlyingType() }
+    /**
+     * Returns the name of the interface
+     */
+    override string getName() { result = serviceInterface.getName() }
+
+    /**
+     * Returns the named type on top of this interface type
+     */
+    NamedType getNamedType() { result = serviceInterface }
   }
 
   /**
@@ -68,7 +81,7 @@ module Twirp {
 
     ServiceClient() {
       exists(ServiceInterface i |
-        pointerType.implements(i.getInterfaceType()) and
+        pointerType.implements(i) and
         this = pointerType.getBaseType() and
         this.getName().toLowerCase() = i.getName().toLowerCase() + ["protobuf", "json"] + "client"
       )
@@ -81,7 +94,7 @@ module Twirp {
   class ServiceServer extends NamedType {
     ServiceServer() {
       exists(ServiceInterface i |
-        this.implements(i.getInterfaceType()) and
+        this.implements(i) and
         this.getName().toLowerCase() = i.getName().toLowerCase() + "server"
       )
     }
@@ -106,9 +119,9 @@ module Twirp {
    */
   class ServerConstructor extends Function {
     ServerConstructor() {
-      exists(ServiceServer c |
+      exists(ServiceServer c, ServiceInterface i |
         this.getName().toLowerCase() = "new" + c.getName().toLowerCase() and
-        this.getParameter(0).getType() instanceof ServiceInterface
+        this.getParameter(0).getType() = i.getNamedType()
       )
     }
   }
@@ -139,10 +152,10 @@ module Twirp {
       exists(DataFlow::CallNode call, Type handlerType, ServiceInterface i |
         call.getTarget() instanceof ServerConstructor and
         call.getArgument(0).getType() = handlerType and
-        handlerType.implements(i.getInterfaceType()) and
+        handlerType.implements(i) and
         this = handlerType.getMethod(_) and
         this.implements(m) and
-        i.getMethod(_) = m
+        i.getNamedType().getMethod(_) = m
       )
     }
   }

go/ql/test/library-tests/semmle/go/frameworks/Twirp/tests.ql

@@ -52,7 +52,7 @@ query predicate passingPositiveTests(string res, string expectation, InlineTest
     exists(Twirp::ProtobufMessage n | t.inType(n))
     or
     expectation = "serviceInterface" and
-    exists(Twirp::ServiceInterface n | t.inType(n))
+    exists(Twirp::ServiceInterface n | t.inType(n.getNamedType()))
     or
     expectation = "serviceClient" and
     exists(Twirp::ServiceClient n | t.inType(n))
@@ -85,7 +85,7 @@ query predicate failingPositiveTests(string res, string expectation, InlineTest
     not exists(Twirp::ProtobufMessage n | t.inType(n))
     or
     expectation = "serviceInterface" and
-    not exists(Twirp::ServiceInterface n | t.inType(n))
+    not exists(Twirp::ServiceInterface n | t.inType(n.getNamedType()))
     or
     expectation = "serviceClient" and
     not exists(Twirp::ServiceClient n | t.inType(n))