Normalise naming of fields when unpacking coordinates across the repo

docs/BUILD

@@ -19,7 +19,10 @@ stardoc(
         "maven_install",
     ],
     visibility = ["//scripts:__pkg__"],
-    deps = ["//:implementation"],
+    deps = [
+        "//:implementation",
+        "@bazel_skylib//lib:structs",
+    ],
 )
 
 stardoc(
@@ -32,5 +35,8 @@ stardoc(
         "maven.exclusion",
     ],
     visibility = ["//scripts:__pkg__"],
-    deps = ["//:implementation"],
+    deps = [
+        "//:implementation",
+        "@bazel_skylib//lib:structs",
+    ],
 )

private/lib/coordinates.bzl

@@ -1,103 +1,83 @@
 def unpack_coordinates(coords):
     """Takes a maven coordinate and unpacks it into a struct with fields
-    `groupId`, `artifactId`, `version`, `type`, `scope`
-    where type and scope are optional.
+    `group`, `artifact`, `version`, `packaging`, `classifier`
+    where `version,` `packaging` and `classifier` may be `None`
 
     Assumes `coords` is in one of the following syntaxes:
-     * groupId:artifactId[:type[:scope]]:version
-     * groupId:artifactId[:version][@classifier][:type]
+     * group:artifact[:packaging[:classifier]]:version
+     * group:artifact[:version][:classifier][@packaging]
     """
     if not coords:
         return None
 
-    parts = coords.split(":")
-    nparts = len(parts)
-
-    if nparts < 2:
-        fail("Unparsed: %s" % coords)
-
-    # Both formats look the same for just `group:artifact`
-    if nparts == 2:
-        return struct(
-            groupId = parts[0],
-            artifactId = parts[1],
-            type = None,
-            scope = None,
-            version = None,
-            classifier = None,
-        )
-
-    # From here, we can be sure we have at least three `parts`
-    if _is_version_number(parts[2]):
-        return _unpack_gradle_format(coords)
-
-    return _unpack_rje_format(coords, parts)
+    pieces = coords.split(":")
+    group = pieces[0]
+    artifact = pieces[1]
+
+    if len(pieces) == 2:
+        return struct(group = group, artifact = artifact, version = "", packaging = None, classifier = None)
+
+    # Unambiguously the original format
+    if len(pieces) == 5:
+        packaging = pieces[2]
+        classifier = pieces[3]
+        version = pieces[4]
+        return struct(group = group, artifact = artifact, packaging = packaging, classifier = classifier, version = version)
+
+    # If we're using BOMs, the version is optional. That means at this point
+    # we could be dealing with g:a:p or g:a:v
+    is_gradle = pieces[2][0].isdigit()
+
+    if len(pieces) == 3:
+        if is_gradle:
+            if "@" in pieces[2]:
+                (version, packaging) = pieces[2].split("@", 2)
+                return struct(group = group, artifact = artifact, packaging = packaging, version = version, classifier = None)
+            version = pieces[2]
+            return struct(group = group, artifact = artifact, version = version, packaging = None, classifier = None)
+        else:
+            packaging = pieces[2]
+            return struct(group = group, artifact = artifact, packaging = packaging, version = "", classifier = None)
+
+    if len(pieces) == 4:
+        if is_gradle:
+            version = pieces[2]
+            if "@" in pieces[3]:
+                (classifier, packaging) = pieces[3].split("@", 2)
+                return struct(group = group, artifact = artifact, packaging = packaging, classifier = classifier, version = version)
+            classifier = pieces[3]
+            return struct(group = group, artifact = artifact, classifier = classifier, version = version, packaging = None)
+        else:
+            packaging = pieces[2]
+            version = pieces[3]
+            return struct(group = group, artifact = artifact, packaging = packaging, version = version, classifier = None)
+
+    fail("Could not parse maven coordinate: %s" % coords)
 
 def _is_version_number(part):
     return part[0].isdigit()
 
-def _unpack_rje_format(coords, parts):
-    nparts = len(parts)
-
-    if nparts < 3 or nparts > 5:
-        fail("Unparsed: %s" % coords)
-
-    version = parts[-1]
-    parts = dict(enumerate(parts[:-1]))
-    return struct(
-        groupId = parts.get(0),
-        artifactId = parts.get(1),
-        type = parts.get(2),
-        scope = parts.get(3),
-        classifier = None,
-        version = version,
-    )
-
-def _unpack_gradle_format(coords):
-    idx = coords.find("@")
-    type = None
-    if idx != -1:
-        type = coords[idx + 1:]
-        coords = coords[0:idx]
-
-    parts = coords.split(":")
-    nparts = len(parts)
-
-    if nparts < 3 or nparts > 4:
-        fail("Unparsed: %s" % coords)
-
-    parts = dict(enumerate(parts))
-
-    return struct(
-        groupId = parts.get(0),
-        artifactId = parts.get(1),
-        version = parts.get(2),
-        classifier = parts.get(3),
-        type = type,
-        scope = None,
-    )
-
 def to_external_form(coords):
     """Formats `coords` as a string suitable for use by tools such as Gradle.
 
     The returned format matches Gradle's "external dependency" short-form
-    syntax: `group:name:version:classifier@type`
+    syntax: `group:name:version:classifier@packaging`
     """
 
     if type(coords) == "string":
         unpacked = unpack_coordinates(coords)
     else:
         unpacked = coords
 
