Include return param names in modified namespaced compilation (#1050)

OpenZeppelin · Jul 22, 2024 · 742415c · 742415c
1 parent 6a9aef6
commit 742415c
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 8 deletions.
diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## 1.34.4 (2024-07-22)
+
+- Fix Hardhat compile error when return parameter names are documented as param. ([#1050](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1050))
+
 ## 1.34.3 (2024-07-19)
 
 - Fix Hardhat compile error when multiple return parameters are documented. ([#1048](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1048))

diff --git a/packages/core/contracts/test/NamespacedToModify.sol b/packages/core/contracts/test/NamespacedToModify.sol
@@ -231,6 +231,13 @@ function hasMultipleReturns() pure returns (uint, uint) {
 function hasMultipleNamedReturns() pure returns (uint a, uint b) {
 }
 
+/**
+ * @param a first
+ * @param b second
+ */
+function hasReturnsDocumentedAsParams() pure returns (uint a, uint b) {
+}
+
 contract HasNatSpecWithMultipleReturns {
     /**
      * @return uint 1
@@ -245,4 +252,11 @@ contract HasNatSpecWithMultipleReturns {
      */
     function hasMultipleNamedReturnsInContract() public pure returns (uint a, uint b) {
     }
+
+    /**
+     * @param a first
+     * @param b second
+     */
+    function hasReturnsDocumentedAsParamsInContract() public pure returns (uint a, uint b) {
+    }
 }
diff --git a/packages/core/package.json b/packages/core/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openzeppelin/upgrades-core",
-  "version": "1.34.3",
+  "version": "1.34.4",
   "description": "",
   "repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/core",
   "license": "MIT",

diff --git a/packages/core/src/utils/make-namespaced.test.ts.md b/packages/core/src/utils/make-namespaced.test.ts.md
@@ -59,7 +59,7 @@ Generated by [AVA](https://avajs.dev).
               bytes32 private constant MAIN_STORAGE_LOCATION =␊
                   0x183a6125c38840424c4a85fa12bab2ab606c4b6d0e7cc73c0c06ba5300eab500;␊
           ␊
-              function _getMainStorage() private pure returns (bool) {}␊
+              function _getMainStorage() private pure returns (bool $) {}␊
           ␊
               function _getXTimesY() internal view returns (bool) {}␊
           ␊
@@ -221,26 +221,38 @@ Generated by [AVA](https://avajs.dev).
            * @return uint 1␊
            * @return uint 2␊
            */␊
-          function hasMultipleReturns() pure returns (bool,bool) {}␊
+          function hasMultipleReturns() pure returns (bool, bool) {}␊
           ␊
           /**␊
            * @return a first␊
            * @return b second␊
            */␊
-          function hasMultipleNamedReturns() pure returns (bool,bool) {}␊
+          function hasMultipleNamedReturns() pure returns (bool a, bool b) {}␊
+          ␊
+          /**␊
+           * @param a first␊
+           * @param b second␊
+           */␊
+          function hasReturnsDocumentedAsParams() pure returns (bool a, bool b) {}␊
           ␊
           contract HasNatSpecWithMultipleReturns {␊
               /**␊
                * @return uint 1␊
                * @return uint 2␊
                */␊
-              function hasMultipleReturnsInContract() public pure returns (bool,bool) {}␊
+              function hasMultipleReturnsInContract() public pure returns (bool, bool) {}␊
           ␊
               /**␊
                * @return a first␊
                * @return b second␊
                */␊
-              function hasMultipleNamedReturnsInContract() public pure returns (bool,bool) {}␊
+              function hasMultipleNamedReturnsInContract() public pure returns (bool a, bool b) {}␊
+          ␊
+              /**␊
+               * @param a first␊
+               * @param b second␊
+               */␊
+              function hasReturnsDocumentedAsParamsInContract() public pure returns (bool a, bool b) {}␊
           }␊
           `,
         },

diff --git a/packages/core/src/utils/make-namespaced.test.ts.snap b/packages/core/src/utils/make-namespaced.test.ts.snap
diff --git a/packages/core/src/utils/make-namespaced.ts b/packages/core/src/utils/make-namespaced.ts
@@ -3,7 +3,7 @@ import { Node } from 'solidity-ast/node';
 import { SolcInput, SolcOutput } from '../solc-api';
 import { getStorageLocationAnnotation } from '../storage/namespace';
 import { assert } from './assert';
-import { FunctionDefinition } from 'solidity-ast';
+import { FunctionDefinition, VariableDeclaration } from 'solidity-ast';
 
 const OUTPUT_SELECTION = {
   '*': {
@@ -176,7 +176,11 @@ function replaceFunction(node: FunctionDefinition, orig: Buffer, modifications:
 
     if (node.returnParameters.parameters.length > 0) {
       modifications.push(
-        makeReplace(node.returnParameters, orig, `(${node.returnParameters.parameters.map(() => 'bool').join(',')})`),
+        makeReplace(
+          node.returnParameters,
+          orig,
+          `(${node.returnParameters.parameters.map(param => toReturnParameterReplacement(param)).join(', ')})`,
+        ),
       );
     }
 
@@ -186,6 +190,14 @@ function replaceFunction(node: FunctionDefinition, orig: Buffer, modifications:
   }
 }
 
+function toReturnParameterReplacement(param: VariableDeclaration) {
+  if (param.name.length > 0) {
+    return `bool ${param.name}`;
+  } else {
+    return 'bool';
+  }
+}
+
 function getPositions(node: Node) {
   const [start, length] = node.src.split(':').map(Number);
   const end = start + length;