go/analysis: add Pass.ReadFile

The ReadFile function enables an analyzer to read the contents
of a file through the driver. The allows the driver to provide
the analyzer with a view of the file system that is consistent
with that seen by the parser and type checker, and enables
drivers to reject attempts to read arbitrary files, so that
their dependency analysis (if any) can be sound and precise.

+ a test

Fixes golang/go#62292

Change-Id: I9f301cbfd4a705a259f9e213fc2e63d74424294a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/581555
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: adonovan

go/analysis/analysis.go

@@ -112,6 +112,19 @@ type Pass struct {
 	// analysis's ResultType.
 	ResultOf map[*Analyzer]interface{}
 
+	// ReadFile returns the contents of the named file.
+	//
+	// The only valid file names are the elements of OtherFiles
+	// and IgnoredFiles, and names returned by
+	// Fset.File(f.FileStart).Name() for each f in Files.
+	//
+	// Analyzers must use this function (if provided) instead of
+	// accessing the file system directly. This allows a driver to
+	// provide a virtualized file tree (including, for example,
+	// unsaved editor buffers) and to track dependencies precisely
+	// to avoid unnecessary recomputation.
+	ReadFile func(filename string) ([]byte, error)
+
 	// -- facts --
 
 	// ImportObjectFact retrieves a fact associated with obj.

go/analysis/doc.go

@@ -32,7 +32,7 @@ bases, and so on.
 
 # Analyzer
 
-The primary type in the API is Analyzer. An Analyzer statically
+The primary type in the API is [Analyzer]. An Analyzer statically
 describes an analysis function: its name, documentation, flags,
 relationship to other analyzers, and of course, its logic.
 
@@ -72,7 +72,7 @@ help that describes the analyses it performs.
 The doc comment contains a brief one-line summary,
 optionally followed by paragraphs of explanation.
 
-The Analyzer type has more fields besides those shown above:
+The [Analyzer] type has more fields besides those shown above:
 
 	type Analyzer struct {
 		Name             string
@@ -114,7 +114,7 @@ instance of the Pass type.
 
 # Pass
 
-A Pass describes a single unit of work: the application of a particular
+A [Pass] describes a single unit of work: the application of a particular
 Analyzer to a particular package of Go code.
 The Pass provides information to the Analyzer's Run function about the
 package being analyzed, and provides operations to the Run function for
@@ -135,16 +135,14 @@ reporting diagnostics and other information back to the driver.
 The Fset, Files, Pkg, and TypesInfo fields provide the syntax trees,
 type information, and source positions for a single package of Go code.
 
-The OtherFiles field provides the names, but not the contents, of non-Go
-files such as assembly that are part of this package. See the "asmdecl"
-or "buildtags" analyzers for examples of loading non-Go files and reporting
-diagnostics against them.
-
-The IgnoredFiles field provides the names, but not the contents,
-of ignored Go and non-Go source files that are not part of this package
-with the current build configuration but may be part of other build
-configurations. See the "buildtags" analyzer for an example of loading
-and checking IgnoredFiles.
+The OtherFiles field provides the names of non-Go
+files such as assembly that are part of this package.
+Similarly, the IgnoredFiles field provides the names of Go and non-Go
+source files that are not part of this package with the current build
+configuration but may be part of other build configurations.
+The contents of these files may be read using Pass.ReadFile;
+see the "asmdecl" or "buildtags" analyzers for examples of loading
+non-Go files and reporting diagnostics against them.
 
 The ResultOf field provides the results computed by the analyzers
 required by this one, as expressed in its Analyzer.Requires field. The
@@ -177,7 +175,7 @@ Diagnostic is defined as:
 The optional Category field is a short identifier that classifies the
 kind of message when an analysis produces several kinds of diagnostic.
 
-The Diagnostic struct does not have a field to indicate its severity
+The [Diagnostic] struct does not have a field to indicate its severity
 because opinions about the relative importance of Analyzers and their
 diagnostics vary widely among users. The design of this framework does
 not hold each Analyzer responsible for identifying the severity of its
@@ -191,7 +189,7 @@ and buildtag, inspect the raw text of Go source files or even non-Go
 files such as assembly. To report a diagnostic against a line of a
 raw text file, use the following sequence:
 
-	content, err := os.ReadFile(filename)
+	content, err := pass.ReadFile(filename)
 	if err != nil { ... }
 	tf := fset.AddFile(filename, -1, len(content))
 	tf.SetLinesForContent(content)
@@ -216,7 +214,7 @@ addition, it records which functions are printf wrappers for use by
 later analysis passes to identify other printf wrappers by induction.
 A result such as “f is a printf wrapper” that is not interesting by
 itself but serves as a stepping stone to an interesting result (such as
-a diagnostic) is called a "fact".
+a diagnostic) is called a [Fact].
 
 The analysis API allows an analysis to define new types of facts, to
 associate facts of these types with objects (named entities) declared

go/analysis/internal/checker/checker.go

@@ -31,6 +31,7 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/internal/analysisflags"
 	"golang.org/x/tools/go/packages"
+	"golang.org/x/tools/internal/analysisinternal"
 	"golang.org/x/tools/internal/diff"
 	"golang.org/x/tools/internal/robustio"
 )
