Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(go_indexer): fix builtin type references #5812

Merged
merged 1 commit into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 0 additions & 10 deletions kythe/go/extractors/govname/govname.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,16 +185,6 @@ func ForPackage(pkg *build.Package, opts *PackageVNameOptions) *spb.VName {
return v
}

// ForBuiltin returns a VName for a Go built-in with the given signature.
func ForBuiltin(signature string) *spb.VName {
return &spb.VName{
Corpus: GolangCorpus,
Language: Language,
Root: "ref/spec",
Signature: signature,
}
}

// ForStandardLibrary returns a VName for a standard library package with the
// given import path.
func ForStandardLibrary(importPath string) *spb.VName {
Expand Down
14 changes: 0 additions & 14 deletions kythe/go/extractors/govname/govname_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,20 +139,6 @@ func TestNotStandardLib(t *testing.T) {
}
}

func TestForBuiltin(t *testing.T) {
const signature = "blah"
want := &spb.VName{
Corpus: GolangCorpus,
Language: Language,
Root: "ref/spec",
Signature: signature,
}
got := ForBuiltin("blah")
if !proto.Equal(got, want) {
t.Errorf("ForBuiltin(%q): got %+v, want %+v", signature, got, want)
}
}

func TestForStandardLibrary(t *testing.T) {
tests := []struct {
input string
Expand Down
1 change: 1 addition & 0 deletions kythe/go/indexer/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ go_indexer_test(
has_marked_source = True,
import_path = "rendered",
resolve_code_facts = True,
deps = [":builtin_test"],
)

go_indexer_test(
Expand Down
1 change: 1 addition & 0 deletions kythe/go/indexer/cmd/go_indexer/go_indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ func indexGo(ctx context.Context, unit *apb.CompilationUnit, f indexer.Fetcher)
UseFileAsTopLevelScope: *useFileAsTopLevelScope,
OverrideStdlibCorpus: *overrideStdlibCorpus,
EmitRefCallOverIdentifier: *emitRefCallOverIdentifier,
Verbose: *verbose,
})
}

Expand Down
16 changes: 14 additions & 2 deletions kythe/go/indexer/emit.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ type EmitOptions struct {
// over function identifiers (or the legacy behavior of over the entire
// callsite).
EmitRefCallOverIdentifier bool

// Verbose determines whether verbose logging is enabled.
Verbose bool
}

