buildtools icon indicating copy to clipboard operation
buildtools copied to clipboard

False positive: Fix mode is adding to_list() method

Open davido opened this issue 6 years ago • 1 comments

Running buildifier in fix mode is adding to_list() method, that breaks the code:

diff --git a/closure/compiler/closure_js_library.bzl b/closure/compiler/closure_js_library.bzl
index e3cc389..9139942 100644
--- a/closure/compiler/closure_js_library.bzl
+++ b/closure/compiler/closure_js_library.bzl
@@ -307,7 +307,7 @@ def _closure_js_library_impl(
     )
 
     if type(internal_descriptors) == "list":
-        internal_descriptors = depset(internal_descriptors)
+        internal_descriptors = depset(internal_descriptors.to_list())
 
     # We now export providers to any parent Target. This is considered a public
     # interface because other Skylark rules can be designed to do things with

To reproduce run this buildlifier version aef5cc1e9c5bdb8c42473b4a974e9c38d54cc088 on rules_closure repository.

davido avatar Sep 07 '19 06:09 davido

It seems that buidifier assumes that internal_descriptios is a depset, because its default value in the function header is depset(). That can be useful in some cases, but here it leads to a wrong fix.

vladmos avatar Sep 07 '19 10:09 vladmos