fix: escaping in languages & meaningful src lines (#7)

* fix: escaping in languages & meaningful src lines

* add: a lot more tests

* add: more tests

* add: yet more tests

* add: moooore tests

* chore: cleanup the tests

* chore: cleanup package description

* chore: code cleanup and more tests

* fix: add description to Dracula

Signed-off-by: Swarnim Arun <[email protected]>

* fix: add description to pydracula

Signed-off-by: Swarnim Arun <[email protected]>

* Apply suggestions from code review

Signed-off-by: raghav-deepsource <[email protected]>

* fix: repeat dsl

* fix: c tests as per updated lang parsing rules

* fix: as suggested

* add: support for more types of python `str`s

* fix: let `tests.rs` showup in gitdiff

* fix: as suggested

Co-authored-by: Prajwal S N <[email protected]>
Co-authored-by: raghav-deepsource <[email protected]>
Signed-off-by: Swarnim Arun <[email protected]>

* add: more tests for python

and,
- fix line-ending on EOF bug with pre-exact
- add golden files for testing cleaned_src for python

---------

Signed-off-by: Swarnim Arun <[email protected]>
Signed-off-by: raghav-deepsource <[email protected]>
Co-authored-by: raghav-deepsource <[email protected]>
Co-authored-by: Prajwal S N <[email protected]>

Author: 3 people

Cargo.lock

@@ -4,21 +4,32 @@ members = [
     "pydracula"
 ]
 
-[package]
-name = "dracula"
+[workspace.package]
+edition = "2021"
 description = "🧛 Count-ing lines, AH AH AHH!"
-version = "0.1.2"
+version = "0.1.3"
 authors = ["Swarnim Arun <[email protected]>"]
-edition = "2021"
 license-file = "LICENSE"
 documentation = "https://docs.rs/dracula"
 homepage = "https://github.com/deepsourcelabs/dracula"
 repository = "https://github.com/deepsourcelabs/dracula"
 keywords = ["parsing", "line-count"]
 categories = ["command-line-utilities", "text-processing"]
 
+
+[package]
+name = "dracula"
+version.workspace = true
+authors.workspace = true
+edition.workspace = true
+
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
+[dev-dependencies]
+ra_ap_syntax = "0.0.149"
+pretty_assertions = "1.3.0"
+letr = "0.2.1"
+
 [dependencies.log]
 version = "0.4"
 optional = true

Cargo.toml

@@ -1,9 +1,9 @@
 [package]
 name = "cdracula"
-description = "🧛 Count-ing lines, AH AH AHH!"
-version = "0.1.2"
-authors = ["Swarnim Arun <[email protected]>"]
-edition = "2021"
+version.workspace = true
+description.workspace = true
+authors.workspace = true
+edition.workspace = true
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 [lib]

cdracula/Cargo.toml

@@ -21,7 +21,7 @@ macro_rules! languages_supported {
             $(
                 if idx == $num {
                     return Some(
-                        dracula::count::get_cleaned_source_code::<dracula::langs::$name>(src)
+                        dracula::count::get_cleaned_source_code::<dracula::langs::$name>(src).unwrap_or_else(|| src.to_string())
                     );
                 }
             )+

cdracula/src/util_macros.rs

@@ -139,7 +139,7 @@ int main() {
                 ) + "\0")
                     .into(),
             );
-            assert_eq!(get_meaningful_line_count(src.as_ptr(), C_LANG, 0), 4);
+            assert_eq!(get_meaningful_line_count(src.as_ptr(), C_LANG, 0), 3);
         }
     }
 
@@ -164,7 +164,7 @@ int main() {
             let mut len = 0u64;
             let ptr = meaningful_lines(src.as_ptr(), C_LANG, 0, &mut len as *mut u64);
             let v = Vec::from_raw_parts(ptr, len as _, len as _);
-            assert_eq!(&v, &[2, 4, 7, 8]);
+            assert_eq!(&v, &[2, 4, 7]);
         }
     }
 
@@ -193,7 +193,7 @@ int main() {
 r#"int main() {
     return 0;
  int x = 10;
-}"#,)
+"#,)
             );
         }
     }

cdracula/tests/test_capi.rs

@@ -1,8 +1,9 @@
 [package]
 name = "pydracula"
-version = "0.1.2"
-authors = ["Swarnim Arun <[email protected]>"]
-edition = "2018"
+version.workspace = true
+authors.workspace = true
+description.workspace = true
+edition.workspace = true
 
 [lib]
 name = "pydracula"

pydracula/Cargo.toml

@@ -22,14 +22,27 @@ impl Lang {
             Lang::Java => get_meaningful_line_indices::<Java>(src).flatten().collect(),
         }
     }
