From cc1c9542a996373a2ee9f72507b004d927357fc7 Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Sat, 18 Dec 2021 14:41:28 -0500 Subject: [PATCH 1/2] nit: correct slice allocation --- cli/internal/login/link.go | 6 +++--- cli/internal/run/run.go | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cli/internal/login/link.go b/cli/internal/login/link.go index 4f6162f5aca79..8281fb1fbd23e 100644 --- a/cli/internal/login/link.go +++ b/cli/internal/login/link.go @@ -100,11 +100,11 @@ func (c *LinkCommand) Run(args []string) int { var chosenTeam client.Team - teamOptions := make([]string, len(teamsResponse.Teams)) + teamOptions := make([]string, 0, len(teamsResponse.Teams)) // Gather team options - for i, team := range teamsResponse.Teams { - teamOptions[i] = team.Name + for _, team := range teamsResponse.Teams { + teamOptions = append(teamOptions, team.Name) } var chosenTeamName string diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index d7b415fa559c1..a70641ece46c3 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -286,13 +286,13 @@ func (c *RunCommand) Run(args []string) int { topoVisit = append(topoVisit, v) pack := ctx.PackageInfos[v] - ancestralHashes := make([]string, len(pack.InternalDeps)) + ancestralHashes := make([]string, 0, len(pack.InternalDeps)) if len(pack.InternalDeps) > 0 { - for i, ancestor := range pack.InternalDeps { - ancestralHashes[i] = ctx.PackageInfos[ancestor].Hash + for _, ancestor := range pack.InternalDeps { + ancestralHashes = append(ancestralHashes, ctx.PackageInfos[ancestor].Hash) } + sort.Strings(ancestralHashes) } - sort.Strings(ancestralHashes) var hashable = struct { hashOfFiles string ancestralHashes []string @@ -845,13 +845,13 @@ func parseRunArgs(args []string, cwd string) (*RunOptions, error) { // convertStringsToGlobs converts string glob patterns to an array glob.Glob instances. func convertStringsToGlobs(patterns []string) (globss []glob.Glob, err error) { - var globs = make([]glob.Glob, len(patterns)) - for i, pattern := range patterns { + var globs = make([]glob.Glob, 0, len(patterns)) + for _, pattern := range patterns { g, err := glob.Compile(pattern) if err != nil { return nil, err } - globs[i] = g + globs = append(globs, g) } return globs, nil From 4ab43a8def9644d1a35db930a93fabf276b4f3db Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Sat, 18 Dec 2021 15:30:28 -0500 Subject: [PATCH 2/2] Handle nil slices --- cli/internal/cache/cache_http.go | 2 +- cli/internal/context/context.go | 73 ++++++++++++++++++++------------ cli/internal/run/run.go | 6 ++- 3 files changed, 50 insertions(+), 31 deletions(-) diff --git a/cli/internal/cache/cache_http.go b/cli/internal/cache/cache_http.go index 11252f563ab25..b50a45fb203c7 100644 --- a/cli/internal/cache/cache_http.go +++ b/cli/internal/cache/cache_http.go @@ -115,7 +115,7 @@ func (cache *httpCache) retrieve(key string) (bool, []string, error) { return false, nil, err } defer resp.Body.Close() - files := []string{} + var files []string if resp.StatusCode == http.StatusNotFound { return false, files, nil // doesn't exist - not an error } else if resp.StatusCode != http.StatusOK { diff --git a/cli/internal/context/context.go b/cli/internal/context/context.go index 8bd32fe655923..1f0552b7673fd 100644 --- a/cli/internal/context/context.go +++ b/cli/internal/context/context.go @@ -300,19 +300,23 @@ func (c *Context) ResolveWorkspaceRootDeps() (*fs.PackageJSON, error) { pkg.SubLockfile = make(fs.YarnLockfile) c.ResolveDepGraph(&lockfileWg, pkg.UnresolvedExternalDeps, depSet, seen, pkg) lockfileWg.Wait() - pkg.ExternalDeps = make([]string, 0, depSet.Cardinality()) - for v := range depSet.ToSlice() { - pkg.ExternalDeps = append(pkg.ExternalDeps, fmt.Sprintf("%v", v)) - } - sort.Strings(pkg.ExternalDeps) - hashOfExternalDeps, err := fs.HashObject(pkg.ExternalDeps) - if err != nil { - return nil, err + externalDepCount := depSet.Cardinality() + var externalDeps []string + var externalDepsHash string + if externalDepCount > 0 { + externalDeps = make([]string, 0, externalDepCount) + for v := range depSet.ToSlice() { + externalDeps = append(externalDeps, fmt.Sprintf("%v", v)) + } + sort.Strings(externalDeps) + hashOfExternalDeps, err := fs.HashObject(externalDeps) + if err != nil { + return nil, err + } + externalDepsHash = hashOfExternalDeps } - pkg.ExternalDepsHash = hashOfExternalDeps - } else { - pkg.ExternalDeps = []string{} - pkg.ExternalDepsHash = "" + pkg.ExternalDepsHash = externalDepsHash + pkg.ExternalDeps = externalDeps } return pkg, nil @@ -397,25 +401,38 @@ func (c *Context) populateTopologicGraphForPackageJson(pkg *fs.PackageJSON) erro c.ResolveDepGraph(&lockfileWg, pkg.UnresolvedExternalDeps, externalDepSet, seen, pkg) lockfileWg.Wait() - // when there are no internal dependencies, we need to still add these leafs to the graph - if internalDepsSet.Len() == 0 { + internalDepsSetCount := internalDepsSet.Len() + var internalDeps []string + if internalDepsSetCount > 0 { + internalDeps = make([]string, 0, internalDepsSet.Len()) + for v := range internalDepsSet.List() { + internalDeps = append(internalDeps, fmt.Sprintf("%v", v)) + } + sort.Strings(internalDeps) + } else { + // when there are no internal dependencies, we need to still add these leafs to the graph c.TopologicalGraph.Connect(dag.BasicEdge(pkg.Name, ROOT_NODE_NAME)) } - pkg.ExternalDeps = make([]string, 0, externalDepSet.Cardinality()) - for v := range externalDepSet.ToSlice() { - pkg.ExternalDeps = append(pkg.ExternalDeps, fmt.Sprintf("%v", v)) - } - pkg.InternalDeps = make([]string, 0, internalDepsSet.Len()) - for v := range internalDepsSet.List() { - pkg.ExternalDeps = append(pkg.InternalDeps, fmt.Sprintf("%v", v)) - } - sort.Strings(pkg.InternalDeps) - sort.Strings(pkg.ExternalDeps) - hashOfExternalDeps, err := fs.HashObject(pkg.ExternalDeps) - if err != nil { - return err + pkg.InternalDeps = internalDeps + + externalDepCount := externalDepSet.Cardinality() + var externalDeps []string + var externalDepsHash string + if externalDepCount > 0 { + externalDeps = make([]string, 0, externalDepCount) + for v := range externalDepSet.ToSlice() { + externalDeps = append(externalDeps, fmt.Sprintf("%v", v)) + } + sort.Strings(externalDeps) + hashOfExternalDeps, err := fs.HashObject(externalDeps) + if err != nil { + return err + } + externalDepsHash = hashOfExternalDeps } - pkg.ExternalDepsHash = hashOfExternalDeps + pkg.ExternalDeps = externalDeps + pkg.ExternalDepsHash = externalDepsHash + return nil } diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index a70641ece46c3..33513097cc6bb 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -289,7 +289,9 @@ func (c *RunCommand) Run(args []string) int { ancestralHashes := make([]string, 0, len(pack.InternalDeps)) if len(pack.InternalDeps) > 0 { for _, ancestor := range pack.InternalDeps { - ancestralHashes = append(ancestralHashes, ctx.PackageInfos[ancestor].Hash) + if h, ok := ctx.PackageInfos[ancestor]; ok { + ancestralHashes = append(ancestralHashes, h.Hash) + } } sort.Strings(ancestralHashes) } @@ -315,7 +317,7 @@ func (c *RunCommand) Run(args []string) int { vertexSet.Add(v) } // We remove nodes that aren't in the final filter set - for _, toRemove := range util.Set(vertexSet).Difference(filteredPkgs) { + for _, toRemove := range vertexSet.Difference(filteredPkgs) { if toRemove != ctx.RootNode { ctx.TopologicalGraph.Remove(toRemove) }