Skip to content

Commit

Permalink
Split ColorConfig off of HumanReadableErrorType
Browse files Browse the repository at this point in the history
The previous setup tied two unrelated things together. Splitting these two is a better model.
  • Loading branch information
estebank committed Aug 8, 2024
1 parent ce20e15 commit 9ee3a14
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 54 deletions.
17 changes: 3 additions & 14 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,9 @@ const DEFAULT_COLUMN_WIDTH: usize = 140;
/// Describes the way the content of the `rendered` field of the json output is generated
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum HumanReadableErrorType {
Default(ColorConfig),
AnnotateSnippet(ColorConfig),
Short(ColorConfig),
}

impl HumanReadableErrorType {
/// Returns a (`short`, `color`) tuple
pub fn unzip(self) -> (bool, ColorConfig) {
match self {
HumanReadableErrorType::Default(cc) => (false, cc),
HumanReadableErrorType::Short(cc) => (true, cc),
HumanReadableErrorType::AnnotateSnippet(cc) => (false, cc),
}
}
Default,
AnnotateSnippet,
Short,
}

#[derive(Clone, Copy, Debug)]
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub struct JsonEmitter {
ignored_directories_in_source_blocks: Vec<String>,
#[setters(skip)]
json_rendered: HumanReadableErrorType,
color_config: ColorConfig,
diagnostic_width: Option<usize>,
macro_backtrace: bool,
track_diagnostics: bool,
Expand All @@ -68,6 +69,7 @@ impl JsonEmitter {
fallback_bundle: LazyFallbackBundle,
pretty: bool,
json_rendered: HumanReadableErrorType,
color_config: ColorConfig,
) -> JsonEmitter {
JsonEmitter {
dst: IntoDynSyncSend(dst),
Expand All @@ -79,6 +81,7 @@ impl JsonEmitter {
ui_testing: false,
ignored_directories_in_source_blocks: Vec::new(),
json_rendered,
color_config,
diagnostic_width: None,
macro_backtrace: false,
track_diagnostics: false,
Expand Down Expand Up @@ -173,7 +176,7 @@ impl Emitter for JsonEmitter {
}

fn should_show_explain(&self) -> bool {
!matches!(self.json_rendered, HumanReadableErrorType::Short(_))
!matches!(self.json_rendered, HumanReadableErrorType::Short)
}
}

Expand Down Expand Up @@ -353,8 +356,8 @@ impl Diagnostic {

let buf = BufWriter::default();
let mut dst: Destination = Box::new(buf.clone());
let (short, color_config) = je.json_rendered.unzip();
match color_config {
let short = matches!(je.json_rendered, HumanReadableErrorType::Short);
match je.color_config {
ColorConfig::Always | ColorConfig::Auto => dst = Box::new(termcolor::Ansi::new(dst)),
ColorConfig::Never => {}
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_errors/src/json/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) {
sm,
fallback_bundle,
true, // pretty
HumanReadableErrorType::Short(ColorConfig::Never),
HumanReadableErrorType::Short,
ColorConfig::Never,
);

let span = Span::with_root_ctxt(BytePos(span.0), BytePos(span.1));
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,8 @@ fn test_search_paths_tracking_hash_different_order() {
let early_dcx = EarlyDiagCtxt::new(JSON);
const JSON: ErrorOutputType = ErrorOutputType::Json {
pretty: false,
json_rendered: HumanReadableErrorType::Default(ColorConfig::Never),
json_rendered: HumanReadableErrorType::Default,
json_color: ColorConfig::Never,
};

let push = |opts: &mut Options, search_path| {
Expand Down
38 changes: 23 additions & 15 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,20 +602,21 @@ impl OutputType {
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum ErrorOutputType {
/// Output meant for the consumption of humans.
HumanReadable(HumanReadableErrorType),
HumanReadable(HumanReadableErrorType, ColorConfig),
/// Output that's consumed by other tools such as `rustfix` or the `RLS`.
Json {
/// Render the JSON in a human readable way (with indents and newlines).
pretty: bool,
/// The JSON output includes a `rendered` field that includes the rendered
/// human output.
json_rendered: HumanReadableErrorType,
color_config: ColorConfig,
},
}

impl Default for ErrorOutputType {
fn default() -> Self {
Self::HumanReadable(HumanReadableErrorType::Default(ColorConfig::Auto))
Self::HumanReadable(HumanReadableErrorType::Default, ColorConfig::Auto)
}
}

Expand Down Expand Up @@ -1631,6 +1632,7 @@ pub fn parse_color(early_dcx: &EarlyDiagCtxt, matches: &getopts::Matches) -> Col
/// Possible json config files
pub struct JsonConfig {
pub json_rendered: HumanReadableErrorType,
pub json_color: ColorConfig,
json_artifact_notifications: bool,
pub json_unused_externs: JsonUnusedExterns,
json_future_incompat: bool,
Expand Down Expand Up @@ -1668,8 +1670,7 @@ impl JsonUnusedExterns {
/// The first value returned is how to render JSON diagnostics, and the second
/// is whether or not artifact notifications are enabled.
pub fn parse_json(early_dcx: &EarlyDiagCtxt, matches: &getopts::Matches) -> JsonConfig {
let mut json_rendered: fn(ColorConfig) -> HumanReadableErrorType =
HumanReadableErrorType::Default;
let mut json_rendered = HumanReadableErrorType::Default;
let mut json_color = ColorConfig::Never;
let mut json_artifact_notifications = false;
let mut json_unused_externs = JsonUnusedExterns::No;
Expand All @@ -1696,7 +1697,8 @@ pub fn parse_json(early_dcx: &EarlyDiagCtxt, matches: &getopts::Matches) -> Json
}

JsonConfig {
json_rendered: json_rendered(json_color),
json_rendered,
json_color,
json_artifact_notifications,
json_unused_externs,
json_future_incompat,
Expand All @@ -1708,6 +1710,7 @@ pub fn parse_error_format(
early_dcx: &mut EarlyDiagCtxt,
matches: &getopts::Matches,
color: ColorConfig,
json_color: ColorConfig,
json_rendered: HumanReadableErrorType,
) -> ErrorOutputType {
// We need the `opts_present` check because the driver will send us Matches
Expand All @@ -1717,18 +1720,22 @@ pub fn parse_error_format(
let error_format = if matches.opts_present(&["error-format".to_owned()]) {
match matches.opt_str("error-format").as_deref() {
None | Some("human") => {
ErrorOutputType::HumanReadable(HumanReadableErrorType::Default(color))
ErrorOutputType::HumanReadable(HumanReadableErrorType::Default, color)
}
Some("human-annotate-rs") => {
ErrorOutputType::HumanReadable(HumanReadableErrorType::AnnotateSnippet(color))
ErrorOutputType::HumanReadable(HumanReadableErrorType::AnnotateSnippet, color)
}
Some("json") => ErrorOutputType::Json { pretty: false, json_rendered },
Some("pretty-json") => ErrorOutputType::Json { pretty: true, json_rendered },
Some("short") => ErrorOutputType::HumanReadable(HumanReadableErrorType::Short(color)),

Some("json") => {
ErrorOutputType::Json { pretty: false, json_rendered, color_config: json_color }
}
Some("pretty-json") => {
ErrorOutputType::Json { pretty: true, json_rendered, color_config: json_color }
}
Some("short") => ErrorOutputType::HumanReadable(HumanReadableErrorType::Short, color),
Some(arg) => {
early_dcx.abort_if_error_and_set_error_format(ErrorOutputType::HumanReadable(
HumanReadableErrorType::Default(color),
HumanReadableErrorType::Default,
color,
));
early_dcx.early_fatal(format!(
"argument for `--error-format` must be `human`, `json` or \
Expand All @@ -1737,7 +1744,7 @@ pub fn parse_error_format(
}
}
} else {
ErrorOutputType::HumanReadable(HumanReadableErrorType::Default(color))
ErrorOutputType::HumanReadable(HumanReadableErrorType::Default, color)
};

match error_format {
Expand Down Expand Up @@ -1791,7 +1798,7 @@ fn check_error_format_stability(
if let ErrorOutputType::Json { pretty: true, .. } = error_format {
early_dcx.early_fatal("`--error-format=pretty-json` is unstable");
}
if let ErrorOutputType::HumanReadable(HumanReadableErrorType::AnnotateSnippet(_)) =
if let ErrorOutputType::HumanReadable(HumanReadableErrorType::AnnotateSnippet, _) =
error_format
{
early_dcx.early_fatal("`--error-format=human-annotate-rs` is unstable");
Expand Down Expand Up @@ -2392,12 +2399,13 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M

let JsonConfig {
json_rendered,
json_color,
json_artifact_notifications,
json_unused_externs,
json_future_incompat,
} = parse_json(early_dcx, matches);

let error_format = parse_error_format(early_dcx, matches, color, json_rendered);
let error_format = parse_error_format(early_dcx, matches, color, json_color, json_rendered);

early_dcx.abort_if_error_and_set_error_format(error_format);

Expand Down
30 changes: 17 additions & 13 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -950,10 +950,10 @@ fn default_emitter(
t => t,
};
match sopts.error_format {
config::ErrorOutputType::HumanReadable(kind) => {
let (short, color_config) = kind.unzip();
config::ErrorOutputType::HumanReadable(kind, color_config) => {
let short = matches!(kind, HumanReadableErrorType::Short);

if let HumanReadableErrorType::AnnotateSnippet(_) = kind {
if let HumanReadableErrorType::AnnotateSnippet = kind {
let emitter = AnnotateSnippetEmitter::new(
Some(source_map),
bundle,
Expand All @@ -978,13 +978,14 @@ fn default_emitter(
Box::new(emitter.ui_testing(sopts.unstable_opts.ui_testing))
}
}
config::ErrorOutputType::Json { pretty, json_rendered } => Box::new(
config::ErrorOutputType::Json { pretty, json_rendered, color_config } => Box::new(
JsonEmitter::new(
Box::new(io::BufWriter::new(io::stderr())),
source_map,
fallback_bundle,
pretty,
json_rendered,
color_config,
)
.registry(Some(registry))
.fluent_bundle(bundle)
Expand Down Expand Up @@ -1425,20 +1426,23 @@ fn mk_emitter(output: ErrorOutputType) -> Box<DynEmitter> {
let fallback_bundle =
fallback_fluent_bundle(vec![rustc_errors::DEFAULT_LOCALE_RESOURCE], false);
let emitter: Box<DynEmitter> = match output {
config::ErrorOutputType::HumanReadable(kind) => {
let (short, color_config) = kind.unzip();
config::ErrorOutputType::HumanReadable(kind, color_config) => {
let short = matches!(kind, HumanReadableErrorType::Short);
Box::new(
HumanEmitter::new(stderr_destination(color_config), fallback_bundle)
.short_message(short),
)
}
config::ErrorOutputType::Json { pretty, json_rendered } => Box::new(JsonEmitter::new(
Box::new(io::BufWriter::new(io::stderr())),
Lrc::new(SourceMap::new(FilePathMapping::empty())),
fallback_bundle,
pretty,
json_rendered,
)),
config::ErrorOutputType::Json { pretty, json_rendered, color_config } => {
Box::new(JsonEmitter::new(
Box::new(io::BufWriter::new(io::stderr())),
Lrc::new(SourceMap::new(FilePathMapping::empty())),
fallback_bundle,
pretty,
json_rendered,
color_config,
))
}
};
emitter
}
Expand Down
5 changes: 3 additions & 2 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,10 @@ impl Options {
}

let color = config::parse_color(early_dcx, matches);
let config::JsonConfig { json_rendered, json_unused_externs, .. } =
let config::JsonConfig { json_rendered, json_unused_externs, json_color, .. } =
config::parse_json(early_dcx, matches);
let error_format = config::parse_error_format(early_dcx, matches, color, json_rendered);
let error_format =
config::parse_error_format(early_dcx, matches, color, json_color, json_rendered);
let diagnostic_width = matches.opt_get("diagnostic-width").unwrap_or_default();

let codegen_options = CodegenOptions::build(early_dcx, matches);
Expand Down
7 changes: 4 additions & 3 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ pub(crate) fn new_dcx(
false,
);
let emitter: Box<DynEmitter> = match error_format {
ErrorOutputType::HumanReadable(kind) => {
let (short, color_config) = kind.unzip();
ErrorOutputType::HumanReadable(kind, color_config) => {
let short = matches!(kind, HumanReadableErrorType::Short);
Box::new(
HumanEmitter::new(stderr_destination(color_config), fallback_bundle)
.sm(source_map.map(|sm| sm as _))
Expand All @@ -150,7 +150,7 @@ pub(crate) fn new_dcx(
.ui_testing(unstable_opts.ui_testing),
)
}
ErrorOutputType::Json { pretty, json_rendered } => {
ErrorOutputType::Json { pretty, json_rendered, color_config } => {
let source_map = source_map.unwrap_or_else(|| {
Lrc::new(source_map::SourceMap::new(source_map::FilePathMapping::empty()))
});
Expand All @@ -161,6 +161,7 @@ pub(crate) fn new_dcx(
fallback_bundle,
pretty,
json_rendered,
color_config,
)
.ui_testing(unstable_opts.ui_testing)
.diagnostic_width(diagnostic_width)
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/doctest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,8 +422,8 @@ fn run_test(
path_for_rustdoc.to_str().expect("target path must be valid unicode")
}
});
if let ErrorOutputType::HumanReadable(kind) = rustdoc_options.error_format {
let (short, color_config) = kind.unzip();
if let ErrorOutputType::HumanReadable(kind, color_config) = rustdoc_options.error_format {
let short = matches!(kind, HumanReadableErrorType::Short);

if short {
compiler.arg("--error-format").arg("short");
Expand Down

0 comments on commit 9ee3a14

Please sign in to comment.