Openzeppelin bugs

.circleci/config.yml

@@ -23,12 +23,14 @@ orbs:
 jobs:
   unit-tests:
     docker:
-      - image: cimg/rust:1.64.0
+      - image: cimg/rust:1.65.0
     steps:
       - checkout
       - run:
           name: Install ripgrep
-          command: sudo apt-get install ripgrep
+          command: |
+            curl -LO https://github.com/BurntSushi/ripgrep/releases/download/13.0.0/ripgrep_13.0.0_amd64.deb
+            sudo dpkg -i ripgrep_13.0.0_amd64.deb
       - run:
           name: Check version matches of Cargo.toml, pyproject.toml and CHANGELOG
           command: |

.gitignore

@@ -1,5 +1,5 @@
 **/target
 **/Cargo.lock
 **/.DS_Store
-
-**/python_wrapper/.env
+BUGS
+**/python_wrapper/.env

CHANGELOG.md

@@ -1,4 +1,11 @@
 # Changelog
+## [0.5.0] - 2022-11-16
+### Fixed
+- Handle lines of the form /*****/ (for any amount of *)
+- Ignore /*****/ (for any amount of *) when it is a separator between elements
+- Fixed element span to contain both the documentation block and the associated element
+
+
 ## [0.4.2] - 2022-10-02
 ### Fixed
 - Fix issue with recognition of `rule` blocks.

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "cvldoc_parser_core"
-version = "0.4.2"
+version = "0.5.0"
 edition = "2021"
 
 [dependencies]

src/parse/associated_element.rs

@@ -64,29 +64,6 @@ fn mandatory_sep<'src>() -> BoxedParser<'src, char, (), Simple<char>> {
     mandatory_ws.ignore_then(optional_sep()).boxed()
 }
 
-/// when parsing the block associated with the documentation, we are dealing with
-/// a stream of tokens. tokens may be separated by some combination of whitespace or comments.
-/// since we do not go through a lexing stage that filters them out, we must assume
-/// that they may exist (possibly repeatedly) between any valid token of the associated block.
-fn optional_sep_immediately_after_doc<'src>() -> BoxedParser<'src, char, (), Simple<char>> {
-    let single_line_comment_between_tokens = just("//")
-        .then(none_of('/').rewind())
-        .then(take_to_newline_or_end())
-        .ignored();
-
-    //we cannot use the usual multi-line comment parser here, since it is
-    //now allowed to have "/**" as a comment starter.
-    let multi_line_comment_between_tokens = just("/*").then(take_to_starred_terminator()).ignored();
-
-    let comment = choice((
-        single_line_comment_between_tokens,
-        multi_line_comment_between_tokens,
-    ))
-    .padded();
-
-    comment.repeated().ignored().padded().boxed()
-}
-
 fn optional_sep<'src>() -> BoxedParser<'src, char, (), Simple<char>> {
     //we cannot use the usual multi-line comment parser here, since it is
     //now allowed to have "/**" as a comment starter.
@@ -339,18 +316,14 @@ fn definition_decl<'src>() -> BoxedParser<'src, char, AssociatedElement, Simple<
         .boxed()
 }
 
-pub(super) fn associated_element<'src>() -> BoxedParser<'src, char, AssociatedElement, Simple<char>>
-{
-    let decl = choice([
+pub(super) fn element_parser<'src>() -> BoxedParser<'src, char, AssociatedElement, Simple<char>> {
+    choice([
         rule_decl(),
         methods_decl(),
         invariant_decl(),
         function_decl(),
         ghost_decl(),
         definition_decl(),
-    ]);
-
-    optional_sep_immediately_after_doc()
-        .ignore_then(decl)
-        .boxed()
+    ])
+    .boxed()
 }

src/parse/helpers.rs

@@ -74,3 +74,27 @@ pub(super) fn multi_line_cvl_comment() -> impl Parser<char, (), Error = Simple<c
         .then(take_to_starred_terminator())
         .ignored()
 }
+
+/// when parsing the block associated with the documentation, we are dealing with
+/// a stream of tokens. tokens may be separated by some combination of whitespace or comments.
+/// since we do not go through a lexing stage that filters them out, we must assume
+/// that they may exist (possibly repeatedly) between any valid token of the associated block.
+pub(super) fn optional_sep_immediately_after_doc<'src>() -> BoxedParser<'src, char, (), Simple<char>>
+{
+    let single_line_comment_between_tokens = just("//")
+        .then(none_of('/').rewind())
+        .then(take_to_newline_or_end())
+        .ignored();
+
+    //we cannot use the usual multi-line comment parser here, since it is
+    //now allowed to have "/**" as a comment starter.
+    let multi_line_comment_between_tokens = just("/*").then(take_to_starred_terminator()).ignored();
+
+    let comment = choice((
+        single_line_comment_between_tokens,
+        multi_line_comment_between_tokens,
+    ))
+    .padded();
+
+    comment.repeated().ignored().padded().boxed()
+}

