Skip to content
This repository has been archived by the owner on Apr 10, 2019. It is now read-only.

structcheck *.go different than structcheck . #34

Closed
cep21 opened this issue Sep 24, 2015 · 12 comments
Closed

structcheck *.go different than structcheck . #34

cep21 opened this issue Sep 24, 2015 · 12 comments

Comments

@cep21
Copy link

cep21 commented Sep 24, 2015

Calling structcheck *.go is different than calling structcheck ., which is causing issues running the meta linter.

Particularly, if you have a struct defined in one file and in another file inside the same package you add a method to that struct, then structcheck *.go (and gometalinter) will report undeclared name on that method while structcheck . will report no errors.

@cep21
Copy link
Author

cep21 commented Sep 24, 2015

Is there a particular reason gometalinter was written to operate on *.go rather than assuming operation on directories at a time?

@alecthomas
Copy link
Owner

Yes. If you have a directory foo then structcheck foo will error out with: cannot find package "foo" (as it assumes an absolute package path). If you do structcheck ./foo it works, or if you do ./foo/*.go it works (though apparently not ideally). However, gometalinter normalises directory names for de-duplication, so even if you use gometalinter ./foo it will be normalised to foo. To work around this, gometalinter would need to either not normalise paths, or normalise, then ensure the path looks like a path again. The latter may be possible.

@cep21
Copy link
Author

cep21 commented Sep 24, 2015

This is reproducible even running gometalinter on gometalinter

regression_tests/deadcode_test.go:13:14:warning: unused struct field undeclared name: Issues (structcheck)
regression_tests/deadcode_test.go:16:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/defercheck_test.go:17:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/dupl_test.go:32:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/errcheck_test.go:14:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/gocyclo_test.go:65:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/golint_test.go:14:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/gotype_test.go:15:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/ineffassign_test.go:14:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/structcheck_test.go:15:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/varcheck_test.go:13:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/vet_shadow_test.go:19:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)
regression_tests/vet_test.go:10:2:warning: unused struct field undeclared name: ExpectIssues (structcheck)

One option is to cd ./foo && structcheck .

@alecthomas
Copy link
Owner

That worked!

@alecthomas
Copy link
Owner

Hmm, except it broke the tests.

@cep21
Copy link
Author

cep21 commented Sep 24, 2015

    Error:      Not equal: regression_tests.Issues{regression_tests.Issue{Linter:"structcheck", Severity:"warning", Path:"test.go", Line:4, Col:2, Message:"unused struct field test.test.unused"}} (expected)
                    != regression_tests.Issues{regression_tests.Issue{Linter:"structcheck", Severity:"warning", Path:"test.go", Line:4, Col:2, Message:"unused struct field github.com/alecthomas/gometalinter/regression_tests/.test.unused"}} (actual)

:( I prefer the older version.

In general though, isn't cd $1 && {$CMD} . a probably better model than *.go? I imagine most tools will work better with it.

@cep21
Copy link
Author

cep21 commented Sep 24, 2015

At least the tools that don't recurse into subdirectories.

@cep21
Copy link
Author

cep21 commented Sep 24, 2015

That would modify vet and varcheck.

@alecthomas
Copy link
Owner

Well, there's no "better" here, they all work completely differently. Even tools from the same author work differently. So I basically just try different approaches and use whatever works.

@cep21
Copy link
Author

cep21 commented Sep 24, 2015

What are your thoughts on changing vet and varcheck to match the "cd &&" model? I think it's closer to what most people do so is probably more future proof.

alecthomas added a commit that referenced this issue Sep 24, 2015
@cep21
Copy link
Author

cep21 commented Sep 24, 2015

The dupl change may be too aggressive. It's the output we eventually want (with dupl running on ./...) but metalinter runs it on subdirectories so you end up dupl-ing into subdirectories multiple times.

@alecthomas
Copy link
Owner

Ah yes, got a little carried away there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants