diff options
Diffstat (limited to 'cli')
| -rw-r--r-- | cli/Cargo.toml | 1 | ||||
| -rw-r--r-- | cli/fmt_errors.rs | 72 | ||||
| -rw-r--r-- | cli/main.rs | 27 | ||||
| -rw-r--r-- | cli/ops/errors.rs | 48 | ||||
| -rw-r--r-- | cli/proc_state.rs | 2 | ||||
| -rw-r--r-- | cli/source_maps.rs | 264 | ||||
| -rw-r--r-- | cli/standalone.rs | 2 | ||||
| -rw-r--r-- | cli/tests/integration/test_tests.rs | 2 | ||||
| -rw-r--r-- | cli/tests/testdata/future_check1.out | 2 | ||||
| -rw-r--r-- | cli/tests/testdata/inline_js_source_map_2_with_inline_contents.js.out | 2 | ||||
| -rw-r--r-- | cli/tests/testdata/test/aggregate_error.out | 1 | ||||
| -rw-r--r-- | cli/tools/coverage/mod.rs | 4 |
12 files changed, 28 insertions, 399 deletions
diff --git a/cli/Cargo.toml b/cli/Cargo.toml index c044d6c72..ed0885c0e 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -90,7 +90,6 @@ secure_tempfile = { version = "=3.2.0", package = "tempfile" } # different name semver-parser = "=0.10.2" serde = { version = "=1.0.136", features = ["derive"] } shell-escape = "=0.1.5" -sourcemap = "=6.0.1" text-size = "=1.1.0" text_lines = "=0.4.1" tokio = { version = "=1.17", features = ["full"] } diff --git a/cli/fmt_errors.rs b/cli/fmt_errors.rs index 106ecaaf2..fb7ccdb86 100644 --- a/cli/fmt_errors.rs +++ b/cli/fmt_errors.rs @@ -134,17 +134,17 @@ fn format_stack( message_line: &str, cause: Option<&str>, source_line: Option<&str>, - start_column: Option<i64>, - end_column: Option<i64>, + source_line_frame_index: Option<usize>, frames: &[JsStackFrame], level: usize, ) -> String { let mut s = String::new(); s.push_str(&format!("{:indent$}{}", "", message_line, indent = level)); + let column_number = + source_line_frame_index.and_then(|i| frames.get(i).unwrap().column_number); s.push_str(&format_maybe_source_line( source_line, - start_column, - end_column, + column_number, is_error, level, )); @@ -171,12 +171,11 @@ fn format_stack( /// a pretty printed version of that line. fn format_maybe_source_line( source_line: Option<&str>, - start_column: Option<i64>, - end_column: Option<i64>, + column_number: Option<i64>, is_error: bool, level: usize, ) -> String { - if source_line.is_none() || start_column.is_none() || end_column.is_none() { + if source_line.is_none() || column_number.is_none() { return "".to_string(); } @@ -191,37 +190,24 @@ fn format_maybe_source_line( return format!("\n{}", source_line); } - assert!(start_column.is_some()); - assert!(end_column.is_some()); let mut s = String::new(); - let start_column = start_column.unwrap(); - let end_column = end_column.unwrap(); + let column_number = column_number.unwrap(); - if start_column as usize >= source_line.len() { + if column_number as usize > source_line.len() { return format!( "\n{} Couldn't format source line: Column {} is out of bounds (source may have changed at runtime)", - crate::colors::yellow("Warning"), start_column + 1, + crate::colors::yellow("Warning"), column_number, ); } - // 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..start_column { + for _i in 0..(column_number - 1) { if source_line.chars().nth(_i as usize).unwrap() == '\t' { s.push('\t'); } else { s.push(' '); } } - for _i in 0..(end_column - start_column) { - s.push(underline_char); - } + s.push('^'); let color_underline = if is_error { red(&s).to_string() } else { @@ -254,24 +240,6 @@ impl Deref for PrettyJsError { impl fmt::Display for PrettyJsError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let mut frames = self.0.frames.clone(); - - // When the stack frame array is empty, but the source location given by - // (script_resource_name, line_number, start_column + 1) exists, this is - // likely a syntax error. For the sake of formatting we treat it like it was - // given as a single stack frame. - if frames.is_empty() - && self.0.script_resource_name.is_some() - && self.0.line_number.is_some() - && self.0.start_column.is_some() - { - frames = vec![JsStackFrame::from_location( - self.0.script_resource_name.clone(), - self.0.line_number, - self.0.start_column.map(|n| n + 1), - )]; - } - let cause = self .0 .cause @@ -286,9 +254,8 @@ impl fmt::Display for PrettyJsError { &self.0.exception_message, cause.as_deref(), self.0.source_line.as_deref(), - self.0.start_column, - self.0.end_column, - &frames, + self.0.source_line_frame_index, + &self.0.frames, 0 ) )?; @@ -305,22 +272,17 @@ mod tests { #[test] fn test_format_none_source_line() { - let actual = format_maybe_source_line(None, None, None, false, 0); + let actual = format_maybe_source_line(None, None, false, 0); assert_eq!(actual, ""); } #[test] fn test_format_some_source_line() { - let actual = format_maybe_source_line( - Some("console.log('foo');"), - Some(8), - Some(11), - true, - 0, - ); + let actual = + format_maybe_source_line(Some("console.log('foo');"), Some(9), true, 0); assert_eq!( strip_ansi_codes(&actual), - "\nconsole.log(\'foo\');\n ~~~" + "\nconsole.log(\'foo\');\n ^" ); } } diff --git a/cli/main.rs b/cli/main.rs index 689d5c634..320e4e8c3 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -29,7 +29,6 @@ mod module_loader; mod ops; mod proc_state; mod resolver; -mod source_maps; mod standalone; mod text_encoding; mod tools; @@ -71,7 +70,6 @@ use crate::module_loader::CliModuleLoader; use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; -use crate::source_maps::apply_source_map; use deno_ast::MediaType; use deno_core::error::generic_error; use deno_core::error::AnyError; @@ -127,13 +125,6 @@ fn create_web_worker_preload_module_callback( fn create_web_worker_callback(ps: ProcState) -> Arc<CreateWebWorkerCb> { Arc::new(move |args| { - let global_state_ = ps.clone(); - let js_error_create_fn = Rc::new(move |core_js_error| { - let source_mapped_error = - apply_source_map(&core_js_error, global_state_.clone()); - PrettyJsError::create(source_mapped_error) - }); - let maybe_inspector_server = ps.maybe_inspector_server.clone(); let module_loader = CliModuleLoader::new_for_worker( @@ -149,7 +140,6 @@ fn create_web_worker_callback(ps: ProcState) -> Arc<CreateWebWorkerCb> { let options = WebWorkerOptions { bootstrap: BootstrapOptions { args: ps.flags.argv.clone(), - apply_source_maps: true, cpu_count: std::thread::available_parallelism() .map(|p| p.get()) .unwrap_or(1), @@ -176,7 +166,8 @@ fn create_web_worker_callback(ps: ProcState) -> Arc<CreateWebWorkerCb> { module_loader, create_web_worker_cb, preload_module_cb, - js_error_create_fn: Some(js_error_create_fn), + source_map_getter: Some(Box::new(ps.clone())), + js_error_create_fn: Some(Rc::new(PrettyJsError::create)), use_deno_namespace: args.use_deno_namespace, worker_type: args.worker_type, maybe_inspector_server, @@ -206,14 +197,6 @@ pub fn create_main_worker( ) -> MainWorker { let module_loader = CliModuleLoader::new(ps.clone()); - let global_state_ = ps.clone(); - - let js_error_create_fn = Rc::new(move |core_js_error| { - let source_mapped_error = - apply_source_map(&core_js_error, global_state_.clone()); - PrettyJsError::create(source_mapped_error) - }); - let maybe_inspector_server = ps.maybe_inspector_server.clone(); let should_break_on_first_statement = ps.flags.inspect_brk.is_some(); @@ -252,7 +235,6 @@ pub fn create_main_worker( let options = WorkerOptions { bootstrap: BootstrapOptions { - apply_source_maps: true, args: ps.flags.argv.clone(), cpu_count: std::thread::available_parallelism() .map(|p| p.get()) @@ -274,7 +256,8 @@ pub fn create_main_worker( root_cert_store: ps.root_cert_store.clone(), user_agent: version::get_user_agent(), seed: ps.flags.seed, - js_error_create_fn: Some(js_error_create_fn), + source_map_getter: Some(Box::new(ps.clone())), + js_error_create_fn: Some(Rc::new(PrettyJsError::create)), create_web_worker_cb, web_worker_preload_module_cb, maybe_inspector_server, @@ -1168,7 +1151,7 @@ async fn run_command( run_flags: RunFlags, ) -> Result<i32, AnyError> { if !flags.has_check_flag && flags.type_check_mode == TypeCheckMode::All { - info!("{} In future releases `deno run` will not automatically type check without the --check flag. + info!("{} In future releases `deno run` will not automatically type check without the --check flag. To opt into this new behavior now, specify DENO_FUTURE_CHECK=1.", colors::yellow("Warning")); } diff --git a/cli/ops/errors.rs b/cli/ops/errors.rs index a219b462d..f0f5fce04 100644 --- a/cli/ops/errors.rs +++ b/cli/ops/errors.rs @@ -2,70 +2,22 @@ use crate::diagnostics::Diagnostics; use crate::fmt_errors::format_file_name; -use crate::proc_state::ProcState; -use crate::source_maps::get_orig_position; -use crate::source_maps::CachedMaps; use deno_core::error::AnyError; use deno_core::op; use deno_core::serde_json; use deno_core::serde_json::json; use deno_core::serde_json::Value; use deno_core::Extension; -use deno_core::OpState; -use serde::Deserialize; -use serde::Serialize; -use std::collections::HashMap; pub fn init() -> Extension { Extension::builder() .ops(vec![ - op_apply_source_map::decl(), op_format_diagnostic::decl(), op_format_file_name::decl(), ]) .build() } -#[derive(Deserialize)] -#[serde(rename_all = "camelCase")] -struct ApplySourceMap { - file_name: String, - line_number: i32, - column_number: i32, -} - -#[derive(Serialize)] -#[serde(rename_all = "camelCase")] -struct AppliedSourceMap { - file_name: String, - line_number: u32, - column_number: u32, -} - -#[op] -fn op_apply_source_map( - state: &mut OpState, - args: ApplySourceMap, -) -> Result<AppliedSourceMap, AnyError> { - let mut mappings_map: CachedMaps = HashMap::new(); - let ps = state.borrow::<ProcState>().clone(); - - let (orig_file_name, orig_line_number, orig_column_number, _) = - get_orig_position( - args.file_name, - args.line_number.into(), - args.column_number.into(), - &mut mappings_map, - ps, - ); - - Ok(AppliedSourceMap { - file_name: orig_file_name, - line_number: orig_line_number as u32, - column_number: orig_column_number as u32, - }) -} - #[op] fn op_format_diagnostic(args: Value) -> Result<Value, AnyError> { let diagnostic: Diagnostics = serde_json::from_value(args)?; diff --git a/cli/proc_state.rs b/cli/proc_state.rs index fc132dadc..d4173ee2e 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -20,7 +20,6 @@ use crate::lockfile::as_maybe_locker; use crate::lockfile::Lockfile; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; -use crate::source_maps::SourceMapGetter; use crate::version; use deno_ast::MediaType; @@ -38,6 +37,7 @@ use deno_core::ModuleSource; use deno_core::ModuleSpecifier; use deno_core::ModuleType; use deno_core::SharedArrayBufferStore; +use deno_core::SourceMapGetter; use deno_graph::create_graph; use deno_graph::source::CacheInfo; use deno_graph::source::LoadFuture; diff --git a/cli/source_maps.rs b/cli/source_maps.rs deleted file mode 100644 index d92caf7f4..000000000 --- a/cli/source_maps.rs +++ /dev/null @@ -1,264 +0,0 @@ -// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. - -//! This mod provides functions to remap a `JsError` based on a source map. - -use deno_core::error::JsError; -use sourcemap::SourceMap; -use std::collections::HashMap; -use std::str; - -pub trait SourceMapGetter: Sync + Send + Clone { - /// Returns the raw source map file. - fn get_source_map(&self, file_name: &str) -> Option<Vec<u8>>; - fn get_source_line( - &self, - file_name: &str, - line_number: usize, - ) -> Option<String>; -} - -/// Cached filename lookups. The key can be None if a previous lookup failed to -/// find a SourceMap. -pub type CachedMaps = HashMap<String, Option<SourceMap>>; - -/// Apply a source map to a `deno_core::JsError`, returning a `JsError` where -/// file names and line/column numbers point to the location in the original -/// source, rather than the transpiled source code. -pub fn apply_source_map<G: SourceMapGetter>( - js_error: &JsError, - getter: G, -) -> JsError { - // Note that js_error.frames has already been source mapped in - // prepareStackTrace(). - let mut mappings_map: CachedMaps = HashMap::new(); - - let (script_resource_name, line_number, start_column, source_line) = - get_maybe_orig_position( - js_error.script_resource_name.clone(), - js_error.line_number, - // start_column is 0-based, we need 1-based here. - js_error.start_column.map(|n| n + 1), - &mut mappings_map, - getter.clone(), - ); - let start_column = start_column.map(|n| n - 1); - // It is better to just move end_column to be the same distance away from - // start column because sometimes the code point is not available in the - // source file map. - let end_column = match js_error.end_column { - Some(ec) => { - start_column.map(|sc| ec - (js_error.start_column.unwrap() - sc)) - } - _ => None, - }; - - let mut script_resource_name = script_resource_name; - let mut line_number = line_number; - let mut start_column = start_column; - let mut end_column = end_column; - let mut source_line = source_line; - if script_resource_name - .as_ref() - .map_or(true, |s| s.starts_with("deno:")) - { - for frame in &js_error.frames { - if let (Some(file_name), Some(line_number_)) = - (&frame.file_name, frame.line_number) - { - if !file_name.starts_with("deno:") { - script_resource_name = Some(file_name.clone()); - line_number = Some(line_number_); - start_column = frame.column_number.map(|n| n - 1); - end_column = frame.column_number; - // Lookup expects 0-based line and column numbers, but ours are 1-based. - source_line = - getter.get_source_line(file_name, (line_number_ - 1) as usize); - break; - } - } - } - } - - let cause = js_error - .cause - .clone() - .map(|cause| Box::new(apply_source_map(&*cause, getter))); - - JsError { - name: js_error.name.clone(), - message: js_error.message.clone(), - exception_message: js_error.exception_message.clone(), - cause, - source_line, - script_resource_name, - line_number, - start_column, - end_column, - frames: js_error.frames.clone(), - stack: None, - } -} - -fn get_maybe_orig_position<G: SourceMapGetter>( - file_name: Option<String>, - line_number: Option<i64>, - column_number: Option<i64>, - mappings_map: &mut CachedMaps, - getter: G, -) -> (Option<String>, Option<i64>, Option<i64>, Option<String>) { - match (file_name, line_number, column_number) { - (Some(file_name_v), Some(line_v), Some(column_v)) => { - let (file_name, line_number, column_number, source_line) = - get_orig_position(file_name_v, line_v, column_v, mappings_map, getter); - ( - Some(file_name), - Some(line_number), - Some(column_number), - source_line, - ) - } - _ => (None, None, None, None), - } -} - -pub fn get_orig_position<G: SourceMapGetter>( - file_name: String, - line_number: i64, - column_number: i64, - mappings_map: &mut CachedMaps, - getter: G, -) -> (String, i64, i64, Option<String>) { - // Lookup expects 0-based line and column numbers, but ours are 1-based. - let line_number = line_number - 1; - let column_number = column_number - 1; - - let default_pos = (file_name.clone(), line_number, column_number, None); - let maybe_source_map = get_mappings(&file_name, mappings_map, getter.clone()); - let (file_name, line_number, column_number, source_line) = - match maybe_source_map { - None => default_pos, - Some(source_map) => { - match source_map.lookup_token(line_number as u32, column_number as u32) - { - None => default_pos, - Some(token) => match token.get_source() { - None => default_pos, - Some(source_file_name) => { - // The `source_file_name` written by tsc in the source map is - // sometimes only the basename of the URL, or has unwanted `<`/`>` - // around it. Use the `file_name` we get from V8 if - // `source_file_name` does not parse as a URL. - let file_name = match deno_core::resolve_url(source_file_name) { - Ok(m) if m.scheme() == "blob" => file_name, - Ok(m) => m.to_string(), - Err(_) => file_name, - }; - let source_line = - if let Some(source_view) = token.get_source_view() { - source_view - .get_line(token.get_src_line()) - .map(|s| s.to_string()) - } else { - None - }; - ( - file_name, - i64::from(token.get_src_line()), - i64::from(token.get_src_col()), - source_line, - ) - } - }, - } - } - }; - let source_line = source_line - .or_else(|| getter.get_source_line(&file_name, line_number as usize)); - (file_name, line_number + 1, column_number + 1, source_line) -} - -fn get_mappings<'a, G: SourceMapGetter>( - file_name: &str, - mappings_map: &'a mut CachedMaps, - getter: G, -) -> &'a Option<SourceMap> { - mappings_map - .entry(file_name.to_string()) - .or_insert_with(|| parse_map_string(file_name, getter)) -} - -// TODO(kitsonk) parsed source maps should probably be cached in state in -// the module meta data. -fn parse_map_string<G: SourceMapGetter>( - file_name: &str, - getter: G, -) -> Option<SourceMap> { - getter - .get_source_map(file_name) - .and_then(|raw_source_map| SourceMap::from_slice(&raw_source_map).ok()) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[derive(Clone)] - struct MockSourceMapGetter {} - - impl SourceMapGetter for MockSourceMapGetter { - fn get_source_map(&self, file_name: &str) -> Option<Vec<u8>> { - let s = match file_name { - "foo_bar.ts" => { - r#"{"sources": ["foo_bar.ts"], "mappings":";;;IAIA,OAAO,CAAC,GAAG,CAAC,qBAAqB,EAAE,EAAE,CAAC,OAAO,CAAC,CAAC;IAC/C,OAAO,CAAC,GAAG,CAAC,eAAe,EAAE,IAAI,CAAC,QAAQ,CAAC,IAAI,CAAC,CAAC;IACjD,OAAO,CAAC,GAAG,CAAC,WAAW,EAAE,IAAI,CAAC,QAAQ,CAAC,EAAE,CAAC,CAAC;IAE3C,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC"}"# - } - "bar_baz.ts" => { - r#"{"sources": ["bar_baz.ts"], "mappings":";;;IAEA,CAAC,KAAK,IAAI,EAAE;QACV,MAAM,GAAG,GAAG,sDAAa,OAAO,2BAAC,CAAC;QAClC,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC;IACnB,CAAC,CAAC,EAAE,CAAC;IAEQ,QAAA,GAAG,GAAG,KAAK,CAAC;IAEzB,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC"}"# - } - _ => return None, - }; - Some(s.as_bytes().to_owned()) - } - - fn get_source_line( - &self, - file_name: &str, - line_number: usize, - ) -> Option<String> { - let s = match file_name { - "foo_bar.ts" => vec![ - "console.log('foo');", - "console.log('foo');", - "console.log('foo');", - "console.log('foo');", - "console.log('foo');", - ], - _ => return None, - }; - if s.len() > line_number { - Some(s[line_number].to_string()) - } else { - None - } - } - } - - #[test] - fn apply_source_map_line() { - let e = JsError { - name: Some("TypeError".to_string()), - message: Some("baz".to_string()), - exception_message: "TypeError: baz".to_string(), - cause: None, - source_line: Some("foo".to_string()), - script_resource_name: Some("foo_bar.ts".to_string()), - line_number: Some(4), - start_column: Some(16), - end_column: None, - frames: vec![], - stack: None, - }; - let getter = MockSourceMapGetter {}; - let actual = apply_source_map(&e, getter); - assert_eq!(actual.source_line, Some("console.log('foo');".to_string())); - } -} diff --git a/cli/standalone.rs b/cli/standalone.rs index 5005fd142..9f2aba9bd 100644 --- a/cli/standalone.rs +++ b/cli/standalone.rs @@ -273,7 +273,6 @@ pub async fn run( let options = WorkerOptions { bootstrap: BootstrapOptions { - apply_source_maps: false, args: metadata.argv, cpu_count: std::thread::available_parallelism() .map(|p| p.get()) @@ -293,6 +292,7 @@ pub async fn run( .unsafely_ignore_certificate_errors, root_cert_store: Some(root_cert_store), seed: metadata.seed, + source_map_getter: None, js_error_create_fn: None, create_web_worker_cb, web_worker_preload_module_cb, diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs index bf0ceb038..6a0d5c1ab 100644 --- a/cli/tests/integration/test_tests.rs +++ b/cli/tests/integration/test_tests.rs @@ -246,7 +246,7 @@ itest!(shuffle_with_seed { }); itest!(aggregate_error { - args: "test test/aggregate_error.ts", + args: "test --quiet test/aggregate_error.ts", exit_code: 1, output: "test/aggregate_error.out", }); diff --git a/cli/tests/testdata/future_check1.out b/cli/tests/testdata/future_check1.out index 9c7592fc5..7505efcd4 100644 --- a/cli/tests/testdata/future_check1.out +++ b/cli/tests/testdata/future_check1.out @@ -1,3 +1,3 @@ -Warning In future releases `deno run` will not automatically type check without the --check flag. +Warning In future releases `deno run` will not automatically type check without the --check flag. To opt into this new behavior now, specify DENO_FUTURE_CHECK=1. Check [WILDCARD]/future_check.ts diff --git a/cli/tests/testdata/inline_js_source_map_2_with_inline_contents.js.out b/cli/tests/testdata/inline_js_source_map_2_with_inline_contents.js.out index 9280361f7..4f9127da6 100644 --- a/cli/tests/testdata/inline_js_source_map_2_with_inline_contents.js.out +++ b/cli/tests/testdata/inline_js_source_map_2_with_inline_contents.js.out @@ -1,4 +1,2 @@ error: Uncaught Error: Hello world! -throw new Error("Hello world!" as unknown as string); - ^ at http://localhost:4545/inline_js_source_map_2.ts:6:7 diff --git a/cli/tests/testdata/test/aggregate_error.out b/cli/tests/testdata/test/aggregate_error.out index 1fbdcb350..35349e67c 100644 --- a/cli/tests/testdata/test/aggregate_error.out +++ b/cli/tests/testdata/test/aggregate_error.out @@ -1,4 +1,3 @@ -Check [WILDCARD]/testdata/test/aggregate_error.ts running 1 test from test/aggregate_error.ts aggregate ... FAILED ([WILDCARD]) diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index 092f6c825..338779583 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -5,7 +5,6 @@ use crate::flags::CoverageFlags; use crate::flags::Flags; use crate::fs_util::collect_files; use crate::proc_state::ProcState; -use crate::source_maps::SourceMapGetter; use crate::tools::fmt::format_json; use deno_ast::MediaType; @@ -14,10 +13,11 @@ use deno_core::anyhow::anyhow; use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::serde_json; +use deno_core::sourcemap::SourceMap; use deno_core::url::Url; use deno_core::LocalInspectorSession; +use deno_core::SourceMapGetter; use regex::Regex; -use sourcemap::SourceMap; use std::fs; use std::fs::File; use std::io::BufWriter; |