Fix incorrect renaming of versioned packages #11
Conversation
This correctly accounts for versioned packages e.g. foo/v3, which is currently treated as if its package name is v3.
tckastanek
left a comment
There was a problem hiding this comment.
Great catch on this. Can confirm this is working for me in https://github.com/nicheinc/lead/pull/977
| } | ||
|
|
||
| // LocalName returns the name that can be used to reference the imported package | ||
| // locally: [Name] if nonempty or else the last element of [Path]. |
There was a problem hiding this comment.
Not related to this PR specifically: Is this an indication that I should give up on trying to get these Go comment links to work? I still add them sometimes and I have no idea why anymore a they never do anything 😅
There was a problem hiding this comment.
Godoc links should work, but they need a sufficiently qualified name for something that actually appears as a heading in the rendered Godoc page. 👀 I mindlessly threw brackets around Name last time, but it occurred to me that struct fields aren't linkable - they're just shown in the definition for the containing type.
Conversely, I could have linked to the [Method] type or to the [Import.String] method, for instance, because those do have their own entries!
Documentation
DELTA-2596: Mock treats foo/v2 and bar/v2 as conflicting imports (aliasing one to v3)Description
This fixes a bug introduced in #10, in which
mockconsiders different packages with the same version number to have the same package name, causing it to alias one of them to an incremented version number in the mock file. For example, the following imports......could be aliased to...
This PR addresses this by using package import info to look up each import's real package name, rather than assuming that the base of the import path is the package name.
Testing Considerations
This is a little difficult to write an integration test for in the
examplesdirectory because I would need to import some versioned modules. I didn't want to introduce otherwise irrelevant dependencies tomockjust to test this bug fix.So instead I've just confirmed locally that this fixes the issues reported here and here.
Versioning
Patch.