From ba239ce4ab6066d8a8dfc6bba8ce8886d54ab391 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 24 Sep 2020 15:03:51 -0700 Subject: [PATCH] Make query error line numbers consistently display 1-indexed --- cli/src/loader.rs | 88 ++++++++++++++++++++++++------------- cli/src/tags.rs | 7 ++- cli/src/tests/query_test.rs | 28 ++++++------ lib/binding_rust/lib.rs | 14 +++--- tags/src/lib.rs | 33 +++++++------- 5 files changed, 102 insertions(+), 68 deletions(-) diff --git a/cli/src/loader.rs b/cli/src/loader.rs index 3d026f21..3d5a9377 100644 --- a/cli/src/loader.rs +++ b/cli/src/loader.rs @@ -13,7 +13,7 @@ use std::time::SystemTime; use std::{fs, mem}; use tree_sitter::{Language, QueryError}; use tree_sitter_highlight::HighlightConfiguration; -use tree_sitter_tags::TagsConfiguration; +use tree_sitter_tags::{Error as TagsError, TagsConfiguration}; #[cfg(unix)] const DYLIB_EXTENSION: &'static str = "so"; @@ -544,25 +544,8 @@ impl Loader { impl<'a> LanguageConfiguration<'a> { pub fn highlight_config(&self, language: Language) -> Result> { - fn include_path_in_error<'a>( - mut error: QueryError, - ranges: &'a Vec<(String, Range)>, - source: &str, - start_offset: usize, - ) -> (&'a str, QueryError) { - let offset = error.offset - start_offset; - let (path, range) = ranges - .iter() - .find(|(_, range)| range.contains(&offset)) - .unwrap(); - error.row = source[range.start..offset] - .chars() - .filter(|c| *c == '\n') - .count(); - (path.as_ref(), error) - } - - self.highlight_config + return self + .highlight_config .get_or_try_init(|| { let (highlights_query, highlight_ranges) = self.read_queries(&self.highlights_filenames, "highlights.scm")?; @@ -582,16 +565,21 @@ impl<'a> LanguageConfiguration<'a> { ) .map_err(|error| { if error.offset < injections_query.len() { - include_path_in_error(error, &injection_ranges, &injections_query, 0) + Self::include_path_in_query_error( + error, + &injection_ranges, + &injections_query, + 0, + ) } else if error.offset < injections_query.len() + locals_query.len() { - include_path_in_error( + Self::include_path_in_query_error( error, &locals_ranges, &locals_query, injections_query.len(), ) } else { - include_path_in_error( + Self::include_path_in_query_error( error, &highlight_ranges, &highlights_query, @@ -611,27 +599,67 @@ impl<'a> LanguageConfiguration<'a> { Ok(Some(result)) } }) - .map(Option::as_ref) + .map(Option::as_ref); } pub fn tags_config(&self, language: Language) -> Result> { self.tags_config .get_or_try_init(|| { - let (tags_query, _) = self.read_queries(&self.tags_filenames, "tags.scm")?; - let (locals_query, _) = self.read_queries(&self.locals_filenames, "locals.scm")?; + let (tags_query, tags_ranges) = + self.read_queries(&self.tags_filenames, "tags.scm")?; + let (locals_query, locals_ranges) = + self.read_queries(&self.locals_filenames, "locals.scm")?; if tags_query.is_empty() { Ok(None) } else { TagsConfiguration::new(language, &tags_query, &locals_query) - .map_err(Error::wrap(|| { - format!("Failed to load queries in {:?}", self.root_path) - })) - .map(|config| Some(config)) + .map(Some) + .map_err(|error| { + if let TagsError::Query(error) = error { + if error.offset < locals_query.len() { + Self::include_path_in_query_error( + error, + &locals_ranges, + &locals_query, + 0, + ) + } else { + Self::include_path_in_query_error( + error, + &tags_ranges, + &tags_query, + locals_query.len(), + ) + } + .into() + } else { + error.into() + } + }) } }) .map(Option::as_ref) } + fn include_path_in_query_error<'b>( + mut error: QueryError, + ranges: &'b Vec<(String, Range)>, + source: &str, + start_offset: usize, + ) -> (&'b str, QueryError) { + let offset_within_section = error.offset - start_offset; + let (path, range) = ranges + .iter() + .find(|(_, range)| range.contains(&offset_within_section)) + .unwrap(); + error.offset = offset_within_section - range.start; + error.row = source[range.start..offset_within_section] + .chars() + .filter(|c| *c == '\n') + .count(); + (path.as_ref(), error) + } + fn read_queries( &self, paths: &Option>, diff --git a/cli/src/tags.rs b/cli/src/tags.rs index 122b58d2..802d8d06 100644 --- a/cli/src/tags.rs +++ b/cli/src/tags.rs @@ -53,7 +53,10 @@ pub fn generate_tags( let source = fs::read(path)?; let t0 = Instant::now(); - for tag in context.generate_tags(tags_config, &source, Some(&cancellation_flag))?.0 { + for tag in context + .generate_tags(tags_config, &source, Some(&cancellation_flag))? + .0 + { let tag = tag?; if !quiet { write!( @@ -69,7 +72,7 @@ pub fn generate_tags( )?; if let Some(docs) = tag.docs { if docs.len() > 120 { - write!(&mut stdout, "\t{:?}...", &docs[0..120])?; + write!(&mut stdout, "\t{:?}...", docs.get(0..120).unwrap_or(""))?; } else { write!(&mut stdout, "\t{:?}", &docs)?; } diff --git a/cli/src/tests/query_test.rs b/cli/src/tests/query_test.rs index 2b816bbc..efdaf780 100644 --- a/cli/src/tests/query_test.rs +++ b/cli/src/tests/query_test.rs @@ -130,7 +130,7 @@ fn test_query_errors_on_invalid_symbols() { assert_eq!( Query::new(language, "(clas)").unwrap_err(), QueryError { - row: 1, + row: 0, offset: 1, column: 1, kind: QueryErrorKind::NodeType, @@ -140,7 +140,7 @@ fn test_query_errors_on_invalid_symbols() { assert_eq!( Query::new(language, "(if_statement (arrayyyyy))").unwrap_err(), QueryError { - row: 1, + row: 0, offset: 15, column: 15, kind: QueryErrorKind::NodeType, @@ -150,7 +150,7 @@ fn test_query_errors_on_invalid_symbols() { assert_eq!( Query::new(language, "(if_statement condition: (non_existent3))").unwrap_err(), QueryError { - row: 1, + row: 0, offset: 26, column: 26, kind: QueryErrorKind::NodeType, @@ -160,7 +160,7 @@ fn test_query_errors_on_invalid_symbols() { assert_eq!( Query::new(language, "(if_statement condit: (identifier))").unwrap_err(), QueryError { - row: 1, + row: 0, offset: 14, column: 14, kind: QueryErrorKind::Field, @@ -170,7 +170,7 @@ fn test_query_errors_on_invalid_symbols() { assert_eq!( Query::new(language, "(if_statement conditioning: (identifier))").unwrap_err(), QueryError { - row: 1, + row: 0, offset: 14, column: 14, kind: QueryErrorKind::Field, @@ -189,7 +189,7 @@ fn test_query_errors_on_invalid_predicates() { Query::new(language, "((identifier) @id (@id))").unwrap_err(), QueryError { kind: QueryErrorKind::Syntax, - row: 1, + row: 0, column: 19, offset: 19, message: [ @@ -214,7 +214,7 @@ fn test_query_errors_on_invalid_predicates() { Query::new(language, "((identifier) @id (#eq? @id @ok))").unwrap_err(), QueryError { kind: QueryErrorKind::Capture, - row: 1, + row: 0, column: 29, offset: 29, message: "ok".to_string(), @@ -236,7 +236,7 @@ fn test_query_errors_on_impossible_patterns() { ), Err(QueryError { kind: QueryErrorKind::Structure, - row: 1, + row: 0, offset: 38, column: 38, message: [ @@ -256,7 +256,7 @@ fn test_query_errors_on_impossible_patterns() { Query::new(js_lang, "(function_declaration name: (statement_block))"), Err(QueryError { kind: QueryErrorKind::Structure, - row: 1, + row: 0, offset: 22, column: 22, message: [ @@ -272,7 +272,7 @@ fn test_query_errors_on_impossible_patterns() { Query::new(rb_lang, "(call receiver:(binary))"), Err(QueryError { kind: QueryErrorKind::Structure, - row: 1, + row: 0, offset: 6, column: 6, message: [ @@ -303,7 +303,7 @@ fn test_query_errors_on_impossible_patterns() { ), Err(QueryError { kind: QueryErrorKind::Structure, - row: 3, + row: 2, offset: 88, column: 42, message: [ @@ -318,7 +318,7 @@ fn test_query_errors_on_impossible_patterns() { Query::new(js_lang, "(identifier (identifier))",), Err(QueryError { kind: QueryErrorKind::Structure, - row: 1, + row: 0, offset: 12, column: 12, message: [ @@ -332,7 +332,7 @@ fn test_query_errors_on_impossible_patterns() { Query::new(js_lang, "(true (true))",), Err(QueryError { kind: QueryErrorKind::Structure, - row: 1, + row: 0, offset: 6, column: 6, message: [ @@ -354,7 +354,7 @@ fn test_query_errors_on_impossible_patterns() { Query::new(js_lang, "(if_statement condition: (_expression))",), Err(QueryError { kind: QueryErrorKind::Structure, - row: 1, + row: 0, offset: 14, column: 14, message: [ diff --git a/lib/binding_rust/lib.rs b/lib/binding_rust/lib.rs index 75ed361f..97d10d13 100644 --- a/lib/binding_rust/lib.rs +++ b/lib/binding_rust/lib.rs @@ -1174,16 +1174,16 @@ impl Query { let offset = error_offset as usize; let mut line_start = 0; let mut row = 0; - let line_containing_error = source.split("\n").find_map(|line| { - row += 1; + let mut line_containing_error = None; + for line in source.split("\n") { let line_end = line_start + line.len() + 1; if line_end > offset { - Some(line) - } else { - line_start = line_end; - None + line_containing_error = Some(line); + break; } - }); + line_start = line_end; + row += 1; + } let column = offset - line_start; let kind; diff --git a/tags/src/lib.rs b/tags/src/lib.rs index dd55d4be..576b04f8 100644 --- a/tags/src/lib.rs +++ b/tags/src/lib.rs @@ -271,21 +271,24 @@ impl TagsContext { .matches(&config.query, tree_ref.root_node(), move |node| { &source[node.byte_range()] }); - Ok((TagsIter { - _tree: tree, - matches, - source, - config, - cancellation_flag, - prev_line_info: None, - tag_queue: Vec::new(), - iter_count: 0, - scopes: vec![LocalScope { - range: 0..source.len(), - inherits: false, - local_defs: Vec::new(), - }], - }, tree_ref.root_node().has_error())) + Ok(( + TagsIter { + _tree: tree, + matches, + source, + config, + cancellation_flag, + prev_line_info: None, + tag_queue: Vec::new(), + iter_count: 0, + scopes: vec![LocalScope { + range: 0..source.len(), + inherits: false, + local_defs: Vec::new(), + }], + }, + tree_ref.root_node().has_error(), + )) } }