Hide external buffer types used by primitive::Encoder in aws-smithy-types

.github/workflows/ci-sdk.yaml

@@ -68,7 +68,7 @@ jobs:
         - name: Clippy
           run: cargo clippy --all-features
         - name: Unused Dependencies
-          run: cargo +nightly-2022-02-08 udeps
+          run: cargo +nightly udeps
         - name: Additional per-crate checks
           run: ../tools/additional-per-crate-checks.sh ./sdk/ ../tools/ci-cdk/
     env:
@@ -87,7 +87,7 @@ jobs:
         default: true
     - uses: actions-rs/toolchain@v1
       with:
-        toolchain: nightly-2022-02-08
+        toolchain: nightly
         default: false
     - name: Cache cargo bin
       uses: actions/cache@v2
@@ -97,11 +97,15 @@ jobs:
     - name: Install additional cargo binaries
       run: |
         if [[ ! -f ~/.cargo/bin/cargo-udeps ]]; then
-          cargo +nightly-2022-02-08 install cargo-udeps
+          cargo +nightly install cargo-udeps
         fi
         if [[ ! -f ~/.cargo/bin/cargo-hack ]]; then
           cargo install cargo-hack
         fi
+        # Install the api-linter tool for finding external types in public APIs
+        pushd tools/api-linter &>/dev/null
+        cargo install --debug --path .
+        popd &>/dev/null
     - name: Generate a name for the SDK
       id: gen-name
       run: echo "name=${GITHUB_REF##*/}" >> $GITHUB_ENV

rust-runtime/aws-smithy-types/additional-ci

@@ -0,0 +1,15 @@
+#!/bin/bash
+#
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# SPDX-License-Identifier: Apache-2.0.
+#
+
+# This script contains additional CI checks to run for this specific package
+
+set -e
+
+echo "### Checking for duplicate dependency versions in the normal dependency graph with all features enabled"
+cargo tree -d --edges normal --all-features
+
+echo "### Running api-linter"
+cargo +nightly api-linter --all-features

rust-runtime/aws-smithy-types/src/primitive.rs

@@ -94,6 +94,18 @@ impl Parse for f64 {
     }
 }
 
+/// This type exists to hide `itoa::Buffer` implementation detail from the public API
+#[allow(missing_debug_implementations)]
+#[doc(hidden)]
+#[derive(Default)]
+pub struct IntegerEncoder(itoa::Buffer);
+
+/// This type exists to hide `ryu::Buffer` implementation detail from the public API
+#[allow(missing_debug_implementations)]
+#[doc(hidden)]
+#[derive(Default)]
+pub struct FloatingEncoder(ryu::Buffer);
+
 /// Primitive Type Encoder
 ///
 /// Encodes primitive types in Smithy's specified format. For floating-point numbers,
