gopls: Add test cases for references in assembly and Go files

Improve code comments for better readability.Remove type checking logic from assembly file reference lookup.Update xrefs index retrieval for consistency

Updates golang/go#71754

Author: groot-guo

gopls/internal/cache/metadata/metadata.go

@@ -51,8 +51,7 @@ type Package struct {
 	IgnoredFiles    []protocol.DocumentURI
 	OtherFiles      []protocol.DocumentURI
 
-	// These fields ares as defined by asm.File
-	AsmFiles []protocol.DocumentURI
+	AsmFiles []protocol.DocumentURI // *.s subset of OtherFiles
 
 	ForTest       PackagePath // q in a "p [q.test]" package, else ""
 	TypesSizes    types.Sizes

gopls/internal/cache/package.go

@@ -227,7 +227,7 @@ func (pkg *syntaxPackage) AsmFile(uri protocol.DocumentURI) (*asm.File, error) {
 	for _, af := range pkg.asmFiles {
 		if af.URI == uri {
 			return af, nil
-		}	
+		}
 	}
 
 	return nil, fmt.Errorf("no parsed file for %s in %v", uri, pkg.id)

gopls/internal/cache/xrefs/xrefs.go

@@ -11,6 +11,7 @@ package xrefs
 import (
 	"go/ast"
 	"go/types"
+	"slices"
 	"sort"
 
 	"golang.org/x/tools/go/types/objectpath"
@@ -72,7 +73,7 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles
 								// (e.g. local const/var/type).
 								continue
 							}
-							gobObj = &gobObject{Path: path, isAsm: false}
+							gobObj = &gobObject{Path: path}
 							objects[obj] = gobObj
 						}
 
@@ -114,29 +115,21 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles
 		}
 	}
 
