Make PackageRef validate package names.

Apparently labels for GN packages can be system-absolute.

Change-Id: Id1d9cbaa1b8aea21764ba08e5e5280986a6a6964
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/23840
Reviewed-by: Matt Stark <msta@google.com>
Commit-Queue: Matt Stark <msta@google.com>
Reviewed-by: Philipp Wollermann <philwo@google.com>

src/gn/starlark/crates/types/src/errors.rs

diff --git a/src/gn/starlark/crates/types/src/errors.rs b/src/gn/starlark/crates/types/src/errors.rs
index e8b2f07..c88aa1d 100644
--- a/src/gn/starlark/crates/types/src/errors.rs
+++ b/src/gn/starlark/crates/types/src/errors.rs
@@ -22,6 +22,8 @@
     /// The referenced file does not exist on disk.
     #[error("File {1} does not exist in {0}")]
     FileNotFound(Package, String),
+    #[error("Invalid package, must start with \"//\": \"{0}\"")]
+    NotAPackage(String),
 }
 
 impl From<Error> for starlark::Error {

src/gn/starlark/crates/types/src/label.rs

diff --git a/src/gn/starlark/crates/types/src/label.rs b/src/gn/starlark/crates/types/src/label.rs
index bef5dc7..4728d5a 100644
--- a/src/gn/starlark/crates/types/src/label.rs
+++ b/src/gn/starlark/crates/types/src/label.rs
@@ -198,7 +198,7 @@
 
     #[test]
     fn test_label_parsing() {
-        let current_pkg = PackageRef::new_for_testing("//foo/bar");
+        let current_pkg = PackageRef::new("//foo/bar").unwrap();
 
         let lbl = Label::parse("//foo/bar:baz", PackageRef::root()).unwrap();
         assert_eq!(lbl.to_string(), "//foo/bar:baz");
@@ -215,10 +215,7 @@
     #[test]
     fn test_label_attributes() {
         let lbl = Label::parse("//foo/bar:baz", PackageRef::root()).unwrap();
-        assert_eq!(
-            lbl.package.as_ref(),
-            PackageRef::new_for_testing("//foo/bar")
-        );
+        assert_eq!(lbl.package.as_ref(), PackageRef::new("//foo/bar").unwrap());
         assert_eq!(lbl.name, "baz");
         assert_eq!(format!("{lbl:?}"), "Label(\"//foo/bar:baz\")");
     }
@@ -230,7 +227,7 @@
             builder.set(
                 "my_label",
                 Label::new(
-                    PackageRef::new_for_testing("//foo/bar").to_owned(),
+                    PackageRef::new("//foo/bar").unwrap().to_owned(),
                     "baz".to_owned(),
                 ),
             );

src/gn/starlark/crates/types/src/package_ref.rs

diff --git a/src/gn/starlark/crates/types/src/package_ref.rs b/src/gn/starlark/crates/types/src/package_ref.rs
index 83034d2..349d08d 100644
--- a/src/gn/starlark/crates/types/src/package_ref.rs
+++ b/src/gn/starlark/crates/types/src/package_ref.rs
@@ -16,12 +16,14 @@
         unsafe { Self::new_unchecked("//") }
     }
 
-    /// Creates a new `PackageRef` for testing purposes.
-    /// Not #[cfg(test)] because we use it in tests for downstream crates.
-    pub fn new_for_testing(s: &str) -> &Self {
-        assert!(s.starts_with("//"), "Package name must start with //");
-        // Safety: checked above
-        unsafe { Self::new_unchecked(s) }
+    // Validates and creates a PackageRef for the given string.
+    pub fn new(s: &str) -> starlark::Result<&Self> {
+        if s.starts_with("//") {
+            // Safety: checked above
+            unsafe { Ok(Self::new_unchecked(s)) }
+        } else {
+            Err(crate::Error::NotAPackage(s.to_owned()).into())
+        }
     }
 
     /// Creates a new `PackageRef` from a string slice.