@@ -107,25 +119,25 @@ pub enum Encoder {
     Bool(bool),
     /// 8-bit signed integer
     #[non_exhaustive]
-    I8(i8, itoa::Buffer),
+    I8(i8, IntegerEncoder),
     /// 16-bit signed integer
     #[non_exhaustive]
-    I16(i16, itoa::Buffer),
+    I16(i16, IntegerEncoder),
     /// 32-bit signed integer
     #[non_exhaustive]
-    I32(i32, itoa::Buffer),
+    I32(i32, IntegerEncoder),
     /// 64-bit signed integer
     #[non_exhaustive]
-    I64(i64, itoa::Buffer),
+    I64(i64, IntegerEncoder),
     /// 64-bit unsigned integer
     #[non_exhaustive]
-    U64(u64, itoa::Buffer),
+    U64(u64, IntegerEncoder),
     #[non_exhaustive]
     /// 32-bit IEEE 754 single-precision floating-point number
-    F32(f32, ryu::Buffer),
+    F32(f32, FloatingEncoder),
     /// 64-bit IEEE 754 double-precision floating-point number
     #[non_exhaustive]
-    F64(f64, ryu::Buffer),
+    F64(f64, FloatingEncoder),
 }
 
 impl Debug for Encoder {
@@ -149,12 +161,12 @@ impl Encoder {
         match self {
             Encoder::Bool(true) => "true",
             Encoder::Bool(false) => "false",
-            Encoder::I8(v, buf) => buf.format(*v),
-            Encoder::I16(v, buf) => buf.format(*v),
-            Encoder::I32(v, buf) => buf.format(*v),
-            Encoder::I64(v, buf) => buf.format(*v),
-            Encoder::U64(v, buf) => buf.format(*v),
-            Encoder::F32(v, buf) => {
+            Encoder::I8(v, IntegerEncoder(buf)) => buf.format(*v),
+            Encoder::I16(v, IntegerEncoder(buf)) => buf.format(*v),
+            Encoder::I32(v, IntegerEncoder(buf)) => buf.format(*v),
+            Encoder::I64(v, IntegerEncoder(buf)) => buf.format(*v),
+            Encoder::U64(v, IntegerEncoder(buf)) => buf.format(*v),
+            Encoder::F32(v, FloatingEncoder(buf)) => {
                 if v.is_nan() {
                     float::NAN
                 } else if *v == f32::INFINITY {
@@ -165,7 +177,7 @@ impl Encoder {
                     buf.format_finite(*v)
                 }
             }
-            Encoder::F64(v, buf) => {
+            Encoder::F64(v, FloatingEncoder(buf)) => {
                 if v.is_nan() {
                     float::NAN
                 } else if *v == f64::INFINITY {
@@ -188,43 +200,43 @@ impl From<bool> for Encoder {
 
 impl From<i8> for Encoder {
     fn from(input: i8) -> Self {
-        Self::I8(input, itoa::Buffer::new())
+        Self::I8(input, Default::default())
     }
 }
 
 impl From<i16> for Encoder {
     fn from(input: i16) -> Self {
-        Self::I16(input, itoa::Buffer::new())
+        Self::I16(input, Default::default())
     }
 }
 
 impl From<i32> for Encoder {
     fn from(input: i32) -> Self {
-        Self::I32(input, itoa::Buffer::new())
+        Self::I32(input, Default::default())
     }
 }
 
 impl From<i64> for Encoder {
     fn from(input: i64) -> Self {
-        Self::I64(input, itoa::Buffer::new())
+        Self::I64(input, Default::default())
     }
 }
 
 impl From<u64> for Encoder {
     fn from(input: u64) -> Self {
-        Self::U64(input, itoa::Buffer::new())
+        Self::U64(input, Default::default())
     }
 }
 
 impl From<f32> for Encoder {
     fn from(input: f32) -> Self {
-        Self::F32(input, ryu::Buffer::new())
+        Self::F32(input, Default::default())
     }
 }
 
 impl From<f64> for Encoder {
     fn from(input: f64) -> Self {
-        Self::F64(input, ryu::Buffer::new())
+        Self::F64(input, Default::default())
     }
 }
 

tools/api-linter/Cargo.toml

@@ -10,7 +10,7 @@ publish = false
 [dependencies]
 anyhow = "1"
 cargo_metadata = "0.14"
-clap = { version = "3", features = ["derive"] }
+clap = { version = "3.1", features = ["derive"] }
 owo-colors = { version = "3", features = ["supports-colors"] }
 pest = "2" # For pretty error formatting
 rustdoc-types = "0.6"

tools/api-linter/src/main.rs

@@ -15,6 +15,7 @@ use smithy_rs_tool_common::shell::ShellOperation;
 use std::fmt;
 use std::fs;
 use std::path::PathBuf;
+use std::process;
 use std::str::FromStr;
 use tracing_subscriber::prelude::*;
 use tracing_subscriber::EnvFilter;
@@ -64,7 +65,7 @@ struct ApiLinterArgs {
     #[clap(long)]
     no_default_features: bool,
     /// Comma delimited list of features to enable in the crate
-    #[clap(long, use_delimiter = true)]
+    #[clap(long, use_value_delimiter = true)]
     features: Option<Vec<String>>,
     /// Path to the Cargo manifest
     manifest_path: Option<PathBuf>,
@@ -86,7 +87,29 @@ enum Args {
     ApiLinter(ApiLinterArgs),
 }
 
-fn main() -> Result<()> {
+enum Error {
+    ValidationErrors,
+    Failure(anyhow::Error),
+}
+
+impl From<anyhow::Error> for Error {
+    fn from(err: anyhow::Error) -> Self {
+        Error::Failure(err)
+    }
+}
+
+fn main() {
+    process::exit(match run_main() {
+        Ok(_) => 0,
+        Err(Error::ValidationErrors) => 1,
+        Err(Error::Failure(err)) => {
+            println!("{:#}", dbg!(err));
+            2
+        }
+    })
+}
+
+fn run_main() -> Result<(), Error> {
     let Args::ApiLinter(args) = Args::parse();
     if args.verbose {
         let filter_layer = EnvFilter::try_from_default_env()
@@ -124,14 +147,15 @@ fn main() -> Result<()> {
     let crate_path = if let Some(manifest_path) = args.manifest_path {
         cargo_metadata_cmd.manifest_path(&manifest_path);
         manifest_path
-            .canonicalize()?
+            .canonicalize()
+            .context(here!())?
             .parent()
             .expect("parent path")
             .to_path_buf()
     } else {
-        std::env::current_dir()?
+        std::env::current_dir().context(here!())?
     };
-    let cargo_metadata = cargo_metadata_cmd.exec()?;
+    let cargo_metadata = cargo_metadata_cmd.exec().context(here!())?;
     let cargo_features = resolve_features(&cargo_metadata)?;
 
     eprintln!("Running rustdoc to produce json doc output...");
@@ -162,6 +186,7 @@ fn main() -> Result<()> {
                     errors.len(),
                     "errors".if_supports_color(Stream::Stdout, |text| text.red())
                 );
+                return Err(Error::ValidationErrors);
             }
         }
         OutputFormat::MarkdownTable => {

tools/api-linter/tests/integration_test.rs

@@ -17,7 +17,10 @@ fn run_with_args(in_path: impl AsRef<Path>, args: &[&str]) -> String {
         cmd.arg(arg);
     }
     let output = cmd.output().expect("failed to start cargo-api-linter");
-    handle_failure("cargo-api-linter", &output).unwrap();
+    match output.status.code() {
+        Some(1) => { /* expected */ }
+        _ => handle_failure("cargo-api-linter", &output).unwrap(),
+    }
     let (stdout, _) = output_text(&output);
     stdout
 }