From d0a182d328c2ca96a505dcafe29f197526d897a4 Mon Sep 17 00:00:00 2001 From: Julie Qiu Date: Tue, 1 Sep 2020 22:40:52 -0400 Subject: internal: remove Unit.Package Unit.Pacakge is removed, since it isn't need for anything. For golang/go#39629 Change-Id: Ic620603550874de8a0b4c37c67a88bee65ec7933 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/252323 Reviewed-by: Jonathan Amsterdam --- internal/fetch/directory.go | 4 -- internal/fetch/fetchdata_test.go | 88 +++++++++------------------------ internal/fetch/test_helper.go | 47 +++++++++--------- internal/postgres/insert_module.go | 12 ++--- internal/postgres/insert_module_test.go | 3 -- internal/postgres/path_test.go | 5 +- internal/postgres/unit.go | 8 --- internal/postgres/unit_test.go | 3 +- internal/testing/sample/sample.go | 5 -- internal/unit.go | 1 - 10 files changed, 50 insertions(+), 126 deletions(-) (limited to 'internal') diff --git a/internal/fetch/directory.go b/internal/fetch/directory.go index 9d958e17..b1a8003a 100644 --- a/internal/fetch/directory.go +++ b/internal/fetch/directory.go @@ -60,10 +60,6 @@ func moduleUnits(modulePath, version string, } if pkg, ok := pkgLookup[dirPath]; ok { dir.Name = pkg.Name - dir.Package = &internal.Package{ - Path: pkg.Path, - Name: pkg.Name, - } dir.Imports = pkg.Imports dir.Documentation = &internal.Documentation{ GOOS: pkg.GOOS, diff --git a/internal/fetch/fetchdata_test.go b/internal/fetch/fetchdata_test.go index 5df28942..c3ee36bd 100644 --- a/internal/fetch/fetchdata_test.go +++ b/internal/fetch/fetchdata_test.go @@ -53,14 +53,12 @@ var moduleOnePackage = &testModule{ }, { UnitMeta: internal.UnitMeta{ + Name: "foo", Path: "github.com/basic/foo", }, Documentation: &internal.Documentation{ Synopsis: "package foo exports a helpful constant.", }, - Package: &internal.Package{ - Name: "foo", - }, Imports: []string{"net/http"}, }, }, @@ -127,15 +125,13 @@ var moduleMultiPackage = &testModule{ }, { UnitMeta: internal.UnitMeta{ + Name: "bar", Path: "github.com/my/module/bar", }, Readme: &internal.Readme{ Filepath: "bar/README.md", Contents: "Another README FILE FOR TESTING.", }, - Package: &internal.Package{ - Name: "bar", - }, Documentation: &internal.Documentation{ Synopsis: "package bar", HTML: html("Bar returns the string "bar"."), @@ -143,10 +139,8 @@ var moduleMultiPackage = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "github.com/my/module/foo", - }, - Package: &internal.Package{ Name: "foo", + Path: "github.com/my/module/foo", }, Documentation: &internal.Documentation{ Synopsis: "package foo", @@ -189,10 +183,8 @@ var moduleNoGoMod = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "no.mod/module/p", - }, - Package: &internal.Package{ Name: "p", + Path: "no.mod/module/p", }, Documentation: &internal.Documentation{ Synopsis: "Package p is inside a module where a go.mod file hasn't been explicitly added yet.", @@ -254,10 +246,8 @@ var moduleBadPackages = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "bad.mod/module/good", - }, - Package: &internal.Package{ Name: "good", + Path: "bad.mod/module/good", }, Documentation: &internal.Documentation{ Synopsis: "Package good is inside a module that has bad packages.", @@ -321,10 +311,8 @@ var moduleBuildConstraints = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "build.constraints/module/cpu", - }, - Package: &internal.Package{ Name: "cpu", + Path: "build.constraints/module/cpu", }, Documentation: &internal.Documentation{ Synopsis: "Package cpu implements processor feature detection used by the Go standard library.", @@ -416,10 +404,8 @@ var moduleNonRedist = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "nonredistributable.mod/module/bar", - }, - Package: &internal.Package{ Name: "bar", + Path: "nonredistributable.mod/module/bar", }, Documentation: &internal.Documentation{ Synopsis: "package bar", @@ -428,10 +414,8 @@ var moduleNonRedist = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "nonredistributable.mod/module/bar/baz", - }, - Package: &internal.Package{ Name: "baz", + Path: "nonredistributable.mod/module/bar/baz", }, Documentation: &internal.Documentation{ Synopsis: "package baz", @@ -440,15 +424,13 @@ var moduleNonRedist = &testModule{ }, { UnitMeta: internal.UnitMeta{ + Name: "foo", Path: "nonredistributable.mod/module/foo", }, Readme: &internal.Readme{ Filepath: "foo/README.md", Contents: "README FILE SHOW UP HERE BUT WILL BE REMOVED BEFORE DB INSERT", }, - Package: &internal.Package{ - Name: "foo", - }, Documentation: &internal.Documentation{ Synopsis: "package foo", HTML: html("FooBar returns the string"), @@ -494,10 +476,8 @@ var moduleBadImportPath = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "bad.import.path.com/good/import/path", - }, - Package: &internal.Package{ Name: "foo", + Path: "bad.import.path.com/good/import/path", }, Documentation: &internal.Documentation{}, }, @@ -556,10 +536,8 @@ var moduleDocTest = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "doc.test/permalink", - }, - Package: &internal.Package{ Name: "permalink", + Path: "doc.test/permalink", }, Documentation: &internal.Documentation{ Synopsis: "Package permalink is for testing the heading permalink documentation rendering feature.", @@ -594,10 +572,8 @@ var moduleDocTooLarge = &testModule{ Units: []*internal.Unit{ { UnitMeta: internal.UnitMeta{ - Path: "bigdoc.test", - }, - Package: &internal.Package{ Name: "bigdoc", + Path: "bigdoc.test", }, Documentation: &internal.Documentation{ Synopsis: "This documentation is big.", @@ -654,10 +630,8 @@ var moduleWasm = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "github.com/my/module/js/js", - }, - Package: &internal.Package{ Name: "js", + Path: "github.com/my/module/js/js", }, Documentation: &internal.Documentation{ Synopsis: "Package js only works with wasm.", @@ -712,10 +686,8 @@ var moduleStd = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "builtin", - }, - Package: &internal.Package{ Name: "builtin", + Path: "builtin", }, Documentation: &internal.Documentation{ Synopsis: "Package builtin provides documentation for Go's predeclared identifiers.", @@ -728,15 +700,13 @@ var moduleStd = &testModule{ }, { UnitMeta: internal.UnitMeta{ + Name: "main", Path: "cmd/pprof", }, Readme: &internal.Readme{ Filepath: "cmd/pprof/README", Contents: "This directory is the copy of Google's pprof shipped as part of the Go distribution.\n", }, - Package: &internal.Package{ - Name: "main", - }, Documentation: &internal.Documentation{ Synopsis: "Pprof interprets and displays profiles of Go programs.", }, @@ -762,10 +732,8 @@ var moduleStd = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "context", - }, - Package: &internal.Package{ Name: "context", + Path: "context", }, Documentation: &internal.Documentation{ Synopsis: "Package context defines the Context type, which carries deadlines, cancelation signals, and other request-scoped values across API boundaries and between processes.", @@ -779,10 +747,8 @@ var moduleStd = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "encoding/json", - }, - Package: &internal.Package{ Name: "json", + Path: "encoding/json", }, Documentation: &internal.Documentation{ Synopsis: "Package json implements encoding and decoding of JSON as defined in RFC 7159.", @@ -807,10 +773,8 @@ var moduleStd = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "errors", - }, - Package: &internal.Package{ Name: "errors", + Path: "errors", }, Documentation: &internal.Documentation{ Synopsis: "Package errors implements functions to manipulate errors.", @@ -818,12 +782,10 @@ var moduleStd = &testModule{ }, { UnitMeta: internal.UnitMeta{ + Name: "flag", Path: "flag", }, Imports: []string{"errors", "fmt", "io", "os", "reflect", "sort", "strconv", "strings", "time"}, - Package: &internal.Package{ - Name: "flag", - }, Documentation: &internal.Documentation{ Synopsis: "Package flag implements command-line flag parsing.", }, @@ -859,10 +821,8 @@ var moduleMaster = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "github.com/my/module/foo", - }, - Package: &internal.Package{ Name: "foo", + Path: "github.com/my/module/foo", }, Documentation: &internal.Documentation{ Synopsis: "package foo exports a helpful constant.", @@ -899,10 +859,8 @@ var moduleLatest = &testModule{ }, { UnitMeta: internal.UnitMeta{ - Path: "github.com/my/module/foo", - }, - Package: &internal.Package{ Name: "foo", + Path: "github.com/my/module/foo", }, Documentation: &internal.Documentation{ Synopsis: "package foo exports a helpful constant.", @@ -953,10 +911,8 @@ package example_test }, { UnitMeta: internal.UnitMeta{ - Path: path + "/example", - }, - Package: &internal.Package{ Name: "example", + Path: path + "/example", }, Documentation: &internal.Documentation{ Synopsis: "Package example contains examples.", diff --git a/internal/fetch/test_helper.go b/internal/fetch/test_helper.go index 76727df0..dcd0f964 100644 --- a/internal/fetch/test_helper.go +++ b/internal/fetch/test_helper.go @@ -57,39 +57,38 @@ func cleanFetchResult(fr *FetchResult, detector *licenses.Detector) *FetchResult } shouldSetPVS := (fr.PackageVersionStates == nil) - for _, dir := range fr.Module.Units { - dir.UnitMeta = internal.UnitMeta{ + for _, u := range fr.Module.Units { + u.UnitMeta = internal.UnitMeta{ ModulePath: fr.Module.ModulePath, Version: fr.Module.Version, - Path: dir.Path, - IsRedistributable: dir.IsRedistributable, - Licenses: dir.Licenses, + Path: u.Path, + Name: u.Name, + IsRedistributable: u.IsRedistributable, + Licenses: u.Licenses, } - if dir.Documentation != nil { - if dir.Documentation.GOOS == "" { - dir.Documentation.GOOS = "linux" - dir.Documentation.GOARCH = "amd64" + if u.Documentation != nil { + if u.Documentation.GOOS == "" { + u.Documentation.GOOS = "linux" + u.Documentation.GOARCH = "amd64" } } - if dir.Package != nil { - dir.Name = dir.Package.Name - dir.Package.Path = dir.Path + if u.Name != "" { fr.Module.LegacyPackages = append(fr.Module.LegacyPackages, &internal.LegacyPackage{ - Path: dir.Path, - Licenses: dir.Licenses, - V1Path: internal.V1Path(dir.Path, dir.ModulePath), - Name: dir.Package.Name, - Synopsis: dir.Documentation.Synopsis, - DocumentationHTML: dir.Documentation.HTML, - Imports: dir.Imports, - GOOS: dir.Documentation.GOOS, - GOARCH: dir.Documentation.GOARCH, - IsRedistributable: dir.IsRedistributable, + Path: u.Path, + Licenses: u.Licenses, + V1Path: internal.V1Path(u.Path, u.ModulePath), + Name: u.Name, + Synopsis: u.Documentation.Synopsis, + DocumentationHTML: u.Documentation.HTML, + Imports: u.Imports, + GOOS: u.Documentation.GOOS, + GOARCH: u.Documentation.GOARCH, + IsRedistributable: u.IsRedistributable, }) if shouldSetPVS { fr.PackageVersionStates = append( fr.PackageVersionStates, &internal.PackageVersionState{ - PackagePath: dir.Path, + PackagePath: u.Path, ModulePath: fr.Module.ModulePath, Version: fr.Module.Version, Status: http.StatusOK, @@ -176,7 +175,7 @@ func validateDocumentationHTML(t *testing.T, got, want *internal.Module) { checkHTML("LegacyPackages", i, got.LegacyPackages[i].DocumentationHTML, want.LegacyPackages[i].DocumentationHTML) } for i := 0; i < len(want.Units); i++ { - if want.Units[i].Package == nil { + if !want.Units[i].IsPackage() { continue } checkHTML("Directories", i, got.Units[i].Documentation.HTML, want.Units[i].Documentation.HTML) diff --git a/internal/postgres/insert_module.go b/internal/postgres/insert_module.go index bcfabf06..2db1695f 100644 --- a/internal/postgres/insert_module.go +++ b/internal/postgres/insert_module.go @@ -385,10 +385,8 @@ func insertUnits(ctx context.Context, db *database.DB, m *internal.Module, modul sort.Slice(m.Units, func(i, j int) bool { return m.Units[i].Path < m.Units[j].Path }) - for _, d := range m.Units { - if d.Package != nil && len(d.Imports) > 1 { - sort.Strings(d.Imports) - } + for _, u := range m.Units { + sort.Strings(u.Imports) } var ( pathValues []interface{} @@ -415,15 +413,11 @@ func insertUnits(ctx context.Context, db *database.DB, m *internal.Module, modul } } } - var name string - if d.Package != nil { - name = d.Package.Name - } pathValues = append(pathValues, d.Path, moduleID, internal.V1Path(d.Path, m.ModulePath), - name, + d.Name, pq.Array(licenseTypes), pq.Array(licensePaths), d.IsRedistributable, diff --git a/internal/postgres/insert_module_test.go b/internal/postgres/insert_module_test.go index d5b717d6..49f0552a 100644 --- a/internal/postgres/insert_module_test.go +++ b/internal/postgres/insert_module_test.go @@ -121,9 +121,6 @@ func checkModule(ctx context.Context, t *testing.T, want *internal.Module) { Contents: sample.ReadmeContents, } wantu.LicenseContents = sample.Licenses - if wantu.Package != nil { - wantu.Name = wantu.Package.Name - } opts := cmp.Options{ cmpopts.IgnoreFields(internal.LegacyModuleInfo{}, "LegacyReadmeFilePath"), cmpopts.IgnoreFields(internal.LegacyModuleInfo{}, "LegacyReadmeContents"), diff --git a/internal/postgres/path_test.go b/internal/postgres/path_test.go index 86f026a5..44c2ff99 100644 --- a/internal/postgres/path_test.go +++ b/internal/postgres/path_test.go @@ -62,10 +62,7 @@ func TestGetUnitMeta(t *testing.T) { }, } if d == pkgPath { - dir.Package = &internal.Package{ - Path: pkgPath, - Name: pkgName, - } + dir.Name = pkgName dir.Documentation = &internal.Documentation{} } sample.AddUnit(m, dir) diff --git a/internal/postgres/unit.go b/internal/postgres/unit.go index 2e16b7af..268c606b 100644 --- a/internal/postgres/unit.go +++ b/internal/postgres/unit.go @@ -122,14 +122,6 @@ func (db *DB) GetUnit(ctx context.Context, um *internal.UnitMeta, fields interna } u.LicenseContents = lics } - if fields == internal.AllFields { - if u.Name != "" { - u.Package = &internal.Package{ - Path: u.Path, - Name: u.Name, - } - } - } if !db.bypassLicenseCheck { u.RemoveNonRedistributableData() } diff --git a/internal/postgres/unit_test.go b/internal/postgres/unit_test.go index 855be001..fa87fd93 100644 --- a/internal/postgres/unit_test.go +++ b/internal/postgres/unit_test.go @@ -335,7 +335,7 @@ func TestGetUnit(t *testing.T) { Filepath: sample.ReadmeFilePath, Contents: sample.ReadmeContents, } - if test.want.Package != nil { + if test.want.Name != "" { test.want.Imports = sample.Imports } test.want.SourceInfo = pathInfo.SourceInfo @@ -442,7 +442,6 @@ func unit(path, modulePath, version string, readme *internal.Readme, pkg *intern }, LicenseContents: sample.Licenses, Readme: readme, - Package: pkg, } if pkg != nil { u.Documentation = sample.Documentation diff --git a/internal/testing/sample/sample.go b/internal/testing/sample/sample.go index f19c3f08..813d4dc2 100644 --- a/internal/testing/sample/sample.go +++ b/internal/testing/sample/sample.go @@ -199,7 +199,6 @@ func Module(modulePath, version string, suffixes ...string) *internal.Module { } else { m.LegacyPackages = append(m.LegacyPackages, lp) u := UnitForPackage(lp, modulePath, version) - m.Units[0].Package = u.Package m.Units[0].Documentation = u.Documentation } } @@ -285,10 +284,6 @@ func UnitForPackage(pkg *internal.LegacyPackage, modulePath, version string) *in GOOS: pkg.GOOS, GOARCH: pkg.GOARCH, }, - Package: &internal.Package{ - Name: pkg.Name, - Path: pkg.Path, - }, } } diff --git a/internal/unit.go b/internal/unit.go index ff5e0c19..3dfe7750 100644 --- a/internal/unit.go +++ b/internal/unit.go @@ -47,7 +47,6 @@ type Unit struct { UnitMeta Readme *Readme Documentation *Documentation - Package *Package Imports []string LicenseContents []*licenses.License } -- cgit v1.3