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

Combine CLI Errors. #2487

Merged
merged 10 commits into from
Jun 20, 2019
Merged
Show file tree
Hide file tree
Changes from 5 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
8 changes: 0 additions & 8 deletions cli/ansi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,6 @@ pub fn red(s: String) -> impl fmt::Display {
style.paint(s)
}

pub fn grey(s: String) -> impl fmt::Display {
let mut style = Style::new();
if use_color() {
style = style.fg(Fixed(8));
}
style.paint(s)
}

pub fn bold(s: String) -> impl fmt::Display {
let mut style = Style::new();
if use_color() {
Expand Down
27 changes: 14 additions & 13 deletions cli/compiler.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
use crate::diagnostics::Diagnostic;
use crate::errors::err_check;
use crate::errors::DenoError;
use crate::msg;
use crate::resources;
use crate::startup_data;
use crate::state::*;
use crate::tokio_util;
use crate::worker::Worker;
use deno::js_check;
use deno::Buf;
use futures::Future;
use futures::Stream;
Expand Down Expand Up @@ -92,7 +93,7 @@ pub fn bundle_async(
state: ThreadSafeState,
module_name: String,
out_file: String,
) -> impl Future<Item = (), Error = Diagnostic> {
) -> impl Future<Item = (), Error = DenoError> {
debug!(
"Invoking the compiler to bundle. module_name: {}",
module_name
Expand All @@ -112,9 +113,9 @@ pub fn bundle_async(
// as was done previously.
state.clone(),
);
js_check(worker.execute("denoMain()"));
js_check(worker.execute("workerMain()"));
js_check(worker.execute("compilerMain()"));
err_check(worker.execute("denoMain()"));
err_check(worker.execute("workerMain()"));
err_check(worker.execute("compilerMain()"));

let resource = worker.state.resource.clone();
let compiler_rid = resource.rid;
Expand All @@ -140,7 +141,7 @@ pub fn bundle_async(
let json_str = std::str::from_utf8(&msg).unwrap();
debug!("Message: {}", json_str);
if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) {
return Err(diagnostics);
return Err(DenoError::from(diagnostics));
}
}

Expand All @@ -152,7 +153,7 @@ pub fn bundle_async(
pub fn compile_async(
state: ThreadSafeState,
module_meta_data: &ModuleMetaData,
) -> impl Future<Item = ModuleMetaData, Error = Diagnostic> {
) -> impl Future<Item = ModuleMetaData, Error = DenoError> {
let module_name = module_meta_data.module_name.clone();

debug!(
Expand All @@ -174,9 +175,9 @@ pub fn compile_async(
// as was done previously.
state.clone(),
);
js_check(worker.execute("denoMain()"));
js_check(worker.execute("workerMain()"));
js_check(worker.execute("compilerMain()"));
err_check(worker.execute("denoMain()"));
err_check(worker.execute("workerMain()"));
err_check(worker.execute("compilerMain()"));

let compiling_job = state.progress.add(format!("Compiling {}", module_name));

Expand Down Expand Up @@ -205,7 +206,7 @@ pub fn compile_async(
let json_str = std::str::from_utf8(&msg).unwrap();
debug!("Message: {}", json_str);
if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) {
return Err(diagnostics);
return Err(DenoError::from(diagnostics));
}
}

Expand Down Expand Up @@ -235,7 +236,7 @@ pub fn compile_async(
pub fn compile_sync(
state: ThreadSafeState,
module_meta_data: &ModuleMetaData,
) -> Result<ModuleMetaData, Diagnostic> {
) -> Result<ModuleMetaData, DenoError> {
tokio_util::block_on(compile_async(state, module_meta_data))
}

Expand Down Expand Up @@ -306,6 +307,6 @@ mod tests {
]);
let out =
bundle_async(state, module_name, String::from("$deno$/bundle.js"));
assert_eq!(tokio_util::block_on(out), Ok(()));
assert!(tokio_util::block_on(out).is_ok());
}
}
16 changes: 15 additions & 1 deletion cli/deno_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use crate::errors::DenoResult;
use crate::errors::ErrorKind;
use crate::fs as deno_fs;
use crate::http_util;
use crate::js_errors::SourceMapGetter;
use crate::msg;
use crate::progress::Progress;
use crate::source_maps::SourceMapGetter;
use crate::tokio_util;
use crate::version;
use dirs;
Expand Down Expand Up @@ -349,6 +349,20 @@ impl SourceMapGetter for DenoDir {
},
}
}

fn get_source_line(&self, script_name: &str, line: usize) -> Option<String> {
match self.fetch_module_meta_data(script_name, ".", true, true) {
Ok(out) => match str::from_utf8(&out.source_code) {
Ok(v) => {
let lines: Vec<&str> = v.lines().collect();
assert!(lines.len() > line);
Some(lines[line].to_string())
}
_ => None,
},
_ => None,
}
}
}

