-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix redunant-import-alias rule - unused alias is also redundant #950
fix redunant-import-alias rule - unused alias is also redundant #950
Conversation
…eful for sdk packages)
8825aa7
to
03826dd
Compare
_Configuration_: N/A | ||
_Configuration_: | ||
|
||
* `ignoreUsed` : (bool) ignore aliases used in code (useful when using popular sdk packages). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please improve the wording? It's not really clear for me what exactly is to be ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import apiv2 "cloud.google.com/go/run/apiv2"
import v1 "https://pkg.go.dev/k8s.io/api/core/v1"
technically above aliases are correct if they are used min one time later in code. What is your sugestion to describe that flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem here is that if it's not used, the code won't compile.
package main
import apiv2 "cloud.google.com/go/run/apiv2"
import v1 "k8s.io/api/core/v1"
import "fmt"
func main() {
fmt.Println("dummy")
}
$ go build .
# poc
./main.go:3:8: "cloud.google.com/go/run/apiv2" imported and not used
./main.go:4:8: "k8s.io/api/core/v1" imported and not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no no no, your case is not that case, because this code is not used and make "noise" after compile
but in case mentioned in #936:
package main
import (
"fmt"
redundant "example.com/app/redundant"
)
func main() {
fmt.Println(redundant.Lovely)
}
=$ ./app
123
above code is working well, but linter shows warning:
=$ cat ~/revive-aliases.toml
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0
[rule.blank-imports]
[rule.dot-imports]
[rule.redundant-import-alias]
revive -config ~/revive-aliases.toml -formatter stylish ./...
main.go
(5, 2) https://revive.run/r#redundant-import-alias Import alias "redundant" is redundant
✖ 1 problem (0 errors) (1 warnings)
after set ignoreUsed = true
[rule.redundant-import-alias]
arguments = [{ignoreUsed = true}]
linter dont produce warnings that is the case :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is indeed redundant. This is the purpose of the linter. If you think it's fine for your project, you should just disable this linter rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes i know, it is specific case, because in one way you should think how you use packages and aliases for them. On the other side we can leave this rule with that argument for more flexibility, but this is not my decision, what you think @chavacava
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is specific about this case? ignoreUsed
seems to be essentially disabling the rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@denisvmedia ok, I think that issue #936 and this pull request is to close, because in standard scenario this rule is used to detect redundant aliases, and results should used to fix/cleanup codebase of your project (change redundant aliases to proper form)
…nd change usage of getImportPackageName
f769aa6
to
0ef6c87
Compare
related with: #936
I think we should change definition of redundant alias, because import like this:
import apiv2 "cloud.google.com/go/run/apiv2" is ok i one condition: when alias apiv2 is used min one time in code below import, otherwise is redundant because is not used
of course this rule checks only imports with aliases, imports without aliases are ignored in this rule :)