Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

[WIP] Report Ineffectual Rules #1494

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ IMPROVEMENTS:
* Skip empty constraints during import ([#1414](https://github.com/golang/dep/pull/1349))
* Handle errors when writing status output ([#1420](https://github.com/golang/dep/pull/1420))
* Add constraint for locked projects in `status`. (#962)
* Report ineffectual rules during dep `ensure` [#1494](https://github.com/golang/dep/pull/1494)

# v0.3.2

Expand Down
4 changes: 4 additions & 0 deletions cmd/dep/ensure.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
return err
}

if err := dep.ValidatePackageRules(p, sm); err != nil {
return err
}

params := p.MakeParams()
if ctx.Verbose {
params.TraceLogger = ctx.Err
Expand Down
29 changes: 7 additions & 22 deletions cmd/dep/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import (

"github.com/golang/dep"
"github.com/golang/dep/gps"
"github.com/golang/dep/gps/paths"
"github.com/golang/dep/gps/pkgtree"
"github.com/golang/dep/internal/fs"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -137,7 +135,13 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
if ctx.Verbose {
ctx.Out.Println("Getting direct dependencies...")
}
pkgT, directDeps, err := getDirectDependencies(sm, p)

pkgT, err := p.ParseRootPackageTree()
if err != nil {
return errors.Wrap(err, "init failed: unable to determine root packages")
}

directDeps, err := p.DirectDependencies(sm)
if err != nil {
return errors.Wrap(err, "init failed: unable to determine direct dependencies")
}
Expand Down Expand Up @@ -227,25 +231,6 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
return nil
}

func getDirectDependencies(sm gps.SourceManager, p *dep.Project) (pkgtree.PackageTree, map[string]bool, error) {
pkgT, err := p.ParseRootPackageTree()
if err != nil {
return pkgtree.PackageTree{}, nil, err
}

directDeps := map[string]bool{}
rm, _ := pkgT.ToReachMap(true, true, false, nil)
for _, ip := range rm.FlattenFn(paths.IsStandardImportPath) {
pr, err := sm.DeduceProjectRoot(ip)
if err != nil {
return pkgtree.PackageTree{}, nil, err
}
directDeps[string(pr)] = true
}

return pkgT, directDeps, nil
}

// TODO solve failures can be really creative - we need to be similarly creative
// in handling them and informing the user appropriately
func handleAllTheFailuresOfTheWorld(err error) error {
Expand Down
2 changes: 1 addition & 1 deletion cmd/dep/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestGetDirectDependencies_ConsolidatesRootProjects(t *testing.T) {
testpath := h.Path(testdir)
prj := &dep.Project{AbsRoot: testpath, ResolvedAbsRoot: testpath, ImportRoot: gps.ProjectRoot(testprj)}

_, dd, err := getDirectDependencies(sm, prj)
dd, err := prj.DirectDependencies(sm)
h.Must(err)

wantpr := "github.com/carolynvs/deptest-subpkg"
Expand Down
2 changes: 1 addition & 1 deletion cmd/dep/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ func collectConstraints(ctx *dep.Ctx, p *dep.Project, sm gps.SourceManager) (con
constraintCollection := make(constraintsCollection)

// Get direct deps of the root project.
_, directDeps, err := getDirectDependencies(sm, p)
directDeps, err := p.DirectDependencies(sm)
if err != nil {
// Return empty collection, not nil, if we fail here.
return constraintCollection, []error{errors.Wrap(err, "failed to get direct dependencies")}
Expand Down
84 changes: 84 additions & 0 deletions manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,90 @@ func ValidateProjectRoots(c *Ctx, m *Manifest, sm gps.SourceManager) error {
return valErr
}

type errInefficientRules struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ineffectual, not inefficient

constraints []gps.ProjectRoot
ignores []gps.ProjectRoot
}

func (e errInefficientRules) notEmpty() bool {
return len(e.constraints) > 0 || len(e.ignores) > 0
}

func (e errInefficientRules) Error() string {
var err bytes.Buffer
if len(e.constraints) > 0 {
err.WriteString("The following constraints aren't imported:\n\n")
for _, pr := range e.constraints {
str := fmt.Sprintf(" ✗ %s\n", pr)
err.WriteString(str)
}
}
if len(e.ignores) > 0 {
err.WriteString("The following ignored packages aren't imported:\n\n")
for _, pr := range e.ignores {
str := fmt.Sprintf(" ✗ %s\n", pr)
err.WriteString(str)
}
}
return err.String()
}

// ValidatePackageRules ensure that there are no ineffectual package declarations
// in the constraints, overrides, and ignored rules in the manifest.
func ValidatePackageRules(proj *Project, sm gps.SourceManager) error {
directDeps, err := proj.DirectDependencies(sm)
if err != nil {
return err
}

var errRules errInefficientRules
ic := findIneffectualConstraints(proj.Manifest, directDeps)
errRules.constraints = append(errRules.constraints, ic...)

ig := findIneffectualIgnores(proj.Manifest, directDeps)
errRules.ignores = append(errRules.ignores, ig...)

if errRules.notEmpty() {
return errRules
}

return nil
}

// findIneffectualConstraints looks for constraints decleared in the project
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decleared

// manifest that aren't directly used in the project.
func findIneffectualConstraints(m *Manifest, directDeps map[string]bool) []gps.ProjectRoot {
ineffectuals := make([]gps.ProjectRoot, 0)
// Check constraints in the manifest.
for pr := range m.DependencyConstraints() {
if !directDeps[string(pr)] {
ineffectuals = append(ineffectuals, pr)
}
}

// Check overrides in the manifest.
for pr := range m.Overrides() {
if !directDeps[string(pr)] {
ineffectuals = append(ineffectuals, pr)
}
}

return ineffectuals
}

// findIneffectualIgnores looks for ignored packages decleared in the project
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declared

// manifest that aren't directly used in the project.
func findIneffectualIgnores(m *Manifest, directDeps map[string]bool) []gps.ProjectRoot {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not possible to determine if ignores are ineffectual at this point

ineffectuals := make([]gps.ProjectRoot, 0)
ignoredPkgs := m.IgnoredPackages()
for _, pr := range ignoredPkgs.ToSlice() {
if !directDeps[pr] {
ineffectuals = append(ineffectuals, gps.ProjectRoot(pr))
}
}
return ineffectuals
}

// readManifest returns a Manifest read from r and a slice of validation warnings.
func readManifest(r io.Reader) (*Manifest, []error, error) {
buf := &bytes.Buffer{}
Expand Down
21 changes: 21 additions & 0 deletions project.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"

"github.com/golang/dep/gps"
"github.com/golang/dep/gps/paths"
"github.com/golang/dep/gps/pkgtree"
"github.com/golang/dep/internal/fs"
"github.com/pkg/errors"
Expand Down Expand Up @@ -151,6 +152,26 @@ func (p *Project) ParseRootPackageTree() (pkgtree.PackageTree, error) {
return ptree.TrimHiddenPackages(true, true, ig), nil
}

// DirectDependencies creates a map of direct dependencies imported in the project.
func (p *Project) DirectDependencies(sm gps.SourceManager) (map[string]bool, error) {
pkgT, err := p.ParseRootPackageTree()
if err != nil {
return nil, err
}

directDeps := make(map[string]bool)
rm, _ := pkgT.ToReachMap(true, true, false, nil)
for _, ip := range rm.FlattenFn(paths.IsStandardImportPath) {
pr, err := sm.DeduceProjectRoot(ip)
if err != nil {
return nil, err
}
directDeps[string(pr)] = true
}

return directDeps, nil
}

// BackupVendor looks for existing vendor directory and if it's not empty,
// creates a backup of it to a new directory with the provided suffix.
func BackupVendor(vpath, suffix string) (string, error) {
Expand Down