@@ -432,6 +433,8 @@ func applyFixes(roots []*action) error {
 
 	// Now we've got a set of valid edits for each file. Apply them.
 	for path, edits := range editsByPath {
+		// TODO(adonovan): this should really work on the same
+		// gulp from the file system that fed the analyzer (see #62292).
 		contents, err := os.ReadFile(path)
 		if err != nil {
 			return err
@@ -766,6 +769,7 @@ func (act *action) execOnce() {
 		AllObjectFacts:    act.allObjectFacts,
 		AllPackageFacts:   act.allPackageFacts,
 	}
+	pass.ReadFile = analysisinternal.MakeReadFile(pass)
 	act.pass = pass
 
 	var err error

go/analysis/internal/checker/checker_test.go

@@ -10,6 +10,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"testing"
 
 	"golang.org/x/tools/go/analysis"
@@ -18,6 +19,8 @@ import (
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/internal/testenv"
+	"golang.org/x/tools/internal/testfiles"
+	"golang.org/x/tools/txtar"
 )
 
 func TestApplyFixes(t *testing.T) {
@@ -254,3 +257,90 @@ func TestURL(t *testing.T) {
 		t.Errorf("Expected Diagnostics.URLs %v. got %v", want, urls)
 	}
 }
+
+// TestPassReadFile exercises the Pass.ReadFile function.
+func TestPassReadFile(t *testing.T) {
+	cwd, _ := os.Getwd()
+
+	const src = `
+-- go.mod --
+module example.com
+
+-- p/file.go --
+package p
+
+-- p/ignored.go --
+//go:build darwin && mips64
+
+package p
+
+hello from ignored
+
+-- p/other.s --
+hello from other
+`
+
+	// Expand archive into tmp tree.
+	tmpdir := t.TempDir()
+	if err := testfiles.ExtractTxtar(tmpdir, txtar.Parse([]byte(src))); err != nil {
+		t.Fatal(err)
+	}
+
+	ran := false
+	a := &analysis.Analyzer{
+		Name:     "a",
+		Requires: []*analysis.Analyzer{inspect.Analyzer},
+		Doc:      "doc",
+		Run: func(pass *analysis.Pass) (any, error) {
+			if len(pass.OtherFiles)+len(pass.IgnoredFiles) == 0 {
+				t.Errorf("OtherFiles and IgnoredFiles are empty")
+				return nil, nil
+			}
+
+			for _, test := range []struct {
+				filename string
+				want     string // substring of file content or error message
+			}{
+				{
+					pass.OtherFiles[0], // [other.s]
+					"hello from other",
+				},
+				{
+					pass.IgnoredFiles[0], // [ignored.go]
+					"hello from ignored",
+				},
+				{
+					"nonesuch",
+					"nonesuch is not among OtherFiles, ", // etc
+				},
+				{
+					filepath.Join(cwd, "checker_test.go"),
+					"checker_test.go is not among OtherFiles, ", // etc
+				},
+			} {
+				content, err := pass.ReadFile(test.filename)
+				var got string
+				if err != nil {
+					got = err.Error()
+				} else {
+					got = string(content)
+					if len(got) > 100 {
+						got = got[:100] + "..."
+					}
+				}
+				if !strings.Contains(got, test.want) {
+					t.Errorf("Pass.ReadFile(%q) did not contain %q; got:\n%s",
+						test.filename, test.want, got)
+				}
+			}
+			ran = true
+			return nil, nil
+		},
+	}
+
+	analysistest.Run(t, tmpdir, a, "example.com/p")
+
+	if !ran {
+		t.Error("analyzer did not run")
+	}
+}

go/analysis/passes/asmdecl/asmdecl.go

@@ -173,7 +173,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 
 Files:
 	for _, fname := range sfiles {
-		content, tf, err := analysisutil.ReadFile(pass.Fset, fname)
+		content, tf, err := analysisutil.ReadFile(pass, fname)
 		if err != nil {
 			return nil, err
 		}

go/analysis/passes/buildtag/buildtag.go

@@ -89,7 +89,7 @@ func checkOtherFile(pass *analysis.Pass, filename string) error {
 
 	// We cannot use the Go parser, since this may not be a Go source file.
 	// Read the raw bytes instead.
-	content, tf, err := analysisutil.ReadFile(pass.Fset, filename)
+	content, tf, err := analysisutil.ReadFile(pass, filename)
 	if err != nil {
 		return err
 	}

go/analysis/passes/buildtag/buildtag_old.go

@@ -83,7 +83,7 @@ func checkGoFile(pass *analysis.Pass, f *ast.File) {
 }
 
 func checkOtherFile(pass *analysis.Pass, filename string) error {
-	content, tf, err := analysisutil.ReadFile(pass.Fset, filename)
+	content, tf, err := analysisutil.ReadFile(pass, filename)
 	if err != nil {
 		return err
 	}

go/analysis/passes/directive/directive.go

@@ -90,7 +90,7 @@ func checkGoFile(pass *analysis.Pass, f *ast.File) {
 func checkOtherFile(pass *analysis.Pass, filename string) error {
 	// We cannot use the Go parser, since is not a Go source file.
 	// Read the raw bytes instead.
-	content, tf, err := analysisutil.ReadFile(pass.Fset, filename)
+	content, tf, err := analysisutil.ReadFile(pass, filename)
 	if err != nil {
 		return err
 	}

go/analysis/passes/framepointer/framepointer.go

@@ -48,7 +48,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	}
 
 	for _, fname := range sfiles {
-		content, tf, err := analysisutil.ReadFile(pass.Fset, fname)
+		content, tf, err := analysisutil.ReadFile(pass, fname)
 		if err != nil {
 			return nil, err
 		}

go/analysis/passes/internal/analysisutil/util.go

@@ -14,6 +14,7 @@ import (
 	"go/types"
 	"os"
 
+	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/analysisinternal"
 )
@@ -60,12 +61,16 @@ func HasSideEffects(info *types.Info, e ast.Expr) bool {
 
 // ReadFile reads a file and adds it to the FileSet
 // so that we can report errors against it using lineStart.
-func ReadFile(fset *token.FileSet, filename string) ([]byte, *token.File, error) {
-	content, err := os.ReadFile(filename)
+func ReadFile(pass *analysis.Pass, filename string) ([]byte, *token.File, error) {
+	readFile := pass.ReadFile
+	if readFile == nil {
+		readFile = os.ReadFile
+	}
+	content, err := readFile(filename)
 	if err != nil {
 		return nil, nil, err
 	}
-	tf := fset.AddFile(filename, -1, len(content))
+	tf := pass.Fset.AddFile(filename, -1, len(content))
 	tf.SetLinesForContent(content)
 	return content, tf, nil
 }

go/analysis/unitchecker/unitchecker.go

@@ -49,6 +49,7 @@ import (
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/internal/analysisflags"
+	"golang.org/x/tools/internal/analysisinternal"
 	"golang.org/x/tools/internal/facts"
 	"golang.org/x/tools/internal/versions"
 )
@@ -377,6 +378,7 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re
 				ExportPackageFact: facts.ExportPackageFact,
 				AllPackageFacts:   func() []analysis.PackageFact { return facts.AllPackageFacts(factFilter) },
 			}
+			pass.ReadFile = analysisinternal.MakeReadFile(pass)
 
 			t0 := time.Now()
 			act.result, act.err = a.Run(pass)

gopls/internal/cache/analysis.go

@@ -44,6 +44,7 @@ import (
 	"golang.org/x/tools/gopls/internal/util/bug"
 	"golang.org/x/tools/gopls/internal/util/frob"
 	"golang.org/x/tools/gopls/internal/util/maps"
+	"golang.org/x/tools/internal/analysisinternal"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/facts"
 	"golang.org/x/tools/internal/gcimporter"
@@ -1363,6 +1364,9 @@ func (act *action) exec() (interface{}, *actionSummary, error) {
 		AllObjectFacts:    func() []analysis.ObjectFact { return factset.AllObjectFacts(factFilter) },
 		AllPackageFacts:   func() []analysis.PackageFact { return factset.AllPackageFacts(factFilter) },
 	}
+	// TODO(adonovan): integrate this into the snapshot's file
+	// cache and its dependency analysis.
+	pass.ReadFile = analysisinternal.MakeReadFile(pass)
 
 	// Recover from panics (only) within the analyzer logic.
 	// (Use an anonymous function to limit the recover scope.)

internal/analysisinternal/analysis.go

@@ -12,8 +12,10 @@ import (
 	"go/ast"
 	"go/token"
 	"go/types"
+	"os"
 	"strconv"
 
+	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/internal/aliases"
 )
 
@@ -393,3 +395,38 @@ func equivalentTypes(want, got types.Type) bool {
 	}
 	return types.AssignableTo(want, got)
 }
+
+// MakeReadFile returns a simple implementation of the Pass.ReadFile function.
+func MakeReadFile(pass *analysis.Pass) func(filename string) ([]byte, error) {
+	return func(filename string) ([]byte, error) {
+		if err := checkReadable(pass, filename); err != nil {
+			return nil, err
+		}
+		return os.ReadFile(filename)
+	}
+}
+
+// checkReadable enforces the access policy defined by the ReadFile field of [analysis.Pass].
+func checkReadable(pass *analysis.Pass, filename string) error {
+	if slicesContains(pass.OtherFiles, filename) ||
+		slicesContains(pass.IgnoredFiles, filename) {
+		return nil
+	}
+	for _, f := range pass.Files {
+		// TODO(adonovan): use go1.20 f.FileStart
+		if pass.Fset.File(f.Pos()).Name() == filename {
+			return nil
+		}
+	}
+	return fmt.Errorf("Pass.ReadFile: %s is not among OtherFiles, IgnoredFiles, or names of Files", filename)
+}
+
+// TODO(adonovan): use go1.21 slices.Contains.
+func slicesContains[S ~[]E, E comparable](slice S, x E) bool {
+	for _, elem := range slice {
+		if elem == x {
+			return true
+		}
+	}
+	return false
+}