[meta] more minor cleanups (#81)

Author: sunshowers

crates/dropshot-api-manager-types/src/validation.rs

@@ -134,7 +134,8 @@ impl LockstepApiSpecFileName {
 
 impl fmt::Display for LockstepApiSpecFileName {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str(self.path().as_str())
+        // For lockstep files, path == basename (no directory prefix).
+        f.write_str(&self.basename())
     }
 }
 
@@ -203,7 +204,12 @@ impl VersionedApiSpecFileName {
 
     /// Returns the base name of this file path.
     pub fn basename(&self) -> String {
-        match self.kind {
+        self.basename_for_kind(self.kind)
+    }
+
+    /// Returns the base name for a specific storage kind.
+    fn basename_for_kind(&self, kind: VersionedApiSpecKind) -> String {
+        match kind {
             VersionedApiSpecKind::Json => {
                 format!("{}-{}-{}.json", self.ident, self.version, self.hash)
             }
@@ -245,21 +251,22 @@ impl VersionedApiSpecFileName {
     /// - If already a Git stub, returns `basename()` directly.
     /// - If JSON, returns `basename() + ".gitstub"`.
     pub fn git_stub_basename(&self) -> String {
-        self.to_git_stub().basename()
+        self.basename_for_kind(VersionedApiSpecKind::GitStub)
     }
 
     /// Returns the basename as a JSON filename.
     ///
     /// - If already JSON, returns `basename()` directly.
     /// - If Git stub, returns the basename without `.gitstub`.
     pub fn json_basename(&self) -> String {
-        self.to_json().basename()
+        self.basename_for_kind(VersionedApiSpecKind::Json)
     }
 }
 
 impl fmt::Display for VersionedApiSpecFileName {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str(self.path().as_str())
+        // path = "{ident}/{basename}".
+        write!(f, "{}/{}", self.ident, self.basename())
     }
 }
 
@@ -288,7 +295,10 @@ pub enum ApiSpecFileName {
 
 impl fmt::Display for ApiSpecFileName {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str(self.path().as_str())
+        match self {
+            ApiSpecFileName::Lockstep(l) => fmt::Display::fmt(l, f),
+            ApiSpecFileName::Versioned(v) => fmt::Display::fmt(v, f),
+        }
     }
 }
 

crates/dropshot-api-manager-types/src/versions.rs

@@ -294,28 +294,10 @@ macro_rules! api_versions {
             )*
             $(,)?
         ] ) => {
-        dropshot_api_manager_types::paste! {
-            pub const [<VERSION_ $latest_name>]: $crate::semver::Version =
-                $crate::semver::Version::new($latest_major, 0, 0);
-
-            $(
-                pub const [<VERSION_ $name>]: $crate::semver::Version =
-                    $crate::semver::Version::new($major, 0, 0);
-            )*
-
-            pub fn supported_versions() -> $crate::SupportedVersions {
-                let mut literal_versions = vec![
-                    $crate::SupportedVersion::new([<VERSION_ $latest_name>], stringify!($latest_name)),
-                    $( $crate::SupportedVersion::new([<VERSION_ $name>], stringify!($name)) ),*
-                ];
-                literal_versions.reverse();
-                $crate::SupportedVersions::new(literal_versions)
-            }
-
-            pub const fn latest_version() -> $crate::semver::Version {
-                [<VERSION_ $latest_name>]
-            }
-        }
+        $crate::api_versions_picky!([
+            ($latest_major, 0, 0, $latest_name)
+            $(, ($major, 0, 0, $name))*
+        ]);
     };
 }
 

crates/dropshot-api-manager/src/resolved.rs

@@ -1712,17 +1712,6 @@ fn resolve_api_version_blessed<'a>(
                 git_stub: None,
             });
         }
-
-        // Report non-matching local files as extra.
-        problems.extend(non_matching.into_iter().map(|s| {
-            Problem::BlessedVersionExtraLocalSpec {
-                spec_file_name: s
-                    .spec_file_name()
-                    .as_versioned()
-                    .expect("blessed extra spec is versioned")
-                    .clone(),
-            }
-        }));
     } else if !use_git_stub_storage || is_latest {
         // Fast path: Git stub storage disabled or this is the latest version.
         // We know we always want JSON in this case, so we can avoid computing
@@ -1756,16 +1745,6 @@ fn resolve_api_version_blessed<'a>(
                     .push(Problem::GitStubShouldBeJson { local_file, blessed });
             }
         }
-
-        problems.extend(non_matching.into_iter().map(|s| {
-            Problem::BlessedVersionExtraLocalSpec {
-                spec_file_name: s
-                    .spec_file_name()
-                    .as_versioned()
-                    .expect("blessed extra spec is versioned")
-                    .clone(),
-            }
-        }));
     } else {
         // Slow path: Git stub storage enabled and not latest. Compute what
         // storage format this version should use.
@@ -1876,18 +1855,19 @@ fn resolve_api_version_blessed<'a>(
                 }
             }
         }
-
-        problems.extend(non_matching.into_iter().map(|s| {
-            Problem::BlessedVersionExtraLocalSpec {
-                spec_file_name: s
-                    .spec_file_name()
-                    .as_versioned()
-                    .expect("blessed extra spec is versioned")
-                    .clone(),
-            }
-        }));
     }
 
+    // Report non-matching local files as extra.
+    problems.extend(non_matching.into_iter().map(|s| {
+        Problem::BlessedVersionExtraLocalSpec {
+            spec_file_name: s
+                .spec_file_name()
+                .as_versioned()
+                .expect("blessed extra spec is versioned")
+                .clone(),
+        }
+    }));
+
     Resolution::new_blessed(problems)
 }
 

crates/dropshot-api-manager/src/spec_files_blessed.rs

@@ -226,16 +226,27 @@ struct UnrecognizedPath;
 impl<'a> BlessedPathKind<'a> {
     /// Parse a path from git ls-tree output into its structural kind.
     fn parse(path: &'a Utf8Path) -> Result<Self, UnrecognizedPath> {
-        let parts: Vec<_> = path.iter().collect();
-        match parts.as_slice() {
-            [_basename] => Ok(BlessedPathKind::Lockstep),
-            [api_dir, basename] if basename.ends_with(".json.gitstub") => {
-                Ok(BlessedPathKind::GitStubFile { api_dir, basename })
-            }
-            [api_dir, basename] => {
-                Ok(BlessedPathKind::VersionedFile { api_dir, basename })
-            }
-            _ => Err(UnrecognizedPath),
+        let mut iter = path.iter();
+        let first = iter.next().ok_or(UnrecognizedPath)?;
+        let Some(second) = iter.next() else {
+            // Single-component path: lockstep file.
+            return Ok(BlessedPathKind::Lockstep);
+        };
+        if iter.next().is_some() {
+            // Three or more components: not recognized.
+            return Err(UnrecognizedPath);
+        }
+        // Two-component path: versioned file or Git stub.
+        if second.ends_with(".json.gitstub") {
+            Ok(BlessedPathKind::GitStubFile {
+                api_dir: first,
+                basename: second,
+            })
+        } else {
+            Ok(BlessedPathKind::VersionedFile {
+                api_dir: first,
+                basename: second,
+            })
         }
     }
 }