Merge branch 'main' into test/bes-207-improve-test-suite

.npmignore

@@ -0,0 +1,5 @@
+.github
+.husky
+
+test
+sample-data

README.md

@@ -47,7 +47,7 @@ npx @defi-wonderland/natspec-smells --include solidity --exclude solidity/(test|
 
 3. Run
    ```bash
-   yarn @defi-wonderland/natspec-smells
+   yarn natspec-smells
    ```
 
 ## Verify your natspec in CI
@@ -71,4 +71,3 @@ Natspec Smells was built with ❤️ by [Wonderland](https://defi.sucks).
 Wonderland the largest core development group in web3. Our commitment is to a financial future that's open, decentralized, and accessible to all.
 
 [DeFi sucks](https://defi.sucks), but Wonderland is here to make it better.
-``

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@defi-wonderland/natspec-smells",
-  "version": "1.0.0",
+  "version": "1.0.2",
   "description": "Automatically identify missing or incomplete natspec",
   "homepage": "https://github.com/defi-wonderland/natspec-smells#readme",
   "repository": {

sample-data/BasicSample.sol

@@ -76,6 +76,15 @@ contract BasicSample is AbstractBasic {
         return (true, 111);
     }
 
+    /**
+     * @notice External function that returns a bool
+     * @dev A dev comment
+     * @return Some return data
+     */
+    function externalSimpleMultipleUnnamedReturn() external pure returns (bool, uint256) {
+        return (true, 111);
+    }
+
     /**
      * @notice This function should have an inheritdoc tag
      */

sample-data/ParserTest.sol

@@ -179,5 +179,12 @@ contract ParserTestFunny is IParserTest {
   function _viewLinterFail() internal pure {
 
   }
+
+  /// fun fact: there are extra spaces after the 1st return
+  /// @return       
+  /// @return
+  function functionUnnamedEmptyReturn() external view returns (uint256, bool){
+    return (1, true);
+  }
 }
 // forgefmt: disable-end

src/main.ts

@@ -18,6 +18,11 @@ import { Validator } from './validator';
   const processor = new Processor(validator);
   const warnings = await processor.processSources(sourceUnits);
 
+  if (!warnings.length) {
+    console.warn('No issues found');
+    return;
+  }
+
   warnings.forEach(({ location, messages }) => {
     console.warn(location);
     messages.forEach((message) => {

src/utils.ts

@@ -95,7 +95,7 @@ export function parseNodeNatspec(node: NodeToProcess): Natspec {
           result.inheritdoc = { content: tagMatch[2] };
         }
       } else if (tagName === 'param' || tagName === 'return') {
-        const tagMatch = line.match(/^\s*@(\w+) *(\w+) (.*)$/);
+        const tagMatch = line.match(/^\s*@(\w+) *(\w*) *(.*)$/);
         if (tagMatch) {
           currentTag = { name: tagMatch[2], content: tagMatch[3].trim() };
           result[tagName === 'param' ? 'params' : 'returns'].push(currentTag);

src/validator.ts

@@ -75,17 +75,13 @@ export class Validator {
     let functionReturns = node.vReturnParameters.vParameters.map((p) => p.name);
 
     // Make sure all defined returns have natspec
-    for (let paramName of functionReturns) {
-      if (!natspecReturns.includes(paramName)) {
-        let message = paramName === '' ? '@return missing for unnamed return' : `@return ${paramName} is missing`;
+    for (let [paramIndex, paramName] of functionReturns.entries()) {
+      if (paramIndex > natspecReturns.length - 1) {
+        let message = paramName === '' ? `@return missing for unnamed return №${paramIndex + 1}` : `@return ${paramName} is missing`;
+        alerts.push(message);
+      } else if (natspecReturns[paramIndex] !== paramName && paramName !== '') {
+        let message = `@return ${paramName} is missing`;
         alerts.push(message);
-      }
-    }
-
-    // Make sure there is no natspec defined for non-existing returns
-    for (let paramName of natspecReturns) {
-      if (paramName && !functionReturns.includes(paramName)) {
-        alerts.push(`Missing named return for: @return ${paramName}`);
       }
     }
 

test/parser.test.ts

@@ -420,5 +420,27 @@ describe('Parser', () => {
         })
       );
     });
+
+    it('should correctly parse empty return tag', async () => {
+      const node = contract.vFunctions.find(({ name }) => name === 'functionUnnamedEmptyReturn')!;
+      const result = parseNodeNatspec(node);
+
+      expect(result).toEqual(
+        mockNatspec({
+          tags: [],
+          params: [],
+          returns: [
+            {
+              name: '',
+              content: '',
+            },
+            {
+              name: '',
+              content: '',
+            },
+          ],
+        })
+      );
+    });
   });
 });

test/validator.test.ts

@@ -144,7 +144,103 @@ describe('Validator', () => {
     };
 
     const result = validator.validate(node, natspec);
-    expect(result).toContainEqual(`@return missing for unnamed return`);
+    expect(result).toContainEqual(`@return missing for unnamed return №2`);
+  });
+
+  it('should warn of a missing unnamed return', () => {
+    node = contract.vFunctions.find(({ name }) => name === 'externalSimpleMultipleUnnamedReturn')!;
+    let natspec = {
+      tags: [
+        {
+          name: 'notice',
+          content: 'External function that returns a bool',
+        },
+        {
+          name: 'dev',
+          content: 'A dev comment',
+        },
+      ],
+      params: [],
+      returns: [
+        {
+          name: 'Some',
+          content: 'return data',
+        },
+      ],
+    };
+
+    const result = validator.validate(node, natspec);
+    expect(result).toEqual([`@return missing for unnamed return №2`]); // only 1 warning
+  });
+
+  it('should warn all returns if the first natspec tag is missing', () => {
+    node = contract.vFunctions.find(({ name }) => name === 'externalSimpleMultipleReturn')!;
+    let natspec = {
+      tags: [
+        {
+          name: 'notice',
+          content: 'External function that returns a bool',
+        },
+        {
+          name: 'dev',
+          content: 'A dev comment',
+        },
+      ],
+      params: [
+        {
+          name: '_magicNumber',
+          content: 'A parameter description',
+        },
+        {
+          name: '_name',
+          content: 'Another parameter description',
+        },
+      ],
+      returns: [
+        {
+          name: 'Some',
+          content: 'return data',
+        },
+      ],
+    };
+
+    const result = validator.validate(node, natspec);
+    expect(result).toEqual(['@return _isMagic is missing', '@return missing for unnamed return №2']); // 2 warnings
+  });
+
+  it('should warn if the last natspec tag is missing', () => {
+    node = contract.vFunctions.find(({ name }) => name === 'externalSimpleMultipleReturn')!;
+    let natspec = {
+      tags: [
+        {
+          name: 'notice',
+          content: 'External function that returns a bool',
+        },
+        {
+          name: 'dev',
+          content: 'A dev comment',
+        },
+      ],
+      params: [
+        {
+          name: '_magicNumber',
+          content: 'A parameter description',
+        },
+        {
+          name: '_name',
+          content: 'Another parameter description',
+        },
+      ],
+      returns: [
+        {
+          name: '_isMagic',
+          content: 'Some return data',
+        },
+      ],
+    };
+
+    const result = validator.validate(node, natspec);
+    expect(result).toEqual(['@return missing for unnamed return №2']); // 1 warnings
   });
 
   // TODO: Check overridden functions, virtual, etc?