[meta] minor cleanups (#80)

One small functional change where we now reject Git stubs that aren't
valid UTF-8. (These probably wouldn't have parsed earlier anyway, and in
any case accepting them is incorrect.)

Author: sunshowers

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

@@ -198,7 +198,7 @@ impl VersionedApiSpecFileName {
     /// Returns the path of this file relative to the root of the OpenAPI
     /// documents.
     pub fn path(&self) -> Utf8PathBuf {
-        Utf8PathBuf::from_iter([self.ident.deref().clone(), self.basename()])
+        Utf8PathBuf::from_iter([self.ident.as_str(), &self.basename()])
     }
 
     /// Returns the base name of this file path.
@@ -216,54 +216,44 @@ impl VersionedApiSpecFileName {
         }
     }
 
-    /// Converts this filename to its JSON equivalent.
-    ///
-    /// If already JSON, returns a clone of self.
-    pub fn to_json(&self) -> Self {
+    /// Returns a copy of this filename with the given storage kind.
+    fn with_kind(&self, kind: VersionedApiSpecKind) -> Self {
         Self {
             ident: self.ident.clone(),
             version: self.version.clone(),
             hash: self.hash.clone(),
-            kind: VersionedApiSpecKind::Json,
+            kind,
         }
     }
 
+    /// Converts this filename to its JSON equivalent.
+    ///
+    /// If already JSON, returns a clone of self.
+    pub fn to_json(&self) -> Self {
+        self.with_kind(VersionedApiSpecKind::Json)
+    }
+
     /// Converts this filename to its Git stub equivalent.
     ///
     /// If already a Git stub, returns a clone of self.
     pub fn to_git_stub(&self) -> Self {
-        Self {
-            ident: self.ident.clone(),
-            version: self.version.clone(),
-            hash: self.hash.clone(),
-            kind: VersionedApiSpecKind::GitStub,
-        }
+        self.with_kind(VersionedApiSpecKind::GitStub)
     }
 
     /// Returns the basename as a Git stubname.
     ///
     /// - If already a Git stub, returns `basename()` directly.
     /// - If JSON, returns `basename() + ".gitstub"`.
     pub fn git_stub_basename(&self) -> String {
-        match self.kind {
-            VersionedApiSpecKind::GitStub => self.basename(),
-            VersionedApiSpecKind::Json => {
-                format!("{}.gitstub", self.basename())
-            }
-        }
+        self.to_git_stub().basename()
     }
 
     /// 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 {
-        match self.kind {
-            VersionedApiSpecKind::Json => self.basename(),
-            VersionedApiSpecKind::GitStub => {
-                format!("{}-{}-{}.json", self.ident, self.version, self.hash)
-            }
-        }
+        self.to_json().basename()
     }
 }
 
@@ -497,8 +487,10 @@ impl ApiIdent {
     }
 
     /// Given an API identifier and a file name, determine if we're looking at
-    /// this API's "latest" symlink
+    /// this API's "latest" symlink.
     pub fn versioned_api_is_latest_symlink(&self, base_name: &str) -> bool {
-        base_name == self.versioned_api_latest_symlink()
+        base_name
+            .strip_prefix(self.0.as_str())
+            .is_some_and(|rest| rest == "-latest.json")
     }
 }

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

