Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: add directness to gopls's CommandAddDependency #38914

Closed
stamblerre opened this issue May 7, 2020 · 5 comments
Closed

x/tools/gopls: add directness to gopls's CommandAddDependency #38914

stamblerre opened this issue May 7, 2020 · 5 comments
Assignees
Labels
Milestone

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented May 7, 2020

Repro case:

-- mod/go.mod --
module mod.com

go 1.14

require github.com/esimov/caire v1.0.1
-- mod/main.go --
package main

import (
	"github.com/esimov/caire"
)

func _() {
	caire.RemoveTempImage()
}

Running go mod tidy results in:

module mod.com

go 1.14

require (
	github.com/esimov/caire v1.0.1
	github.com/pkg/errors v0.9.1 // indirect
	golang.org/x/image v0.0.0-20200430140353-33d19683fad8 // indirect
)
@stamblerre stamblerre added this to the gopls/v0.5.0 milestone May 7, 2020
@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v1.0.0 Jul 22, 2020
@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Oct 28, 2020

The extra dependencies are now suggested, but they are not labeled as // indirect. We should modify the diagnostic message to contain the word indirect and add the // indirect automatically.

@stamblerre stamblerre changed the title x/tools/gopls: go.mod suggested fixes do not add indirect dependencies x/tools/gopls: add directness to gopls's CommandAddDependency Oct 29, 2020
@heschik
Copy link
Contributor

@heschik heschik commented Oct 30, 2020

We have a test case in https://github.com/golang/tools/blob/4fc0492b8eca8657dd80eff56bf885b7258ddd73/gopls/internal/regtest/modfile_test.go#L402 that verifies that dependencies like this are added without an // indirect comment. The test was confusing enough that I didn't try to figure out whether it was correct. It seems wrong to me too, though, and if you agree I'll try to clean up that test and fix it.

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Nov 2, 2020

Yeah, it looks like the comment on the test doesn't actually match what the test checks for.

@heschik
Copy link
Contributor

@heschik heschik commented Nov 3, 2020

I added the comment recently because I didn't understand what was going on. I will change the behavior to match my expectations.

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 3, 2020

Change https://golang.org/cl/267578 mentions this issue: internal/lsp: set correct directness when adding new requires

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.