Skip to content

Commit

Permalink
Rollup merge of #128806 - estebank:color-config, r=jieyouxu
Browse files Browse the repository at this point in the history
Split `ColorConfig` off of `HumanReadableErrorType`

The previous setup tied two unrelated things together. Splitting these two is a better model.

Identified by https://github.com/rust-lang/rust/pull/126597/files#r1667800754
  • Loading branch information
matthiaskrgr authored Aug 8, 2024
2 parents bcf6f9f + 95c1c34 commit f106496
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 50 deletions.
15 changes: 5 additions & 10 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,14 @@ 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),
Default,
AnnotateSnippet,
Short,
}

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),
}
pub fn short(&self) -> bool {
*self == HumanReadableErrorType::Short
}
}

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(_))
!self.json_rendered.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 = je.json_rendered.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,
color_config: 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 = kind.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 = kind.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 = kind.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 = kind.short();

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

0 comments on commit f106496

Please sign in to comment.