Handle all known variants used in gradle version catalogues (#1468)

Author: shs96c

private/extensions/maven.bzl

@@ -337,29 +337,52 @@ def _coordinates_match(artifact, coordinates):
     return (artifact.group == coords.group and
             artifact.artifact == coords.artifact)
 
-def _process_gradle_versions_file(parsed, bom_modules):
+def process_gradle_versions_file(parsed, bom_modules):
     artifacts = []
     boms = []
 
-    for value in parsed.get("libraries", {}).values():
-        if not "module" in value.keys():
-            continue
-        coords = value["module"]
+    for alias, value in parsed.get("libraries", {}).items():
+        # Handle different dependency declaration formats
+        coords = None
+
+        # Case 1: Simple string notation: "group:artifact:version"
+        if type(value) == "string":
+            coords = value
+        # Case 2: Map notation
+        elif type(value) == "dict":
+            # Case 2a: Map with "module" key
+            if "module" in value.keys():
+                coords = value["module"]
+            # Case 2b: Map with "group" and "name" keys
+            elif "group" in value.keys() and "name" in value.keys():
+                coords = "%s:%s" % (value["group"], value["name"])
+            else:
+                fail("Library '%s' must have either 'module' or both 'group' and 'name' keys" % alias)
+
+            # Handle version (applies to both module and group+name formats)
+            if "version.ref" in value.keys():
+                version = parsed.get("versions", {}).get(value["version.ref"])
+                if not version:
+                    fail("Unable to resolve version.ref %s" % value["version.ref"])
+                coords += ":%s" % version
+            elif "version" in value.keys():
+                coords += ":%s" % value["version"]
+
+            # Handle packaging (e.g., "aar" for Android libraries)
+            # Note: Gradle uses "package" but we'll check common variants
+            packaging = value.get("package", value.get("packaging", "jar"))
+            if packaging != "jar":
+                coords += "@%s" % packaging
+        else:
+            fail("Library '%s' has unsupported format: %s" % (alias, type(value)))
 
-        if "version.ref" in value.keys():
-            version = parsed.get("versions", {}).get(value["version.ref"])
-            if not version:
-                fail("Unable to resolve version.ref %s" % value["version.ref"])
-            coords += ":%s" % version
-        elif "version" in value.keys():
-            coords += ":%s" % value["version"]
+        # Determine the module identifier for BOM checking
+        # Extract just group:artifact for comparison with bom_modules
+        module_id = coords.split(":")[0] + ":" + coords.split(":")[1] if ":" in coords else coords
 
-        if value["module"] in bom_modules:
+        if module_id in bom_modules:
             boms.append(unpack_coordinates(coords))
         else:
-            packaging = value.get("package", "jar")
-            if packaging != "jar":
-                coords += "@%s" % packaging
             artifacts.append(unpack_coordinates(coords))
 
     return artifacts, boms
@@ -376,7 +399,7 @@ def _process_module_tags(mctx, mod, target_repos, repo_name_2_module_name):
         content = mctx.read(mctx.path(from_toml_tag.libs_versions_toml))
         parsed = parse_toml(content)
 
-        (new_artifacts, new_boms) = _process_gradle_versions_file(parsed, from_toml_tag.bom_modules)
+        (new_artifacts, new_boms) = process_gradle_versions_file(parsed, from_toml_tag.bom_modules)
 
         repo["artifacts"] = repo.get("artifacts", []) + new_artifacts
         repo["boms"] = repo.get("boms", []) + new_boms

private/lib/toml_parser.bzl

@@ -321,14 +321,7 @@ def _parse_string(value_str, line_num, section_path):
         # Basic string - handle escape sequences
         # Double-quoted TOML strings have the same spec as JSON strings <3
 
-        result = json.decode(value_str, default = JSON_DECODE_ERROR)
-        if result == JSON_DECODE_ERROR:
-            print("Invalid string %s: '%s'" % (_parser_location(line_num, section_path), value_str))
-
-            # Re-run json.decode() without defaults to get the error message.
-            json.decode(value_str)
-
-        return result
+        return json.decode(value_str)
     else:
         fail("Invalid string format %s: %'s'" % (_parser_location(line_num, section_path), value_str))
 

tests/unit/BUILD

@@ -5,6 +5,7 @@ load(":coursier_utilities_test.bzl", "coursier_utilities_test_suite")
 load(":java_utilities_test.bzl", "java_utilities_test_suite")
 load(":proxy_test.bzl", "proxy_test_suite")
 load(":specs_test.bzl", "artifact_specs_test_suite")
+load(":version_catalogs_test.bzl", "version_catalogs_test_suite")
 
 artifact_specs_test_suite()
 
@@ -19,3 +20,5 @@ coursier_utilities_test_suite()
 java_utilities_test_suite()
 
 proxy_test_suite()
+
+version_catalogs_test_suite()

tests/unit/version_catalogs_test.bzl

@@ -0,0 +1,273 @@
+load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest")
+load("//private/extensions:maven.bzl", "process_gradle_versions_file")
+load("//private/lib:toml_parser.bzl", "parse_toml")
+
+def _simple_string_notation_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[libraries]
+commons-lang = "org.apache.commons:commons-lang3:3.12.0"
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, [])
+
+    asserts.equals(env, 1, len(artifacts))
+    asserts.equals(env, 0, len(boms))
+    asserts.equals(env, "org.apache.commons", artifacts[0].group)
+    asserts.equals(env, "commons-lang3", artifacts[0].artifact)
+    asserts.equals(env, "3.12.0", artifacts[0].version)
+
+    return unittest.end(env)
+
+simple_string_notation_test = unittest.make(_simple_string_notation_impl)
+
+def _map_with_module_and_inline_version_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[libraries]
+guava = { module = "com.google.guava:guava", version = "32.1.0-jre" }
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, [])
+
+    asserts.equals(env, 1, len(artifacts))
+    asserts.equals(env, 0, len(boms))
+    asserts.equals(env, "com.google.guava", artifacts[0].group)
+    asserts.equals(env, "guava", artifacts[0].artifact)
+    asserts.equals(env, "32.1.0-jre", artifacts[0].version)
+
+    return unittest.end(env)
+
+map_with_module_and_inline_version_test = unittest.make(_map_with_module_and_inline_version_impl)
+
+def _map_with_module_and_version_ref_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[versions]
+junit = "5.10.0"
+
+[libraries]
+junit-api = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "junit" }
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, [])
+
+    asserts.equals(env, 1, len(artifacts))
+    asserts.equals(env, 0, len(boms))
+    asserts.equals(env, "org.junit.jupiter", artifacts[0].group)
+    asserts.equals(env, "junit-jupiter-api", artifacts[0].artifact)
+    asserts.equals(env, "5.10.0", artifacts[0].version)
+
+    return unittest.end(env)
+
+map_with_module_and_version_ref_test = unittest.make(_map_with_module_and_version_ref_impl)
+
+def _map_with_module_no_version_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[libraries]
+guava-from-bom = { module = "com.google.guava:guava" }
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, [])
+
+    asserts.equals(env, 1, len(artifacts))
+    asserts.equals(env, 0, len(boms))
+    asserts.equals(env, "com.google.guava", artifacts[0].group)
+    asserts.equals(env, "guava", artifacts[0].artifact)
+    asserts.equals(env, "", artifacts[0].version)
+
+    return unittest.end(env)
+
+map_with_module_no_version_test = unittest.make(_map_with_module_no_version_impl)
+
+def _map_with_group_name_and_inline_version_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[libraries]
+androidx-core = { group = "androidx.core", name = "core-ktx", version = "1.12.0" }
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, [])
+
+    asserts.equals(env, 1, len(artifacts))
+    asserts.equals(env, 0, len(boms))
+    asserts.equals(env, "androidx.core", artifacts[0].group)
+    asserts.equals(env, "core-ktx", artifacts[0].artifact)
+    asserts.equals(env, "1.12.0", artifacts[0].version)
+
+    return unittest.end(env)
+
+map_with_group_name_and_inline_version_test = unittest.make(_map_with_group_name_and_inline_version_impl)
+
+def _map_with_group_name_and_version_ref_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[versions]
+kotlin = "1.9.0"
+
+[libraries]
+kotlin-stdlib = { group = "org.jetbrains.kotlin", name = "kotlin-stdlib", version.ref = "kotlin" }
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, [])
+
+    asserts.equals(env, 1, len(artifacts))
+    asserts.equals(env, 0, len(boms))
+    asserts.equals(env, "org.jetbrains.kotlin", artifacts[0].group)
+    asserts.equals(env, "kotlin-stdlib", artifacts[0].artifact)
+    asserts.equals(env, "1.9.0", artifacts[0].version)
+
+    return unittest.end(env)
+
+map_with_group_name_and_version_ref_test = unittest.make(_map_with_group_name_and_version_ref_impl)
+
+def _map_with_group_name_no_version_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[versions]
+compose = "1.5.0"
+
+[libraries]
+compose-ui = { group = "androidx.compose.ui", name = "ui", version.ref = "compose" }
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, [])
+
+    asserts.equals(env, 1, len(artifacts))
+    asserts.equals(env, 0, len(boms))
+    asserts.equals(env, "androidx.compose.ui", artifacts[0].group)
+    asserts.equals(env, "ui", artifacts[0].artifact)
+    asserts.equals(env, "1.5.0", artifacts[0].version)
+
+    return unittest.end(env)
+
+map_with_group_name_no_version_test = unittest.make(_map_with_group_name_no_version_impl)
+
+def _map_with_module_and_packaging_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[libraries]
+play-services = { module = "com.google.android.gms:play-services-tasks", package = "aar", version = "18.1.0" }
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, [])
+
+    asserts.equals(env, 1, len(artifacts))
+    asserts.equals(env, 0, len(boms))
+    asserts.equals(env, "com.google.android.gms", artifacts[0].group)
+    asserts.equals(env, "play-services-tasks", artifacts[0].artifact)
+    asserts.equals(env, "18.1.0", artifacts[0].version)
+    asserts.equals(env, "aar", artifacts[0].packaging)
+
+    return unittest.end(env)
+
+map_with_module_and_packaging_test = unittest.make(_map_with_module_and_packaging_impl)
+
+def _map_with_group_name_and_packaging_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[libraries]
+android-material = { group = "com.google.android.material", name = "material", version = "1.10.0", package = "aar" }
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, [])
+
+    asserts.equals(env, 1, len(artifacts))
+    asserts.equals(env, 0, len(boms))
+    asserts.equals(env, "com.google.android.material", artifacts[0].group)
+    asserts.equals(env, "material", artifacts[0].artifact)
+    asserts.equals(env, "1.10.0", artifacts[0].version)
+    asserts.equals(env, "aar", artifacts[0].packaging)
+
+    return unittest.end(env)
+
+map_with_group_name_and_packaging_test = unittest.make(_map_with_group_name_and_packaging_impl)
+
+def _bom_handling_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[libraries]
+guava-bom = { module = "com.google.guava:guava-bom", version = "32.1.0-jre" }
+guava = { module = "com.google.guava:guava", version = "32.1.0-jre" }
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, ["com.google.guava:guava-bom"])
+
+    asserts.equals(env, 1, len(artifacts))
+    asserts.equals(env, 1, len(boms))
+
+    # Check the artifact
+    asserts.equals(env, "com.google.guava", artifacts[0].group)
+    asserts.equals(env, "guava", artifacts[0].artifact)
+    asserts.equals(env, "32.1.0-jre", artifacts[0].version)
+
+    # Check the BOM
+    asserts.equals(env, "com.google.guava", boms[0].group)
+    asserts.equals(env, "guava-bom", boms[0].artifact)
+    asserts.equals(env, "32.1.0-jre", boms[0].version)
+
+    return unittest.end(env)
+
+bom_handling_test = unittest.make(_bom_handling_impl)
+
+def _multiple_libraries_impl(ctx):
+    env = unittest.begin(ctx)
+
+    toml_content = """\
+[versions]
+kotlin = "1.9.0"
+junit = "5.10.0"
+
+[libraries]
+commons-lang = "org.apache.commons:commons-lang3:3.12.0"
+guava = { module = "com.google.guava:guava", version = "32.1.0-jre" }
+junit-api = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "junit" }
+kotlin-stdlib = { group = "org.jetbrains.kotlin", name = "kotlin-stdlib", version.ref = "kotlin" }
+"""
+
+    parsed = parse_toml(toml_content)
+    artifacts, boms = process_gradle_versions_file(parsed, [])
+
+    asserts.equals(env, 4, len(artifacts))
+    asserts.equals(env, 0, len(boms))
+
+    return unittest.end(env)
+
+multiple_libraries_test = unittest.make(_multiple_libraries_impl)
+
+def version_catalogs_test_suite():
+    unittest.suite(
+        "version_catalogs_tests",
+        simple_string_notation_test,
+        map_with_module_and_inline_version_test,
+        map_with_module_and_version_ref_test,
+        map_with_module_no_version_test,
+        map_with_group_name_and_inline_version_test,
+        map_with_group_name_and_version_ref_test,
+        map_with_group_name_no_version_test,
+        map_with_module_and_packaging_test,
+        map_with_group_name_and_packaging_test,
+        bom_handling_test,
+        multiple_libraries_test,
+    )