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

…-types (#1209)

* Fix exit status in `api-linter`

* Run `api-linter` against `aws-smithy-types` in CI

* Hide external buffer types used by `primitive::Encoder`

* Unpin nightly in CI

* Make `Encoder` a struct and split out an inner enum

* Update changelog

.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

CHANGELOG.next.toml

@@ -10,3 +10,9 @@
 # references = ["smithy-rs#920"]
 # meta = { "breaking" = false, "tada" = false, "bug" = false }
 # author = "rcoh"
+
+[[smithy-rs]]
+message = "`aws_smithy_types::primitive::Encoder` is now a struct rather than an enum, but its usage remains the same."
+references = ["smithy-rs#1209"]
+meta = { "breaking" = true, "tada" = false, "bug" = false }
+author = "jdisanti"

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,41 +94,26 @@ impl Parse for f64 {
     }
 }
 
-/// Primitive Type Encoder
-///
-/// Encodes primitive types in Smithy's specified format. For floating-point numbers,
-/// Smithy requires that NaN and Infinity values be specially encoded.
-///
-/// This type implements `From<T>` for all Smithy primitive types.
-#[non_exhaustive]
-pub enum Encoder {
+enum Inner {
     /// Boolean
-    #[non_exhaustive]
     Bool(bool),
     /// 8-bit signed integer
-    #[non_exhaustive]
     I8(i8, itoa::Buffer),
     /// 16-bit signed integer
-    #[non_exhaustive]
     I16(i16, itoa::Buffer),
     /// 32-bit signed integer
-    #[non_exhaustive]
     I32(i32, itoa::Buffer),
     /// 64-bit signed integer
-    #[non_exhaustive]
     I64(i64, itoa::Buffer),
     /// 64-bit unsigned integer
-    #[non_exhaustive]
     U64(u64, itoa::Buffer),
-    #[non_exhaustive]
     /// 32-bit IEEE 754 single-precision floating-point number
     F32(f32, ryu::Buffer),
     /// 64-bit IEEE 754 double-precision floating-point number
-    #[non_exhaustive]
     F64(f64, ryu::Buffer),
 }
 
-impl Debug for Encoder {
+impl Debug for Inner {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         match self {
             Self::Bool(v) => write!(f, "Bool({})", v),
@@ -143,18 +128,28 @@ impl Debug for Encoder {
     }
 }
 
+/// Primitive Type Encoder
+///
+/// Encodes primitive types in Smithy's specified format. For floating-point numbers,
+/// Smithy requires that NaN and Infinity values be specially encoded.
+///
+/// This type implements `From<T>` for all Smithy primitive types.
+#[non_exhaustive]
+#[derive(Debug)]
+pub struct Encoder(Inner);
+
 impl Encoder {
     /// Encodes a Smithy primitive as a string.
     pub fn encode(&mut self) -> &str {
-        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) => {
+        match &mut self.0 {
+            Inner::Bool(true) => "true",
+            Inner::Bool(false) => "false",
+            Inner::I8(v, buf) => buf.format(*v),
+            Inner::I16(v, buf) => buf.format(*v),
+            Inner::I32(v, buf) => buf.format(*v),
+            Inner::I64(v, buf) => buf.format(*v),
+            Inner::U64(v, buf) => buf.format(*v),
+            Inner::F32(v, buf) => {
                 if v.is_nan() {
                     float::NAN
                 } else if *v == f32::INFINITY {
@@ -165,7 +160,7 @@ impl Encoder {
                     buf.format_finite(*v)
                 }
             }
-            Encoder::F64(v, buf) => {
+            Inner::F64(v, buf) => {
                 if v.is_nan() {
                     float::NAN
                 } else if *v == f64::INFINITY {
@@ -182,49 +177,49 @@ impl Encoder {
 
 impl From<bool> for Encoder {
     fn from(input: bool) -> Self {
-        Self::Bool(input)
+        Self(Inner::Bool(input))
     }
 }
 
 impl From<i8> for Encoder {
     fn from(input: i8) -> Self {
-        Self::I8(input, itoa::Buffer::new())
+        Self(Inner::I8(input, itoa::Buffer::new()))
     }
 }
 
 impl From<i16> for Encoder {
     fn from(input: i16) -> Self {
-        Self::I16(input, itoa::Buffer::new())
+        Self(Inner::I16(input, itoa::Buffer::new()))
     }
 }
 
 impl From<i32> for Encoder {
     fn from(input: i32) -> Self {
-        Self::I32(input, itoa::Buffer::new())
+        Self(Inner::I32(input, itoa::Buffer::new()))
     }
 }
 
 impl From<i64> for Encoder {
     fn from(input: i64) -> Self {
-        Self::I64(input, itoa::Buffer::new())
+        Self(Inner::I64(input, itoa::Buffer::new()))
     }
 }
 
 impl From<u64> for Encoder {
     fn from(input: u64) -> Self {
-        Self::U64(input, itoa::Buffer::new())
+        Self(Inner::U64(input, itoa::Buffer::new()))
     }
 }
 
 impl From<f32> for Encoder {
     fn from(input: f32) -> Self {
-        Self::F32(input, ryu::Buffer::new())
+        Self(Inner::F32(input, ryu::Buffer::new()))
     }
 }
 
 impl From<f64> for Encoder {
     fn from(input: f64) -> Self {
-        Self::F64(input, ryu::Buffer::new())
+        Self(Inner::F64(input, ryu::Buffer::new()))
     }
 }
 

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
 }