From ac0b72cd04f02de836b080586780fae77ef095f5 Mon Sep 17 00:00:00 2001 From: Travis Thieman Date: Thu, 26 Mar 2026 15:44:23 -0400 Subject: [PATCH 1/5] feat(allowedsymbols): add go/analysis.Analyzer and refactor checker internals Replace the duplicated AST-walking logic in symbols_test.go with shared helper functions in a new analyzer.go file that also exposes a proper go/analysis.Analyzer (NewAnalyzer) for go vet integration. - Add allowedsymbols/analyzer.go with NewAnalyzer, buildAllowlistSets, checkFileImports, checkFileSelectors, reportUnused, and fileLineReporter helpers - Rewrite checkAllowedSymbols in symbols_test.go to use those helpers instead of its own inline AST walking; all existing test signatures and behaviors are preserved - Add golang.org/x/tools v0.43.0 as a direct dependency All 35 existing tests continue to pass. Co-Authored-By: Claude Sonnet 4.6 --- allowedsymbols/analyzer.go | 223 +++++++++++++++++++++++++++++++++ allowedsymbols/symbols_test.go | 98 +++------------ go.mod | 5 +- go.sum | 10 +- 4 files changed, 248 insertions(+), 88 deletions(-) create mode 100644 allowedsymbols/analyzer.go diff --git a/allowedsymbols/analyzer.go b/allowedsymbols/analyzer.go new file mode 100644 index 00000000..bd5ccb05 --- /dev/null +++ b/allowedsymbols/analyzer.go @@ -0,0 +1,223 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +// Package allowedsymbols provides a go/analysis.Analyzer that enforces +// symbol-level import restrictions on Go source files. +// +// The analyzer checks that every imported symbol is in a given allowlist, that +// no permanently banned packages are imported, and that every symbol in the +// allowlist is actually used. It reports violations with file:line:col +// diagnostics and integrates with go vet. +package allowedsymbols + +import ( + "fmt" + "go/ast" + "go/token" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" +) + +// AnalyzerConfig configures a single instance of the allowed-symbols analyzer. +type AnalyzerConfig struct { + // Symbols is the allowlist to enforce (e.g. builtinAllowedSymbols). + // Each entry must be in "importpath.Symbol" form. + Symbols []string + // ExemptImport returns true for import paths that are auto-allowed and + // should not be checked against the allowlist (e.g. same-module imports). + ExemptImport func(importPath string) bool + // ListName is used in diagnostic messages (e.g. "builtinAllowedSymbols"). + ListName string +} + +// NewAnalyzer returns a go/analysis.Analyzer that enforces the symbol-level +// import restrictions described by cfg. The analyzer uses the inspect pass for +// efficient AST traversal and reports violations via pass.Reportf so they +// appear as go vet diagnostics with proper file:line:col positions. +func NewAnalyzer(cfg AnalyzerConfig) *analysis.Analyzer { + run := func(pass *analysis.Pass) (interface{}, error) { + allowedSyms, allowedPkgs := buildAllowlistSets(cfg.Symbols) + usedSymbols := make(map[string]bool, len(cfg.Symbols)) + + for _, f := range pass.Files { + localToPath := checkFileImports(f, allowedPkgs, cfg.ExemptImport, + func(pos token.Pos, format string, args ...any) { + pass.Reportf(pos, format, args...) + }) + + checkFileSelectors(f, localToPath, allowedSyms, usedSymbols, + func(pos token.Pos, format string, args ...any) { + pass.Reportf(pos, format, args...) + }) + } + + if len(pass.Files) > 0 { + reportUnused(cfg.Symbols, usedSymbols, cfg.ListName, + func(entry string) { + pass.Reportf(pass.Files[0].Package, + "allowlist symbol %q is not used by any file — remove it from %s", + entry, cfg.ListName) + }) + } + + return nil, nil + } + + return &analysis.Analyzer{ + Name: "allowedsymbols", + Doc: "enforces symbol-level import restrictions via an allowlist", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, + } +} + +// buildAllowlistSets constructs lookup maps from a flat allowlist. +// It returns (allowedSymbols, allowedPackages) where allowedSymbols maps +// "pkg.Symbol" → true and allowedPackages maps "importpath" → true. +func buildAllowlistSets(symbols []string) (map[string]bool, map[string]bool) { + allowedSyms := make(map[string]bool, len(symbols)) + allowedPkgs := make(map[string]bool) + for _, entry := range symbols { + dot := strings.LastIndexByte(entry, '.') + if dot <= 0 { + continue + } + allowedSyms[entry] = true + allowedPkgs[entry[:dot]] = true + } + return allowedSyms, allowedPkgs +} + +// checkFileImports validates each import in f against the permanently banned +// list, the exempt predicate, and allowedPkgs. It calls report for each +// violation and returns a localName→importPath map for the file's valid, +// non-exempt imports. +// +// report is called with a token.Pos and a pre-formatted message (the pos +// argument is valid only when fset is non-nil; callers using the token.Pos +// form must pass the actual position). For callers that use the file:line +// string form (symbols_test.go), token.Pos is set to token.NoPos and the +// message already encodes position info. +func checkFileImports( + f *ast.File, + allowedPkgs map[string]bool, + exemptImport func(string) bool, + report func(pos token.Pos, format string, args ...any), +) map[string]string { + localToPath := make(map[string]string) + + for _, imp := range f.Imports { + importPath := strings.Trim(imp.Path.Value, `"`) + + banned := false + for key, reason := range permanentlyBanned { + if strings.HasSuffix(key, "/") { + if strings.HasPrefix(importPath, key) { + report(imp.Pos(), "import of %q is permanently banned (%s)", importPath, reason) + banned = true + break + } + } else if importPath == key { + report(imp.Pos(), "import of %q is permanently banned (%s)", importPath, reason) + banned = true + break + } + } + if banned { + continue + } + + if exemptImport != nil && exemptImport(importPath) { + continue + } + + var localName string + if imp.Name != nil { + localName = imp.Name.Name + } else { + parts := strings.Split(importPath, "/") + localName = parts[len(parts)-1] + } + + if localName == "_" || localName == "." { + report(imp.Pos(), "blank/dot import of %q is not allowed", importPath) + continue + } + + if !allowedPkgs[importPath] { + report(imp.Pos(), "import of %q is not in the allowlist", importPath) + continue + } + + localToPath[localName] = importPath + } + + return localToPath +} + +// checkFileSelectors walks all ast.SelectorExpr nodes in f and checks each +// package-qualified symbol against allowedSyms. Allowed symbols are recorded +// in usedSymbols. Violations are sent to report. +func checkFileSelectors( + f *ast.File, + localToPath map[string]string, + allowedSyms map[string]bool, + usedSymbols map[string]bool, + report func(pos token.Pos, format string, args ...any), +) { + ast.Inspect(f, func(n ast.Node) bool { + sel, ok := n.(*ast.SelectorExpr) + if !ok { + return true + } + ident, ok := sel.X.(*ast.Ident) + if !ok { + return true + } + importPath, ok := localToPath[ident.Name] + if !ok { + return true // not a package-level selector + } + key := importPath + "." + sel.Sel.Name + if !allowedSyms[key] { + report(sel.Pos(), "%s is not in the allowlist", key) + } else { + usedSymbols[key] = true + } + return true + }) +} + +// reportUnused calls onUnused for each symbol in symbols that is not present +// in usedSymbols. +func reportUnused(symbols []string, usedSymbols map[string]bool, listName string, onUnused func(entry string)) { + for _, entry := range symbols { + if !usedSymbols[entry] { + onUnused(entry) + } + } +} + +// fileLineReporter returns a report function suitable for use with +// checkFileImports and checkFileSelectors when doing file-level AST analysis +// (without type information). It translates token.Pos into file:line strings +// using fset and collects messages via errorf. +// +// The returned function ignores the pos argument and produces messages already +// prefixed with relPath:line via fset. When pos is token.NoPos, only the +// format+args message is emitted (no location prefix). +func fileLineReporter(fset *token.FileSet, relPath string, errorf func(string, ...any)) func(token.Pos, string, ...any) { + return func(pos token.Pos, format string, args ...any) { + msg := fmt.Sprintf(format, args...) + if pos != token.NoPos && fset != nil { + p := fset.Position(pos) + errorf("%s:%d: %s", relPath, p.Line, msg) + } else { + errorf("%s: %s", relPath, msg) + } + } +} diff --git a/allowedsymbols/symbols_test.go b/allowedsymbols/symbols_test.go index 550611b1..1c089f50 100644 --- a/allowedsymbols/symbols_test.go +++ b/allowedsymbols/symbols_test.go @@ -7,7 +7,6 @@ package allowedsymbols import ( "fmt" - "go/ast" "go/parser" "go/token" "os" @@ -45,20 +44,23 @@ type allowedSymbolsConfig struct { // Go source files. It verifies that every imported symbol is in the allowlist, // that no permanently banned packages are imported, and that every symbol in // the allowlist is actually used. +// +// The core checking logic is provided by the analyzer helpers in analyzer.go +// (checkFileImports, checkFileSelectors, reportUnused) — this function handles +// file discovery, AST parsing, and test-framework integration. func checkAllowedSymbols(t *testing.T, cfg allowedSymbolsConfig) { t.Helper() // Build lookup sets from the allowlist. - allowedSymbols := make(map[string]bool, len(cfg.Symbols)) + allowedSyms, allowedPkgs := buildAllowlistSets(cfg.Symbols) usedSymbols := make(map[string]bool, len(cfg.Symbols)) - allowedPackages := make(map[string]bool) + + // Validate allowlist entries are well-formed. for _, entry := range cfg.Symbols { dot := strings.LastIndexByte(entry, '.') if dot <= 0 { t.Fatalf("malformed allowlist entry (no dot): %q", entry) } - allowedSymbols[entry] = true - allowedPackages[entry[:dot]] = true } // reportErr collects errors into cfg.Errors when set, otherwise calls t.Errorf. @@ -102,90 +104,22 @@ func checkAllowedSymbols(t *testing.T, cfg allowedSymbolsConfig) { continue } - // Build a map from local package name → import path and validate each import. - localToPath := make(map[string]string) - for _, imp := range f.Imports { - importPath := strings.Trim(imp.Path.Value, `"`) - - banned := false - for key, reason := range permanentlyBanned { - if strings.HasSuffix(key, "/") { - if strings.HasPrefix(importPath, key) { - reportErr("%s: import of %q is permanently banned (%s)", rel, importPath, reason) - banned = true - break - } - } else if importPath == key { - reportErr("%s: import of %q is permanently banned (%s)", rel, importPath, reason) - banned = true - break - } - } - if banned { - continue - } - - if cfg.ExemptImport(importPath) { - continue - } - - // Determine the local name used to reference this package. - var localName string - if imp.Name != nil { - localName = imp.Name.Name - } else { - parts := strings.Split(importPath, "/") - localName = parts[len(parts)-1] - } - - if localName == "_" || localName == "." { - reportErr("%s: blank/dot import of %q is not allowed", rel, importPath) - continue - } - - if !allowedPackages[importPath] { - reportErr("%s: import of %q is not in the allowlist", rel, importPath) - continue - } - - localToPath[localName] = importPath - } - - // Walk all selector expressions and verify each pkg.Symbol is allowed. - ast.Inspect(f, func(n ast.Node) bool { - sel, ok := n.(*ast.SelectorExpr) - if !ok { - return true - } - ident, ok := sel.X.(*ast.Ident) - if !ok { - return true - } - importPath, ok := localToPath[ident.Name] - if !ok { - return true // not a package-level selector - } - key := importPath + "." + sel.Sel.Name - if !allowedSymbols[key] { - pos := fset.Position(sel.Pos()) - reportErr("%s:%d: %s is not in the allowlist", rel, pos.Line, key) - } else { - usedSymbols[key] = true - } - return true - }) + // Use analyzer helpers for import checking and selector walking. + reporter := fileLineReporter(fset, rel, reportErr) + localToPath := checkFileImports(f, allowedPkgs, cfg.ExemptImport, reporter) + checkFileSelectors(f, localToPath, allowedSyms, usedSymbols, reporter) } + if checked < cfg.MinFiles { t.Fatalf("expected at least %d files in %s, found %d", cfg.MinFiles, cfg.TargetDir, checked) } // Verify every symbol in the allowlist is actually used by at least one // file. Unused entries should be removed to keep the allowlist minimal. - for _, entry := range cfg.Symbols { - if !usedSymbols[entry] { - reportErr("allowlist symbol %q is not used by any file in %s — remove it from %s", entry, cfg.TargetDir, cfg.ListName) - } - } + reportUnused(cfg.Symbols, usedSymbols, cfg.ListName, func(entry string) { + reportErr("allowlist symbol %q is not used by any file in %s — remove it from %s", + entry, cfg.TargetDir, cfg.ListName) + }) } // perBuiltinConfig holds the configuration for checkPerBuiltinAllowedSymbols. diff --git a/go.mod b/go.mod index 42a88c05..8209300d 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 golang.org/x/sys v0.42.0 + golang.org/x/tools v0.43.0 gopkg.in/yaml.v3 v3.0.1 mvdan.cc/sh/v3 v3.13.0 ) @@ -17,6 +18,6 @@ require ( github.com/google/uuid v1.6.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/net v0.49.0 // indirect - golang.org/x/sync v0.19.0 // indirect + golang.org/x/net v0.52.0 // indirect + golang.org/x/sync v0.20.0 // indirect ) diff --git a/go.sum b/go.sum index 930144f8..9793ac39 100644 --- a/go.sum +++ b/go.sum @@ -28,12 +28,14 @@ github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3A github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= -golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= -golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= -golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/net v0.52.0 h1:He/TN1l0e4mmR3QqHMT2Xab3Aj3L9qjbhRm78/6jrW0= +golang.org/x/net v0.52.0/go.mod h1:R1MAz7uMZxVMualyPXb+VaqGSa3LIaUqk0eEt3w36Sw= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/tools v0.43.0 h1:12BdW9CeB3Z+J/I/wj34VMl8X+fEXBxVR90JeMX5E7s= +golang.org/x/tools v0.43.0/go.mod h1:uHkMso649BX2cZK6+RpuIPXS3ho2hZo4FVwfoy1vIk0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From 35d8b771b393336c4ec9b11409ff0ed5c610c61a Mon Sep 17 00:00:00 2001 From: Travis Thieman Date: Thu, 26 Mar 2026 15:55:52 -0400 Subject: [PATCH 2/5] fix(allowedsymbols): resolve interface{}, unused param, and misplaced helper Three code-quality fixes to analyzer.go and symbols_test.go: - Replace interface{} with any in the NewAnalyzer run func signature - Remove unused listName parameter from reportUnused; callers already embed the list name in the closure message they pass in - Move fileLineReporter from analyzer.go (production code) into symbols_test.go where it is actually used; keeps production API clean All 35 tests continue to pass. Co-Authored-By: Claude Sonnet 4.6 --- allowedsymbols/analyzer.go | 36 +++++++--------------------------- allowedsymbols/symbols_test.go | 19 +++++++++++++++++- 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/allowedsymbols/analyzer.go b/allowedsymbols/analyzer.go index bd5ccb05..95e27abe 100644 --- a/allowedsymbols/analyzer.go +++ b/allowedsymbols/analyzer.go @@ -13,7 +13,6 @@ package allowedsymbols import ( - "fmt" "go/ast" "go/token" "strings" @@ -39,7 +38,7 @@ type AnalyzerConfig struct { // efficient AST traversal and reports violations via pass.Reportf so they // appear as go vet diagnostics with proper file:line:col positions. func NewAnalyzer(cfg AnalyzerConfig) *analysis.Analyzer { - run := func(pass *analysis.Pass) (interface{}, error) { + run := func(pass *analysis.Pass) (any, error) { allowedSyms, allowedPkgs := buildAllowlistSets(cfg.Symbols) usedSymbols := make(map[string]bool, len(cfg.Symbols)) @@ -56,12 +55,11 @@ func NewAnalyzer(cfg AnalyzerConfig) *analysis.Analyzer { } if len(pass.Files) > 0 { - reportUnused(cfg.Symbols, usedSymbols, cfg.ListName, - func(entry string) { - pass.Reportf(pass.Files[0].Package, - "allowlist symbol %q is not used by any file — remove it from %s", - entry, cfg.ListName) - }) + reportUnused(cfg.Symbols, usedSymbols, func(entry string) { + pass.Reportf(pass.Files[0].Package, + "allowlist symbol %q is not used by any file — remove it from %s", + entry, cfg.ListName) + }) } return nil, nil @@ -194,30 +192,10 @@ func checkFileSelectors( // reportUnused calls onUnused for each symbol in symbols that is not present // in usedSymbols. -func reportUnused(symbols []string, usedSymbols map[string]bool, listName string, onUnused func(entry string)) { +func reportUnused(symbols []string, usedSymbols map[string]bool, onUnused func(entry string)) { for _, entry := range symbols { if !usedSymbols[entry] { onUnused(entry) } } } - -// fileLineReporter returns a report function suitable for use with -// checkFileImports and checkFileSelectors when doing file-level AST analysis -// (without type information). It translates token.Pos into file:line strings -// using fset and collects messages via errorf. -// -// The returned function ignores the pos argument and produces messages already -// prefixed with relPath:line via fset. When pos is token.NoPos, only the -// format+args message is emitted (no location prefix). -func fileLineReporter(fset *token.FileSet, relPath string, errorf func(string, ...any)) func(token.Pos, string, ...any) { - return func(pos token.Pos, format string, args ...any) { - msg := fmt.Sprintf(format, args...) - if pos != token.NoPos && fset != nil { - p := fset.Position(pos) - errorf("%s:%d: %s", relPath, p.Line, msg) - } else { - errorf("%s: %s", relPath, msg) - } - } -} diff --git a/allowedsymbols/symbols_test.go b/allowedsymbols/symbols_test.go index 1c089f50..3a1c6342 100644 --- a/allowedsymbols/symbols_test.go +++ b/allowedsymbols/symbols_test.go @@ -116,7 +116,7 @@ func checkAllowedSymbols(t *testing.T, cfg allowedSymbolsConfig) { // Verify every symbol in the allowlist is actually used by at least one // file. Unused entries should be removed to keep the allowlist minimal. - reportUnused(cfg.Symbols, usedSymbols, cfg.ListName, func(entry string) { + reportUnused(cfg.Symbols, usedSymbols, func(entry string) { reportErr("allowlist symbol %q is not used by any file in %s — remove it from %s", entry, cfg.TargetDir, cfg.ListName) }) @@ -297,3 +297,20 @@ func collectFlatGoFiles(dir string) ([]string, error) { } return files, nil } + +// fileLineReporter returns a report function suitable for use with +// checkFileImports and checkFileSelectors in the test harness. It translates +// token.Pos into file:line strings using fset and forwards messages via errorf. +// When pos is token.NoPos, only the format+args message is emitted without a +// location prefix. +func fileLineReporter(fset *token.FileSet, relPath string, errorf func(string, ...any)) func(token.Pos, string, ...any) { + return func(pos token.Pos, format string, args ...any) { + msg := fmt.Sprintf(format, args...) + if pos != token.NoPos && fset != nil { + p := fset.Position(pos) + errorf("%s:%d: %s", relPath, p.Line, msg) + } else { + errorf("%s: %s", relPath, msg) + } + } +} From 4d97ff19fd0ab4cfb1c58ad3a8d49348e4f77e12 Mon Sep 17 00:00:00 2001 From: Travis Thieman Date: Fri, 27 Mar 2026 12:33:20 -0400 Subject: [PATCH 3/5] fix(compliance): add golang.org/x/tools to LICENSE-3rdparty.csv The new allowedsymbols package imports golang.org/x/tools, which was added to go.mod but missing from the third-party license file. Co-Authored-By: Claude Sonnet 4.6 --- LICENSE-3rdparty.csv | 1 + 1 file changed, 1 insertion(+) diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index d3485289..12804cb4 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -12,3 +12,4 @@ github.com/spf13/pflag,https://github.com/spf13/pflag,BSD-3-Clause,"Copyright (c golang.org/x/net,https://github.com/golang/net,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. golang.org/x/sync,https://github.com/golang/sync,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. golang.org/x/sys,https://github.com/golang/sys,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. +golang.org/x/tools,https://github.com/golang/tools,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. From a437e3061c182f84e83524a2698cf8cf49449143 Mon Sep 17 00:00:00 2001 From: Travis Thieman Date: Fri, 27 Mar 2026 12:47:04 -0400 Subject: [PATCH 4/5] fix(allowedsymbols): remove unused inspect dep, validate allowlist, add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues from code review: 1. inspect.Analyzer was declared in Requires but never used — the run function uses ast.Inspect directly. Remove the Requires entry and the inspect import to avoid unnecessary framework overhead. 2. Malformed allowlist entries (no dot separator) were silently dropped by buildAllowlistSets in the analyzer path, unlike the test harness which calls t.Fatalf. NewAnalyzer now panics on construction if any entry is malformed, matching the test-harness behaviour and surfacing programmer errors early. 3. NewAnalyzer had no tests. Add analysistest.Run-based tests covering: metadata (correct Name, empty Requires), malformed-entry panic, allowed imports (no diagnostics), banned imports, disallowed imports, and unused allowlist symbols. Co-Authored-By: Claude Sonnet 4.6 --- allowedsymbols/analyzer.go | 34 +++++--- allowedsymbols/analyzer_test.go | 86 +++++++++++++++++++ .../testdata/src/bannedimport/bannedimport.go | 13 +++ .../src/disallowedimport/disallowedimport.go | 13 +++ allowedsymbols/testdata/src/good/good.go | 9 ++ .../testdata/src/unusedsymbol/unusedsymbol.go | 9 ++ go.mod | 1 + go.sum | 2 + 8 files changed, 153 insertions(+), 14 deletions(-) create mode 100644 allowedsymbols/analyzer_test.go create mode 100644 allowedsymbols/testdata/src/bannedimport/bannedimport.go create mode 100644 allowedsymbols/testdata/src/disallowedimport/disallowedimport.go create mode 100644 allowedsymbols/testdata/src/good/good.go create mode 100644 allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go diff --git a/allowedsymbols/analyzer.go b/allowedsymbols/analyzer.go index 95e27abe..6afb25ea 100644 --- a/allowedsymbols/analyzer.go +++ b/allowedsymbols/analyzer.go @@ -9,16 +9,16 @@ // The analyzer checks that every imported symbol is in a given allowlist, that // no permanently banned packages are imported, and that every symbol in the // allowlist is actually used. It reports violations with file:line:col -// diagnostics and integrates with go vet. +// diagnostics. package allowedsymbols import ( + "fmt" "go/ast" "go/token" "strings" "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" ) // AnalyzerConfig configures a single instance of the allowed-symbols analyzer. @@ -34,10 +34,18 @@ type AnalyzerConfig struct { } // NewAnalyzer returns a go/analysis.Analyzer that enforces the symbol-level -// import restrictions described by cfg. The analyzer uses the inspect pass for -// efficient AST traversal and reports violations via pass.Reportf so they -// appear as go vet diagnostics with proper file:line:col positions. +// import restrictions described by cfg. Violations are reported via +// pass.Reportf and appear as diagnostics with proper file:line:col positions. +// +// NewAnalyzer panics if any entry in cfg.Symbols is malformed (no dot +// separator), matching the behaviour of the test-harness variant. func NewAnalyzer(cfg AnalyzerConfig) *analysis.Analyzer { + for _, entry := range cfg.Symbols { + if strings.LastIndexByte(entry, '.') <= 0 { + panic(fmt.Sprintf("allowedsymbols.NewAnalyzer: malformed allowlist entry (no dot): %q", entry)) + } + } + run := func(pass *analysis.Pass) (any, error) { allowedSyms, allowedPkgs := buildAllowlistSets(cfg.Symbols) usedSymbols := make(map[string]bool, len(cfg.Symbols)) @@ -66,10 +74,9 @@ func NewAnalyzer(cfg AnalyzerConfig) *analysis.Analyzer { } return &analysis.Analyzer{ - Name: "allowedsymbols", - Doc: "enforces symbol-level import restrictions via an allowlist", - Requires: []*analysis.Analyzer{inspect.Analyzer}, - Run: run, + Name: "allowedsymbols", + Doc: "enforces symbol-level import restrictions via an allowlist", + Run: run, } } @@ -95,11 +102,10 @@ func buildAllowlistSets(symbols []string) (map[string]bool, map[string]bool) { // violation and returns a localName→importPath map for the file's valid, // non-exempt imports. // -// report is called with a token.Pos and a pre-formatted message (the pos -// argument is valid only when fset is non-nil; callers using the token.Pos -// form must pass the actual position). For callers that use the file:line -// string form (symbols_test.go), token.Pos is set to token.NoPos and the -// message already encodes position info. +// report is called with a token.Pos and a pre-formatted message. Callers that +// use the file:line string form (e.g. fileLineReporter) set pos to +// token.NoPos; callers that surface diagnostics via pass.Reportf pass the +// actual source position. func checkFileImports( f *ast.File, allowedPkgs map[string]bool, diff --git a/allowedsymbols/analyzer_test.go b/allowedsymbols/analyzer_test.go new file mode 100644 index 00000000..e192822c --- /dev/null +++ b/allowedsymbols/analyzer_test.go @@ -0,0 +1,86 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package allowedsymbols + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +// TestNewAnalyzer_Metadata verifies that the returned Analyzer has the correct +// name and does not declare unnecessary framework dependencies. +func TestNewAnalyzer_Metadata(t *testing.T) { + a := NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"fmt.Println"}, + ListName: "test", + }) + if a.Name != "allowedsymbols" { + t.Errorf("Name = %q, want \"allowedsymbols\"", a.Name) + } + if len(a.Requires) != 0 { + t.Errorf("Requires has %d entries, want 0", len(a.Requires)) + } +} + +// TestNewAnalyzer_MalformedEntry verifies that NewAnalyzer panics immediately +// when given an allowlist entry with no dot separator, matching the test-harness +// behaviour (t.Fatalf on the same condition). +func TestNewAnalyzer_MalformedEntry(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("expected panic for malformed allowlist entry, got none") + } + }() + NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"nodot"}, + ListName: "test", + }) +} + +// TestNewAnalyzer_AllowedImport runs the analyzer against a package that uses +// only allowlisted symbols; no diagnostics should be emitted. +func TestNewAnalyzer_AllowedImport(t *testing.T) { + testdata := analysistest.TestData() + a := NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"fmt.Println"}, + ListName: "test", + }) + analysistest.Run(t, testdata, a, "good") +} + +// TestNewAnalyzer_BannedImport verifies that a permanently banned import is +// reported at the import site. +func TestNewAnalyzer_BannedImport(t *testing.T) { + testdata := analysistest.TestData() + a := NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"fmt.Println"}, + ListName: "test", + }) + analysistest.Run(t, testdata, a, "bannedimport") +} + +// TestNewAnalyzer_DisallowedImport verifies that an import not in the +// allowlist is reported at the import site. +func TestNewAnalyzer_DisallowedImport(t *testing.T) { + testdata := analysistest.TestData() + a := NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"fmt.Println"}, + ListName: "test", + }) + analysistest.Run(t, testdata, a, "disallowedimport") +} + +// TestNewAnalyzer_UnusedSymbol verifies that an allowlisted symbol that is +// never referenced in the package is reported at the package declaration. +func TestNewAnalyzer_UnusedSymbol(t *testing.T) { + testdata := analysistest.TestData() + a := NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"fmt.Println", "fmt.Sprintf"}, + ListName: "test", + }) + analysistest.Run(t, testdata, a, "unusedsymbol") +} diff --git a/allowedsymbols/testdata/src/bannedimport/bannedimport.go b/allowedsymbols/testdata/src/bannedimport/bannedimport.go new file mode 100644 index 00000000..5555b30f --- /dev/null +++ b/allowedsymbols/testdata/src/bannedimport/bannedimport.go @@ -0,0 +1,13 @@ +// Package bannedimport imports a permanently banned package. +package bannedimport + +import ( + "fmt" + "os/exec" // want `import of "os/exec" is permanently banned` +) + +// Run uses a banned import. +func Run() { + fmt.Println("run") + _ = exec.Command("ls") +} diff --git a/allowedsymbols/testdata/src/disallowedimport/disallowedimport.go b/allowedsymbols/testdata/src/disallowedimport/disallowedimport.go new file mode 100644 index 00000000..48f8ef48 --- /dev/null +++ b/allowedsymbols/testdata/src/disallowedimport/disallowedimport.go @@ -0,0 +1,13 @@ +// Package disallowedimport imports a package not in the allowlist. +package disallowedimport + +import ( + "bufio" // want `import of "bufio" is not in the allowlist` + "fmt" +) + +// Read uses an import that is not allowlisted. +func Read() { + fmt.Println("read") + _ = bufio.NewScanner(nil) +} diff --git a/allowedsymbols/testdata/src/good/good.go b/allowedsymbols/testdata/src/good/good.go new file mode 100644 index 00000000..a6244a15 --- /dev/null +++ b/allowedsymbols/testdata/src/good/good.go @@ -0,0 +1,9 @@ +// Package good uses only allowlisted symbols; no diagnostics expected. +package good + +import "fmt" + +// Hello prints a greeting. +func Hello() { + fmt.Println("hello") +} diff --git a/allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go b/allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go new file mode 100644 index 00000000..a8ff65eb --- /dev/null +++ b/allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go @@ -0,0 +1,9 @@ +// Package unusedsymbol has an allowlisted symbol that is never used. +package unusedsymbol // want `allowlist symbol "fmt.Sprintf" is not used` + +import "fmt" + +// Hello uses only fmt.Println; fmt.Sprintf is in the allowlist but unused. +func Hello() { + fmt.Println("hello") +} diff --git a/go.mod b/go.mod index 8209300d..7be42efe 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( github.com/google/uuid v1.6.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + golang.org/x/mod v0.34.0 // indirect golang.org/x/net v0.52.0 // indirect golang.org/x/sync v0.20.0 // indirect ) diff --git a/go.sum b/go.sum index 9793ac39..a8069899 100644 --- a/go.sum +++ b/go.sum @@ -28,6 +28,8 @@ github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3A github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= +golang.org/x/mod v0.34.0 h1:xIHgNUUnW6sYkcM5Jleh05DvLOtwc6RitGHbDk4akRI= +golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY= golang.org/x/net v0.52.0 h1:He/TN1l0e4mmR3QqHMT2Xab3Aj3L9qjbhRm78/6jrW0= golang.org/x/net v0.52.0/go.mod h1:R1MAz7uMZxVMualyPXb+VaqGSa3LIaUqk0eEt3w36Sw= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= From 406df81bcda7dcde3f8313e64b8bd2a525e6aecd Mon Sep 17 00:00:00 2001 From: Travis Thieman Date: Fri, 27 Mar 2026 12:58:51 -0400 Subject: [PATCH 5/5] fix(compliance): add license headers to testdata files, add golang.org/x/mod The four testdata Go files added for analysistest were missing the required Apache 2.0 license header. Also adds golang.org/x/mod (pulled in as an indirect dependency of analysistest) to LICENSE-3rdparty.csv. Co-Authored-By: Claude Sonnet 4.6 --- LICENSE-3rdparty.csv | 1 + allowedsymbols/testdata/src/bannedimport/bannedimport.go | 5 +++++ .../testdata/src/disallowedimport/disallowedimport.go | 5 +++++ allowedsymbols/testdata/src/good/good.go | 5 +++++ allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go | 5 +++++ 5 files changed, 21 insertions(+) diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index 12804cb4..36d206a7 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -12,4 +12,5 @@ github.com/spf13/pflag,https://github.com/spf13/pflag,BSD-3-Clause,"Copyright (c golang.org/x/net,https://github.com/golang/net,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. golang.org/x/sync,https://github.com/golang/sync,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. golang.org/x/sys,https://github.com/golang/sys,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. +golang.org/x/mod,https://github.com/golang/mod,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. golang.org/x/tools,https://github.com/golang/tools,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. diff --git a/allowedsymbols/testdata/src/bannedimport/bannedimport.go b/allowedsymbols/testdata/src/bannedimport/bannedimport.go index 5555b30f..d088b546 100644 --- a/allowedsymbols/testdata/src/bannedimport/bannedimport.go +++ b/allowedsymbols/testdata/src/bannedimport/bannedimport.go @@ -1,3 +1,8 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + // Package bannedimport imports a permanently banned package. package bannedimport diff --git a/allowedsymbols/testdata/src/disallowedimport/disallowedimport.go b/allowedsymbols/testdata/src/disallowedimport/disallowedimport.go index 48f8ef48..1e25f753 100644 --- a/allowedsymbols/testdata/src/disallowedimport/disallowedimport.go +++ b/allowedsymbols/testdata/src/disallowedimport/disallowedimport.go @@ -1,3 +1,8 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + // Package disallowedimport imports a package not in the allowlist. package disallowedimport diff --git a/allowedsymbols/testdata/src/good/good.go b/allowedsymbols/testdata/src/good/good.go index a6244a15..52e1ca46 100644 --- a/allowedsymbols/testdata/src/good/good.go +++ b/allowedsymbols/testdata/src/good/good.go @@ -1,3 +1,8 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + // Package good uses only allowlisted symbols; no diagnostics expected. package good diff --git a/allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go b/allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go index a8ff65eb..13bd6b3e 100644 --- a/allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go +++ b/allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go @@ -1,3 +1,8 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + // Package unusedsymbol has an allowlisted symbol that is never used. package unusedsymbol // want `allowlist symbol "fmt.Sprintf" is not used`