-    to_return = "%s:%s:%s" % (unpacked.groupId, unpacked.artifactId, unpacked.version)
+    to_return = "%s:%s:%s" % (unpacked.group, unpacked.artifact, unpacked.version)
 
     if hasattr(unpacked, "classifier"):
         if unpacked.classifier and unpacked.classifier != "jar":
             to_return += ":" + unpacked.classifier
 
-    if hasattr(unpacked, "type"):
-        if unpacked.type and unpacked.type != "jar":
-            to_return += "@" + unpacked.type
+    if hasattr(unpacked, "packaging"):
+        if unpacked.packaging and unpacked.packaging != "jar":
+            to_return += "@" + unpacked.packaging
 
     return to_return
 
@@ -113,16 +93,16 @@ def to_purl(coords, repository):
 
     unpacked = unpack_coordinates(coords)
     to_return += "{group}:{artifact}@{version}".format(
-        artifact = unpacked.artifactId,
-        group = unpacked.groupId,
+        artifact = unpacked.artifact,
+        group = unpacked.group,
         version = unpacked.version,
     )
 
     suffix = []
     if unpacked.classifier:
         suffix.append("classifier=" + unpacked.classifier)
-    if unpacked.type:
-        suffix.append("type=" + unpacked.type)
+    if unpacked.packaging and "jar" != unpacked.packaging:
+        suffix.append("type=" + unpacked.packaging)
     if repository and repository not in _DEFAULT_PURL_REPOS:
         # Default repository name is pulled from https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst
         suffix.append("repository=" + repository)

private/rules/maven_bom.bzl

@@ -75,7 +75,7 @@ def _maven_dependencies_bom_impl(ctx):
     dependencies_bom = generate_pom(
         ctx,
         coordinates = ctx.attr.maven_coordinates,
-        versioned_dep_coordinates = combined_deps + ["%s:%s:pom:import:%s" % (unpacked.groupId, unpacked.artifactId, unpacked.version)],
+        versioned_dep_coordinates = combined_deps + ["%s:%s:pom:%s" % (unpacked.group, unpacked.artifact, unpacked.version)],
         pom_template = ctx.file.pom_template,
         out_name = "%s.xml" % ctx.label.name,
         indent = 12,

private/rules/maven_utils.bzl

@@ -11,16 +11,16 @@ def _whitespace(indent):
         whitespace = whitespace + " "
     return whitespace
 
-def format_dep(unpacked, indent = 8, include_version = True):
+def format_dep(unpacked, scope = None, indent = 8, include_version = True):
     whitespace = _whitespace(indent)
 
     dependency = [
         whitespace,
         "<dependency>\n",
         whitespace,
-        "    <groupId>%s</groupId>\n" % unpacked.groupId,
+        "    <groupId>%s</groupId>\n" % unpacked.group,
         whitespace,
-        "    <artifactId>%s</artifactId>\n" % unpacked.artifactId,
+        "    <artifactId>%s</artifactId>\n" % unpacked.artifact,
     ]
 
     if include_version:
@@ -29,16 +29,22 @@ def format_dep(unpacked, indent = 8, include_version = True):
             "    <version>%s</version>\n" % unpacked.version,
         ])
 
-    if unpacked.type and unpacked.type != "jar":
+    if unpacked.classifier and unpacked.classifier != "jar":
         dependency.extend([
             whitespace,
-            "    <type>%s</type>\n" % unpacked.type,
+            "    <classifier>%s</classifier>\n" % unpacked.classifier,
         ])
 