+	// For each asm file, record references to identifiers.
 	for fileIndex, af := range asmFiles {
 		for _, id := range af.Idents {
 			_, name, ok := morestrings.CutLast(id.Name, ".")
 			if !ok {
 				continue
 			}
-			if id.Kind != asm.Text {
-				continue
-			}
 			obj := pkg.Scope().Lookup(name)
 			if obj == nil {
 				continue
 			}
 			objects := getObjects(pkg)
 			gobObj, ok := objects[obj]
 			if !ok {
-				path, err := objectpathFor(obj)
-				if err != nil {
-					// Capitalized but not exported
-					// (e.g. local const/var/type).
-					continue
-				}
-				gobObj = &gobObject{Path: path, isAsm: true}
+				gobObj = &gobObject{Path: objectpath.Path(obj.Name())}
 				objects[obj] = gobObj
 			}
 			if rng, err := af.NodeRange(id); err == nil {
@@ -183,10 +176,7 @@ func Lookup(mp *metadata.Package, data []byte, targets map[metadata.PackagePath]
 			for _, gobObj := range gp.Objects {
 				if _, ok := objectSet[gobObj.Path]; ok {
 					for _, ref := range gobObj.Refs {
-						uri := mp.CompiledGoFiles[ref.FileIndex]
-						if gobObj.isAsm {
-							uri = mp.AsmFiles[ref.FileIndex]
-						}
+						uri := slices.Concat(mp.CompiledGoFiles, mp.AsmFiles)[ref.FileIndex]
 						locs = append(locs, protocol.Location{
 							URI:   uri,
 							Range: ref.Range,
@@ -223,9 +213,8 @@ type gobPackage struct {
 
 // A gobObject records all references to a particular symbol.
 type gobObject struct {
-	Path  objectpath.Path // symbol name within package; "" => import of package itself
-	Refs  []gobRef        // locations of references within P, in lexical order
-	isAsm bool            // true if this is an assembly object
+	Path objectpath.Path // symbol name within package; "" => import of package itself
+	Refs []gobRef        // locations of references within P, in lexical order
 }
 
 type gobRef struct {

gopls/internal/goasm/references.go

@@ -20,29 +20,36 @@ import (
 	"golang.org/x/tools/internal/event"
 )
 
-func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, position protocol.Position) ([]protocol.Location, error) {
+// References returns a list of locations (file and position) where the symbol under the cursor in an assembly file is referenced,
+// including both Go source files and assembly files within the same package.
+func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, position protocol.Position, includeDeclaration bool) ([]protocol.Location, error) {
 	ctx, done := event.Start(ctx, "goasm.References")
 	defer done()
 
-	pkg, asmFile, err := GetPackageID(ctx, snapshot, fh.URI())
+	mps, err := snapshot.MetadataForFile(ctx, fh.URI())
 	if err != nil {
 		return nil, err
 	}
-
-	// Read the file.
-	content, err := fh.Content()
+	metadata.RemoveIntermediateTestVariants(&mps)
+	if len(mps) == 0 {
+		return nil, fmt.Errorf("no package metadata for file %s", fh.URI())
+	}
+	mp := mps[0]
+	pkgs, err := snapshot.TypeCheck(ctx, mp.ID)
 	if err != nil {
 		return nil, err
 	}
-	mapper := protocol.NewMapper(fh.URI(), content)
-	offset, err := mapper.PositionOffset(position)
+	pkg := pkgs[0]
+	asmFile, err := pkg.AsmFile(fh.URI())
+	if err != nil {
+		return nil, err // "can't happen"
+	}
+
+	offset, err := asmFile.Mapper.PositionOffset(position)
 	if err != nil {
 		return nil, err
 	}
 
-	// // Parse the assembly.
-	// file := asm.Parse(fh.URI(), content)
-
 	// Figure out the selected symbol.
 	// For now, just find the identifier around the cursor.
 	var found *asm.Ident
@@ -62,7 +69,10 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p
 	if !ok {
 		return nil, fmt.Errorf("not found")
 	}
-	// return localReferences(pkg, targets, true, report)
+
+	// TODO(grootguo): Currently, only references to the symbol within the package are found (i.e., only Idents in this package's Go files are searched).
+	// It is still necessary to implement cross-package reference lookup: that is, to find all references to this symbol in other packages that import the current package.
+	// Refer to the global search logic in golang.References, and add corresponding test cases for verification.
 	for _, pgf := range pkg.CompiledGoFiles() {
 		for curId := range pgf.Cursor.Preorder((*ast.Ident)(nil)) {
 			id := curId.Node().(*ast.Ident)
@@ -76,6 +86,11 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p
 		}
 	}
 
+	// If includeDeclaration is false, return only reference locations (exclude declarations).
+	if !includeDeclaration {
+		return locations, nil
+	}
+
 	for _, asmFile := range pkg.AsmFiles() {
 		for _, id := range asmFile.Idents {
 			if id.Name != sym {
@@ -93,25 +108,3 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p
 
 	return locations, nil
 }
-
-func GetPackageID(ctx context.Context, snapshot *cache.Snapshot, uri protocol.DocumentURI) (*cache.Package, *asm.File, error) {
-	mps, err := snapshot.MetadataForFile(ctx, uri)
-	if err != nil {
-		return nil, nil, err
-	}
-	metadata.RemoveIntermediateTestVariants(&mps)
-	if len(mps) == 0 {
-		return nil, nil, fmt.Errorf("no package metadata for file %s", uri)
-	}
-	mp := mps[0]
-	pkgs, err := snapshot.TypeCheck(ctx, mp.ID)
-	if err != nil {
-		return nil, nil, err
-	}
-	pkg := pkgs[0]
-	asmFile, err := pkg.AsmFile(uri)
-	if err != nil {
-		return nil, nil, err // "can't happen"
-	}
-	return pkg, asmFile, nil
-}

gopls/internal/golang/references.go

@@ -32,7 +32,6 @@ import (
 	"golang.org/x/tools/gopls/internal/cache/parsego"
 	"golang.org/x/tools/gopls/internal/file"
 	"golang.org/x/tools/gopls/internal/protocol"
-	"golang.org/x/tools/gopls/internal/util/asm"
 	"golang.org/x/tools/gopls/internal/util/morestrings"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
 	"golang.org/x/tools/internal/event"
@@ -612,15 +611,13 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo
 		}
 	}
 
+	// Iterate over all assembly files and find all references to the target object.
 	for _, pgf := range pkg.AsmFiles() {
 		for _, id := range pgf.Idents {
 			_, name, ok := morestrings.CutLast(id.Name, ".")
 			if !ok {
 				continue
 			}
-			if id.Kind != asm.Text {
-				continue
-			}
 			obj := pkg.Types().Scope().Lookup(name)
 			if obj == nil {
 				continue
@@ -630,7 +627,7 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo
 					URI:   pgf.URI,
 					Range: rng,
 				}
-				report(asmLocation, true)
+				report(asmLocation, false)
 			}
 		}
 	}

gopls/internal/server/references.go

@@ -37,7 +37,7 @@ func (s *server) References(ctx context.Context, params *protocol.ReferenceParam
 	case file.Go:
 		return golang.References(ctx, snapshot, fh, params.Position, params.Context.IncludeDeclaration)
 	case file.Asm:
-		return goasm.References(ctx, snapshot, fh, params.Position)
+		return goasm.References(ctx, snapshot, fh, params.Position, params.Context.IncludeDeclaration)
 	}
 	return nil, nil // empty result
 }

gopls/internal/test/marker/testdata/references/asm_ref.txt

@@ -0,0 +1,25 @@
+
+-- go.mod --
+module example.com
+go 1.24
+
+-- foo/foo.go --
+package foo
+
+func Add(a, b int) int //@ loc(use, "Add"), refs("Add", use, def)
+func sub(a, b int) int //@ loc(useSub, "sub"), refs("sub", useSub, defSub, refSub)
+var _ = sub //@loc(refSub, "sub"), refs("sub", useSub, defSub, refSub)
+
+-- foo/foo.s --
+// portable assembly
+#include "textflag.h"
+
+TEXT ·Add(SB), NOSPLIT, $0-24 //@ loc(def, "Add"), refs("Add", def, use)
+    MOVQ a+0(FP), AX
+    ADDQ b+8(FP), AX
+    RET
+
+TEXT ·sub(SB), NOSPLIT, $0-24 //@ loc(defSub, "sub"), refs("sub", defSub, useSub, refSub)
+    MOVQ a+0(FP), AX
+    SUBQ b+8(FP), AX
+    RET

gopls/internal/util/asm/parse.go

@@ -48,14 +48,14 @@ type File struct {
 	URI    protocol.DocumentURI
 	Idents []Ident
 
-	Mapper *protocol.Mapper // may map fixed Src, not file content
+	Mapper *protocol.Mapper
 
 	// TODO(adonovan): use token.File? This may be important in a
 	// future in which analyzers can report diagnostics in .s files.
 }
 
 func (f *File) NodeRange(ident Ident) (protocol.Range, error) {
-	return f.Mapper.OffsetRange(ident.Offset, ident.End())
+	return f.Mapper.OffsetRange(ident.Offset+2, ident.End()+1)
 }
 
 // Ident represents an identifier in an assembly file.