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

--show-coverage json #66472

Merged
merged 6 commits into from
Mar 11, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
65 changes: 57 additions & 8 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::BTreeMap;
use std::convert::TryFrom;
use std::ffi::OsStr;
use std::fmt;
use std::path::PathBuf;
Expand All @@ -24,6 +25,33 @@ use crate::opts;
use crate::passes::{self, Condition, DefaultPassOption};
use crate::theme;

#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub enum OutputFormat {
Json,
Html,
}

impl OutputFormat {
pub fn is_json(&self) -> bool {
match self {
OutputFormat::Json => true,
_ => false,
}
}
}

impl TryFrom<&str> for OutputFormat {
type Error = String;

fn try_from(value: &str) -> Result<Self, Self::Error> {
match value {
"json" => Ok(OutputFormat::Json),
"html" => Ok(OutputFormat::Html),
_ => Err(format!("unknown output format `{}`", value)),
}
}
}

/// Configuration options for rustdoc.
#[derive(Clone)]
pub struct Options {
Expand Down Expand Up @@ -115,6 +143,8 @@ pub struct Options {
pub crate_version: Option<String>,
/// Collected options specific to outputting final pages.
pub render_options: RenderOptions,
/// Output format rendering (used only for "show-coverage" option for the moment)
pub output_format: Option<OutputFormat>,
}

impl fmt::Debug for Options {
Expand Down Expand Up @@ -425,14 +455,6 @@ impl Options {
}
}

match matches.opt_str("w").as_ref().map(|s| &**s) {
Some("html") | None => {}
Some(s) => {
diag.struct_err(&format!("unknown output format: {}", s)).emit();
return Err(1);
}
}

let index_page = matches.opt_str("index-page").map(|s| PathBuf::from(&s));
if let Some(ref index_page) = index_page {
if !index_page.is_file() {
Expand Down Expand Up @@ -469,6 +491,29 @@ impl Options {
}
};

let output_format = match matches.opt_str("output-format") {
Some(s) => match OutputFormat::try_from(s.as_str()) {
Ok(o) => {
if o.is_json() && !show_coverage {
diag.struct_err("json output format isn't supported for doc generation")
.emit();
return Err(1);
} else if !o.is_json() && show_coverage {
diag.struct_err(
"html output format isn't supported for the --show-coverage option",
)
.emit();
return Err(1);
}
Some(o)
}
Err(e) => {
diag.struct_err(&e).emit();
return Err(1);
}
},
None => None,
};
let crate_name = matches.opt_str("crate-name");
let proc_macro_crate = crate_types.contains(&CrateType::ProcMacro);
let playground_url = matches.opt_str("playground-url");
Expand Down Expand Up @@ -553,6 +598,7 @@ impl Options {
generate_search_filter,
generate_redirect_pages,
},
output_format,
})
}

