diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 5403144b..8b72e191 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -31,6 +31,8 @@ jobs: --allow clippy::unknown_clippy_lints \ --allow clippy::unnecessary_cast \ --allow clippy::block_in_if_condition_stmt + # Prevent regressions of https://github.com/trishume/syntect/issues/98 + cargo clippy --all-features --lib -- --deny clippy::panic - name: Run cargo check run: | cargo check --all-features --all-targets diff --git a/examples/synhtml-css-classes.rs b/examples/synhtml-css-classes.rs index 80d8436a..fb7289c2 100644 --- a/examples/synhtml-css-classes.rs +++ b/examples/synhtml-css-classes.rs @@ -106,7 +106,7 @@ int main() { let css_dark_file = File::create(Path::new("theme-dark.css"))?; let mut css_dark_writer = BufWriter::new(&css_dark_file); - let css_dark = css_for_theme_with_class_style(dark_theme, ClassStyle::Spaced); + let css_dark = css_for_theme_with_class_style(dark_theme, ClassStyle::Spaced).unwrap(); writeln!(css_dark_writer, "{}", css_dark)?; // create light color scheme css @@ -114,7 +114,7 @@ int main() { let css_light_file = File::create(Path::new("theme-light.css"))?; let mut css_light_writer = BufWriter::new(&css_light_file); - let css_light = css_for_theme_with_class_style(light_theme, ClassStyle::Spaced); + let css_light = css_for_theme_with_class_style(light_theme, ClassStyle::Spaced).unwrap(); writeln!(css_light_writer, "{}", css_light)?; Ok(()) diff --git a/examples/synstats.rs b/examples/synstats.rs index 92f108b0..8d455398 100644 --- a/examples/synstats.rs +++ b/examples/synstats.rs @@ -85,7 +85,7 @@ fn count_line(ops: &[(usize, ScopeStackOp)], line: &str, stack: &mut ScopeStack, let mut line_has_doc_comment = false; let mut line_has_code = false; for (s, op) in ScopeRegionIterator::new(ops, line) { - stack.apply(op); + stack.apply(op).unwrap(); if s.is_empty() { // in this case we don't care about blank tokens continue; } diff --git a/examples/syntest.rs b/examples/syntest.rs index a2df166e..fc1a15e2 100644 --- a/examples/syntest.rs +++ b/examples/syntest.rs @@ -283,7 +283,7 @@ fn test_file( } let mut col: usize = 0; for (s, op) in ScopeRegionIterator::new(&ops, &line) { - stack.apply(op); + stack.apply(op).unwrap(); if s.is_empty() { // in this case we don't care about blank tokens continue; diff --git a/src/easy.rs b/src/easy.rs index 80985217..012e8ccc 100644 --- a/src/easy.rs +++ b/src/easy.rs @@ -298,7 +298,7 @@ mod tests { let mut stack = ScopeStack::new(); let mut token_count = 0; for (s, op) in ScopeRegionIterator::new(&ops, line) { - stack.apply(op); + stack.apply(op).expect("#[cfg(test)]"); if s.is_empty() { // in this case we don't care about blank tokens continue; } @@ -326,7 +326,7 @@ mod tests { let mut iterated_ops: Vec<&ScopeStackOp> = Vec::new(); for (_, op) in ScopeRegionIterator::new(&ops, line) { - stack.apply(op); + stack.apply(op).expect("#[cfg(test)]"); iterated_ops.push(op); println!("{:?}", op); } diff --git a/src/highlighting/highlighter.rs b/src/highlighting/highlighter.rs index d16dcaad..1e144c34 100644 --- a/src/highlighting/highlighter.rs +++ b/src/highlighting/highlighter.rs @@ -181,7 +181,7 @@ impl<'a, 'b> Iterator for RangedHighlightIterator<'a, 'b> { m_caches.pop(); } } - }); + }).ok()?; } self.pos = end; self.index += 1; diff --git a/src/html.rs b/src/html.rs index 28a15da5..be2f6b59 100644 --- a/src/html.rs +++ b/src/html.rs @@ -94,7 +94,7 @@ impl<'a> ClassedHTMLGenerator<'a> { parsed_line.as_slice(), self.style, &mut self.scope_stack, - ); + )?; self.open_spans += delta; self.html.push_str(formatted_line.as_str()); @@ -128,11 +128,11 @@ impl<'a> ClassedHTMLGenerator<'a> { #[deprecated(since="4.2.0", note="Please use `css_for_theme_with_class_style` instead.")] pub fn css_for_theme(theme: &Theme) -> String { - css_for_theme_with_class_style(theme, ClassStyle::Spaced) + css_for_theme_with_class_style(theme, ClassStyle::Spaced).expect("Please use `css_for_theme_with_class_style` instead.") } /// Create a complete CSS for a given theme. Can be used inline, or written to a CSS file. -pub fn css_for_theme_with_class_style(theme: &Theme, style: ClassStyle) -> String { +pub fn css_for_theme_with_class_style(theme: &Theme, style: ClassStyle) -> Result { let mut css = String::new(); css.push_str("/*\n"); @@ -201,7 +201,7 @@ pub fn css_for_theme_with_class_style(theme: &Theme, style: ClassStyle) -> Strin css.push_str("}\n"); } - css + Ok(css) } #[derive(Debug, PartialEq, Eq, Clone, Copy)] @@ -337,7 +337,7 @@ pub fn line_tokens_to_classed_spans( ops: &[(usize, ScopeStackOp)], style: ClassStyle, stack: &mut ScopeStack, -) -> (String, isize) { +) -> Result<(String, isize), Error> { let mut s = String::with_capacity(line.len() + ops.len() * 8); // a guess let mut cur_index = 0; let mut span_delta = 0; @@ -349,7 +349,7 @@ pub fn line_tokens_to_classed_spans( for &(i, ref op) in ops { if i > cur_index { span_empty = false; - write!(s, "{}", Escape(&line[cur_index..i])).unwrap(); + write!(s, "{}", Escape(&line[cur_index..i]))?; cur_index = i } stack.apply_with_hook(op, |basic_op, _| match basic_op { @@ -370,10 +370,10 @@ pub fn line_tokens_to_classed_spans( span_delta -= 1; span_empty = false; } - }); + })?; } - write!(s, "{}", Escape(&line[cur_index..line.len()])).unwrap(); - (s, span_delta) + write!(s, "{}", Escape(&line[cur_index..line.len()]))?; + Ok((s, span_delta)) } /// Preserved for compatibility, always use `line_tokens_to_classed_spans` @@ -385,7 +385,7 @@ pub fn tokens_to_classed_spans( ops: &[(usize, ScopeStackOp)], style: ClassStyle, ) -> (String, isize) { - line_tokens_to_classed_spans(line, ops, style, &mut ScopeStack::new()) + line_tokens_to_classed_spans(line, ops, style, &mut ScopeStack::new()).expect("Use `line_tokens_to_classed_spans` instead, this can panic and highlight incorrectly") } #[deprecated(since="3.1.0", note="Use `line_tokens_to_classed_spans` instead to avoid incorrect highlighting and panics")] @@ -393,7 +393,7 @@ pub fn tokens_to_classed_html(line: &str, ops: &[(usize, ScopeStackOp)], style: ClassStyle) -> String { - line_tokens_to_classed_spans(line, ops, style, &mut ScopeStack::new()).0 + line_tokens_to_classed_spans(line, ops, style, &mut ScopeStack::new()).expect("Use `line_tokens_to_classed_spans` instead to avoid incorrect highlighting and panics").0 } /// Determines how background color attributes are generated @@ -543,7 +543,7 @@ mod tests { // use util::debug_print_ops; // debug_print_ops(line, &ops); - let (html, _) = line_tokens_to_classed_spans(line, &ops[..], ClassStyle::Spaced, &mut stack); + let (html, _) = line_tokens_to_classed_spans(line, &ops[..], ClassStyle::Spaced, &mut stack).expect("#[cfg(test)]"); println!("{}", html); assert_eq!(html, include_str!("../testdata/test2.html").trim_end()); diff --git a/src/lib.rs b/src/lib.rs index 29eb02cd..bf2bd918 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -58,6 +58,9 @@ pub enum Error { #[cfg(feature = "parsing")] #[error("Parsing error: {0}")] ParsingError(#[from] crate::parsing::ParsingError), + /// Scope error + #[error("Scope error: {0}")] + ScopeError(#[from] crate::parsing::ScopeError), /// Formatting error #[error("Formatting error: {0}")] Fmt(#[from] std::fmt::Error), diff --git a/src/parsing/parser.rs b/src/parsing/parser.rs index c8071217..30154060 100644 --- a/src/parsing/parser.rs +++ b/src/parsing/parser.rs @@ -25,10 +25,16 @@ use crate::parsing::syntax_definition::ContextId; #[derive(Debug, thiserror::Error)] #[non_exhaustive] pub enum ParsingError { + #[error("Somehow main context was popped from the stack")] + MissingMainContext, /// A context is missing. Usually caused by a syntax referencing a another /// syntax that is not known to syntect. See e.g. #[error("Missing context with ID '{0:?}'")] MissingContext(ContextId), + #[error("Bad index to match_at: {0}")] + BadMatchIndex(usize), + #[error("Tried to use a ContextReference that has not bee resolved yet: {0:?}")] + UnresolvedContextReference(ContextReference), } /// Keeps the current parser state (the internal syntax interpreter stack) between lines of parsing. @@ -210,8 +216,9 @@ impl ParseState { line: &str, syntax_set: &SyntaxSet, ) -> Result, ParsingError> { - assert!(!self.stack.is_empty(), - "Somehow main context was popped from the stack"); + if self.stack.is_empty() { + return Err(ParsingError::MissingMainContext) + } let mut match_start = 0; let mut res = Vec::new(); @@ -300,7 +307,7 @@ impl ParseState { // check the next "pop" for loops. Otherwise leave the state, // e.g. non-consuming "set" could also result in a loop. let context = reg_match.context; - let match_pattern = context.match_at(reg_match.pat_index); + let match_pattern = context.match_at(reg_match.pat_index)?; if let MatchOperation::Push(_) = match_pattern.operation { *non_consuming_push_at = (match_end, self.stack.len() + 1); } @@ -362,7 +369,7 @@ impl ParseState { for (from_with_proto, ctx, captures) in context_chain { for (pat_context, pat_index) in context_iter(syntax_set, syntax_set.get_context(ctx)?) { - let match_pat = pat_context.match_at(pat_index); + let match_pat = pat_context.match_at(pat_index)?; if let Some(match_region) = self.search( line, start, match_pat, captures, search_cache, regions @@ -475,10 +482,10 @@ impl ParseState { ) -> Result { let (match_start, match_end) = reg_match.regions.pos(0).unwrap(); let context = reg_match.context; - let pat = context.match_at(reg_match.pat_index); + let pat = context.match_at(reg_match.pat_index)?; // println!("running pattern {:?} on '{}' at {}, operation {:?}", pat.regex_str, line, match_start, pat.operation); - self.push_meta_ops(true, match_start, level_context, &pat.operation, syntax_set, ops); + self.push_meta_ops(true, match_start, level_context, &pat.operation, syntax_set, ops)?; for s in &pat.scope { // println!("pushing {:?} at {}", s, match_start); ops.push((match_start, ScopeStackOp::Push(*s))); @@ -511,7 +518,7 @@ impl ParseState { // println!("popping at {}", match_end); ops.push((match_end, ScopeStackOp::Pop(pat.scope.len()))); } - self.push_meta_ops(false, match_end, &*level_context, &pat.operation, syntax_set, ops); + self.push_meta_ops(false, match_end, &*level_context, &pat.operation, syntax_set, ops)?; self.perform_op(line, ®_match.regions, pat, syntax_set) } @@ -524,7 +531,7 @@ impl ParseState { match_op: &MatchOperation, syntax_set: &'a SyntaxSet, ops: &mut Vec<(usize, ScopeStackOp)>, - ) { + ) -> Result<(), ParsingError>{ // println!("metas ops for {:?}, initial: {}", // match_op, // initial); @@ -542,7 +549,7 @@ impl ParseState { // cleared scopes are restored after the scopes from match pattern that invoked the pop are applied if !initial && cur_context.clear_scopes != None { - ops.push((index, ScopeStackOp::Restore)); + ops.push((index, ScopeStackOp::Restore)) } }, // for some reason the ST3 behaviour of set is convoluted and is inconsistent with the docs and other ops @@ -560,7 +567,7 @@ impl ParseState { } // add each context's meta scope for r in context_refs.iter() { - let ctx = r.resolve(syntax_set); + let ctx = r.resolve(syntax_set)?; if !is_set { if let Some(clear_amount) = ctx.clear_scopes { @@ -574,14 +581,14 @@ impl ParseState { } } else { let repush = (is_set && (!cur_context.meta_scope.is_empty() || !cur_context.meta_content_scope.is_empty())) || context_refs.iter().any(|r| { - let ctx = r.resolve(syntax_set); + let ctx = r.resolve(syntax_set).unwrap(); !ctx.meta_content_scope.is_empty() || (ctx.clear_scopes.is_some() && is_set) }); if repush { // remove previously pushed meta scopes, so that meta content scopes will be applied in the correct order let mut num_to_pop : usize = context_refs.iter().map(|r| { - let ctx = r.resolve(syntax_set); + let ctx = r.resolve(syntax_set).unwrap(); ctx.meta_scope.len() }).sum(); @@ -597,7 +604,7 @@ impl ParseState { // now we push meta scope and meta context scope for each context pushed for r in context_refs { - let ctx = r.resolve(syntax_set); + let ctx = r.resolve(syntax_set)?; // for some reason, contrary to my reading of the docs, set does this after the token if is_set { @@ -618,6 +625,8 @@ impl ParseState { }, MatchOperation::None => (), } + + Ok(()) } /// Returns true if the stack was changed @@ -657,10 +666,10 @@ impl ParseState { // referred to as the "target" of the push by sublimehq - see // https://forum.sublimetext.com/t/dev-build-3111/19240/17 for more info if let Some(ref p) = pat.with_prototype { - proto_ids.push(p.id()); + proto_ids.push(p.id()?); } } - let context_id = r.id(); + let context_id = r.id()?; let context = syntax_set.get_context(&context_id)?; let captures = { let mut uses_backrefs = context.uses_backrefs; @@ -770,7 +779,7 @@ mod tests { let mut stack = ScopeStack::new(); for &(_, ref op) in ops.iter() { - stack.apply(op); + stack.apply(op).expect("#[cfg(test)]"); } assert_eq!(stack, test_stack); } @@ -1827,7 +1836,7 @@ contexts: let mut states = Vec::new(); let mut stack = ScopeStack::new(); for &(_, ref op) in ops.iter() { - stack.apply(op); + stack.apply(op).expect("#[cfg(test)]"); let scopes: Vec = stack.as_slice().iter().map(|s| format!("{:?}", s)).collect(); let stack_str = scopes.join(", "); states.push(stack_str); diff --git a/src/parsing/scope.rs b/src/parsing/scope.rs index 13059ade..6c3ae035 100644 --- a/src/parsing/scope.rs +++ b/src/parsing/scope.rs @@ -11,6 +11,14 @@ use std::mem; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde::de::{Error, Visitor}; +/// Scope related errors +#[derive(Debug, thiserror::Error)] +#[non_exhaustive] +pub enum ScopeError { + #[error("Tried to restore cleared scopes, but none were cleared")] + NoClearedScopesToRestore, +} + /// Multiplier on the power of 2 for MatchPower. This is only useful if you compute your own /// [`MatchPower`] scores /// @@ -219,6 +227,7 @@ impl Scope { /// I can't think of any reason you'd find this useful. It is used internally for turning a /// scope back into a string. pub fn atom_at(self, index: usize) -> u16 { + #[allow(clippy::panic)] // The below panic is too much of an edge-case for it to be worth propagating let shifted = if index < 4 { self.a >> ((3 - index) * 16) } else if index < 8 { @@ -398,7 +407,7 @@ impl ScopeStack { /// Modifies this stack according to the operation given /// /// Use this to create a stack from a `Vec` of changes given by the parser. - pub fn apply(&mut self, op: &ScopeStackOp) { + pub fn apply(&mut self, op: &ScopeStackOp) -> Result<(), ScopeError> { self.apply_with_hook(op, |_,_|{}) } @@ -410,7 +419,7 @@ impl ScopeStack { /// [`apply`]: #method.apply /// [`BasicScopeStackOp`]: enum.BasicScopeStackOp.html #[inline] - pub fn apply_with_hook(&mut self, op: &ScopeStackOp, mut hook: F) + pub fn apply_with_hook(&mut self, op: &ScopeStackOp, mut hook: F) -> Result<(), ScopeError> where F: FnMut(BasicScopeStackOp, &[Scope]) { match *op { @@ -451,11 +460,13 @@ impl ScopeStack { hook(BasicScopeStackOp::Push(*s), self.as_slice()); } } - None => panic!("tried to restore cleared scopes, but none were cleared"), + None => return Err(ScopeError::NoClearedScopesToRestore), } } ScopeStackOp::Noop => (), } + + Ok(()) } /// Prints out each scope in the stack separated by spaces diff --git a/src/parsing/syntax_definition.rs b/src/parsing/syntax_definition.rs index ed32493b..b0855eb9 100644 --- a/src/parsing/syntax_definition.rs +++ b/src/parsing/syntax_definition.rs @@ -6,7 +6,7 @@ use std::collections::{BTreeMap, HashMap}; use std::hash::Hash; -use super::scope::*; +use super::{scope::*, ParsingError}; use super::regex::{Regex, Region}; use regex_syntax::escape; use serde::{Serialize, Serializer}; @@ -192,29 +192,29 @@ pub fn context_iter<'a>(syntax_set: &'a SyntaxSet, context: &'a Context) -> Matc } impl Context { - /// Returns the match pattern at an index, panics if the thing isn't a match pattern - pub fn match_at(&self, index: usize) -> &MatchPattern { + /// Returns the match pattern at an index + pub fn match_at(&self, index: usize) -> Result<&MatchPattern, ParsingError> { match self.patterns[index] { - Pattern::Match(ref match_pat) => match_pat, - _ => panic!("bad index to match_at"), + Pattern::Match(ref match_pat) => Ok(match_pat), + _ => Err(ParsingError::BadMatchIndex(index)), } } } impl ContextReference { - /// find the pointed to context, panics if ref is not linked - pub fn resolve<'a>(&self, syntax_set: &'a SyntaxSet) -> &'a Context { + /// find the pointed to context + pub fn resolve<'a>(&self, syntax_set: &'a SyntaxSet) -> Result<&'a Context, ParsingError> { match *self { - ContextReference::Direct(ref context_id) => syntax_set.get_context(context_id).unwrap(), - _ => panic!("Can only call resolve on linked references: {:?}", self), + ContextReference::Direct(ref context_id) => syntax_set.get_context(context_id), + _ => Err(ParsingError::UnresolvedContextReference(self.clone())), } } - /// get the context ID this reference points to, panics if ref is not linked - pub fn id(&self) -> ContextId { + /// get the context ID this reference points to + pub fn id(&self) -> Result { match *self { - ContextReference::Direct(ref context_id) => *context_id, - _ => panic!("Can only get ContextId of linked references: {:?}", self), + ContextReference::Direct(ref context_id) => Ok(*context_id), + _ => Err(ParsingError::UnresolvedContextReference(self.clone())), } } }