buildtools icon indicating copy to clipboard operation
buildtools copied to clipboard

Don't move comments when moving `load()`

Open cpovirk opened this issue 3 years ago • 1 comments

I don't know if I quite believe that I want this or not, but:

$ cat BUILD
foo(name = "foo")

# some comment
load("//some:file.bzl", "bar")

bar(name = "bar")
$ buildifier --lint=fix BUILD
$ cat BUILD
# some comment
load("//some:file.bzl", "bar")

foo(name = "foo")

bar(name = "bar")

I might prefer for the comment to stay where it was, as in:

load("//some:file.bzl", "bar")

foo(name = "foo")

# some comment

bar(name = "bar")

Very anecdotally, it seems that a comment before a load() is typically intended to refer to the next actual target after the comment, rather than to the load. So it's mildly sad to see, e.g., a comment about the Android tests in a package moved to the top of the file (with the load) rather than kept near the android_test. (Inside Google, you can see some examples of this in CL 371415986.)

Again, though, I wouldn't expect you to change the behavior based on an anecdote, and I doubt it's worth the effort to do real research. Plus, as a workaround, it's straightforward to move load() statements to the top of the file with perl or something.

cpovirk avatar May 03 '21 15:05 cpovirk

It does make sense to not move comments in these examples, however it's easy to imagine one where a comment is attached to a load statement and should be moved together. Buildozer acts the simplest way here because in general it's not possible to distinguish these situations automatically.

vladmos avatar May 07 '21 01:05 vladmos