Avoid duplicate file level functions and variables in namespace rewri…

…te (#918) Co-authored-by: Eric Lau <[email protected]>
OpenZeppelin · Nov 28, 2023 · 5da44b1 · 5da44b1
1 parent f8745df
commit 5da44b1
Show file tree

Hide file tree

Showing 7 changed files with 263 additions and 86 deletions.
diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- Fix Hardhat compile errors when contracts have overloaded functions or standalone NatSpec documentation. ([#918](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/918))
+
 ## 1.31.2 (2023-11-28)
 
 - Fix `upgradeProxy` in Hardhat from an implementation that has a fallback function and is not using OpenZeppelin Contracts 5.0. ([#926](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/926))

diff --git a/packages/core/contracts/test/NamespacedToModify.sol b/packages/core/contracts/test/NamespacedToModify.sol
@@ -61,6 +61,11 @@ contract Example {
      * @notice some natspec
      */
     function foo4() public {}
+
+    /**
+     * @dev a custom error inside a contract
+     */
+    error CustomErrorInsideContract(address a);
 }
 
 contract HasFunction {
@@ -89,6 +94,8 @@ library Lib {
 }
 
 contract Consumer {
+  bytes4 public variableFromConstant = CONSTANT_USING_SELECTOR;
+
   function usingFreeFunction() pure public returns (bytes4) {
     return FreeFunctionUsingSelector();
   }
@@ -106,6 +113,34 @@ function plusTwo(uint x) pure returns (uint) {
   return x + 2;
 }
 
+/**
+ * @notice originally orphaned natspec
+ */
+
+/**
+ * @dev plusThree
+ * @param x x
+ */
+function plusThree(uint x) pure returns (uint) {
+  return x + 3;
+}
+
+/// @notice originally orphaned natspec 2
+
+/**
+ * @dev plusThree overloaded
+ * @param x x
+ * @param y y
+ */
+function plusThree(uint x, uint y) pure returns (uint) {
+  return x + y + 3;
+}
+
+function originallyNoDocumentation() pure {}
+
+/**
+ * @param foo foo
+ */
 using {plusTwo} for uint;
 
 contract UsingForDirectives {
@@ -114,4 +149,22 @@ contract UsingForDirectives {
   function usingFor(uint x) pure public returns (uint) {
     return x.plusOne() + x.plusTwo();
   }
-}
+}
+
+/**
+ * @title a
+ * @author a
+ * @inheritdoc Example
+ * @dev a
+ * @custom:a a
+ * @notice a
+ * @param a a
+ * @return a a
+ */
+enum FreeEnum { MyEnum }
+
+/**
+ * @dev a custom error outside a contract
+ * @param example example parameter
+ */
+error CustomErrorOutsideContract(Example example);
diff --git a/packages/core/contracts/test/NamespacedToModifyImported.sol b/packages/core/contracts/test/NamespacedToModifyImported.sol
@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {CONSTANT_USING_SELECTOR, plusTwo, plusThree, CustomErrorOutsideContract} from "./NamespacedToModify.sol";
+
+contract Example {}
diff --git a/packages/core/src/utils/make-namespaced.test.ts b/packages/core/src/utils/make-namespaced.test.ts
@@ -12,7 +12,7 @@ import { SolcInput, SolcOutput } from '../solc-api';
 import { BuildInfo } from 'hardhat/types';
 
 test('make namespaced input', async t => {
-  const origBuildInfo = await artifacts.getBuildInfo('contracts/test/NamespacedToModify.sol:Example');
+  const origBuildInfo = await artifacts.getBuildInfo('contracts/test/NamespacedToModifyImported.sol:Example');
   await testMakeNamespaced(origBuildInfo, t, '0.8.20');
 });
 
@@ -35,23 +35,25 @@ async function testMakeNamespaced(
   const origInput = JSON.parse(JSON.stringify(origBuildInfo.input));
 
   const modifiedInput = makeNamespacedInput(origBuildInfo.input, origBuildInfo.output);
-  normalizeStateVariableNames(modifiedInput);
-  t.snapshot(modifiedInput);
-
-  t.deepEqual(origBuildInfo.input, origInput);
-  t.notDeepEqual(modifiedInput, origInput);
 
   // Run hardhat compile on the modified input and make sure it has no errors
   const modifiedOutput = await hardhatCompile(modifiedInput, solcVersion);
   t.is(modifiedOutput.errors, undefined);
+
+  normalizeIdentifiers(modifiedInput);
+  t.snapshot(modifiedInput);
+
+  t.deepEqual(origBuildInfo.input, origInput);
+  t.notDeepEqual(modifiedInput, origInput);
 }
 
-function normalizeStateVariableNames(input: SolcInput): void {
+function normalizeIdentifiers(input: SolcInput): void {
   for (const source of Object.values(input.sources)) {
     if (source.content !== undefined) {
       source.content = source.content
-        .replace(/\$MainStorage_\d{1,6};/g, '$MainStorage_random;')
-        .replace(/\$SecondaryStorage_\d{1,6}/g, '$SecondaryStorage_random');
+        .replace(/\$MainStorage_\d{1,6}/g, '$MainStorage_random')
+        .replace(/\$SecondaryStorage_\d{1,6}/g, '$SecondaryStorage_random')
+        .replace(/\$astId_\d+_\d{1,6}/g, '$astId_id_random');
     }
   }
 }

diff --git a/packages/core/src/utils/make-namespaced.test.ts.md b/packages/core/src/utils/make-namespaced.test.ts.md
@@ -45,77 +45,142 @@ Generated by [AVA](https://avajs.dev).
                   uint256 b;␊
               } SecondaryStorage $SecondaryStorage_random;␊
           ␊
-              ␊
-              ␊
+              /// @notice some natspec␊
+              enum $astId_id_random { dummy }␊
           ␊
-              ␊
-              ␊
+              /// @param a docs␊
+              enum $astId_id_random { dummy }␊
           ␊
-              ␊
-              ␊
+              /// @param a docs␊
+              enum $astId_id_random { dummy }␊
               struct MyStruct { uint b; }␊
           ␊
               // keccak256(abi.encode(uint256(keccak256("example.main")) - 1)) & ~bytes32(uint256(0xff));␊
-              ␊
+              enum $astId_id_random { dummy }␊
           ␊
-              ␊
+              enum $astId_id_random { dummy }␊
           ␊
-              ␊
+              enum $astId_id_random { dummy }␊
           ␊
               /// @notice standlone natspec␊
           ␊
-              ␊
-              ␊
+              /// @notice natspec for var␊
+              enum $astId_id_random { dummy }␊
           ␊
               // standalone doc␊
           ␊
               /**␊
                * standlone doc block␊
                */␊
           ␊
-              ␊
-              ␊
+              /**␊
+               * doc block without natspec␊
+               */␊
+              enum $astId_id_random { dummy }␊
+          ␊
+              /**␊
+               * doc block with natspec␊
+               *␊
+               * @notice some natspec␊
+               */␊
+              enum $astId_id_random { dummy }␊
           ␊
-              ␊
-              ␊
+              /**␊
+               * @dev a custom error inside a contract␊
+               */␊
+              enum $astId_id_random { dummy }␊
           }␊
           ␊
           contract HasFunction {␊
-            ␊
-            ␊
+            enum $astId_id_random { dummy }␊
+            enum $astId_id_random { dummy }␊
           }␊
           ␊
           contract UsingFunction is HasFunction {␊
-            ␊
+            enum $astId_id_random { dummy }␊
           }␊
           ␊
-          uint256 constant FreeFunctionUsingSelector = 0;␊
+          enum FreeFunctionUsingSelector { dummy }␊
           ␊
-          uint256 constant CONSTANT_USING_SELECTOR = 0;␊
+          enum CONSTANT_USING_SELECTOR { dummy }␊
           ␊
           library Lib {␊
-            ␊
+            enum $astId_id_random { dummy }␊
           ␊
-            ␊
+            enum $astId_id_random { dummy }␊
           }␊
           ␊
           contract Consumer {␊
-            ␊
+            enum $astId_id_random { dummy }␊
+          ␊
+            enum $astId_id_random { dummy }␊
           ␊
-            ␊
+            enum $astId_id_random { dummy }␊
           ␊
-            ␊
+            enum $astId_id_random { dummy }␊
           }␊
           ␊
-          uint256 constant plusTwo = 0;␊
+          enum plusTwo { dummy }␊
+          ␊
+          /**␊
+           * @notice originally orphaned natspec␊
+           */␊
+          ␊
+          /**␊
+           * @dev plusThree␊
+           * @param x x␊
+           */␊
+          enum plusThree { dummy }␊
+          ␊
+          /// @notice originally orphaned natspec 2␊
           ␊
+          /**␊
+           * @dev plusThree overloaded␊
+           * @param x x␊
+           * @param y y␊
+           */␊
+          enum $astId_id_random { dummy }␊
           ␊
+          enum originallyNoDocumentation { dummy }␊
+          ␊
+          /**␊
+           * @param foo foo␊
+           */␊
+          enum $astId_id_random { dummy }␊
           ␊
           contract UsingForDirectives {␊
-            ␊
+            enum $astId_id_random { dummy }␊
+          ␊
+            enum $astId_id_random { dummy }␊
+          }␊
+          ␊
+          /**␊
+           * @title a␊
+           * @author a␊
+           * @inheritdoc Example␊
+           * @dev a␊
+           * @custom:a a␊
+           * @notice a␊
+           * @param a a␊
+           * @return a a␊
+           */␊
+          enum FreeEnum { MyEnum }␊
+          ␊
+          /**␊
+           * @dev a custom error outside a contract␊
+           * @param example example parameter␊
+           */␊
+          enum CustomErrorOutsideContract { dummy }␊
+          `,
+        },
+        'contracts/test/NamespacedToModifyImported.sol': {
+          content: `// SPDX-License-Identifier: MIT␊
+          pragma solidity ^0.8.20;␊
           ␊
-            ␊
-          }`,
+          import {CONSTANT_USING_SELECTOR, plusTwo, plusThree, CustomErrorOutsideContract} from "./NamespacedToModify.sol";␊
+          ␊
+          contract Example {}␊
+          `,
         },
       },
     }
@@ -148,12 +213,12 @@ Generated by [AVA](https://avajs.dev).
           pragma solidity 0.7.6;␊
            ␊
           contract HasFunction {␊
-            ␊
-            ␊
+            enum $astId_id_random { dummy }␊
+            enum $astId_id_random { dummy }␊
           }␊
           ␊
           contract UsingFunction is HasFunction {␊
-            ␊
+            enum $astId_id_random { dummy }␊
           }␊
           `,
         },

diff --git a/packages/core/src/utils/make-namespaced.test.ts.snap b/packages/core/src/utils/make-namespaced.test.ts.snap