/// This fetches source code, locally or remotely.
Expand Down
125 changes: 32 additions & 93 deletions cli/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,13 @@
//! This module encodes TypeScript errors (diagnostics) into Rust structs and
//! contains code for printing them to the console.
use crate::ansi;
use crate::fmt_errors::format_maybe_source_line;
use crate::fmt_errors::format_maybe_source_name;
use crate::fmt_errors::DisplayFormatter;
use serde_json;
use serde_json::value::Value;
use std::fmt;

// A trait which specifies parts of a diagnostic like item needs to be able to
// generate to conform its display to other diagnostic like items
pub trait DisplayFormatter {
fn format_category_and_code(&self) -> String;
fn format_message(&self, level: usize) -> String;
fn format_related_info(&self) -> String;
fn format_source_line(&self, level: usize) -> String;
fn format_source_name(&self, level: usize) -> String;
}

#[derive(Debug, PartialEq, Clone)]
pub struct Diagnostic {
pub items: Vec<DiagnosticItem>,
Expand Down Expand Up @@ -179,23 +172,21 @@ impl DiagnosticItem {
}
}

// TODO should chare logic with cli/js_errors, possibly with JSError
// implementing the `DisplayFormatter` trait.
impl DisplayFormatter for DiagnosticItem {
fn format_category_and_code(&self) -> String {
let category = match self.category {
DiagnosticCategory::Error => {
format!("- {}", ansi::red("error".to_string()))
format!("{}", ansi::red_bold("error".to_string()))
}
DiagnosticCategory::Warning => "- warn".to_string(),
DiagnosticCategory::Debug => "- debug".to_string(),
DiagnosticCategory::Info => "- info".to_string(),
DiagnosticCategory::Warning => "warn".to_string(),
DiagnosticCategory::Debug => "debug".to_string(),
DiagnosticCategory::Info => "info".to_string(),
_ => "".to_string(),
};

let code = ansi::grey(format!(" TS{}:", self.code.to_string())).to_string();
let code = ansi::bold(format!(" TS{}", self.code.to_string())).to_string();

format!("{}{} ", category, code)
format!("{}{}: ", category, code)
}

fn format_message(&self, level: usize) -> String {
Expand Down Expand Up @@ -229,85 +220,35 @@ impl DisplayFormatter for DiagnosticItem {
for related_diagnostic in related_information {
let rd = &related_diagnostic;
s.push_str(&format!(
"\n{}{}{}\n",
rd.format_source_name(2),
"\n{}\n\n ► {}{}\n",
rd.format_message(2),
rd.format_source_name(),
rd.format_source_line(4),
rd.format_message(4),
));
}

s
}

fn format_source_line(&self, level: usize) -> String {
if self.source_line.is_none() {
return "".to_string();
}

let source_line = self.source_line.as_ref().unwrap();
// sometimes source_line gets set with an empty string, which then outputs
// an empty source line when displayed, so need just short circuit here
if source_line.is_empty() {
return "".to_string();
}

assert!(self.line_number.is_some());
assert!(self.start_column.is_some());
assert!(self.end_column.is_some());
let line = (1 + self.line_number.unwrap()).to_string();
let line_color = ansi::black_on_white(line.to_string());
let line_len = line.clone().len();
let line_padding =
ansi::black_on_white(format!("{:indent$}", "", indent = line_len))
.to_string();
let mut s = String::new();
let start_column = self.start_column.unwrap();
let end_column = self.end_column.unwrap();
// TypeScript uses `~` always, but V8 would utilise `^` always, even when
// doing ranges, so here, if we only have one marker (very common with V8
// errors) we will use `^` instead.
let underline_char = if (end_column - start_column) <= 1 {
'^'
} else {
'~'
};
for i in 0..end_column {
if i >= start_column {
s.push(underline_char);
} else {
s.push(' ');
}
}
let color_underline = match self.category {
DiagnosticCategory::Error => ansi::red(s).to_string(),
_ => ansi::cyan(s).to_string(),
};

let indent = format!("{:indent$}", "", indent = level);

format!(
"\n\n{}{} {}\n{}{} {}\n",
indent, line_color, source_line, indent, line_padding, color_underline
format_maybe_source_line(
self.source_line.clone(),
self.line_number,
self.start_column,
self.end_column,
match self.category {
DiagnosticCategory::Error => true,
_ => false,
},
level,
)
}

fn format_source_name(&self, level: usize) -> String {
if self.script_resource_name.is_none() {
return "".to_string();
}

let script_name = ansi::cyan(self.script_resource_name.clone().unwrap());
assert!(self.line_number.is_some());
assert!(self.start_column.is_some());
let line = ansi::yellow((1 + self.line_number.unwrap()).to_string());
let column = ansi::yellow((1 + self.start_column.unwrap()).to_string());
format!(
"{:indent$}{}:{}:{} ",
"",
script_name,
line,
column,
indent = level
fn format_source_name(&self) -> String {
format_maybe_source_name(
self.script_resource_name.clone(),
self.line_number,
self.start_column,
)
}
}
Expand All @@ -316,15 +257,13 @@ impl fmt::Display for DiagnosticItem {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
f,
"{}{}{}{}{}",
self.format_source_name(0),
"{}{}\n\n► {}{}{}",
self.format_category_and_code(),
self.format_message(0),
self.format_source_name(),
self.format_source_line(0),
self.format_related_info(),
)?;

Ok(())
)
}
}

Expand Down Expand Up @@ -655,14 +594,14 @@ mod tests {
#[test]
fn diagnostic_to_string1() {
let d = diagnostic1();
let expected = "deno/tests/complex_diagnostics.ts:19:3 - error TS2322: Type \'(o: T) => { v: any; f: (x: B) => string; }[]\' is not assignable to type \'(r: B) => Value<B>[]\'.\n Types of parameters \'o\' and \'r\' are incompatible.\n Type \'B\' is not assignable to type \'T\'.\n\n19 values: o => [\n ~~~~~~\n\n deno/tests/complex_diagnostics.ts:7:3 \n\n 7 values?: (r: T) => Array<Value<T>>;\n ~~~~~~\n The expected type comes from property \'values\' which is declared here on type \'SettingsInterface<B>\'\n";
let expected = "error TS2322: Type \'(o: T) => { v: any; f: (x: B) => string; }[]\' is not assignable to type \'(r: B) => Value<B>[]\'.\n Types of parameters \'o\' and \'r\' are incompatible.\n Type \'B\' is not assignable to type \'T\'.\n\n► deno/tests/complex_diagnostics.ts:19:3\n\n19 values: o => [\n ~~~~~~\n\n The expected type comes from property \'values\' which is declared here on type \'SettingsInterface<B>\'\n\n ► deno/tests/complex_diagnostics.ts:7:3\n\n 7 values?: (r: T) => Array<Value<T>>;\n ~~~~~~\n\n";
assert_eq!(expected, strip_ansi_codes(&d.to_string()));
}

#[test]
fn diagnostic_to_string2() {
let d = diagnostic2();
let expected = "deno/tests/complex_diagnostics.ts:19:3 - error TS2322: Example 1\n\n19 values: o => [\n ~~~~~~\n\n/foo/bar.ts:129:3 - error TS2000: Example 2\n\n129 values: undefined,\n ~~~~~~\n\n\nFound 2 errors.\n";
let expected = "error TS2322: Example 1\n\n► deno/tests/complex_diagnostics.ts:19:3\n\n19 values: o => [\n ~~~~~~\n\nerror TS2000: Example 2\n\n► /foo/bar.ts:129:3\n\n129 values: undefined,\n ~~~~~~\n\n\nFound 2 errors.\n";
assert_eq!(expected, strip_ansi_codes(&d.to_string()));
}
}
Loading