+    #[rustfmt::skip]
     fn get_cleaned_source_code(&self, src: &str) -> String {
         use dracula::count::*;
         use dracula::langs::*;
         match self {
-            Lang::Python => get_cleaned_source_code::<Python>(src),
-            Lang::Rust => get_cleaned_source_code::<Rust>(src),
-            Lang::C => get_cleaned_source_code::<C>(src),
-            Lang::Java => get_cleaned_source_code::<Java>(src),
+            Lang::Python => {
+                get_cleaned_source_code::<Python>(src)
+                    .unwrap_or_else(|| src.to_string())
+            }
+            Lang::Rust => {
+                get_cleaned_source_code::<Rust>(src)
+                    .unwrap_or_else(|| src.to_string())
+            },
+            Lang::C => {
+                get_cleaned_source_code::<C>(src)
+                    .unwrap_or_else(|| src.to_string())
+            },
+            Lang::Java => {
+                get_cleaned_source_code::<Java>(src)
+                    .unwrap_or_else(|| src.to_string())
+            },
         }
     }
     fn get_count_of_meaningful_lines(&self, src: &str) -> usize {

pydracula/src/lib.rs

@@ -15,13 +15,24 @@ pub struct ParseLineMeaningfulIndexIter<'a, L: Language> {
     line_span: Span,
     parse_span: Span,
     line_index: usize,
+    max_lines: usize,
     last_parsed_output: Option<ParseOutput<'a>>,
+    failed: bool,
 }
 
 impl<'a, L: Language> Iterator for ParseLineMeaningfulIndexIter<'a, L> {
     type Item = Option<usize>;
 
     fn next(&mut self) -> Option<Self::Item> {
+        // Self::Item is Option<usize> to signify that a line may also not be meaningful.
+        if self.failed {
+            let li = self.line_index;
+            if li > self.max_lines {
+                return None;
+            } else {
+                return Some(Some(li));
+            }
+        }
         if self.line_span.end >= self.src.len() {
             return None;
         }
@@ -70,6 +81,8 @@ impl<'a, L: Language> Iterator for ParseLineMeaningfulIndexIter<'a, L> {
     }
 }
 
+/// Builds the iterator [`ParseLineMeaningfulIndexIter`] to run over lines of src while
+/// figuring out meaningful lines from it
 pub fn get_meaningful_line_indices<L: Language + 'static>(
     src: &str,
 ) -> ParseLineMeaningfulIndexIter<L> {
@@ -80,14 +93,20 @@ pub fn get_meaningful_line_indices<L: Language + 'static>(
         parse_span: Span::default(),
         line_index: 0,
         last_parsed_output: None,
+        failed: false,
+        max_lines: src.lines().count(),
     }
 }
 