func (e *EmitOptions) emitMarkedSource() bool {
Expand Down Expand Up @@ -132,6 +135,13 @@ func (e *EmitOptions) docURL(pi *PackageInfo) string {
return u.String()
}

func (e *EmitOptions) verbose() bool {
if e == nil {
return false
}
return e.Verbose
}

// An impl records that a type A implements an interface B.
type impl struct{ A, B types.Object }

Expand Down Expand Up @@ -210,8 +220,10 @@ func (pi *PackageInfo) Emit(ctx context.Context, sink Sink, opts *EmitOptions) e
e.emitSatisfactions()

// TODO(fromberger): Add diagnostics for type-checker errors.
for _, err := range pi.Errors {
log.Warningf("Type resolution error: %v", err)
if opts.verbose() {
for _, err := range pi.Errors {
log.Warningf("Type resolution error: %v", err)
}
}
return e.firstErr
}
Expand Down
2 changes: 1 addition & 1 deletion kythe/go/indexer/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ func (pi *PackageInfo) ObjectVName(obj types.Object) *spb.VName {
if base := pi.PackageVName[pkg]; base != nil {
vname = proto.Clone(base).(*spb.VName)
} else if pkg == nil {
return govname.ForBuiltin(sig)
return govname.Builtin(obj.Name())
} else {
// This is an indirect import, that is, a name imported but not
// mentioned explicitly by the package being indexed.
Expand Down
6 changes: 3 additions & 3 deletions kythe/go/indexer/testdata/builtin.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package builtin

//- BoolBuiltin=vname("bool#builtin", "golang.org", "", "", "go").node/kind tbuiltin
//- BoolBuiltin.code/rendered/callsite_signature "bool"
//- BoolBuiltin.code/rendered/signature "bool"
//- IntBuiltin=vname("int#builtin", "golang.org", "", "", "go").node/kind tbuiltin
//- IntBuiltin.code/rendered/callsite_signature "int"
//- IntBuiltin.code/rendered/signature "int"

//- ErrorBuiltin=vname("error#builtin", "golang.org", "", "", "go").node/kind tbuiltin
//- ErrorBuiltin.code/rendered/callsite_signature "error"
//- ErrorBuiltin.code/rendered/signature "error"
10 changes: 10 additions & 0 deletions kythe/go/indexer/testdata/code/rendered.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,13 @@ func (s *S) M() {}
// - @arg defines/binding Arg
// - Arg.code/rendered/qualified_name "rendered.S.MArg.arg"
func (s *S) MArg(arg int) {}

//- StringBuiltin=vname("string#builtin", "golang.org", "", "", "go").node/kind tbuiltin
//- StringBuiltin.code/rendered/callsite_signature "string"
//- ErrorBuiltin=vname("error#builtin", "golang.org", "", "", "go").node/kind tbuiltin
//- ErrorBuiltin.code/rendered/callsite_signature "error"

// - @H defines/binding H?
// - H.code/rendered/callsite_signature "H(param)"
// - H.code/rendered/signature "H(param func() (string, error)) error"
func H(param func() (string, error)) error { return nil }
4 changes: 2 additions & 2 deletions kythe/go/util/markedsource/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,13 @@ func (r *Resolver) resolve(ticket string, ms *cpb.MarkedSource) *cpb.MarkedSourc
case cpb.MarkedSource_PARAMETER_LOOKUP_BY_PARAM_WITH_DEFAULTS, cpb.MarkedSource_PARAMETER_LOOKUP_BY_PARAM:
// TODO: handle param/default
params := r.params[ticket]
if int(ms.GetLookupIndex()) >= len(params) {
if int(ms.GetLookupIndex()) > len(params) {
return ensureKind(invalidLookupReplacement, cpb.MarkedSource_PARAMETER)
}
return r.resolveParameters(ms, params[ms.GetLookupIndex():])
case cpb.MarkedSource_PARAMETER_LOOKUP_BY_TPARAM:
tparams := r.tparams[ticket]
if int(ms.GetLookupIndex()) >= len(tparams) {
if int(ms.GetLookupIndex()) > len(tparams) {
return ensureKind(invalidLookupReplacement, cpb.MarkedSource_PARAMETER)
}
return r.resolveParameters(ms, tparams[ms.GetLookupIndex():])
Expand Down
9 changes: 9 additions & 0 deletions kythe/go/util/tools/markedsource/markedsource.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
var (
rewrite = flag.Bool("rewrite", false, "Rewrite all code facts to be fully resolved")

renderSignature = flag.Bool("render_signatures", false, "Whether to emit /kythe/code/rendered/signature facts (requires --rewrite)")
renderCallsiteSignature = flag.Bool("render_callsite_signatures", false, "Whether to emit /kythe/code/rendered/callsite_signature facts (requires --rewrite)")
renderQualifiedName = flag.Bool("render_qualified_names", false, "Whether to emit /kythe/code/rendered/qualified_name facts (requires --rewrite)")
)
Expand Down Expand Up @@ -96,6 +97,14 @@ func main() {
FactValue: []byte(val),
}))
}
if *renderSignature {
val := markedsource.RenderSignature(resolved, markedsource.PlaintextContent, nil)
exitOnErr(wr.PutProto(&spb.Entry{
Source: e.Source,
FactName: renderFactPrefix + "signature",
FactValue: []byte(val),
}))
}
}
exitOnErr(wr.PutProto(e))
}
Expand Down
2 changes: 1 addition & 1 deletion tools/build_rules/verifier_test/verifier_test.sh.in
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ ENTRIES_GZ=(@ENTRIES_GZ@)
if (( ${#ENTRIES_GZ[@]} )); then gunzip -c "${ENTRIES_GZ[@]}"; fi
) | (
if [[ -z "@REWRITE@" ]]; then
@MARKEDSOURCE@ --rewrite --render_callsite_signatures --render_qualified_names
@MARKEDSOURCE@ --rewrite --render_callsite_signatures --render_qualified_names --render_signatures
else
cat
fi
Expand Down