Expand All @@ -568,6 +614,9 @@ fn check_deprecated_options(matches: &getopts::Matches, diag: &rustc_errors::Han

for flag in deprecated_flags.iter() {
if matches.opt_present(flag) {
if *flag == "output-format" && matches.opt_present("show-coverage") {
continue;
}
let mut err =
diag.struct_warn(&format!("the '{}' flag is considered deprecated", flag));
err.warn(
Expand Down
2 changes: 2 additions & 0 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
mut manual_passes,
display_warnings,
render_options,
output_format,
..
} = options;

Expand Down Expand Up @@ -385,6 +386,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt

let mut renderinfo = RenderInfo::default();
renderinfo.access_levels = access_levels;
renderinfo.output_format = output_format;

let mut ctxt = DocContext {
tcx,
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/html/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use serde::ser::SerializeSeq;
use serde::{Serialize, Serializer};

use crate::clean::{self, AttributesExt, Deprecation, GetDefId, SelfTy};
use crate::config::RenderOptions;
use crate::config::{OutputFormat, RenderOptions};
use crate::docfs::{DocFS, ErrorStorage, PathError};
use crate::doctree;
use crate::html::escape::Escape;
Expand Down Expand Up @@ -270,6 +270,7 @@ pub struct RenderInfo {
pub deref_trait_did: Option<DefId>,
pub deref_mut_trait_did: Option<DefId>,
pub owned_box_did: Option<DefId>,
pub output_format: Option<OutputFormat>,
Copy link
Member

Choose a reason for hiding this comment

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

I think the output_format field would fit better on DocContext than RenderInfo.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's impacting the rendering so from my point of view, it fits better here. :)

}

// Helper structs for rendering items/sidebars and carrying along contextual
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ impl Cache {
deref_trait_did,
deref_mut_trait_did,
owned_box_did,
..
} = renderinfo;

let external_paths =
Expand Down
53 changes: 38 additions & 15 deletions src/librustdoc/passes/calculate_doc_coverage.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use crate::clean;
use crate::config::OutputFormat;
use crate::core::DocContext;
use crate::fold::{self, DocFolder};
use crate::passes::Pass;

use rustc_ast::attr;
use rustc_span::symbol::sym;
use rustc_span::FileName;
use serde::Serialize;
use serde_json;

use std::collections::BTreeMap;
use std::ops;
Expand All @@ -16,16 +19,16 @@ pub const CALCULATE_DOC_COVERAGE: Pass = Pass {
description: "counts the number of items with and without documentation",
};

fn calculate_doc_coverage(krate: clean::Crate, _: &DocContext<'_>) -> clean::Crate {
let mut calc = CoverageCalculator::default();
fn calculate_doc_coverage(krate: clean::Crate, ctx: &DocContext<'_>) -> clean::Crate {
let mut calc = CoverageCalculator::new();
let krate = calc.fold_crate(krate);

calc.print_results();
calc.print_results(ctx.renderinfo.borrow().output_format);

krate
}

#[derive(Default, Copy, Clone)]
#[derive(Default, Copy, Clone, Serialize)]
struct ItemCount {
total: u64,
with_docs: u64,
Expand Down Expand Up @@ -64,13 +67,41 @@ impl ops::AddAssign for ItemCount {
}
}

#[derive(Default)]
struct CoverageCalculator {
items: BTreeMap<FileName, ItemCount>,
}

fn limit_filename_len(filename: String) -> String {
let nb_chars = filename.chars().count();
if nb_chars > 35 {
"...".to_string()
+ &filename[filename.char_indices().nth(nb_chars - 32).map(|x| x.0).unwrap_or(0)..]
} else {
filename
}
}

impl CoverageCalculator {
fn print_results(&self) {
fn new() -> CoverageCalculator {
CoverageCalculator { items: Default::default() }
}

fn to_json(&self) -> String {
Copy link
Member

Choose a reason for hiding this comment

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

It would be cleaner to implement Serialize for CoverageCalculator directly.

serde_json::to_string(
&self
.items
.iter()
.map(|(k, v)| (k.to_string(), v))
.collect::<BTreeMap<String, &ItemCount>>(),
)
.expect("failed to convert JSON data to string")
}

fn print_results(&self, output_format: Option<OutputFormat>) {
if output_format.map(|o| o.is_json()).unwrap_or_else(|| false) {
println!("{}", self.to_json());
return;
}
let mut total = ItemCount::default();

fn print_table_line() {
Expand All @@ -93,15 +124,7 @@ impl CoverageCalculator {

for (file, &count) in &self.items {
if let Some(percentage) = count.percentage() {
let mut name = file.to_string();
// if a filename is too long, shorten it so we don't blow out the table
// FIXME(misdreavus): this needs to count graphemes, and probably also track
// double-wide characters...
if name.len() > 35 {
name = "...".to_string() + &name[name.len() - 32..];
}

print_table_record(&name, count, percentage);
print_table_record(&limit_filename_len(file.to_string()), count, percentage);

total += count;
}
Expand Down
4 changes: 4 additions & 0 deletions src/test/rustdoc-ui/coverage/html.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// compile-flags:-Z unstable-options --output-format html --show-coverage

/// Foo
pub struct Xo;
2 changes: 2 additions & 0 deletions src/test/rustdoc-ui/coverage/html.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
error: html output format isn't supported for the --show-coverage option

27 changes: 27 additions & 0 deletions src/test/rustdoc-ui/coverage/json.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// build-pass
// compile-flags:-Z unstable-options --output-format json --show-coverage

pub mod foo {
/// Hello!
pub struct Foo;
/// Bar
pub enum Bar { A }
}

/// X
pub struct X;

/// Bar
pub mod bar {
/// bar
pub struct Bar;
/// X
pub enum X { Y }
}

/// yolo
pub enum Yolo { X }

pub struct Xo<T: Clone> {
x: T,
}
1 change: 1 addition & 0 deletions src/test/rustdoc-ui/coverage/json.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"$DIR/json.rs":{"total":13,"with_docs":7}}
Copy link
Member

Choose a reason for hiding this comment

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

This test doesn't yet pass on Windows because of how $DIR is normalized. Specifically

if json {
from = from.replace("\\", "\\\\");
}
isn't being triggered but is needed in this case. This can be fixed by adding cflags.contains("--output-format json") || cflags.contains("--output-format=json") to
let json = cflags.contains("--error-format json")
|| cflags.contains("--error-format pretty-json")
|| cflags.contains("--error-format=json")
|| cflags.contains("--error-format=pretty-json");
.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great catch, thanks a lot!

4 changes: 3 additions & 1 deletion src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3204,7 +3204,9 @@ impl<'test> TestCx<'test> {
let json = cflags.contains("--error-format json")
|| cflags.contains("--error-format pretty-json")
|| cflags.contains("--error-format=json")
|| cflags.contains("--error-format=pretty-json");
|| cflags.contains("--error-format=pretty-json")
|| cflags.contains("--output-format json")
|| cflags.contains("--output-format=json");

let mut normalized = output.to_string();

Expand Down