@@ -42,10 +42,7 @@ impl Versions {
 
     /// Returns whether this API is lockstep (as opposed to versioned)
     pub fn is_lockstep(&self) -> bool {
-        match self {
-            Versions::Lockstep { .. } => true,
-            Versions::Versioned { .. } => false,
-        }
+        !self.is_versioned()
     }
 
     /// Iterate over the semver versions of an API that are supported

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

@@ -134,7 +134,7 @@ impl From<ManagedApiConfig> for ManagedApi {
             api_description,
         } = value;
         ManagedApi {
-            ident: ApiIdent::from(ident.to_owned()),
+            ident: ApiIdent::from(ident),
             versions,
             title,
             metadata,

crates/dropshot-api-manager/src/cmd/check.rs

@@ -4,8 +4,8 @@ use crate::{
     apis::ManagedApis,
     environment::{BlessedSource, GeneratedSource, ResolvedEnv},
     output::{
-        CheckResult, OutputOpts, Styles, display_load_problems,
-        display_resolution, headers::*,
+        CheckResult, OutputOpts, display_load_problems, display_resolution,
+        headers::*,
     },
     resolved::Resolved,
 };
@@ -17,10 +17,7 @@ pub(crate) fn check_impl(
     generated_source: &GeneratedSource,
     output: &OutputOpts,
 ) -> anyhow::Result<CheckResult> {
-    let mut styles = Styles::default();
-    if output.use_color(supports_color::Stream::Stderr) {
-        styles.colorize();
-    }
+    let styles = output.styles(supports_color::Stream::Stderr);
 
     eprintln!("{:>HEADER_WIDTH$}", SEPARATOR);
 

crates/dropshot-api-manager/src/cmd/debug.rs

@@ -5,7 +5,7 @@ use crate::{
     environment::{
         BlessedSource, ErrorAccumulator, GeneratedSource, ResolvedEnv,
     },
-    output::{OutputOpts, Styles},
+    output::OutputOpts,
     resolved::Resolved,
     spec_files_generic::{ApiFiles, AsRawFiles},
 };
@@ -19,10 +19,7 @@ pub(crate) fn debug_impl(
     generated_source: &GeneratedSource,
     output: &OutputOpts,
 ) -> anyhow::Result<()> {
-    let mut styles = Styles::default();
-    if output.use_color(supports_color::Stream::Stderr) {
-        styles.colorize();
-    }
+    let styles = output.styles(supports_color::Stream::Stderr);
 
     // Print information about local files.
 

crates/dropshot-api-manager/src/cmd/generate.rs

@@ -38,10 +38,7 @@ pub(crate) fn generate_impl(
     generated_source: &GeneratedSource,
     output: &OutputOpts,
 ) -> Result<GenerateResult> {
-    let mut styles = Styles::default();
-    if output.use_color(supports_color::Stream::Stderr) {
-        styles.colorize();
-    }
+    let styles = output.styles(supports_color::Stream::Stderr);
 
     let (generated, errors) =
         generated_source.load(apis, &styles, &env.repo_root, &env.vcs)?;

crates/dropshot-api-manager/src/cmd/list.rs

@@ -2,7 +2,7 @@
 
 use crate::{
     apis::ManagedApis,
-    output::{OutputOpts, Styles, display_api_spec, display_error, plural},
+    output::{OutputOpts, display_api_spec, display_error, plural},
 };
 use indent_write::io::IndentWriter;
 use openapiv3::OpenAPI;
@@ -14,10 +14,7 @@ pub(crate) fn list_impl(
     verbose: bool,
     output: &OutputOpts,
 ) -> anyhow::Result<()> {
-    let mut styles = Styles::default();
-    if output.use_color(supports_color::Stream::Stdout) {
-        styles.colorize();
-    }
+    let styles = output.styles(supports_color::Stream::Stdout);
     let mut out = std::io::stdout();
 
     let total = apis.len();

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

@@ -150,11 +150,10 @@ fn get_json_value(
 }
 
 fn surround_with_map(
-    pointer: &str,
+    last_component: &str,
     value: &serde_json::Value,
 ) -> serde_json::Value {
     let mut map = serde_json::Map::new();
-    let last_component = pointer.split('/').next_back().unwrap_or("");
     map.insert(unescape_pointer_component(last_component), value.clone());
     serde_json::Value::Object(map)
 }

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

@@ -37,6 +37,16 @@ impl OutputOpts {
             ColorChoice::Never => false,
         }
     }
+
+    /// Creates a `Styles` instance, colorized if color is enabled for the
+    /// given stream.
+    pub(crate) fn styles(&self, stream: supports_color::Stream) -> Styles {
+        let mut styles = Styles::default();
+        if self.use_color(stream) {
+            styles.colorize();
+        }
+        styles
+    }
 }
 
 #[derive(Clone, Debug, Default)]
@@ -123,14 +133,16 @@ where
 }
 
 pub(crate) fn display_api_spec(api: &ManagedApi, styles: &Styles) -> String {
-    let versions: Vec<_> = api.iter_versions_semver().collect();
-    let latest_version = versions.last().expect("must be at least one version");
+    let mut versions = api.iter_versions_semver();
+    let count = versions.len();
+    let latest_version =
+        versions.next_back().expect("must be at least one version");
     if api.is_versioned() {
         format!(
             "{} ({}, versioned ({} supported), latest = {})",
             api.ident().style(styles.filename),
             api.title(),
-            versions.len(),
+            count,
             latest_version,
         )
     } else {

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

@@ -36,7 +36,6 @@ where
     T: Display,
 {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        // slice::join would require the use of unstable Rust.
         let mut iter = self.0.iter();
         if let Some(item) = iter.next() {
             write!(f, "{item}")?;
@@ -1008,7 +1007,7 @@ impl<'a> Resolved<'a> {
                 };
                 let api_local = local.get(&ident);
                 (
-                    api.ident().clone(),
+                    ident,
                     resolve_api(
                         env,
                         api,
@@ -1902,7 +1901,7 @@ fn resolve_api_version_local<'a>(
 ) -> Resolution<'a> {
     let mut problems = Vec::new();
 
-    // alidate the generated API document.
+    // Validate the generated API document.
     validate_generated(env, api, validation, version, generated, &mut problems);
 
     let (matching, non_matching): (Vec<_>, Vec<_>) = local