Apply suggestions from code review

Co-authored-by: Owen Mansel-Chan <[email protected]>

Author: GeekMasher

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

@@ -108,11 +108,10 @@ module GoMicro {
    */
   class ServiceHandler extends Method {
     ServiceHandler() {
-      exists(DataFlow::CallNode call, Type handlerType, ServiceInterfaceType i |
+      exists(DataFlow::CallNode call |
         call.getTarget() instanceof ServiceRegisterHandler and
-        call.getArgument(1).getType() = handlerType and
-        this = handlerType.getMethod(_) and
-        this.implements(i.getNamedType().getMethod(_))
+        this = call.getArgument(1).getType().getMethod(_) and
+        this.implements(any(ServiceInterfaceType i).getNamedType().getMethod(_))
       )
     }
   }
@@ -122,12 +121,10 @@ module GoMicro {
    */
   class ClientService extends Function {
     ClientService() {
-      exists(ClientServiceType c |
-        this.getName().regexpMatch("(?i)new" + c.getName()) and
-        this.getParameterType(0) instanceof StringType and
-        this.getParameterType(1) instanceof GoMicroClientType and
-        this.getDeclaration().getLocation().getFile() instanceof ProtocGeneratedFile
-      )
+      this.getName().regexpMatch("(?i)new" + any(ClientServiceType c).getName()) and
+      this.getParameterType(0) instanceof StringType and
+      this.getParameterType(1) instanceof GoMicroClientType and
+      this.hasLocationInfo(any(ProtocGeneratedFile f).getAbsolutePath(), _, _, _, _)
     }
   }
 
@@ -152,8 +149,8 @@ module GoMicro {
    */
   class Request extends UntrustedFlowSource::Range instanceof DataFlow::ParameterNode {
     Request() {
-      exists(FuncDef c, ServiceHandler handler | handler.getFuncDecl() = c |
-        this.asParameter().isParameterOf(c, 1) and
+      exists(ServiceHandler handler |
+        this.asParameter().isParameterOf(handler.getFuncDecl(), 1) and
         handler.getParameterType(0).hasQualifiedName("context", "Context") and
         this.getType().(PointerType).getBaseType() instanceof ProtocMessageType
       )

go/ql/test/library-tests/semmle/go/frameworks/GoMicro/client/main.go

@@ -16,7 +16,7 @@ func main() {
 	// context
 	ctx := context.Background()
 
-	greeterService := pb.NewGreeterService("http://localhost:8000", service.Client())
+	greeterService := pb.NewGreeterService("http://localhost:8000", service.Client()) // $ clientRequest="http:\/\/localhost:8000"
 	// request
 	req := pb.Request{Name: "Mona"}
 	resp, err := greeterService.Hello(ctx, &req)

go/ql/test/library-tests/semmle/go/frameworks/GoMicro/gomicro.ql

@@ -1,7 +1,26 @@
 import go
+import TestUtilities.InlineExpectationsTest
 
-query predicate serverRequests(DataFlow::ParameterNode node) { node instanceof GoMicro::Request }
+module GoMicroTest implements TestSig {
+  string getARelevantTag() { result = ["serverRequest", "clientRequest"] }
 
-query predicate clientRequests(DataFlow::Node node) {
-  node instanceof GoMicro::ClientRequestUrlAsSink
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(DataFlow::Node node |
+      node.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
+        location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
+      (
+        node instanceof GoMicro::Request and
+        element = node.toString() and
+        value = "\"" + node.toString() + "\"" and
+        tag = "serverRequest"
+        or
+        node instanceof GoMicro::ClientRequestUrlAsSink and
+        element = node.toString() and
+        value = node.toString().replaceAll("/", "\\/") and
+        tag = "clientRequest"
+      )
+    )
+  }
 }
+
+import MakeTest<GoMicroTest>

go/ql/test/library-tests/semmle/go/frameworks/GoMicro/main.go

@@ -15,7 +15,7 @@ import (
 
 type Greeter struct{}
 
-func (g *Greeter) Hello(ctx context.Context, req *pb.Request, rsp *pb.Response) error {
+func (g *Greeter) Hello(ctx context.Context, req *pb.Request, rsp *pb.Response) error { // $ serverRequest="definition of req"
 	// var access
 	name := req.Name
 	fmt.Println("Name :: %s", name)