mockery
mockery copied to clipboard
Removed unused package import logic
Description
@LandonTClipp, I initially thought the current code to build imports list has special logic for vendored files. To prove it, I created a below fixture file:
package test
import (
"github.com/rs/zerolog"
)
type ImportsVendored interface {
A() zerolog.Logger
}
And a test:
func (s *GeneratorSuite) TestPrologueWithImportVendored() {
generator := s.getGenerator(
"imports_vendored.go", "ImportsVendored", false, "",
)
expected := `package mocks
import mock "github.com/stretchr/testify/mock"
import test "github.com/vektra/mockery/v2/pkg/fixtures"
import zerolog "github.com/rs/zerolog"
`
s.checkPrologueGeneration(generator, expected)
}
Moved libs to vendor, to see how vendored dependencies will be handled:
I tried different approaches and did go deeper into the x/tools but I can't make the Go SDK return a file location, it's always Go package path:
The https://pkg.go.dev/golang.org/x/[email protected]/go/packages library that's used to analyze an interface and fetch used types, relies on Go import semantics. There are no references to actual files on disk that are being used by mockery.
Why would there be any references to a file disk? 6 years ago, the tool was actually traversing the GOPATH paths to look for files to scan. The original author had the code that would transform file references to Go import packages aka. localize them. That wasn't perfect, so that’s why issues like #116 were created, with respective fixes.
Because the project doesn't traverse GOPATH anymore but uses only Go import abstractions from x/tools, there's no reason to keep that code anymore.
I marked this PR as bugfix as it removes code that is not used anymore, it only fixes the below issues:
- Fixes #419
- Fixes #249
- Fixes #481
Type of change
- [x] Bug fix (non-breaking change which fixes an issue), see comment below
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update
Version of Golang used when building/testing:
- [ ] 1.11
- [ ] 1.12
- [ ] 1.13
- [ ] 1.14
- [ ] 1.15
- [ ] 1.16
- [ ] 1.17
- [x] 1.18
How Has This Been Tested?
As above.
Checklist
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
Codecov Report
Merging #490 (c82f3c3) into master (4d1f925) will decrease coverage by
0.76%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## master #490 +/- ##
==========================================
- Coverage 72.42% 71.66% -0.77%
==========================================
Files 6 6
Lines 1262 1214 -48
==========================================
- Hits 914 870 -44
+ Misses 304 301 -3
+ Partials 44 43 -1
Impacted Files | Coverage Δ | |
---|---|---|
pkg/generator.go | 93.93% <ø> (+0.15%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 4d1f925...c82f3c3. Read the comment docs.
That's a great find, I am pretty busy over the next few days but if we can axe this logic completely that would be great! I was operating under the assumption that this logic was still needed but honestly I didn't really test that. I think you've shown it's not needed.
That's a great find, I am pretty busy over the next few days but if we can axe this logic completely that would be great! I was operating under the assumption that this logic was still needed but honestly I didn't really test that. I think you've shown it's not needed.
Thanks! Happy to see it merged :)