-    if unpacked.scope and unpacked.scope != "compile":
+    if unpacked.packaging and unpacked.packaging != "jar":
         dependency.extend([
             whitespace,
-            "    <scope>%s</scope>\n" % unpacked.scope,
+            "    <type>%s</type>\n" % unpacked.packaging,
+        ])
+
+    if scope and scope != "compile":
+        dependency.extend([
+            whitespace,
+            "    <scope>%s</scope>\n" % scope,
         ])
 
     dependency.extend([
@@ -60,11 +66,11 @@ def generate_pom(
         indent = 8):
     unpacked_coordinates = _unpack_coordinates(coordinates)
     substitutions = {
-        "{groupId}": unpacked_coordinates.groupId,
-        "{artifactId}": unpacked_coordinates.artifactId,
+        "{groupId}": unpacked_coordinates.group,
+        "{artifactId}": unpacked_coordinates.artifact,
         "{version}": unpacked_coordinates.version,
-        "{type}": unpacked_coordinates.type or "jar",
-        "{scope}": unpacked_coordinates.scope or "compile",
+        "{type}": unpacked_coordinates.packaging or "jar",
+        "{classifier}": unpacked_coordinates.classifier or "jar",
     }
 
     if parent:
@@ -74,9 +80,9 @@ def generate_pom(
         whitespace = _whitespace(indent - 4)
         parts = [
             whitespace,
-            "    <groupId>%s</groupId>\n" % unpacked_parent.groupId,
+            "    <groupId>%s</groupId>\n" % unpacked_parent.group,
             whitespace,
-            "    <artifactId>%s</artifactId>\n" % unpacked_parent.artifactId,
+            "    <artifactId>%s</artifactId>\n" % unpacked_parent.artifact,
             whitespace,
             "    <version>%s</version>" % unpacked_parent.version,
         ]
@@ -86,15 +92,12 @@ def generate_pom(
     for dep in sorted(versioned_dep_coordinates) + sorted(unversioned_dep_coordinates):
         include_version = dep in versioned_dep_coordinates
         unpacked = _unpack_coordinates(dep)
-        new_scope = "runtime" if dep in runtime_deps else unpacked.scope
-        unpacked = struct(
-            groupId = unpacked.groupId,
-            artifactId = unpacked.artifactId,
-            type = unpacked.type,
-            scope = new_scope,
-            version = unpacked.version,
-        )
-        deps.append(format_dep(unpacked, indent = indent, include_version = include_version))
+
+        new_scope = "runtime" if dep in runtime_deps else "compile"
+        if unpacked.packaging == "pom":
+            new_scope = "import"
+
+        deps.append(format_dep(unpacked, scope = new_scope, indent = indent, include_version = include_version))
 
     substitutions.update({"{dependencies}": "\n".join(deps)})
 

private/rules/v2_lock_file.bzl

@@ -47,31 +47,31 @@ def _compute_lock_file_hash(lock_file_contents):
 
 def _to_m2_path(unpacked):
     path = "{group}/{artifact}/{version}/{artifact}-{version}".format(
-        artifact = unpacked["artifactId"],
-        group = unpacked["groupId"].replace(".", "/"),
+        artifact = unpacked["artifact"],
+        group = unpacked["group"].replace(".", "/"),
         version = unpacked["version"],
     )
 
-    classifier = unpacked.get("scope", "jar")
+    classifier = unpacked.get("classifier", "jar")
     if not classifier:
         classifier = "jar"
     if "jar" != classifier:
         path += "-%s" % classifier
 
-    extension = unpacked.get("type", "jar")
+    extension = unpacked.get("packaging", "jar")
     if not extension:
         extension = "jar"
     path += ".%s" % extension
 
     return path
 
 def _to_maven_coordinates(unpacked):
-    coords = "%s:%s" % (unpacked.get("groupId"), unpacked.get("artifactId"))
+    coords = "%s:%s" % (unpacked.get("group"), unpacked.get("artifact"))
 
-    extension = unpacked.get("type", "jar")
+    extension = unpacked.get("packaging", "jar")
     if not extension:
         extension = "jar"
-    classifier = unpacked.get("scope", "jar")
+    classifier = unpacked.get("classifier", "jar")
     if not classifier:
         classifier = "jar"
 
@@ -104,17 +104,17 @@ def _get_artifacts(lock_file_contents):
         parts = root.split(":")
 
         root_unpacked = {
-            "groupId": parts[0],
-            "artifactId": parts[1],
+            "group": parts[0],
+            "artifact": parts[1],
             "version": data["version"],
         }
         if len(parts) > 2:
-            root_unpacked["type"] = parts[2]
+            root_unpacked["packaging"] = parts[2]
         else:
-            root_unpacked["type"] = "jar"
+            root_unpacked["packaging"] = "jar"
 
-        for (scope, shasum) in data.get("shasums", {}).items():
-            root_unpacked["scope"] = scope
+        for (classifier, shasum) in data.get("shasums", {}).items():
+            root_unpacked["classifier"] = classifier
             coordinates = _to_maven_coordinates(root_unpacked)
             key = _to_key(coordinates)