Warn when fix_deprecated_globs_usage.py encounters variables (#9080)

Twitter has a couple examples of BUILD files using variables, rather than raw strings, for globs. The script will currently discard those variables, rather than warning that they can't be safely updated.

(The variables cannot be safely updated because we don't know where they are defined and it would be very complex to try to backtrack that information, especially to update exclude variables. Instead, we warn and accept that the user will need to manually fix, which is not very difficult to do.)

End users will get this update because the `curl` link always points to master.

build-support/migration-support/fix_deprecated_globs_usage.py

@@ -113,7 +113,13 @@ def parse(cls, glob_func: ast.Call, *, build_file: Path) -> Optional["GlobFuncti
         f"{glob_func.lineno}. Please manually update."
       )
       return None
-    include_globs: List[str] = [arg.s for arg in glob_func.args if isinstance(arg, ast.Str)]
+    if not all(isinstance(arg, ast.Str) for arg in glob_func.args):
+      logging.warning(
+        f"Could not parse the globs in {build_file} at line {glob_func.lineno}. Likely, you are "
+        f"using variables instead of raw strings. Please manually update."
+      )
+      return None
+    include_globs: List[str] = [arg.s for arg in glob_func.args]
 
     # Excludes are tricky...The optional `exclude` keyword is guaranteed to have a list as its
     # value, but that list can have any of these elements:
@@ -131,8 +137,17 @@ def parse(cls, glob_func: ast.Call, *, build_file: Path) -> Optional["GlobFuncti
         )
       )
       combined_exclude_elements: List[Union[ast.Call, ast.Str]] = [
-        *exclude_elements, *nested_exclude_elements,
+        element for element in
+        (*exclude_elements, *nested_exclude_elements)
+        # Lists are already flattened, so we want to remove them from this collection.
+        if not isinstance(element, ast.List)
       ]
+      if not all(isinstance(arg, (ast.Call, ast.Str)) for arg in combined_exclude_elements):
+        logging.warning(
+          f"Could not parse the exclude globs in {build_file} at line {glob_func.lineno}. Likely, "
+          f"you are using variables instead of raw strings. Please manually update."
+        )
+        return None
       exclude_globs = [arg.s for arg in combined_exclude_elements if isinstance(arg, ast.Str)]
       exclude_glob_functions = (
         cls.parse(glob, build_file=build_file)
@@ -233,7 +248,7 @@ def generate_possibly_new_build(build_file: Path) -> Optional[List[str]]:
           warning_msg(
             build_file=build_file,
             lineno=lineno,
-            field_name="bundle(filespec=)",
+            field_name="bundle(fileset=)",
             replacement=formatted_replacement,
             script_restriction=SCRIPT_RESTRICTIONS["no_bundles"],
           )

build-support/migration-support/fix_deprecated_globs_usage_test.py

@@ -328,7 +328,7 @@ def assert_no_op(build_file_content: str) -> None:
         )
         """
       ),
-      field_name="bundle(filespec=)",
+      field_name="bundle(fileset=)",
       line_number=3,
       replacement="['foo.java']",
       script_restriction=SCRIPT_RESTRICTIONS["no_bundles"],
@@ -344,7 +344,7 @@ def check_multiple_bad_bundle_entries(
         assert warning == "root: " + warning_msg(
           build_file=build,
           lineno=line_number,
-          field_name="bundle(filespec=)",
+          field_name="bundle(fileset=)",
           replacement=replacement,
           script_restriction=SCRIPT_RESTRICTIONS["no_bundles"],
         )
@@ -372,3 +372,28 @@ def check_multiple_bad_bundle_entries(
       ),
       replacements_and_line_numbers=[("['foo.java']", 2), ("['bar.java']", 2)]
     )
+
+  def test_warns_on_variables(self) -> None:
+    result, build, warnings = self.capture_warnings(
+      build_file_content=dedent(
+        """\
+        files(
+          sources=globs(VARIABLE, VAR2),
+        )
+        """
+      )
+    )
+    assert result is None
+    assert f"Could not parse the globs in {build} at line 2." in warnings[0]
+
+    result, build, warnings = self.capture_warnings(
+      build_file_content=dedent(
+        """\
+        files(
+          sources=globs('foo.py', exclude=[VAR1, [VAR2], glob(VAR3)]),
+        )
+        """
+      )
+    )
+    assert result is None
+    assert f"Could not parse the exclude globs in {build} at line 2." in warnings[0]