Implement unused import warning, and lowering of source code info extensions (#566)

This PR adds two related features:

1. An unused imports warning, which requires additional book-keeping to
track which direct import caused a transitive import to get imported.
2. Codegen for the Buf source code info extension, which includes unused
imports and whether a `syntax` production exists in the file.

In the process of implementing this, I found a bug in how imports are
built, which this PR fixes. It showed up because import causes weren't
being listed correctly, leading to incorrect diagnosis of unused
imports. I also fixed a bug in `yaml.go` where extensions we're being
printed in the output.

Author: mcy

experimental/ir/fdp.go

@@ -17,6 +17,7 @@ package ir
 import (
 	"slices"
 
+	descriptorv1 "buf.build/gen/go/bufbuild/protodescriptor/protocolbuffers/go/buf/descriptor/v1"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/descriptorpb"
 
@@ -25,6 +26,7 @@ import (
 	"github.com/bufbuild/protocompile/experimental/ir/presence"
 	"github.com/bufbuild/protocompile/experimental/seq"
 	"github.com/bufbuild/protocompile/internal/ext/cmpx"
+	"github.com/bufbuild/protocompile/internal/ext/iterx"
 	"github.com/bufbuild/protocompile/internal/ext/slicesx"
 )
 
@@ -66,8 +68,20 @@ func DescriptorProtoBytes(file File, options ...DescriptorOption) ([]byte, error
 // DescriptorOption is an option to pass to [DescriptorSetBytes] or [DescriptorProtoBytes].
 type DescriptorOption func(*descGenerator)
 
+// IncludeDebugInfo sets whether or not to include google.protobuf.SourceCodeInfo in
+// the output.
+func IncludeSourceCodeInfo(flag bool) DescriptorOption {
+	return func(dg *descGenerator) {
+		dg.includeDebugInfo = flag
+	}
+}
+
 type descGenerator struct {
-	currentFile File
+	currentFile      File
+	includeDebugInfo bool
+
+	sourceCodeInfo     *descriptorpb.SourceCodeInfo
+	sourceCodeInfoExtn *descriptorv1.SourceCodeInfoExtension
 }
 
 func (dg *descGenerator) files(files []File, fds *descriptorpb.FileDescriptorSet) {
@@ -77,12 +91,20 @@ func (dg *descGenerator) files(files []File, fds *descriptorpb.FileDescriptorSet
 	for file := range topoSort(files) {
 		fdp := new(descriptorpb.FileDescriptorProto)
 		fds.File = append(fds.File, fdp)
+
 		dg.file(file, fdp)
 	}
 }
 
 func (dg *descGenerator) file(file File, fdp *descriptorpb.FileDescriptorProto) {
 	dg.currentFile = file
+	if dg.includeDebugInfo {
+		dg.sourceCodeInfo = new(descriptorpb.SourceCodeInfo)
+		fdp.SourceCodeInfo = dg.sourceCodeInfo
+
+		dg.sourceCodeInfoExtn = new(descriptorv1.SourceCodeInfoExtension)
+		proto.SetExtension(dg.sourceCodeInfo, descriptorv1.E_BufSourceCodeInfoExtension, dg.sourceCodeInfoExtn)
+	}
 
 	fdp.Name = addr(file.Path())
 	fdp.Package = addr(string(file.Package()))
@@ -94,12 +116,16 @@ func (dg *descGenerator) file(file File, fdp *descriptorpb.FileDescriptorProto)
 		fdp.Syntax = addr(file.Syntax().String())
 	}
 
+	if dg.sourceCodeInfoExtn != nil {
+		dg.sourceCodeInfoExtn.IsSyntaxUnspecified = file.AST().Syntax().IsZero()
+	}
+
 	// Canonicalize import order so that it does not change whenever we refactor
 	// internal structures.
-	// TODO: sort in declaration order to match protoc? Not done currently
-	// since that requires additional book-keeping in [imports].
 	imports := seq.ToSlice(file.Imports())
-	slices.SortFunc(imports, cmpx.Key(Import.Path))
+	slices.SortFunc(imports, cmpx.Key(func(imp Import) int {
+		return imp.Decl.KeywordToken().Span().Start
+	}))
 	for i, imp := range imports {
 		fdp.Dependency = append(fdp.Dependency, imp.Path())
 		if imp.Public {
@@ -108,6 +134,10 @@ func (dg *descGenerator) file(file File, fdp *descriptorpb.FileDescriptorProto)
 		if imp.Weak {
 			fdp.WeakDependency = append(fdp.WeakDependency, int32(i))
 		}
+
+		if dg.sourceCodeInfoExtn != nil && !imp.Used {
+			dg.sourceCodeInfoExtn.UnusedDependency = append(dg.sourceCodeInfoExtn.UnusedDependency, int32(i))
+		}
 	}
 
 	for ty := range seq.Values(file.Types()) {
@@ -139,6 +169,10 @@ func (dg *descGenerator) file(file File, fdp *descriptorpb.FileDescriptorProto)
 		fdp.Options = new(descriptorpb.FileOptions)
 		dg.options(options, fdp.Options)
 	}
+
+	if dg.sourceCodeInfoExtn != nil && iterx.Empty2(dg.sourceCodeInfoExtn.ProtoReflect().Range) {
+		proto.ClearExtension(dg.sourceCodeInfo, descriptorv1.E_BufSourceCodeInfoExtension)
+	}
 }
 
 func (dg *descGenerator) message(ty Type, mdp *descriptorpb.DescriptorProto) {

experimental/ir/ir_imports.go

@@ -17,8 +17,6 @@ package ir
 import (
 	"slices"
 
-	"github.com/bits-and-blooms/bitset"
-
 	"github.com/bufbuild/protocompile/experimental/ast"
 	"github.com/bufbuild/protocompile/experimental/seq"
 	"github.com/bufbuild/protocompile/internal/ext/mapsx"
@@ -32,6 +30,7 @@ type Import struct {
 	Public, Weak bool           // The kind of import this is.
 	Direct       bool           // Whether this is a direct or transitive import.
 	Visible      bool           // Whether this import's symbols are visible in the current file.
+	Used         bool           // Whether this import has been marked as used.
 	Decl         ast.DeclImport // The import declaration.
 }
 
@@ -57,21 +56,22 @@ type imports struct {
 	// 2. Weak imports.
 	// 3. Regular imports.
 	// 4. Transitive public imports.
-	// 4. Transitive imports.
+	// 5. Transitive imports.
 	//
 	// The fields after this one specify where each of these segments ends.
 	//
 	// The last element of this slice is always descriptor.proto, even if it
 	// exists elsewhere as an ordinary import.
 	files []imported
 
-	// Which of the above files we are permitted to import from.
-	visible bitset.BitSet
-
 	// Maps the path of each file to its index in files. This is used for
 	// mapping from one [Context]'s file IDs to another's.
 	byPath intern.Map[uint32]
 
+	// Map of path of each imported file to a direct import which causes it to
+	// be imported. This is used for marking which imports are used.
+	causes intern.Map[uint32]
+
 	// NOTE: public imports always come first. This ensures that when
 	// recursively determining public imports, we consider public imports'
 	// recursive imports first. Consider the following sequence of files:
@@ -105,8 +105,9 @@ type imports struct {
 
 // imported wraps an imported [File] and the import statement declaration [ast.DeclImport].
 type imported struct {
-	file File
-	decl ast.DeclImport
+	file          File
+	decl          ast.DeclImport
+	visible, used bool
 }
 
 // Append appends a direct import to this imports table.
@@ -132,7 +133,7 @@ func (i *imports) AddDirect(imp Import) {
 //
 // Must only be called once, after all direct imports are added.
 func (i *imports) Recurse(dedup intern.Map[ast.DeclImport]) {
-	for file := range seq.Values(i.Directs()) {
+	for _, file := range seq.All(i.Directs()) {
 		for imp := range seq.Values(file.TransitiveImports()) {
 			if !mapsx.AddZero(dedup, imp.InternedPath()) {
 				continue
@@ -150,6 +151,20 @@ func (i *imports) Recurse(dedup intern.Map[ast.DeclImport]) {
 			i.Insert(imp, -1, imp.Public)
 		}
 	}
+
+	// Now, build the path and causes maps.
+	i.byPath = make(intern.Map[uint32])
+	i.causes = make(intern.Map[uint32])
+
+	for n, imp := range i.files {
+		i.byPath[imp.file.InternedPath()] = uint32(n)
+	}
+	for k, file := range seq.All(i.Directs()) {
+		mapsx.Add(i.causes, file.InternedPath(), uint32(k))
+		for imp := range seq.Values(file.TransitiveImports()) {
+			mapsx.Add(i.causes, imp.InternedPath(), uint32(k))
+		}
+	}
 }
 
 // Insert inserts a new import at the given position.
@@ -160,13 +175,19 @@ func (i *imports) Insert(imp Import, pos int, visible bool) {
 		pos = len(i.files)
 	}
 
-	if i.byPath == nil {
-		i.byPath = make(intern.Map[uint32])
-	}
+	i.files = slices.Insert(i.files, pos, imported{
+		file:    imp.File,
+		decl:    imp.Decl,
+		visible: visible,
+	})
+}
 
-	i.files = slices.Insert(i.files, pos, imported{file: imp.File, decl: imp.Decl})
-	i.byPath[imp.File.InternedPath()] = uint32(pos)
-	i.visible.SetTo(uint(pos), visible)
+// MarkUsed records a file as used, which affects the values of [Import].Used.
+func (i *imports) MarkUsed(file File) {
+	idx, ok := i.causes[file.InternedPath()]
+	if ok {
+		i.files[idx].used = true
+	}
 }
 
 // DescriptorProto returns the file for descriptor.proto.
@@ -181,21 +202,25 @@ func (i *imports) Directs() seq.Indexer[Import] {
 		i.files[:i.importEnd],
 		func(j int, imported imported) Import {
 			n := uint32(j)
+			public := n < i.publicEnd
 			return Import{
 				File:    imported.file,
-				Public:  n < i.publicEnd,
-				Weak:    n >= i.publicEnd && n < i.weakEnd,
+				Public:  public,
+				Weak:    !public && n < i.weakEnd,
 				Direct:  true,
 				Visible: true,
 				Decl:    imported.decl,
+
+				// Public imports are implicitly always used.
+				Used: imported.used || public,
 			}
 		},
 	)
 }
 
 // Transitive returns an indexer over the Transitive imports.
 //
-// This function does not report whether those imports are weak or not.
+// This function does not report whether those imports are weak or used.
 func (i *imports) Transitive() seq.Indexer[Import] {
 	return seq.NewFixedSlice(
 		i.files[:max(0, len(i.files)-1)], // Exclude the implicit descriptor.proto.
@@ -206,7 +231,7 @@ func (i *imports) Transitive() seq.Indexer[Import] {
 				Public: n < i.publicEnd ||
 					(n >= i.importEnd && n < i.transPublicEnd),
 				Direct:  n < i.importEnd,
-				Visible: i.visible.Test(uint(j)),
+				Visible: imported.visible,
 				Decl:    imported.decl,
 			}
 		},

experimental/ir/ir_symbol.go

@@ -145,7 +145,7 @@ func (s Symbol) AsMethod() Method {
 func (s Symbol) Visible() bool {
 	return s.ref.file <= 0 ||
 		s.Kind() == SymbolKindPackage || // Packages don't get visibility checks.
-		s.Context().imports.visible.Test(uint(s.ref.file)-1)
+		s.Context().imports.files[uint(s.ref.file)-1].visible
 }
 
 // Definition returns a span for the definition site of this symbol;
@@ -482,24 +482,26 @@ again:
 		candidate = slices.Delete(candidate, len(scope), oldLen)
 	}
 
-	if !scopeSearch {
-		// This was a single identifier name so we're done.
-		return found, expected
+	if scopeSearch {
+		// Now search for the full name inside of the scope we found.
+		candidate = append(candidate, name[len(first):]...)
+		found = s.lookupBytes(c, candidate)
+		if found.ptr.Nil() {
+			// Try again, this time using the full candidate name. This happens
+			// expressly for the purpose of diagnostics.
+			scopeSearch = false
+			// Need to clear the found scope, since otherwise we might get a weird
+			// error message where we say that we found the parent scope.
+			found = ref[rawSymbol]{}
+			expected = FullName(candidate)
+			goto again
+		}
 	}
 
-	// Now search for the full name inside of the scope we found.
-	candidate = append(candidate, name[len(first):]...)
-	r := s.lookupBytes(c, candidate)
-	if r.ptr.Nil() {
-		// Try again, this time using the full candidate name. This happens
-		// expressly for the purpose of diagnostics.
-		scopeSearch = false
-		// Need to clear the found scope, since otherwise we might get a weird
-		// error message where we say that we found the parent scope.
-		found = ref[rawSymbol]{}
-		expected = FullName(candidate)
-		goto again
+	file := found.context(c).File()
+	if file != c.File() {
+		c.imports.MarkUsed(file)
 	}
 
-	return r, expected
+	return found, expected
 }

experimental/ir/ir_test.go

@@ -55,6 +55,7 @@ type Test struct {
 	Files             []File `yaml:"files"`
 	ExcludeWKTSources bool   `yaml:"exclude_wkt_sources"`
 	OutputWKTs        bool   `yaml:"output_wkts"`
+	SourceCodeInfo    bool   `yaml:"source_code_info"`
 }
 
 type File struct {
@@ -130,7 +131,7 @@ func TestIR(t *testing.T) {
 
 		irs := slicesx.Transform(results, func(r incremental.Result[ir.File]) ir.File { return r.Value })
 		irs = slices.DeleteFunc(irs, ir.File.IsZero)
-		bytes, err := ir.DescriptorSetBytes(irs)
+		bytes, err := ir.DescriptorSetBytes(irs, ir.IncludeSourceCodeInfo(test.SourceCodeInfo))
 		require.NoError(t, err)
 
 		fds := new(descriptorpb.FileDescriptorSet)
@@ -176,6 +177,7 @@ func symtabProto(files []ir.File, t *Test) *compilerpb.SymbolSet {
 				Weak:       imp.Weak,
 				Transitive: !imp.Direct,
 				Visible:    imp.Visible,
+				Used:       imp.Used,
 			})
 		}
 		slices.SortFunc(symtab.Imports, cmpx.Key(func(x *compilerpb.Import) string { return x.Path }))

experimental/ir/lower.go

@@ -102,4 +102,6 @@ func lower(c *Context, r *report.Report, importer Importer) {
 	// done in two separate steps.
 	populateOptionTargets(c.File(), r)
 	validateOptionTargets(c.File(), r)
+
+	diagnoseUnusedImports(c.File(), r)
 }

experimental/ir/lower_imports.go

@@ -109,7 +109,7 @@ func buildImports(f File, r *report.Report, importer Importer) {
 	c.imports.Recurse(dedup)
 
 	// Check if descriptor.proto was transitively imported. If not, import it.
-	if idx, ok := c.imports.byPath.Get(&f.Context().session.intern, DescriptorProtoPath); ok {
+	if idx, ok := c.imports.byPath[f.Context().session.builtins.DescriptorFile]; ok {
 		// Copy it to the end so that it's easy to find.
 		c.imports.files = append(c.imports.files, c.imports.files[idx])
 		return
@@ -119,6 +119,8 @@ func buildImports(f File, r *report.Report, importer Importer) {
 	// avoid cycles.
 	if f.IsDescriptorProto() {
 		c.imports.Insert(Import{File: f}, -1, false)
+		c.imports.byPath[f.Context().session.builtins.DescriptorFile] = uint32(len(c.imports.files) - 1)
+		c.imports.causes[f.Context().session.builtins.DescriptorFile] = uint32(len(c.imports.files) - 1)
 		return
 	}
 
@@ -134,6 +136,8 @@ func buildImports(f File, r *report.Report, importer Importer) {
 	}
 
 	c.imports.Insert(Import{File: dproto, Decl: ast.DeclImport{}}, -1, false)
+	c.imports.byPath[f.Context().session.builtins.DescriptorFile] = uint32(len(c.imports.files) - 1)
+	c.imports.causes[f.Context().session.builtins.DescriptorFile] = uint32(len(c.imports.files) - 1)
 }
 
 // diagnoseCycle generates a diagnostic for an import cycle, showing each

experimental/ir/lower_validate.go

@@ -0,0 +1,37 @@
+// Copyright 2020-2025 Buf Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package ir
+
+import (
+	"github.com/bufbuild/protocompile/experimental/report"
+	"github.com/bufbuild/protocompile/experimental/seq"
+)
+
+// diagnoseUnusedImports generates diagnostics for each unused import.
+func diagnoseUnusedImports(f File, r *report.Report) {
+	for imp := range seq.Values(f.Imports()) {
+		if imp.Used {
+			continue
+		}
+
+		r.Warnf("unused import \"%s\"", f.Path()).Apply(
+			report.Snippet(imp.Decl.ImportPath()),
+			report.SuggestEdits(imp.Decl, "delete it", report.Edit{
+				Start: 0, End: imp.Decl.Span().Len(),
+			}),
+			report.Helpf("no symbols from this file are referenced"),
+		)
+	}
+}