From bf24411f6056923f801a6fb119dc071335e533e6 Mon Sep 17 00:00:00 2001 From: Gas <86567384+gas1cent@users.noreply.github.com> Date: Tue, 16 Jan 2024 16:21:51 +0400 Subject: [PATCH 1/4] docs: extra ticks in readme (#18) --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 66d0f7b..8fc11c0 100644 --- a/README.md +++ b/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. -`` From f66c5b0e4582a53b4ee5e2efe5d59726ad69ad7e Mon Sep 17 00:00:00 2001 From: Gas <86567384+gas1cent@users.noreply.github.com> Date: Tue, 16 Jan 2024 16:22:04 +0400 Subject: [PATCH 2/4] fix: post-release fixes (#19) --- .npmignore | 5 +++++ package.json | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .npmignore diff --git a/.npmignore b/.npmignore new file mode 100644 index 0000000..0dab6cb --- /dev/null +++ b/.npmignore @@ -0,0 +1,5 @@ +.github +.husky + +test +sample-data diff --git a/package.json b/package.json index 28a9ae4..41631d2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@defi-wonderland/natspec-smells", - "version": "1.0.0", + "version": "1.0.1", "description": "Automatically identify missing or incomplete natspec", "homepage": "https://github.com/defi-wonderland/natspec-smells#readme", "repository": { From d5c64f78a2150f04d914a6702a100146028fd4be Mon Sep 17 00:00:00 2001 From: Gas <86567384+gas1cent@users.noreply.github.com> Date: Wed, 17 Jan 2024 22:51:33 +0400 Subject: [PATCH 3/4] feat: success message (#21) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # 🤖 Linear Closes BES-224 --- package.json | 2 +- src/main.ts | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 41631d2..5127dd8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@defi-wonderland/natspec-smells", - "version": "1.0.1", + "version": "1.0.2", "description": "Automatically identify missing or incomplete natspec", "homepage": "https://github.com/defi-wonderland/natspec-smells#readme", "repository": { diff --git a/src/main.ts b/src/main.ts index df266a2..7dc256a 100644 --- a/src/main.ts +++ b/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) => { From cf9fe522cc6d6a18f68f2583d3a022bd8ab339e1 Mon Sep 17 00:00:00 2001 From: dristpunk <107591874+dristpunk@users.noreply.github.com> Date: Fri, 26 Jan 2024 14:03:10 +0300 Subject: [PATCH 4/4] feat: unnamed returns (#22) Co-authored-by: Gas <86567384+gas1cent@users.noreply.github.com> --- sample-data/BasicSample.sol | 9 ++++ sample-data/ParserTest.sol | 7 +++ src/utils.ts | 2 +- src/validator.ts | 16 +++--- test/parser.test.ts | 22 +++++++++ test/validator.test.ts | 98 ++++++++++++++++++++++++++++++++++++- 6 files changed, 142 insertions(+), 12 deletions(-) diff --git a/sample-data/BasicSample.sol b/sample-data/BasicSample.sol index 5554dba..489d999 100644 --- a/sample-data/BasicSample.sol +++ b/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 */ diff --git a/sample-data/ParserTest.sol b/sample-data/ParserTest.sol index d33084c..6b215af 100644 --- a/sample-data/ParserTest.sol +++ b/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 diff --git a/src/utils.ts b/src/utils.ts index 980282d..e1de606 100644 --- a/src/utils.ts +++ b/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); diff --git a/src/validator.ts b/src/validator.ts index 2f323bb..3cc50a6 100644 --- a/src/validator.ts +++ b/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}`); } } diff --git a/test/parser.test.ts b/test/parser.test.ts index 5cdbb8d..ef3a2b0 100644 --- a/test/parser.test.ts +++ b/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: '', + }, + ], + }) + ); + }); }); }); diff --git a/test/validator.test.ts b/test/validator.test.ts index 1d19196..26746a6 100644 --- a/test/validator.test.ts +++ b/test/validator.test.ts @@ -153,7 +153,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?