go-cmp
go-cmp copied to clipboard
Support AllowUnexportedWithinModule
For sufficiently large codebases with many struct types defined, it is untenable to specify a large list of all types in the repository that AllowUnexported permits visibility for.
In Go, we observe that code with a shared owner often lives in the same repository, and all sub-packages within a repository often share a common package path prefix. Thus, permit expressing visibility allowances in terms of package path prefixes instead of individual types.
While not explicitly documented, this feature allows the universal comparing of unexported fields on all types: cmp.Equal(x, y, cmp.AllowUnexportedWithinModule(""))
This "feature" is intentionally undocumented (but a natural side-effect of the expressed behavior of PackagePrefix) since it considered to be bad practice.
A problem with PackagePrefix is that it assumes that package paths are universally stable. There are two failure cases I can think of:
- A type is moved out of a repo such that the package it was declared in changed. However, most movement of types is most likely to be within the same repo, so still shares a common package prefix. However, the movement of a type due to type aliases implies that the test author moved the type and can update the test accordingly.
- In non-open-source code-bases, vendoring of third-party packages sometimes re-writes the import paths. However, this seems to me to be more a problem with the vendoring technology.
LGTM. (Is there no button to press for LGTM? If so, I can't find it. I miss Gerrit.)
Apologies for the flurry of changes; was struggling to find a good name. I settled on AllowUnexportedWithinModule
. A module is defined as "a collection of related Go packages that are versioned together as a single unit", which seems to be exactly what we want here.
(Thumbs up if the name is fine with you)
Hm. "Module" has a very specific definition, however, revolving around go.mod files. I think we should stay away from it unless we really mean it.
"PackagePrefix" is verbose, but seems pretty clear.
I will be fond of this feature when it lands. :)
+1 towards "PackagePrefix" for naming. I saw the word "Module" in the commit message and had a brief frission of shock and confusion (e.g. how and why would a library like this know about and read 'go.mod' files?) until reading the full message and patch.