From ae696f847dee523df6afed14468de70fc6479552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 8 Aug 2024 01:46:16 +0000 Subject: [PATCH 1/2] Split `ColorConfig` off of `HumanReadableErrorType` The previous setup tied two unrelated things together. Splitting these two is a better model. --- compiler/rustc_errors/src/emitter.rs | 17 ++--------- compiler/rustc_errors/src/json.rs | 9 ++++-- compiler/rustc_errors/src/json/tests.rs | 3 +- compiler/rustc_interface/src/tests.rs | 3 +- compiler/rustc_session/src/config.rs | 38 +++++++++++++++---------- compiler/rustc_session/src/session.rs | 30 ++++++++++--------- src/librustdoc/config.rs | 5 ++-- src/librustdoc/core.rs | 7 +++-- src/librustdoc/doctest.rs | 4 +-- 9 files changed, 62 insertions(+), 54 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 88ed3128164d1..5bd3b328cb8d0 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -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)] diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 42a28bc789013..02a0de201edd4 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -55,6 +55,7 @@ pub struct JsonEmitter { ignored_directories_in_source_blocks: Vec, #[setters(skip)] json_rendered: HumanReadableErrorType, + color_config: ColorConfig, diagnostic_width: Option, macro_backtrace: bool, track_diagnostics: bool, @@ -68,6 +69,7 @@ impl JsonEmitter { fallback_bundle: LazyFallbackBundle, pretty: bool, json_rendered: HumanReadableErrorType, + color_config: ColorConfig, ) -> JsonEmitter { JsonEmitter { dst: IntoDynSyncSend(dst), @@ -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, @@ -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) } } @@ -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 => {} } diff --git a/compiler/rustc_errors/src/json/tests.rs b/compiler/rustc_errors/src/json/tests.rs index e3549fc3aa537..6af376d7afdb2 100644 --- a/compiler/rustc_errors/src/json/tests.rs +++ b/compiler/rustc_errors/src/json/tests.rs @@ -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)); diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index ce3b2f77f210a..34f2dca7c42ff 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -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, + color_config: ColorConfig::Never, }; let push = |opts: &mut Options, search_path| { diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 20c14a985024c..95d171409d86d 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -602,7 +602,7 @@ 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). @@ -610,12 +610,13 @@ pub enum ErrorOutputType { /// 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) } } @@ -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, @@ -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; @@ -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, @@ -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 @@ -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 \ @@ -1737,7 +1744,7 @@ pub fn parse_error_format( } } } else { - ErrorOutputType::HumanReadable(HumanReadableErrorType::Default(color)) + ErrorOutputType::HumanReadable(HumanReadableErrorType::Default, color) }; match error_format { @@ -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"); @@ -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); diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index be67baf57f6dc..c0fa377f8ba52 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -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, @@ -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) @@ -1425,20 +1426,23 @@ fn mk_emitter(output: ErrorOutputType) -> Box { let fallback_bundle = fallback_fluent_bundle(vec![rustc_errors::DEFAULT_LOCALE_RESOURCE], false); let emitter: Box = 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 } diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index e4549796b3e83..9c7a9f8467f5a 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -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); diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index fea31e7ecbc9a..ae3456e139991 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -138,8 +138,8 @@ pub(crate) fn new_dcx( false, ); let emitter: Box = 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 _)) @@ -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())) }); @@ -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) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 41eb142dd1e29..6804439604814 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -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"); From 95c1c34fff6675910986c93c3fea3c03e9810f6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 8 Aug 2024 14:43:27 +0000 Subject: [PATCH 2/2] review comments --- compiler/rustc_errors/src/emitter.rs | 6 ++++++ compiler/rustc_errors/src/json.rs | 4 ++-- compiler/rustc_session/src/session.rs | 4 ++-- src/librustdoc/core.rs | 2 +- src/librustdoc/doctest.rs | 2 +- 5 files changed, 12 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 5bd3b328cb8d0..9f4d9263f0f40 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -48,6 +48,12 @@ pub enum HumanReadableErrorType { Short, } +impl HumanReadableErrorType { + pub fn short(&self) -> bool { + *self == HumanReadableErrorType::Short + } +} + #[derive(Clone, Copy, Debug)] struct Margin { /// The available whitespace in the left that can be consumed when centering. diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 02a0de201edd4..32e59f9ab036a 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -176,7 +176,7 @@ impl Emitter for JsonEmitter { } fn should_show_explain(&self) -> bool { - !matches!(self.json_rendered, HumanReadableErrorType::Short) + !self.json_rendered.short() } } @@ -356,7 +356,7 @@ impl Diagnostic { let buf = BufWriter::default(); let mut dst: Destination = Box::new(buf.clone()); - let short = matches!(je.json_rendered, HumanReadableErrorType::Short); + let short = je.json_rendered.short(); match je.color_config { ColorConfig::Always | ColorConfig::Auto => dst = Box::new(termcolor::Ansi::new(dst)), ColorConfig::Never => {} diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index c0fa377f8ba52..672dddf871e02 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -951,7 +951,7 @@ fn default_emitter( }; match sopts.error_format { config::ErrorOutputType::HumanReadable(kind, color_config) => { - let short = matches!(kind, HumanReadableErrorType::Short); + let short = kind.short(); if let HumanReadableErrorType::AnnotateSnippet = kind { let emitter = AnnotateSnippetEmitter::new( @@ -1427,7 +1427,7 @@ fn mk_emitter(output: ErrorOutputType) -> Box { fallback_fluent_bundle(vec![rustc_errors::DEFAULT_LOCALE_RESOURCE], false); let emitter: Box = match output { config::ErrorOutputType::HumanReadable(kind, color_config) => { - let short = matches!(kind, HumanReadableErrorType::Short); + let short = kind.short(); Box::new( HumanEmitter::new(stderr_destination(color_config), fallback_bundle) .short_message(short), diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index ae3456e139991..08a4a3f3fb264 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -139,7 +139,7 @@ pub(crate) fn new_dcx( ); let emitter: Box = match error_format { ErrorOutputType::HumanReadable(kind, color_config) => { - let short = matches!(kind, HumanReadableErrorType::Short); + let short = kind.short(); Box::new( HumanEmitter::new(stderr_destination(color_config), fallback_bundle) .sm(source_map.map(|sm| sm as _)) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 6804439604814..08d6a5a52b21a 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -423,7 +423,7 @@ fn run_test( } }); if let ErrorOutputType::HumanReadable(kind, color_config) = rustdoc_options.error_format { - let short = matches!(kind, HumanReadableErrorType::Short); + let short = kind.short(); if short { compiler.arg("--error-format").arg("short");