From 266ff49dc88eda542b6196165a9eacafbc6c125e Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Tue, 13 Aug 2024 10:39:40 +0200
Subject: [PATCH] Preserve canonical labels as such

Previously, parsing and then stringifying a label would turn `@@canonical~name//pkg:target` into `@canonical~name//pkg:target`, which is no longer valid.

The new field on `Label` defaults to `false` if not set, which is preserves the current behavior.
---
 label/label.go      | 25 ++++++++++++++++++-------
 label/label_test.go | 38 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/label/label.go b/label/label.go
index 50a71d0f3..faf431c29 100644
--- a/label/label.go
+++ b/label/label.go
@@ -51,6 +51,12 @@ type Label struct {
 	// Relative indicates whether the label refers to a target in the current
 	// package. Relative is true if and only if Repo and Pkg are both omitted.
 	Relative bool
+
+	// Canonical indicates whether the repository name is canonical. If true,
+	// then the label will be stringified with an extra "@" prefix if it is
+	// absolute.
+	// Note: Label does not apply any kind of repo mapping.
+	Canonical bool
 }
 
 // New constructs a new label from components.
@@ -83,10 +89,11 @@ func Parse(s string) (Label, error) {
 	origStr := s
 
 	relative := true
+	canonical := false
 	var repo string
-	// if target name begins @@ drop the first @
 	if strings.HasPrefix(s, "@@") {
 		s = s[len("@"):]
+		canonical = true
 	}
 	if strings.HasPrefix(s, "@") {
 		relative = false
@@ -140,10 +147,11 @@ func Parse(s string) (Label, error) {
 	}
 
 	return Label{
-		Repo:     repo,
-		Pkg:      pkg,
-		Name:     name,
-		Relative: relative,
+		Repo:      repo,
+		Pkg:       pkg,
+		Name:      name,
+		Relative:  relative,
+		Canonical: canonical,
 	}, nil
 }
 
@@ -160,6 +168,9 @@ func (l Label) String() string {
 		// if l.Repo == "@", the label string will begin with "@//"
 		repo = l.Repo
 	}
+	if l.Canonical && strings.HasPrefix(repo, "@") {
+		repo = "@" + repo
+	}
 
 	if path.Base(l.Pkg) == l.Name {
 		return fmt.Sprintf("%s//%s", repo, l.Pkg)
@@ -213,8 +224,8 @@ func (l Label) Contains(other Label) bool {
 }
 
 func (l Label) BzlExpr() bzl.Expr {
-	return &bzl.StringExpr {
-	    Value: l.String(),
+	return &bzl.StringExpr{
+		Value: l.String(),
 	}
 }
 
diff --git a/label/label_test.go b/label/label_test.go
index 65c5a37e3..b43b9dcb0 100644
--- a/label/label_test.go
+++ b/label/label_test.go
@@ -87,10 +87,10 @@ func TestParse(t *testing.T) {
 		{str: "@a//some/pkg/[someId]:[someId]", want: Label{Repo: "a", Pkg: "some/pkg/[someId]", Name: "[someId]"}},
 		{str: "@rules_python~0.0.0~pip~name_dep//:_pkg", want: Label{Repo: "rules_python~0.0.0~pip~name_dep", Name: "_pkg"}},
 		{str: "@rules_python~0.0.0~pip~name//:dep_pkg", want: Label{Repo: "rules_python~0.0.0~pip~name", Name: "dep_pkg"}},
-		{str: "@@rules_python~0.26.0~python~python_3_10_x86_64-unknown-linux-gnu//:python_runtimes", want: Label{Repo: "rules_python~0.26.0~python~python_3_10_x86_64-unknown-linux-gnu", Name: "python_runtimes"}},
+		{str: "@@rules_python~0.26.0~python~python_3_10_x86_64-unknown-linux-gnu//:python_runtimes", want: Label{Repo: "rules_python~0.26.0~python~python_3_10_x86_64-unknown-linux-gnu", Name: "python_runtimes", Canonical: true}},
 		{str: "@rules_python++pip+name_dep//:_pkg", want: Label{Repo: "rules_python++pip+name_dep", Name: "_pkg"}},
 		{str: "@rules_python++pip+name//:dep_pkg", want: Label{Repo: "rules_python++pip+name", Name: "dep_pkg"}},
-		{str: "@@rules_python++python+python_3_10_x86_64-unknown-linux-gnu//:python_runtimes", want: Label{Repo: "rules_python++python+python_3_10_x86_64-unknown-linux-gnu", Name: "python_runtimes"}},
+		{str: "@@rules_python++python+python_3_10_x86_64-unknown-linux-gnu//:python_runtimes", want: Label{Repo: "rules_python++python+python_3_10_x86_64-unknown-linux-gnu", Name: "python_runtimes", Canonical: true}},
 	} {
 		got, err := Parse(tc.str)
 		if err != nil && !tc.wantErr {
@@ -117,3 +117,37 @@ func TestImportPathToBazelRepoName(t *testing.T) {
 		}
 	}
 }
+
+func TestAbsRoundtrip(t *testing.T) {
+	for _, tc := range []struct {
+		in  string
+		out string
+	}{
+		{in: "target", out: ":target"},
+		{in: ":target"},
+		{in: "//:target"},
+		{in: "//pkg:target"},
+		{in: "@repo//:target"},
+		{in: "@repo//pkg:target"},
+		{in: "@repo", out: "@repo//:repo"},
+		{in: "@//pkg:target"},
+		{in: "@@canonical~name//:target"},
+		{in: "@@//:target"},
+	} {
+		lbl, err := Parse(tc.in)
+		if err != nil {
+			t.Errorf("Parse(%q) failed: %v", tc, err)
+			continue
+		}
+		got := lbl.String()
+		var want string
+		if len(tc.out) == 0 {
+			want = tc.in
+		} else {
+			want = tc.out
+		}
+		if got != want {
+			t.Errorf("Parse(%q).String() = %q; want %q", tc.in, got, want)
+		}
+	}
+}