buildozer: Remove unused loads with comments #896
Conversation
edit/fix.go
Outdated
| continue | ||
| } | ||
|
|
||
| if len(all) == 0 { |
There was a problem hiding this comment.
Why do you test len(all)?
There was a problem hiding this comment.
If all is not empty I can just attach the comments to the previous expression. My idea was that if there's an after-comment and we keep it, it belongs to the whole load-block and shouldn't be separated by an empty line: the following
load(":a.bzl", "used")
load(":b.bzl", "unused")
# foo
becomes
load(":a.bzl", "used")
# foo
There was a problem hiding this comment.
I don't think the comment should be attached to another node. If we keep the comment, that means we consider it was incorrectly attached. So moving it to a block-comment might be better?
edit/fix.go
Outdated
| @@ -396,6 +375,16 @@ func cleanUnusedLoads(f *build.File) bool { | |||
| all = append(all, load) | |||
| } else { | |||
| fixed = true | |||
| // If the load statement contains a post-comment, keep it | |||
| if len(load.Comment().After) == 0 { | |||
There was a problem hiding this comment.
Should we do the same for Comment().Before?
There was a problem hiding this comment.
I'm not sure, there are indeed cases when a before-comment belongs only to one statement and to all of them. The fact that load statements are not separated by empty lines (unless they have comments) and that there's a (non-default) fix that orders them lexicographically makes it hard to determine which is the case:
# Bar definitions:
load(":bar.bzl", "bar")
load(":foo.bzl", "foo")
vs
# All definitions:
load(":bar.bzl", "bar")
load(":foo.bzl", "foo")
There was a problem hiding this comment.
Changed to keeping all comments (except suffix).
Previously buildozer refused to remove an unused load if it had any comment, now it works only with a special "@unused" comment.