src/parse/mod.rs

@@ -3,7 +3,7 @@ pub mod builder;
 mod helpers;
 mod terminated_line;
 
-use self::associated_element::associated_element;
+use self::associated_element::element_parser;
 use crate::parse::terminated_line::JoinToString;
 use builder::CvlDocBuilder;
 use chumsky::{prelude::*, text::whitespace};
@@ -35,17 +35,19 @@ fn free_form_comment<'src>() -> BoxedParser<'src, char, CvlDocBuilder, Simple<ch
     let stars = just('*').repeated().at_least(3);
     let thick_starred_padding = just('/').then(stars).then(just('/')).then(newline_or_end());
 
-    let starred_header = just('/')
-        .ignore_then(stars)
-        .ignore_then(horizontal_ws())
-        .ignore_then(take_to_starred_terminator())
-        .then_ignore(newline_or_end())
-        .collect()
-        .map(|line: String| {
-            let padding = &[' ', '\t', '*'];
-            line.trim_end_matches(padding).to_string()
-        })
-        .boxed();
+    let starred_header = {
+        let endings = choice((just("*/\r\n"), just("*/\n"), just("*/").then_ignore(end())));
+        let content = take_until_without_terminator(endings);
+
+        just("/***")
+            .ignore_then(content)
+            .map(|line| {
+                static PADDING: &[char] = &[' ', '\t', '*'];
+                let line = String::from_iter(line);
+                line.trim_matches(PADDING).to_string()
+            })
+            .boxed()
+    };
 
     let starred_single_line_free_form = starred_header.clone();
     let starred_thick_free_form = starred_header.padded_by(thick_starred_padding).boxed();
@@ -111,15 +113,24 @@ fn cvldoc_documentation<'src>() -> BoxedParser<'src, char, CvlDocBuilder, Simple
     let documentation = choice([slashed_documentation, starred_documentation])
         .map_with_span(|spanned_body, span| (spanned_body, span));
 
+    let optional_associated_element = element_parser()
+        .or_not()
+        .map_with_span(|associated, span| (associated, span))
+        .boxed();
+
     documentation
-        .then(associated_element().or_not())
-        .map(
-            |((spanned_body, span), associated)| CvlDocBuilder::Documentation {
-                span,
+        .then_ignore(optional_sep_immediately_after_doc())
+        .then(optional_associated_element)
+        .map(|(doc, element)| {
+            let (spanned_body, doc_span) = doc;
+            let (associated, associated_span) = element;
+
+            CvlDocBuilder::Documentation {
+                span: doc_span.start()..associated_span.end(),
                 spanned_body,
                 associated,
-            },
-        )
+            }
+        })
         .boxed()
 }
 

src/parse/tests.rs

@@ -3,11 +3,13 @@ use crate::{AssociatedElement, CvlDoc, DocData, Param, Tag};
 use assert_matches::assert_matches;
 use color_eyre::eyre::bail;
 use color_eyre::Report;
+use color_eyre::Result;
 use indoc::indoc;
 use itertools::Itertools;
 use lsp_types::{Position, Range};
 use ropey::Rope;
 use std::iter::zip;
+use std::path::Path;
 
 fn parse_src(src: &str) -> Vec<CvlDoc> {
     let rope = Rope::from_str(src);
@@ -23,6 +25,22 @@ macro_rules! param {
     };
 }
 
+trait PostfixDbg {
+    fn dbg(self) -> Self;
+}
+
+impl<T: std::fmt::Debug> PostfixDbg for T {
+    fn dbg(self) -> Self {
+        dbg!(self)
+    }
+}
+
+impl CvlDoc {
+    fn data(self) -> DocData {
+        self.data
+    }
+}
+
 fn data_of_first(docs: &[CvlDoc]) -> Option<&DocData> {
     docs.first().map(|doc| &doc.data)
 }
@@ -43,6 +61,22 @@ fn compare_params(expected_params: &[Param], actual_params: &[Param]) {
     }
 }
 
