diff --git a/src/comments.rs b/src/comments.rs index 642f950..9a7c38a 100644 --- a/src/comments.rs +++ b/src/comments.rs @@ -10,7 +10,7 @@ use std::fmt; use crate::ast::ParsedSqlFile; /// Represents a line/column location within a source file. -#[derive(Clone, Debug, Eq, PartialEq, PartialOrd)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd)] pub struct Location { line: u64, column: u64, @@ -84,30 +84,12 @@ impl Default for Span { } } -/// Enum for holding the comment content, differentiated by single line `--` and -/// multiline `/* */` -#[derive(Clone, Debug, Eq, PartialEq)] -pub enum CommentKind { - /// Enum variant for Multiline Comments - MultiLine(String), - /// Enum variant for Single Line Comments - SingleLine(String), -} - -impl CommentKind { - /// Getter for returning the comment from within the enum - #[must_use] - pub fn comment(&self) -> &str { - match self { - Self::MultiLine(comment) | Self::SingleLine(comment) => comment, - } - } -} - -/// Structure for containing the [`CommentKind`] and the [`Span`] for a comment -#[derive(Clone, Debug, Eq, PartialEq)] +/// Structure for containing the comment, location and style +#[derive(Clone, Default, Debug, Eq, PartialEq)] pub struct Comment { - kind: CommentKind, + /// The comment content found as a [`String`] + text: String, + /// The location of the comment start/finish as a [`Span`] span: Span, } @@ -115,17 +97,11 @@ impl Comment { /// Method for making a new comment /// /// # Parameters - /// - `kind` where the type of comment is passed as a [`CommentKind`] + /// - `text` the text of the comment as a [`String`] /// - `span` where the [`Span`] of the comment is passed #[must_use] - pub const fn new(kind: CommentKind, span: Span) -> Self { - Self { kind, span } - } - - /// Getter method to get the [`CommentKind`] - #[must_use] - pub const fn kind(&self) -> &CommentKind { - &self.kind + pub const fn new(text: String, span: Span) -> Self { + Self { text, span } } /// Getter method to get the [`Span`] of the comment @@ -134,11 +110,20 @@ impl Comment { &self.span } - /// Getter method that will return the comment content as a [`str`], - /// regardless of [`CommentKind`] + /// Getter method that will return the comment content as a &[`str`] #[must_use] pub fn text(&self) -> &str { - self.kind().comment() + &self.text + } + + /// Setter for text of comment + pub fn set_text(&mut self, text: impl Into) { + self.text = text.into(); + } + + /// Sets new [`Span`] locations for the comment + pub const fn set_span_locations(&mut self, start: Location, end: Location) { + self.span = Span::new(start, end); } } @@ -246,15 +231,16 @@ impl Comments { let mut start_col = 1u64; let mut line_num = 1u64; - let mut col; let mut in_single = false; let mut in_multi = false; + let lines: Vec<&str> = src.lines().collect(); + let mut buf = String::new(); - for line in src.lines() { - col = 1; + for (i, line) in lines.iter().enumerate() { + let mut col = 1; let mut chars = line.chars().peekable(); while let Some(c) = chars.next() { match (in_single, in_multi, c) { @@ -262,9 +248,15 @@ impl Comments { if chars.peek().copied() == Some('-') { chars.next(); in_single = true; - start_line = line_num; - start_col = col; - buf.clear(); + let continuing = i + .checked_sub(1) + .and_then(|j| lines.get(j)) + .is_some_and(|prev| prev.trim().starts_with("--")); + if !continuing { + buf.clear(); + start_line = line_num; + start_col = col; + } col += 1; } } @@ -280,37 +272,30 @@ impl Comments { } (false, false, '*') => { if chars.peek().copied() == Some('/') { - let loc = Location::new(line_num, col); return Err(CommentError::UnmatchedMultilineCommentStart { - location: loc, + location: Location::new(line_num, col), }); } } (false, true, '*') => { - if chars.peek().copied() == Some('/') { - chars.next(); - let end_loc = Location::new(line_num, col + 1); - let normalized_comment = buf - .lines() - .enumerate() - .map(|(i, line)| match i { - 0 => line.trim().to_owned(), - _ => "\n".to_owned() + line.trim(), - }) - .collect(); - comments.push(Comment::new( - CommentKind::MultiLine(normalized_comment), - Span::new( - Location { line: start_line, column: start_col }, - end_loc, - ), - )); - in_multi = false; - buf.clear(); - col += 1; - } else { + if chars.peek().copied() != Some('/') { buf.push('*'); + continue; } + chars.next(); + let normalized_comment = + buf.lines().map(str::trim).collect::>().join("\n"); + + comments.push(Comment::new( + normalized_comment, + Span::new( + Location { line: start_line, column: start_col }, + Location::new(line_num, col + 1), + ), + )); + in_multi = false; + buf.clear(); + col += 1; } (false, true, ch) | (true, false, ch) => { buf.push(ch); @@ -324,24 +309,23 @@ impl Comments { } if in_single { in_single = false; - let end_loc = Location::new(line_num, col); - comments.push(Comment::new( - CommentKind::SingleLine(buf.trim().to_owned()), - Span::new(Location { line: start_line, column: start_col }, end_loc), - )); - buf.clear(); + if lines.get(i + 1).is_some_and(|line| line.trim().starts_with("--")) { + buf.push('\n'); + } else { + comments.push(Comment::new( + buf.trim().replace("\n ", "\n"), + Span::new( + Location { line: start_line, column: start_col }, + Location::new(line_num, col), + ), + )); + buf.clear(); + } } else if in_multi { buf.push('\n'); } line_num += 1; } - // EOF: close any open comments - if in_multi { - return Err(CommentError::UnterminatedMultiLineComment { - start: Location { line: start_line, column: start_col }, - }); - } - Ok(Self { comments }) } @@ -367,7 +351,7 @@ impl Comments { mod tests { use std::{env, fs}; - use crate::comments::{Comment, CommentError, CommentKind, Comments, Location, Span}; + use crate::comments::{Comment, CommentError, Comments, Location, Span}; #[test] fn location_new_and_default() { @@ -398,13 +382,13 @@ mod tests { let raw_comment = "-- a comment"; let len = raw_comment.len() as u64; - let singleline = CommentKind::SingleLine(raw_comment.to_owned()); + let singleline = raw_comment.to_owned(); let mut span = Span::default(); span.end.column = len - 1; let comment = Comment::new(singleline.clone(), span); - assert_eq!(comment.kind, singleline); + assert_eq!(comment.text(), singleline); let expected_span = Span::new(Location { line: 1, column: 1 }, Location { line: 1, column: len - 1 }); @@ -414,12 +398,12 @@ mod tests { #[test] fn multiline_comment_span() { - let kind = CommentKind::MultiLine("/* hello world */".to_owned()); + let text = "/* hello world */".to_owned(); let span = Span::new(Location { line: 1, column: 1 }, Location { line: 2, column: 9 }); - let comment = Comment::new(kind.clone(), span); + let comment = Comment::new(text.clone(), span); - assert_eq!(comment.kind, kind); + assert_eq!(comment.text(), text); assert_eq!(comment.span.start.line, 1); assert_eq!(comment.span.end.line, 2); } @@ -446,7 +430,13 @@ mod tests { let parsed_set = ParsedSqlFileSet::parse_all(set)?; for file in parsed_set.files() { - let parsed_comments = Comments::parse_all_comments_from_file(file)?; + let parsed_comments = Comments::parse_all_comments_from_file(file).map_err(|e| { + format!( + "parse_all_comments_from_file failed for file {:?}: {e}", + file.file().path() + ) + })?; + let filename = file .file() .path() @@ -489,7 +479,7 @@ mod tests { ); for (i, comment) in comments.iter().enumerate() { - assert_eq!(expected[i], comment.kind().comment(), "comment at index {i} did not match"); + assert_eq!(expected[i], comment.text(), "comment at index {i} did not match"); } } @@ -683,11 +673,11 @@ CREATE TABLE posts ( let comments = comments.comments(); assert_eq!(comments.len(), 11); let first = &comments[0]; - assert_eq!(first.kind().comment(), "Users table stores user account information"); + assert_eq!(first.text(), "Users table stores user account information"); assert_eq!(first.span().start(), &Location::new(1, 1)); assert_eq!(first.span().end(), &Location::new(1, 47)); let primary_key = &comments[1]; - assert_eq!(primary_key.kind().comment(), "Primary key"); + assert_eq!(primary_key.text(), "Primary key"); assert_eq!(primary_key.span().start(), &Location::new(3, 5)); assert_eq!(primary_key.span().end(), &Location::new(3, 19)); assert!( @@ -721,10 +711,7 @@ CREATE TABLE posts ( let comments = comments.comments(); assert_eq!(comments.len(), 11); let first = &comments[0]; - assert_eq!( - first.kind().comment(), - "Users table stores user account information\nmultiline" - ); + assert_eq!(first.text(), "Users table stores user account information\nmultiline"); assert_eq!(first.span().start(), &Location::new(1, 1)); assert_eq!(first.span().end().line(), 2); assert!( @@ -732,7 +719,7 @@ CREATE TABLE posts ( "end column should be after start column for first multiline comment", ); let primary_key = &comments[1]; - assert_eq!(primary_key.kind().comment(), "Primary key\nmultiline"); + assert_eq!(primary_key.text(), "Primary key\nmultiline"); assert_eq!(primary_key.span().start(), &Location::new(4, 5)); assert_eq!(primary_key.span().end().line(), 5); assert!( @@ -760,11 +747,11 @@ CREATE TABLE posts ( fn test_comments() { let comment_vec = vec![ Comment::new( - CommentKind::SingleLine("a comment".to_owned()), + "a comment".to_owned(), Span { start: Location::new(1, 1), end: Location::new(1, 12) }, ), Comment::new( - CommentKind::SingleLine("a second comment".to_owned()), + "a second comment".to_owned(), Span { start: Location::new(1, 1), end: Location::new(2, 19) }, ), ]; @@ -772,9 +759,98 @@ CREATE TABLE posts ( let comments = Comments::new(comment_vec.clone()); assert!(comments.comments().len() == length); for (i, comment) in comments.comments().iter().enumerate() { - assert_eq!(comment.kind().comment(), comment_vec[i].kind().comment()); + assert_eq!(comment.text(), comment_vec[i].text()); assert_eq!(comment.span().start(), comment_vec[i].span().start()); assert_eq!(comment.span().end(), comment_vec[i].span().end()); } } + + #[test] + fn single_line_runover_comments_are_combined() -> Result<(), Box> { + use crate::{ast::ParsedSqlFileSet, comments::Comments, files::SqlFileSet}; + use std::{env, fs}; + + let base = env::temp_dir().join("single_line_runover_combined"); + let _ = fs::remove_dir_all(&base); + fs::create_dir_all(&base)?; + + let file = base.join("runover.sql"); + fs::File::create(&file)?; + + let sql = "\ +-- comment 1 +-- comment 2 +SELECT 1; +"; + fs::write(&file, sql)?; + + let set = SqlFileSet::new(&base, &[])?; + let parsed_set = ParsedSqlFileSet::parse_all(set)?; + + let parsed_file = parsed_set + .files() + .iter() + .find(|f| { + f.file().path().and_then(|p| p.to_str()).is_some_and(|p| p.ends_with("runover.sql")) + }) + .ok_or("runover.sql should be present")?; + + let comments = Comments::parse_all_comments_from_file(parsed_file)?; + let comments = comments.comments(); + + assert_eq!(comments.len(), 1, "expected a single combined comment"); + assert_eq!(comments[0].text(), "comment 1\ncomment 2"); + + assert_eq!(comments[0].span().start(), &Location::new(1, 1)); + assert_eq!(comments[0].span().end().line(), 2); + + let _ = fs::remove_dir_all(&base); + Ok(()) + } + + #[test] + fn single_line_comments_with_gap_are_not_combined() -> Result<(), Box> { + use crate::{ast::ParsedSqlFileSet, comments::Comments, files::SqlFileSet}; + use std::{env, fs}; + + let base = env::temp_dir().join("single_line_runover_not_combined"); + let _ = fs::remove_dir_all(&base); + fs::create_dir_all(&base)?; + + let file = base.join("gap.sql"); + fs::File::create(&file)?; + + let sql = "\ +-- comment 1 + +-- comment 2 +SELECT 1; +"; + fs::write(&file, sql)?; + + let set = SqlFileSet::new(&base, &[])?; + let parsed_set = ParsedSqlFileSet::parse_all(set)?; + + let parsed_file = parsed_set + .files() + .iter() + .find(|f| { + f.file().path().and_then(|p| p.to_str()).is_some_and(|p| p.ends_with("gap.sql")) + }) + .ok_or("gap.sql should be present")?; + + let comments = Comments::parse_all_comments_from_file(parsed_file)?; + let comments = comments.comments(); + + assert_eq!(comments.len(), 2, "expected two separate comments"); + assert_eq!(comments[0].text(), "comment 1"); + assert_eq!(comments[1].text(), "comment 2"); + + assert_eq!(comments[0].span().start(), &Location::new(1, 1)); + assert_eq!(comments[0].span().end().line(), 1); + assert_eq!(comments[1].span().start(), &Location::new(3, 1)); + + let _ = fs::remove_dir_all(&base); + Ok(()) + } }