fix: natspec parser and function returns validation (#37)

* test: add insta and similar-asserts

* test: add snapshot tests

* docs: typo

* chore: update deps

* test: rename snapshot test

* test: rename test functions

* fix: natspec parser and function returns validation

Author: beeb

Cargo.lock

@@ -31,10 +31,18 @@ slang_solidity = "0.18.3"
 thiserror = "2.0.11"
 winnow = "0.7.2"
 
+[dev-dependencies]
+insta = "1.42.1"
+similar-asserts = "1.6.1"
+
 [profile.release]
 lto = "thin"
 
 # The profile that 'dist' will build with
 [profile.dist]
 inherits = "release"
 lto = "thin"
+
+[profile.dev.package]
+insta.opt-level = 3
+similar.opt-level = 3

Cargo.toml

@@ -88,7 +88,7 @@ which take precedence over the config file.
 
 ## Usage in GitHub Actions
 
-You can check your code in CI with the lintspec GitHub Action. Any `.lintspec.toml` or `.nsignore` file in
+You can check your code in CI with the lintspec GitHub Action. Any `.lintspec.toml` or `.nsignore` file in the
 repository's root will be used to configure the execution.
 
 The action generates

README.md

@@ -24,6 +24,8 @@
           buildInputs = [
             pkgs.rust-analyzer-unwrapped
             toolchain
+            pkgs.cargo-insta
+            pkgs.cargo-nextest
           ];
 
           RUST_SRC_PATH = "${toolchain}/lib/rustlib/src/rust/library";

flake.nix

@@ -91,6 +91,7 @@ impl Validate for ConstructorDefinition {
 #[cfg(test)]
 mod tests {
     use semver::Version;
+    use similar_asserts::assert_eq;
     use slang_solidity::parser::Parser;
 
     use super::*;

src/definitions/constructor.rs

@@ -106,6 +106,7 @@ fn extract_enum_members(cursor: Cursor) -> Vec<Identifier> {
 #[cfg(test)]
 mod tests {
     use semver::Version;
+    use similar_asserts::assert_eq;
     use slang_solidity::{cst::NonterminalKind, parser::Parser};
 
     use super::*;

src/definitions/enumeration.rs

@@ -94,6 +94,7 @@ impl Validate for ErrorDefinition {
 #[cfg(test)]
 mod tests {
     use semver::Version;
+    use similar_asserts::assert_eq;
     use slang_solidity::{cst::NonterminalKind, parser::Parser};
 
     use super::*;

src/definitions/error.rs

@@ -94,6 +94,7 @@ impl Validate for EventDefinition {
 #[cfg(test)]
 mod tests {
     use semver::Version;
+    use similar_asserts::assert_eq;
     use slang_solidity::{cst::NonterminalKind, parser::Parser};
 
     use super::*;

src/definitions/event.rs

@@ -119,9 +119,13 @@ impl Validate for FunctionDefinition {
         if self.name == "receive" || self.name == "fallback" {
             return out;
         }
-        // raise error if no NatSpec is available (unless there are no params and we don't enforce inheritdoc)
+        // raise error if no NatSpec is available (unless there are no params, no returns, and we don't enforce
+        // inheritdoc)
         let Some(natspec) = &self.natspec else {
-            if self.params.is_empty() && (!options.inheritdoc || !self.requires_inheritdoc()) {
+            if self.returns.is_empty()
+                && self.params.is_empty()
+                && (!options.inheritdoc || !self.requires_inheritdoc())
+            {
                 return out;
             }
             // we require natspec for either inheritdoc or the params
@@ -156,6 +160,7 @@ impl Validate for FunctionDefinition {
 #[cfg(test)]
 mod tests {
     use semver::Version;
+    use similar_asserts::assert_eq;
     use slang_solidity::parser::Parser;
 
     use super::*;
@@ -362,4 +367,17 @@ mod tests {
         assert_eq!(res.diags.len(), 1);
         assert_eq!(res.diags[0].message, "@inheritdoc is missing");
     }
+
+    #[test]
+    fn test_function_internal() {
+        let contents = "contract Test {
+            // @notice
+            function _viewInternal() internal view returns (uint256) {
+                return 1;
+            }
+        }";
+        let res = parse_file(contents).validate(&OPTIONS);
+        assert_eq!(res.diags.len(), 1);
+        assert_eq!(res.diags[0].message, "missing NatSpec");
+    }
 }

src/definitions/function.rs

@@ -576,6 +576,7 @@ pub fn extract_parent_name(mut cursor: Cursor) -> Option<Parent> {
 
 #[cfg(test)]
 mod tests {
+    use similar_asserts::assert_eq;
     use slang_solidity::parser::Parser;
 
     use crate::{

src/definitions/mod.rs

@@ -133,6 +133,7 @@ impl Validate for ModifierDefinition {
 #[cfg(test)]
 mod tests {
     use semver::Version;
+    use similar_asserts::assert_eq;
     use slang_solidity::parser::Parser;
 
     use super::*;

src/definitions/modifier.rs

@@ -113,6 +113,7 @@ fn extract_struct_members(cursor: Cursor) -> Result<Vec<Identifier>> {
 #[cfg(test)]
 mod tests {
     use semver::Version;
+    use similar_asserts::assert_eq;
     use slang_solidity::{cst::NonterminalKind, parser::Parser};
 
     use super::*;

src/definitions/structure.rs

@@ -119,6 +119,7 @@ impl Validate for VariableDeclaration {
 #[cfg(test)]
 mod tests {
     use semver::Version;
+    use similar_asserts::assert_eq;
     use slang_solidity::{cst::NonterminalKind, parser::Parser};
 
     use super::*;

src/definitions/variable.rs

@@ -227,7 +227,11 @@ fn parse_comment_line_single(input: &mut &str) -> Result<NatSpecItem> {
 }
 
 fn parse_single_line_comment(input: &mut &str) -> Result<NatSpec> {
-    let item = preceded("///", parse_comment_line_single).parse_next(input)?;
+    let item = preceded(
+        repeat::<_, _, (), _, _>(3.., '/'),
+        parse_comment_line_single,
+    )
+    .parse_next(input)?;
     if item.is_empty() {
         return Ok(NatSpec::default());
     }
@@ -236,6 +240,8 @@ fn parse_single_line_comment(input: &mut &str) -> Result<NatSpec> {
 
 #[cfg(test)]
 mod tests {
+    use similar_asserts::assert_eq;
+
     use super::*;
 
     #[test]
@@ -434,4 +440,24 @@ mod tests {
         let res = res.unwrap();
         assert_eq!(res, NatSpec::default());
     }
+
+    #[ignore]
+    #[test]
+    fn test_multiline_weird() {
+        // FIXME: this doesn't pass
+        let comment = "/**** @notice Some text
+    ** */";
+        let res = parse_comment.parse(comment);
+        assert!(res.is_ok(), "{res:?}");
+        let res = res.unwrap();
+        assert_eq!(
+            res,
+            NatSpec {
+                items: vec![NatSpecItem {
+                    kind: NatSpecKind::Notice,
+                    comment: "Some text".to_string()
+                },]
+            }
+        );
+    }
 }

src/natspec.rs

@@ -14,6 +14,12 @@ contract BasicSample is AbstractBasic {
         uint256 someNumber;
     }
 
+    /// @notice An enum
+    enum TestEnum {
+        First,
+        Second
+    }
+
     /**
      * @notice Some error missing parameter natspec
      */

test-data/BasicSample.sol

@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: MIT
+pragma solidity =0.8.19;
+
+interface IInterfacedSample {
+    /**
+     * @notice Greets the caller
+     *
+     * @return _greeting The greeting
+     * @return _balance  Current token balance of the caller
+     */
+    function greet() external view returns (string memory _greeting, uint256 _balance);
+}
+
+contract InterfacedSample is IInterfacedSample {
+    /// @inheritdoc IInterfacedSample
+    /// @dev some dev thingy
+    function greet() external view returns (string memory _greeting, uint256 _balance) {}
+}