Repository: sonatype-nexus-community/nancy
Pull request #59 (2019-12-08 19:58:42)

Avoid copying a lock by value, which is invalid.
It is not valid to copy a `sync.Mutex` value. From the [docs](https://golang.org/pkg/sync/#Mutex):

> A Mutex must not be copied after first use.

Doing so can lead to undefined results.

This PR fixes that, by passing a pointer to the value, rather than dereferencing the value before passing it.

sonatypecla[bot]: Thanks for the contribution!  Before we can merge this, we need @flimzy to [sign the Sonatype Contributor License Agreement](https://sonatypecla.herokuapp.com/sign-cla).
flimzy: Incidentally, I discovered this while debugging #58, thanks to running `golangci-lint` in my editor. I notice there are several other linter failures in this repo, some of which may represent real bugs. If you'd find it valuable, I'd be happy to submit fixes for all of them. It might be worth adding `golangci-lint` to your Travis-CI script as well.
fitzoh: looks like `go vet` catches it as well:
```
$ go vet
# github.com/sonatype-nexus-community/nancy
./main.go:102:56: call of packages.ExtractPurlsUsingDep copies lock value: github.com/golang/dep.Project contains sync.Once contains sync.Mutex
```
flimzy: > looks like go vet catches it as well:

`golangci-lint` calls `go vet` (and many other linters), so that's probably what I was running, too :)
zendern: Well we are doing neither in CI and probably makes sense to do something there.

@flimzy what if we open an issue, get a little feedback and see about adding at least `go vet` if not `golangci-lint` to the project. 

@flimzy are you just running the default setup for `golangci-lint` or do you have other linters/checks enabled?? 

^^^I think the biggest thing with introducing that one is we just have to come to a little agreement what we do and don't want enabled. And really once we do it here it's probably going to get copied to other golang projects here like ahab. 
flimzy: > @flimzy what if we open an issue, get a little feedback and see about adding at least go vet if not golangci-lint to the project.

Makes sense to discuss in an issue, rather than here :)

> @flimzy are you just running the default setup for golangci-lint or do you have other linters/checks enabled??

I was using the default configuration. Although in most projects, I do use a custom config to enable a few non-default linters.

DarthHater: @zendern @flimzy this is technically an API change (although I doubt it's going to really hurt anyone), should we bump to 0.1.0 for Nancy? 
zendern: > @zendern @flimzy this is technically an API change (although I doubt it's going to really hurt anyone), should we bump to 0.1.0 for Nancy?

Yeah i think that is reasonable to do. 
DarthHater: I'll try and get this merged tomorrow, I think it'll be Nancy 0.1.0 FTR! cc @zendern @flimzy 

Patch:
diff --git a/main.go b/main.go
index 381b7cd..22b1a8b 100644
--- a/main.go
+++ b/main.go
@@ -113,7 +113,7 @@ func doCheckExistenceAndParse() {
 			customerrors.Check(errors.New("dep failed to parse lock file and returned nil"), "nancy could not continue due to dep failure")
 		}
 
-		purls, invalidPurls := packages.ExtractPurlsUsingDep(*project)
+		purls, invalidPurls := packages.ExtractPurlsUsingDep(project)
 
 		checkOSSIndex(purls, invalidPurls)
 	case strings.Contains(config.Path, "go.sum"):
diff --git a/packages/dep.go b/packages/dep.go
index 9b12539..bcbbaad 100644
--- a/packages/dep.go
+++ b/packages/dep.go
@@ -19,7 +19,7 @@ import (
 	"strings"
 )
 
-func ExtractPurlsUsingDep(project dep.Project) ([]string, []string) {
+func ExtractPurlsUsingDep(project *dep.Project) ([]string, []string) {
 	lockedProjects := project.Lock.P
 	var purls []string
 	var invalidPurls []string


Test patch:
diff --git a/packages/dep_int_test.go b/packages/dep_int_test.go
index cb1524e..c52a533 100644
--- a/packages/dep_int_test.go
+++ b/packages/dep_int_test.go
@@ -37,7 +37,7 @@ func TestExtractPurlsFromManifestUsingDep(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	} else {
-		purls, invalidPurls := ExtractPurlsUsingDep(*project)
+		purls, invalidPurls := ExtractPurlsUsingDep(project)
 		if len(invalidPurls) != 7 {
 			t.Errorf("Number of invalid purls not as expected. Expected : %d, Got %d", 7, len(purls))
 		}