Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to nightly-2022-11-16 #32

Merged
merged 5 commits into from
Nov 28, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ env:
RUSTFLAGS: -Dwarnings
RUSTDOCFLAGS: -Dwarnings
RUST_BACKTRACE: 1
rust_version: nightly-2022-07-26
rust_version: nightly-2022-11-16

jobs:
fmt:
Expand Down
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-check-external-types"
version = "0.1.5"
version = "0.1.6"
authors = ["AWS Rust SDK Team <[email protected]>", "John DiSanti <[email protected]>"]
description = "Static analysis tool to detect external types exposed in a library's public API."
edition = "2021"
Expand All @@ -13,7 +13,7 @@ cargo_metadata = "0.15"
clap = { version = "~3.2.23", features = ["derive"] }
owo-colors = { version = "3", features = ["supports-colors"] }
pest = "2" # For pretty error formatting
rustdoc-types = "0.12"
rustdoc-types = "0.18"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
toml = "0.5"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ How to Use

_Important:_ This tool requires a nightly build of Rust to be installed since it relies on
the [rustdoc JSON output](https://github.com/rust-lang/rust/issues/76578), which hasn't been
stabilized yet. It was last tested against `nightly-2022-07-25`.
stabilized yet. It was last tested against `nightly-2022-11-16`.

To install, run the following from this README path:

Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[toolchain]
channel = "nightly-2022-07-26"
channel = "nightly-2022-11-16"
4 changes: 2 additions & 2 deletions src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl CargoRustDocJson {
pub fn run(&self) -> Result<Crate> {
let cargo = std::env::var("CARGO").unwrap_or_else(|_| "cargo".to_string());

let mut command = Command::new(&cargo);
let mut command = Command::new(cargo);
command.current_dir(&self.crate_path).arg("rustdoc");
if !self.features.is_empty() {
command.arg("--no-default-features").arg("--features");
Expand All @@ -67,7 +67,7 @@ impl CargoRustDocJson {
let output_file_name = self
.target_path
.canonicalize()
.context(here!())?
.context(here!("failed to canonicalize {:?}", self.target_path))?
.join(format!("doc/{}.json", self.crate_name.replace('-', "_")));

let json = fs::read_to_string(output_file_name).context(here!())?;
Expand Down
147 changes: 131 additions & 16 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@
* SPDX-License-Identifier: Apache-2.0
*/

use crate::bug;
use anyhow::{Context, Result};
use owo_colors::{OwoColorize, Stream};
use pest::Position;
use rustdoc_types::Span;
use std::borrow::Cow;
use std::cmp::Ordering;
use std::collections::BTreeSet;
use std::collections::HashMap;
use std::fmt;
use std::iter::Iterator;
use std::path::{Path, PathBuf};

/// Where the error occurred relative to the [`Path`](crate::path::Path).
Expand All @@ -27,6 +30,7 @@ pub enum ErrorLocation {
ClosureOutput,
ConstGeneric,
Constant,
DynTrait,
EnumTupleEntry,
GenericArg,
GenericDefaultBinding,
Expand All @@ -51,6 +55,7 @@ impl fmt::Display for ErrorLocation {
Self::ClosureOutput => "closure output of",
Self::ConstGeneric => "const generic of",
Self::Constant => "constant",
Self::DynTrait => "dyn trait of",
Self::EnumTupleEntry => "enum tuple entry of",
Self::GenericArg => "generic arg of",
Self::GenericDefaultBinding => "generic default binding of",
Expand All @@ -69,6 +74,51 @@ impl fmt::Display for ErrorLocation {
}
}

#[derive(Default)]
pub struct ValidationErrors {
errors: BTreeSet<ValidationError>,
}

impl ValidationErrors {
pub fn new() -> Self {
Default::default()
}

pub fn error_count(&self) -> usize {
self.errors
.iter()
.map(ValidationError::level)
.filter(|&l| l == ErrorLevel::Error)
.count()
}

pub fn warning_count(&self) -> usize {
self.errors
.iter()
.map(ValidationError::level)
.filter(|&l| l == ErrorLevel::Warning)
.count()
}

pub fn add(&mut self, error: ValidationError) {
self.errors.insert(error);
}

pub fn iter(&self) -> impl Iterator<Item = &ValidationError> {
self.errors.iter()
}

pub fn is_empty(&self) -> bool {
self.errors.is_empty()
}
}

#[derive(Copy, Clone, Eq, PartialEq)]
pub enum ErrorLevel {
Error,
Warning,
}

/// Error type for validation errors that get displayed to the user on the CLI.
#[derive(Debug)]
pub enum ValidationError {
Expand All @@ -79,6 +129,9 @@ pub enum ValidationError {
location: Option<Span>,
sort_key: String,
},
FieldsStripped {
type_name: String,
},
}

impl ValidationError {
Expand All @@ -94,6 +147,9 @@ impl ValidationError {
"{}:{type_name}:{what}:{in_what_type}",
location_sort_key(location)
);
if location.is_none() {
bug!("An error is missing a span and will be printed without context, file name, and line number.");
}
Self::UnapprovedExternalTypeRef {
type_name,
what: what.clone(),
Expand All @@ -103,48 +159,63 @@ impl ValidationError {
}
}

pub fn level(&self) -> ErrorLevel {
match self {
Self::UnapprovedExternalTypeRef { .. } => ErrorLevel::Error,
Self::FieldsStripped { .. } => ErrorLevel::Warning,
}
}

pub fn fields_stripped(path: &crate::path::Path) -> Self {
Self::FieldsStripped {
type_name: path.to_string(),
}
}

pub fn type_name(&self) -> &str {
match self {
Self::UnapprovedExternalTypeRef { type_name, .. } => type_name,
Self::UnapprovedExternalTypeRef { type_name, .. }
| Self::FieldsStripped { type_name } => type_name,
}
}

pub fn location(&self) -> Option<&Span> {
match self {
Self::UnapprovedExternalTypeRef { location, .. } => location.as_ref(),
Self::FieldsStripped { .. } => None,
}
}

fn sort_key(&self) -> &str {
match self {
Self::UnapprovedExternalTypeRef { sort_key, .. } => sort_key.as_ref(),
Self::FieldsStripped { type_name } => type_name.as_ref(),
}
}

pub fn fmt_headline(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::UnapprovedExternalTypeRef { type_name, .. } => {
let inner = format!(
"Unapproved external type `{}` referenced in public API",
type_name
);
write!(
f,
"{} {}",
"error:"
.if_supports_color(Stream::Stdout, |text| text.red())
.if_supports_color(Stream::Stdout, |text| text.bold()),
inner.if_supports_color(Stream::Stdout, |text| text.bold())
"Unapproved external type `{type_name}` referenced in public API"
)
}
Self::FieldsStripped { type_name } => {
write!(
f,
"Fields on `{type_name}` marked `#[doc(hidden)]` cannot be checked for external types"
)
}
}
}

pub fn subtext(&self) -> String {
pub fn subtext(&self) -> Cow<'static, str> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

match self {
Self::UnapprovedExternalTypeRef {
what, in_what_type, ..
} => format!("in {} `{}`", what, in_what_type),
} => format!("in {} `{}`", what, in_what_type).into(),
Self::FieldsStripped { .. } => "".into(),
}
}
}
Expand Down Expand Up @@ -215,6 +286,28 @@ impl ErrorPrinter {
Ok(self.file_cache.get(path).unwrap())
}

fn print_error_level(level: ErrorLevel) {
use owo_colors::{OwoColorize, Stream};
match level {
ErrorLevel::Error => {
print!(
"{}",
"error: "
.if_supports_color(Stream::Stdout, |text| text.red())
.if_supports_color(Stream::Stdout, |text| text.bold())
);
}
ErrorLevel::Warning => {
print!(
"{}",
"warning: "
.if_supports_color(Stream::Stdout, |text| text.yellow())
.if_supports_color(Stream::Stdout, |text| text.bold())
);
}
}
}

/// Outputs a human readable error with file location context
///
/// # Example output
Expand All @@ -228,15 +321,17 @@ impl ErrorPrinter {
/// |
/// = in argument named `_one` of `test_crate::external_in_fn_input`
/// ```
pub fn pretty_print_error_context(&mut self, location: &Span, subtext: String) {
pub fn pretty_print_error_context(&mut self, location: &Span, subtext: &str) {
match self.get_file_contents(&location.filename) {
Ok(file_contents) => {
let begin = Self::position_from_line_col(file_contents, location.begin);
let end = Self::position_from_line_col(file_contents, location.end);

// HACK: Using Pest to do the pretty error context formatting for lack of
// knowledge of a smaller library tailored to this use-case
let variant = pest::error::ErrorVariant::<()>::CustomError { message: subtext };
let variant = pest::error::ErrorVariant::<()>::CustomError {
message: subtext.into(),
};
let err_context = match (begin, end) {
(Some(b), Some(e)) => {
Some(pest::error::Error::new_from_span(variant, b.span(&e)))
Expand All @@ -252,7 +347,8 @@ impl ErrorPrinter {
}
}
Err(err) => {
println!("error: {subtext}");
Self::print_error_level(ErrorLevel::Error);
println!("{subtext}");
println!(
" --> {}:{}:{}",
location.filename.to_string_lossy(),
Expand Down Expand Up @@ -283,4 +379,23 @@ impl ErrorPrinter {
}
None
}

pub fn pretty_print_errors(&mut self, errors: &ValidationErrors) {
for error in errors.iter() {
Self::print_error_level(error.level());
println!("{}", error);
if let Some(location) = error.location() {
self.pretty_print_error_context(location, error.subtext().as_ref())
}
}
if !errors.is_empty() {
use owo_colors::{OwoColorize, Stream};
let (error_count, warning_count) = (errors.error_count(), errors.warning_count());
println!(
"{error_count} {errors}, {warning_count} {warnings} emitted",
errors = "errors".if_supports_color(Stream::Stdout, |text| text.red()),
warnings = "warnings".if_supports_color(Stream::Stdout, |text| text.yellow())
);
}
}
}
33 changes: 31 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

pub(crate) const NEW_ISSUE_URL: &str =
"https://github.com/awslabs/cargo-check-external-types/issues/new";

pub mod cargo;
pub mod config;
pub mod error;
Expand All @@ -16,7 +19,33 @@ macro_rules! here {
() => {
concat!("error at ", file!(), ":", line!(), ":", column!())
};
($message:tt) => {
concat!($message, " (", here!(), ")")
($($args:tt)+) => {
format!("{} ({})", format!($($args)+), here!())
};
}

/// Macro that indicates there is a bug in the program, but doesn't panic.
#[macro_export]
macro_rules! bug {
($($args:tt)+) => {
{
use owo_colors::{OwoColorize, Stream};
eprint!("{}",
"BUG: "
.if_supports_color(Stream::Stdout, |text| text.purple())
.if_supports_color(Stream::Stdout, |text| text.bold())
);
eprint!($($args)+);
eprintln!(" This is a bug. Please report it with a piece of Rust code that triggers it at: {}", $crate::NEW_ISSUE_URL);
}
};
}

/// Macro that indicates there is a bug in the program and then panics.
#[macro_export]
macro_rules! bug_panic {
($($args:tt)+) => {
$crate::bug!($($args)+);
panic!("execution cannot continue");
};
}
Loading