+fn find_by_name_of_associated_element<'a>(
+    expected_name: &str,
+    parsed_docs: &'a [CvlDoc],
+) -> Option<&'a CvlDoc> {
+    parsed_docs.iter().find(|doc| {
+        let DocData::Documentation { associated: Some(assoc), .. } = &doc.data else { return false; };
+        let Some(name) = assoc.name() else { return false; };
+        name == expected_name
+    })
+}
+
+fn parse_from_path(path: impl AsRef<Path>) -> Result<Vec<CvlDoc>> {
+    let spec = std::fs::read_to_string(path)?;
+    Ok(parse_src(spec.as_str()))
+}
+
 #[test]
 fn free_form_comments() {
     let src = indoc! {"
@@ -266,7 +300,7 @@ fn commented_out_blocks_are_ignored() {
             */
         "#};
 
-    let parsed = parse_src(src);
+    let parsed = dbg!(parse_src(src));
     assert!(
         parsed.is_empty(),
         "valid CVLDoc blocks were parsed from commented out blocks"
@@ -491,11 +525,76 @@ fn methods_with_whitespace_between_name_and_params() {
     }
         "#};
 
-    let parsed = parse_to_exactly_one_element(&src).unwrap();
+    let parsed = parse_to_exactly_one_element(src).unwrap();
 
     if let DocData::Documentation { associated, .. } = parsed.data {
         assert_matches!(associated, Some(AssociatedElement::Rule { .. }));
     } else {
         panic!("should have been parsed as documentation");
     }
 }
+
+#[test]
+fn freeform_stars_without_text() {
+    // let src = "definition harness_isListed(address a, uint i) returns bool = 0 <= i && i < shadowLenArray() && shadowArray(i) == a ;";
+    let src = indoc! { r#"
+    /******************************************************************************/
+    ghost mapping(uint256 => mathint) sumOfBalances {
+        init_state axiom forall uint256 token . sumOfBalances[token] == 0;
+    }
+    "#};
+
+    let parsed_doc_data = parse_to_exactly_one_element(src).map(CvlDoc::data);
+
+    let Ok(DocData::FreeForm(s)) = parsed_doc_data else { panic!() };
+    assert!(s.is_empty());
+}
+
+#[test]
+fn freeform_stars_before_and_after() {
+    let src = indoc! { r#"
+    /******************************************************************************/
+
+    /// The sum of the balances over all users must equal the total supply for a 
+    /// given token.
+    invariant total_supply_is_sum_of_balances(uint256 token)
+        sumOfBalances[token] == totalSupply(token)
+        {
+            preserved {
+                requireInvariant balanceOfZeroAddressIsZero(token);
+            }
+        }
+
+    /******************************************************************************/
+
+    "#};
+
+    let expected_name = "total_supply_is_sum_of_balances";
+    let parsed_docs = parse_src(src);
+
+    assert!(find_by_name_of_associated_element(expected_name, &parsed_docs).is_some());
+}
+
+#[test]
+fn span_contains_both_doc_and_associated_element() {
+    let src = indoc! { r#"
+    /// If a method call reduces account balances, the caller must be either the 
+    /// holder of the account or approved to act on the holder's behalf.
+    rule onlyHolderOrApprovedCanReduceBalance(method f) 
+    {
+        address holder; uint256 token; uint256 amount;
+        uint256 balanceBefore = balanceOf(holder, token);
+
+        env e; calldataarg args;
+        f(e, args);
+
+        uint256 balanceAfter = balanceOf(holder, token);
+
+        assert balanceAfter < balanceBefore => e.msg.sender == holder || isApprovedForAll(holder, e.msg.sender), 
+            "An account balance may only be reduced by the holder or a holder-approved agent";
+    }
+    "#};
+
+    let Ok(CvlDoc { raw, .. }) = parse_to_exactly_one_element(src) else { panic!() };
+    assert_eq!(raw, src.trim());
+}

src/python_wrapper/CHANGELOG.md

@@ -1,4 +1,9 @@
 # Changelog
+## [0.5.0] - 2022-11-16
+### Added
+- Expose wrapper classes to Python, so their types can be named 
+
+
 ## [0.4.2] - 2022-10-02
 ### Fixed
 - Sync with `cvldoc_parser_core` version `0.4.2`.

src/python_wrapper/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "cvldoc_parser_py"
-version = "0.4.2"
+version = "0.5.0"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

src/python_wrapper/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "maturin"
 
 [project]
 name = "cvldoc_parser"
-version = "0.4.2"
+version = "0.5.0"
 requires-python = ">=3.7"
 classifiers = [
     "Programming Language :: Rust",

src/python_wrapper/src/lib.rs

@@ -10,7 +10,7 @@ use std::{fs::File, io::Read};
 use wrapper_structs::conversions::cvldoc_to_py_object;
 
 fn cvldocs_from_path(path: &str) -> Result<Vec<CvlDoc>> {
-    let mut file = File::open(&path).wrap_err_with(|| format!("file does not exist: {path}"))?;
+    let mut file = File::open(path).wrap_err_with(|| format!("file does not exist: {path}"))?;
 
     let rope = {
         let mut data = String::new();
@@ -52,6 +52,17 @@ fn parse(paths: Vec<&str>) -> Vec<Py<PyAny>> {
 
 #[pymodule]
 fn cvldoc_parser(_py: Python, m: &PyModule) -> PyResult<()> {
+    use wrapper_structs::*;
+
+    m.add_class::<Documentation>()?;
+    m.add_class::<FreeForm>()?;
+    m.add_class::<AssociatedElement>()?;
+    m.add_class::<DocumentationTag>()?;
+    m.add_class::<Diagnostic>()?;
+    m.add_class::<Severity>()?;
+    m.add_class::<Position>()?;
+    m.add_class::<Range>()?;
+
     m.add_function(wrap_pyfunction!(parse, m)?)?;
     Ok(())
 }