Repository: kythe/kythe
Pull request #6037 (2024-02-26 19:27:30)

feat(go_indexer): support vars denoting flags

shahms: ```
ERROR: Traceback (most recent call last):
	File "/workspace/kythe/go/indexer/BUILD", line 411, column 21, in <toplevel>
		flag_constructor(
	File "/workspace/kythe/go/indexer/flags.bzl", line 15, column 29, in flag_constructor
		return proto.encode_text(struct(
Error in encode_text: in struct field .var_arg_position: got NoneType, want string, int, bool, or struct
ERROR: package contains errors: kythe/go/indexer
ERROR: package contains errors: kythe/go/indexer: Traceback (most recent call last):
	File "/workspace/kythe/go/indexer/BUILD", line 411, column 21, in <toplevel>
		flag_constructor(
	File "/workspace/kythe/go/indexer/flags.bzl", line 15, column 29, in flag_constructor
		return proto.encode_text(struct(
Error in encode_text: in struct field .var_arg_position: got NoneType, want string, int, bool, or struct
ERROR: Error evaluating '//...': error loading package 'kythe/go/indexer': Package 'kythe/go/indexer' contains errors
```
schroederc: Seems like behavior differs between Bazel 6 and 7 on this.

Patch:
diff --git a/kythe/go/indexer/BUILD b/kythe/go/indexer/BUILD
index 4fbde348a..7a252caa5 100644
--- a/kythe/go/indexer/BUILD
+++ b/kythe/go/indexer/BUILD
@@ -426,6 +426,7 @@ custom_flags = [
         func_name = "Var",
         name_arg_position = 1,
         pkg_path = "flag",
+        var_arg_position = 0,
     ),
 ]
 
diff --git a/kythe/go/indexer/emit.go b/kythe/go/indexer/emit.go
index e035b0a2c..11d44d8b9 100644
--- a/kythe/go/indexer/emit.go
+++ b/kythe/go/indexer/emit.go
@@ -921,7 +921,11 @@ func (e *emitter) emitFlags(expr *ast.CallExpr, stack stackFunc) {
 		return
 	}
 	// Check for expected arguments.