-pub fn get_cleaned_source_code<L: Language>(src: &str) -> String {
+/// Uses the [`Parser`] to try and figure out the meaningful parts of the source
+pub fn get_cleaned_source_code<L: Language>(src: &str) -> Option<String> {
     let parsed = L::get_parser(src);
     let mut meaningful_src = String::default();
     let mut stack = vec![];
     for p in parsed {
+        if matches!(p, ParseOutput::Invalid(..)) {
+            return None;
+        }
         if matches!(p, ParseOutput::EOL(_) | ParseOutput::EOF) {
             let meaningful_src_len = meaningful_src.len();
             for po in stack.iter() {
@@ -97,61 +116,50 @@ pub fn get_cleaned_source_code<L: Language>(src: &str) -> String {
                     }
                 }
             }
-            if matches!(p, ParseOutput::EOL(_))
-                && meaningful_src_len != meaningful_src.len()
-            {
+            if matches!(p, ParseOutput::EOL(_)) && meaningful_src_len != meaningful_src.len() {
                 meaningful_src.push('\n');
             }
             stack.clear();
         } else {
             stack.push(p);
         }
     }
-    meaningful_src
+    Some(meaningful_src)
 }
 
-pub fn get_count_of_meaningful_lines<L: Language>(src: &str) -> usize {
-    let parsed = L::get_parser(src);
-    let mut line_count: usize = 0;
-    let mut stack = vec![];
-    for p in parsed {
-        if matches!(p, ParseOutput::EOL(_) | ParseOutput::EOF) {
-            if stack.iter().any(L::is_meaningful) {
-                line_count += 1;
-            }
-            // We clear the stack once we reach the end of a line.
-            stack.clear();
-        } else {
-            // we accumulate tokens we see as meaningful tokens for the language.
-            stack.push(p);
-        }
-    }
-    line_count
+/// Uses the [`get_meaningful_line_indices`] function to build an iterator
+/// and count all meaningful lines
+pub fn get_count_of_meaningful_lines<L: Language + 'static>(src: &str) -> usize {
+    get_meaningful_line_indices::<L>(src).flatten().count()
 }
 
-#[test]
-fn test_halting_get_count_of_meaningful_lines() {
-    get_count_of_meaningful_lines::<crate::langs::C>("");
-    get_count_of_meaningful_lines::<crate::langs::Rust>("");
-    get_count_of_meaningful_lines::<crate::langs::Python>("");
-    get_count_of_meaningful_lines::<crate::langs::Java>("");
-}
+/// No halting tests for [`get_count_of_meaningful_lines`] as it uses
+/// Iterator provided by [`get_meaningful_line_indices`]
+#[cfg(test)]
+mod halting_tests_count_api {
+    use super::*;
 
-#[test]
-fn test_halting_get_cleaned_source_code() {
-    get_cleaned_source_code::<crate::langs::C>("");
-    get_cleaned_source_code::<crate::langs::Rust>("");
-    get_cleaned_source_code::<crate::langs::Python>("");
-    get_cleaned_source_code::<crate::langs::Java>("");
-}
+    #[test]
+    fn test_halting_get_cleaned_source_code() {
+        get_cleaned_source_code::<crate::langs::C>("\nint main() {}\n");
+        get_cleaned_source_code::<crate::langs::Rust>("\nfn main() {}\n");
+        get_cleaned_source_code::<crate::langs::Python>("\ndef main():\n\tpass");
+        get_cleaned_source_code::<crate::langs::Java>("\nvoid main() {}\n");
+    }
 
-#[test]
-fn test_halting_get_meaningful_line_indices() {
-    get_meaningful_line_indices::<crate::langs::C>("
-    int main() {}
-    ").flatten().for_each(|_| ());
-    get_meaningful_line_indices::<crate::langs::Rust>("").flatten().for_each(|_| ());
-    get_meaningful_line_indices::<crate::langs::Python>("").flatten().for_each(|_| ());
-    get_meaningful_line_indices::<crate::langs::Java>("").flatten().for_each(|_| ());
+    #[test]
+    fn test_halting_get_meaningful_line_indices() {
+        get_meaningful_line_indices::<crate::langs::C>("\nint main() {}\n")
+            .flatten()
+            .for_each(|_| ());
+        get_meaningful_line_indices::<crate::langs::Rust>("")
+            .flatten()
+            .for_each(|_| ());
+        get_meaningful_line_indices::<crate::langs::Python>("")
+            .flatten()
+            .for_each(|_| ());
+        get_meaningful_line_indices::<crate::langs::Java>("")
+            .flatten()
+            .for_each(|_| ());
+    }
 }
-

src/count.rs

@@ -0,0 +1,41 @@
+# pylint:disable=pointless-string-statement, redundant-u-string-prefix
+"""Test for backslash escapes in byte vs unicode strings"""
+
+# Would be valid in Unicode, but probably not what you want otherwise
+BAD_UNICODE = b'\u0042'  # [anomalous-unicode-escape-in-string]
+BAD_LONG_UNICODE = b'\U00000042'  # [anomalous-unicode-escape-in-string]
+# +1:[anomalous-unicode-escape-in-string]
+BAD_NAMED_UNICODE = b'\N{GREEK SMALL LETTER ALPHA}'
+
+GOOD_UNICODE = u'\u0042'
+GOOD_LONG_UNICODE = u'\U00000042'
+GOOD_NAMED_UNICODE = u'\N{GREEK SMALL LETTER ALPHA}'
+
+
+# Valid raw strings
+RAW_BACKSLASHES = r'raw'
+
+# In a comment you can have whatever you want: \ \\ \n \m
+# even things that look like bad strings: "C:\Program Files"
+
+
+"""Test for anomalous backslash escapes in strings"""
+
+BAD_ESCAPE = '\z'  # [anomalous-backslash-in-string]
+BAD_ESCAPE_NOT_FIRST = 'abc\z'  # [anomalous-backslash-in-string]
+BAD_ESCAPE_WITH_PREFIX = b'abc\z'  # [anomalous-backslash-in-string]
+BAD_ESCAPE_WITH_BACKSLASH = b'a\
+    \z'  # [anomalous-backslash-in-string]
+# +3:[anomalous-backslash-in-string]
+BAD_ESCAPE_BLOCK = b'''
+    abc
+    \z
+'''
+BAD_ESCAPE_PARENS = (b'abc'
+                     b'\z')  # [anomalous-backslash-in-string]
+GOOD_ESCAPE = '\b'
+
+# Valid raw strings
+BAD_ESCAPE_BUT_RAW = r'\z'
+
+# In a comment you can have whatever you want: \z

src/fixtures/more_python_tests.py

@@ -0,0 +1,16 @@
+BAD_UNICODE = 
+BAD_LONG_UNICODE = 
+BAD_NAMED_UNICODE = 
+GOOD_UNICODE = 
+GOOD_LONG_UNICODE = 
+GOOD_NAMED_UNICODE = 
+RAW_BACKSLASHES = 
+BAD_ESCAPE = 
+BAD_ESCAPE_NOT_FIRST = 
+BAD_ESCAPE_WITH_PREFIX = 
+BAD_ESCAPE_WITH_BACKSLASH = 
+BAD_ESCAPE_BLOCK = 
+BAD_ESCAPE_PARENS = (
+)  
+GOOD_ESCAPE = 
+BAD_ESCAPE_BUT_RAW =