zip: document isVendoredPackage false-positives

I had thought that the known bug in isVendoredPackage was strictly
conservative, but it turns out not to be.

Updates golang/go#37397
Updates golang/go#31562

Change-Id: I60f6062c41ec2d116766930f751d7df083535355
Reviewed-on: https://go-review.googlesource.com/c/mod/+/220657
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>

zip/vendor_test.go

@@ -0,0 +1,39 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package zip
+
+import "testing"
+
+func TestIsVendoredPackage(t *testing.T) {
+	for _, tc := range []struct {
+		path          string
+		want          bool
+		falsePositive bool // is this case affected by https://golang.org/issue/37397?
+	}{
+		{path: "vendor/foo/foo.go", want: true},
+		{path: "pkg/vendor/foo/foo.go", want: true},
+		{path: "longpackagename/vendor/foo/foo.go", want: true},
+
+		{path: "vendor/vendor.go", want: false},
+
+		// We ideally want these cases to be false, but they are affected by
+		// https://golang.org/issue/37397, and if we fix them we will invalidate
+		// existing module checksums. We must leave them as-is-for now.
+		{path: "pkg/vendor/vendor.go", falsePositive: true},
+		{path: "longpackagename/vendor/vendor.go", falsePositive: true},
+	} {
+		got := isVendoredPackage(tc.path)
+		want := tc.want
+		if tc.falsePositive {
+			want = true
+		}
+		if got != want {
+			t.Errorf("isVendoredPackage(%q) = %t; want %t", tc.path, got, tc.want)
+			if tc.falsePositive {
+				t.Logf("(Expected a false-positive due to https://golang.org/issue/37397.)")
+			}
+		}
+	}
+}

zip/zip.go

@@ -316,6 +316,12 @@ func (f dirFile) Path() string                 { return f.slashPath }
 func (f dirFile) Lstat() (os.FileInfo, error)  { return f.info, nil }
 func (f dirFile) Open() (io.ReadCloser, error) { return os.Open(f.filePath) }
 
+// isVendoredPackage attempts to report whether the given filename is contained
+// in a package whose import path contains (but does not end with) the component
+// "vendor".
+//
+// Unfortunately, isVendoredPackage reports false positives for files in any
+// non-top-level package whose import path ends in "vendor".
 func isVendoredPackage(name string) bool {
 	var i int
 	if strings.HasPrefix(name, "vendor/") {
@@ -325,15 +331,8 @@ func isVendoredPackage(name string) bool {
 		//
 		// 	i = j + len("/vendor/")
 		//
-		// (See https://golang.org/issue/31562.)
-		//
-		// Unfortunately, we can't fix it without invalidating checksums.
-		// Fortunately, the error appears to be strictly conservative: we'll retain
-		// vendored packages that we should have pruned, but we won't prune
-		// non-vendored packages that we should have retained.
-		//
-		// Since this defect doesn't seem to break anything, it's not worth fixing
-		// for now.
+		// (See https://golang.org/issue/31562 and https://golang.org/issue/37397.)
+		// Unfortunately, we can't fix it without invalidating module checksums.
 		i += len("/vendor/")
 	} else {
 		return false