-	if expected := int(max(ctor.NameArgPosition, ctor.DescriptionArgPosition)); len(expr.Args) <= expected {
+	if expected := int(max(
+		ctor.NameArgPosition,
+		ctor.DescriptionArgPosition,
+		ctor.GetVarArgPosition(),
+	)); len(expr.Args) <= expected {
 		log.Errorf("Expected at least %d arguments for call to %s.%s; found %d", expected, ctor.GetPkgPath(), ctor.GetFuncName(), len(expr.Args))
 		return
 	}
@@ -976,22 +980,24 @@ func (e *emitter) emitFlags(expr *ast.CallExpr, stack stackFunc) {
 	e.writeAnchor(nameArg, anchor, start, end)
 	e.writeEdge(anchor, flagNode, edges.DefinesBinding)
 
-	// TODO: handle *Var typed flag constructors
-	parent := stack(1)
 	var identDef types.Object
-	switch parent := parent.(type) {
-	case *ast.ValueSpec:
-		for i, name := range parent.Names {
-			if expr == parent.Values[i] {
-				identDef = e.pi.Info.Defs[name]
-				break
+	if ctor.VarArgPosition != nil {
+		identDef = e.pi.Info.Uses[findIdentifier(expr.Args[ctor.GetVarArgPosition()])]
+	} else {
+		switch parent := stack(1).(type) {
+		case *ast.ValueSpec:
+			for i, name := range parent.Names {
+				if expr == parent.Values[i] {
+					identDef = e.pi.Info.Defs[name]
+					break
+				}
 			}
-		}
-	case *ast.AssignStmt:
-		for i, v := range parent.Lhs {
-			if name, ok := v.(*ast.Ident); ok && expr == parent.Rhs[i] {
-				identDef = e.pi.Info.Defs[name]
-				break
+		case *ast.AssignStmt:
+			for i, v := range parent.Lhs {
+				if name, ok := v.(*ast.Ident); ok && expr == parent.Rhs[i] {
+					identDef = e.pi.Info.Defs[name]
+					break
+				}
 			}
 		}
 	}
@@ -1004,6 +1010,23 @@ func (e *emitter) emitFlags(expr *ast.CallExpr, stack stackFunc) {
 	e.writeEdge(e.pi.ObjectVName(identDef), flagNode, edges.Denotes)
 }
 
+func findIdentifier(expr ast.Expr) *ast.Ident {
+	for expr != nil {
+		switch e := expr.(type) {
+		case *ast.UnaryExpr:
+			if e.Op != token.AND {
+				return nil
+			}
+			expr = e.X
+		case *ast.Ident:
+			return e
+		default:
+			return nil
+		}
+	}
+	return nil
+}
+
 func (e *emitter) flagConstructor(f *types.Func) *gopb.FlagConstructor {
 	if e.flagConstructors == nil {
 		// Initial the flag constructor lookup table.
diff --git a/kythe/go/indexer/flags.bzl b/kythe/go/indexer/flags.bzl
index 584eb424a..6a9487955 100644
--- a/kythe/go/indexer/flags.bzl
+++ b/kythe/go/indexer/flags.bzl
@@ -1,21 +1,26 @@
 """Starlark library to build FlagConstructors textproto files"""
 
-def flag_constructor(func_name, pkg_path, name_arg_position, description_arg_position):
-    """Produces a text proto encoding of a kythe.proto.FlagConstructor.  Meant
-    to be used as a custom_flags value.
+def flag_constructor(func_name, pkg_path, name_arg_position, description_arg_position, var_arg_position = None):
+    """Produces a text proto encoding of a kythe.proto.FlagConstructor.
+
+    Meant to be used as a custom_flags value.
 
     Args:
       func_name: the name of the flag constructing function
       pkg_path: the package path of the flag constructing function
       name_arg_position: the 0-based position of the name string argument
       description_arg_position: the 0-based position of the description string argument
+      var_arg_position: the 0-based position of the variable pointer argument
     """
-    return proto.encode_text(struct(
-        func_name = func_name,
-        pkg_path = pkg_path,
-        name_arg_position = name_arg_position,
-        description_arg_position = description_arg_position,
-    ))
+    flag = {
+        "func_name": func_name,
+        "pkg_path": pkg_path,
+        "name_arg_position": name_arg_position,
+        "description_arg_position": description_arg_position,
+    }
+    if var_arg_position != None:
+        flag["var_arg_position"] = var_arg_position
+    return proto.encode_text(struct(**flag))
 
 def _flag_constructors(ctx):
     textpb = "# proto-file: kythe/proto/go.proto\n# proto-message: kythe.proto.FlagConstructors\n\n"
@@ -35,6 +40,7 @@ def _flag_constructors(ctx):
                 "func_name": f,
                 "name_arg_position": 1,
                 "description_arg_position": 3,
+                "var_arg_position": 0,
             }))
     textpb += proto.encode_text(struct(**{"flag": flags}))
     for f in ctx.attr.custom_flags:
diff --git a/kythe/proto/go.proto b/kythe/proto/go.proto
index ec4092615..6a6382f97 100644
--- a/kythe/proto/go.proto
+++ b/kythe/proto/go.proto
@@ -57,4 +57,6 @@ message FlagConstructor {
   uint32 name_arg_position = 3;
   // The 0-based position of the flag's description argument.
   uint32 description_arg_position = 4;
+  // The 0-based position of the flag's var argument.
+  optional uint32 var_arg_position = 5;
 }
diff --git a/kythe/proto/go_go_proto/go.pb.go b/kythe/proto/go_go_proto/go.pb.go
index d403215bc..399f53dd7 100644
--- a/kythe/proto/go_go_proto/go.pb.go
+++ b/kythe/proto/go_go_proto/go.pb.go
@@ -214,10 +214,11 @@ type FlagConstructor struct {
 	sizeCache     protoimpl.SizeCache
 	unknownFields protoimpl.UnknownFields
 
-	PkgPath                string `protobuf:"bytes,1,opt,name=pkg_path,json=pkgPath,proto3" json:"pkg_path,omitempty"`
-	FuncName               string `protobuf:"bytes,2,opt,name=func_name,json=funcName,proto3" json:"func_name,omitempty"`
-	NameArgPosition        uint32 `protobuf:"varint,3,opt,name=name_arg_position,json=nameArgPosition,proto3" json:"name_arg_position,omitempty"`
-	DescriptionArgPosition uint32 `protobuf:"varint,4,opt,name=description_arg_position,json=descriptionArgPosition,proto3" json:"description_arg_position,omitempty"`
+	PkgPath                string  `protobuf:"bytes,1,opt,name=pkg_path,json=pkgPath,proto3" json:"pkg_path,omitempty"`
+	FuncName               string  `protobuf:"bytes,2,opt,name=func_name,json=funcName,proto3" json:"func_name,omitempty"`
+	NameArgPosition        uint32  `protobuf:"varint,3,opt,name=name_arg_position,json=nameArgPosition,proto3" json:"name_arg_position,omitempty"`
+	DescriptionArgPosition uint32  `protobuf:"varint,4,opt,name=description_arg_position,json=descriptionArgPosition,proto3" json:"description_arg_position,omitempty"`
+	VarArgPosition         *uint32 `protobuf:"varint,5,opt,name=var_arg_position,json=varArgPosition,proto3,oneof" json:"var_arg_position,omitempty"`
 }
 
 func (x *FlagConstructor) Reset() {
@@ -280,6 +281,13 @@ func (x *FlagConstructor) GetDescriptionArgPosition() uint32 {
 	return 0
 }
 
+func (x *FlagConstructor) GetVarArgPosition() uint32 {
+	if x != nil && x.VarArgPosition != nil {
+		return *x.VarArgPosition
+	}
+	return 0
+}
+
 var File_kythe_proto_go_proto protoreflect.FileDescriptor
 
 var file_kythe_proto_go_proto_rawDesc = []byte{
@@ -305,7 +313,7 @@ var file_kythe_proto_go_proto_rawDesc = []byte{
 	0x30, 0x0a, 0x04, 0x66, 0x6c, 0x61, 0x67, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x1c, 0x2e,
 	0x6b, 0x79, 0x74, 0x68, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x46, 0x6c, 0x61, 0x67,
 	0x43, 0x6f, 0x6e, 0x73, 0x74, 0x72, 0x75, 0x63, 0x74, 0x6f, 0x72, 0x52, 0x04, 0x66, 0x6c, 0x61,
-	0x67, 0x22, 0xaf, 0x01, 0x0a, 0x0f, 0x46, 0x6c, 0x61, 0x67, 0x43, 0x6f, 0x6e, 0x73, 0x74, 0x72,
+	0x67, 0x22, 0xf3, 0x01, 0x0a, 0x0f, 0x46, 0x6c, 0x61, 0x67, 0x43, 0x6f, 0x6e, 0x73, 0x74, 0x72,
 	0x75, 0x63, 0x74, 0x6f, 0x72, 0x12, 0x19, 0x0a, 0x08, 0x70, 0x6b, 0x67, 0x5f, 0x70, 0x61, 0x74,
 	0x68, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x70, 0x6b, 0x67, 0x50, 0x61, 0x74, 0x68,
 	0x12, 0x1b, 0x0a, 0x09, 0x66, 0x75, 0x6e, 0x63, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x02, 0x20,
@@ -316,11 +324,16 @@ var file_kythe_proto_go_proto_rawDesc = []byte{
 	0x63, 0x72, 0x69, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x5f, 0x61, 0x72, 0x67, 0x5f, 0x70, 0x6f, 0x73,
 	0x69, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x16, 0x64, 0x65, 0x73,
 	0x63, 0x72, 0x69, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x41, 0x72, 0x67, 0x50, 0x6f, 0x73, 0x69, 0x74,
-	0x69, 0x6f, 0x6e, 0x42, 0x43, 0x0a, 0x1f, 0x63, 0x6f, 0x6d, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c,
-	0x65, 0x2e, 0x64, 0x65, 0x76, 0x74, 0x6f, 0x6f, 0x6c, 0x73, 0x2e, 0x6b, 0x79, 0x74, 0x68, 0x65,
-	0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x5a, 0x20, 0x6b, 0x79, 0x74, 0x68, 0x65, 0x2e, 0x69, 0x6f,
-	0x2f, 0x6b, 0x79, 0x74, 0x68, 0x65, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x67, 0x6f, 0x5f,
-	0x67, 0x6f, 0x5f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33,
+	0x69, 0x6f, 0x6e, 0x12, 0x2d, 0x0a, 0x10, 0x76, 0x61, 0x72, 0x5f, 0x61, 0x72, 0x67, 0x5f, 0x70,
+	0x6f, 0x73, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x05, 0x20, 0x01, 0x28, 0x0d, 0x48, 0x00, 0x52,
+	0x0e, 0x76, 0x61, 0x72, 0x41, 0x72, 0x67, 0x50, 0x6f, 0x73, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x88,
+	0x01, 0x01, 0x42, 0x13, 0x0a, 0x11, 0x5f, 0x76, 0x61, 0x72, 0x5f, 0x61, 0x72, 0x67, 0x5f, 0x70,
+	0x6f, 0x73, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x42, 0x43, 0x0a, 0x1f, 0x63, 0x6f, 0x6d, 0x2e, 0x67,
+	0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x64, 0x65, 0x76, 0x74, 0x6f, 0x6f, 0x6c, 0x73, 0x2e, 0x6b,
+	0x79, 0x74, 0x68, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x5a, 0x20, 0x6b, 0x79, 0x74, 0x68,
+	0x65, 0x2e, 0x69, 0x6f, 0x2f, 0x6b, 0x79, 0x74, 0x68, 0x65, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f,
+	0x2f, 0x67, 0x6f, 0x5f, 0x67, 0x6f, 0x5f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72,
+	0x6f, 0x74, 0x6f, 0x33,
 }
 
 var (
@@ -406,6 +419,7 @@ func file_kythe_proto_go_proto_init() {
 			}
 		}
 	}
+	file_kythe_proto_go_proto_msgTypes[3].OneofWrappers = []interface{}{}
 	type x struct{}
 	out := protoimpl.TypeBuilder{
 		File: protoimpl.DescBuilder{


Test patch:
diff --git a/kythe/go/indexer/testdata/flags.go b/kythe/go/indexer/testdata/flags.go
index 23ddbe753..668a2b4b0 100644
--- a/kythe/go/indexer/testdata/flags.go
+++ b/kythe/go/indexer/testdata/flags.go
@@ -36,12 +36,14 @@ func init() {
 	//- LocalFlag denotes BoolFlag
 	localFlag := flag.Bool("bool_flag", false, "Bool flag")
 
+	//- @localVar defines/binding LocalVar
 	var localVar bool
 	//- @"\"var_flag\"" defines/binding VarFlag=vname("flag var_flag", _, _, _, _)
 	//- VarFlag.node/kind google/gflag
 	//- VarFlagDoc documents VarFlag
 	//- VarFlagDoc.node/kind doc
 	//- VarFlagDoc.text "Var flag"
+	//- LocalVar denotes VarFlag
 	flag.BoolVar(&localVar, "var_flag", true, "Var flag")
 
 	//- @"\"func_flag\"" defines/binding FuncFlag=vname("flag func_flag", _, _, _, _)