Remove non-struct NatSpec from namespaced storage layout compilation (#…

…1051)

packages/core/CHANGELOG.md

@@ -1,8 +1,9 @@
 # Changelog
 
-## Unreleased
+## 1.35.0 (2024-07-31)
 
 - Fix Hardhat compile error when public variables are used to implement interface functions. ([#1055](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1055))
+- Remove non-struct NatSpec from Hardhat compilation step for namespaced storage layout validations. ([#1051](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1051))
 
 ## 1.34.4 (2024-07-22)
 

packages/core/contracts/test/NamespacedToModify.sol

@@ -14,6 +14,13 @@ contract Example {
         uint256 b;
     }
 
+    /// @custom:storage-location erc7201:example.with.following.comment
+    // some comment
+    struct StorageWithComment {
+        uint256 a;
+        uint256 b;
+    }
+
     /// @notice some natspec
     function foo() public {}
 

packages/core/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@openzeppelin/upgrades-core",
-  "version": "1.34.4",
+  "version": "1.35.0",
   "description": "",
   "repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/core",
   "license": "MIT",
@@ -61,6 +61,7 @@
     "typescript": "^5.0.0"
   },
   "dependencies": {
+    "@nomicfoundation/slang": "^0.15.1",
     "cbor": "^9.0.0",
     "chalk": "^4.1.0",
     "compare-versions": "^6.0.0",

packages/core/src/utils/make-namespaced.test.ts

@@ -16,6 +16,11 @@ test('make namespaced input', async t => {
   await testMakeNamespaced(origBuildInfo, t, '0.8.20');
 });
 
+test('make namespaced input - keep all natspec', async t => {
+  const origBuildInfo = await artifacts.getBuildInfo('contracts/test/NamespacedToModifyImported.sol:Example');
+  await testMakeNamespaced(origBuildInfo, t, '0.8.20', true);
+});
+
 test('make namespaced input - solc 0.7', async t => {
   // The nameNamespacedInput function must work for different solc versions, since it is called before we check whether namespaces are used with solc >= 0.8.20
   const origBuildInfo = await artifacts.getBuildInfo('contracts/test/NamespacedToModify07.sol:HasFunction');
@@ -26,6 +31,7 @@ async function testMakeNamespaced(
   origBuildInfo: BuildInfo | undefined,
   t: ExecutionContext<unknown>,
   solcVersion: string,
+  keepAllNatSpec = false,
 ) {
   if (origBuildInfo === undefined) {
     throw new Error('Build info not found');
@@ -34,7 +40,11 @@ async function testMakeNamespaced(
   // Inefficient, but we want to test that we don't actually modify the original input object
   const origInput = JSON.parse(JSON.stringify(origBuildInfo.input));
 
-  const modifiedInput = makeNamespacedInput(origBuildInfo.input, origBuildInfo.output);
+  const modifiedInput = makeNamespacedInput(
+    origBuildInfo.input,
+    origBuildInfo.output,
+    keepAllNatSpec ? undefined : origBuildInfo.solcVersion,
+  );
 
   // Run hardhat compile on the modified input and make sure it has no errors
   const modifiedOutput = await hardhatCompile(modifiedInput, solcVersion);
@@ -53,6 +63,7 @@ function normalizeIdentifiers(input: SolcInput): void {
       source.content = source.content
         .replace(/\$MainStorage_\d{1,6}/g, '$MainStorage_random')
         .replace(/\$SecondaryStorage_\d{1,6}/g, '$SecondaryStorage_random')
+        .replace(/\$StorageWithComment_\d{1,6}/g, '$StorageWithComment_random')
         .replace(/\$astId_\d+_\d{1,6}/g, '$astId_id_random');
     }
   }

packages/core/src/utils/make-namespaced.test.ts.md

@@ -44,6 +44,251 @@ Generated by [AVA](https://avajs.dev).
                   uint256 a;␊
                   uint256 b;␊
               } SecondaryStorage $SecondaryStorage_random;␊
+          ␊
+              /// @custom:storage-location erc7201:example.with.following.comment␊
+              // some comment␊
+              struct StorageWithComment {␊
+                  uint256 a;␊
+                  uint256 b;␊
+              } StorageWithComment $StorageWithComment_random;␊
+          ␊
+              ␊
+              function foo() public {}␊
+          ␊
+              ␊
+              function foo1(uint a) public {}␊
+          ␊
+              ␊
+              function foo2(uint a) internal {}␊
+              struct MyStruct { uint b; }␊
+          ␊
+              // keccak256(abi.encode(uint256(keccak256("example.main")) - 1)) & ~bytes32(uint256(0xff));␊
+              bytes32 private constant MAIN_STORAGE_LOCATION =␊
+                  0x183a6125c38840424c4a85fa12bab2ab606c4b6d0e7cc73c0c06ba5300eab500;␊
+          ␊
+              function _getMainStorage() private pure returns (bool $) {}␊
+          ␊
+              function _getXTimesY() internal view returns (bool) {}␊
+          ␊
+              ␊
+          ␊
+              ␊
+              enum $astId_id_random { dummy }␊
+          ␊
+              // standalone doc␊
+          ␊
+              ␊
+          ␊
+              ␊
+              function foo3() public {}␊
+          ␊
+              ␊
+              function foo4() public {}␊
+          ␊
+              ␊
+              enum $astId_id_random { dummy }␊
+          ␊
+              enum $astId_id_random { dummy }␊
+              enum $astId_id_random { dummy }␊
+              enum $astId_id_random { dummy }␊
+              enum $astId_id_random { dummy }␊
+              enum $astId_id_random { dummy }␊
+          }␊
+          ␊
+          abstract contract HasFunction {␊
+            ␊
+            function foo() pure public returns (bool) {}␊
+          }␊
+          ␊
+          abstract contract UsingFunction is HasFunction {␊
+            enum $astId_id_random { dummy }␊
+          }␊
+          ␊
+          function FreeFunctionUsingSelector() pure returns (bool) {}␊
+          ␊
+          bytes4 constant CONSTANT_USING_SELECTOR = HasFunction.foo.selector;␊
+          ␊
+          library Lib {␊
+            function usingSelector() pure public returns (bool) {}␊
+          ␊
+            function plusOne(uint x) pure public returns (bool) {}␊
+          }␊
+          ␊
+          abstract contract Consumer {␊
+            enum $astId_id_random { dummy }␊
+          ␊
+            function usingFreeFunction() pure public returns (bool) {}␊
+          ␊
+            function usingConstant() pure public returns (bool) {}␊
+          ␊
+            function usingLibraryFunction() pure public returns (bool) {}␊
+          }␊
+          ␊
+          abstract contract HasConstantWithSelector {␊
+            bytes4 constant CONTRACT_CONSTANT_USING_SELECTOR = HasFunction.foo.selector;␊
+          }␊
+          ␊
+          function plusTwo(uint x) pure returns (bool) {}␊
+          ␊
+          ␊
+          ␊
+          ␊
+          function plusThree(uint x) pure returns (bool) {}␊
+          ␊
+          ␊
+          ␊
+          ␊
+          function plusThree(uint x, uint y) pure returns (bool) {}␊
+          ␊
+          function originallyNoDocumentation() pure {}␊
+          ␊
+          ␊
+          enum $astId_id_random { dummy }␊
+          ␊
+          abstract contract UsingForDirectives {␊
+            enum $astId_id_random { dummy }␊
+          ␊
+            function usingFor(uint x) pure public returns (bool) {}␊
+          }␊
+          ␊
+          ␊
+          enum FreeEnum { MyEnum }␊
+          ␊
+          ␊
+          enum CustomErrorOutsideContract { dummy }␊
+          ␊
+          int8 constant MAX_SIZE_C = 2;␊
+          ␊
+          abstract contract StructArrayUsesConstant {␊
+            uint16 private constant MAX_SIZE = 10;␊
+          ␊
+            struct NotNamespaced {␊
+              uint16 a;␊
+              uint256[MAX_SIZE] b;␊
+              uint256[MAX_SIZE_C] c;␊
+            }␊
+          ␊
+            /// @custom:storage-location erc7201:uses.constant␊
+            struct MainStorage {␊
+              uint256 x;␊
+              uint256[MAX_SIZE] y;␊
+              uint256[MAX_SIZE_C] c;␊
+            } MainStorage $MainStorage_random;␊
+          }␊
+          ␊
+          address constant MY_ADDRESS = address(0);␊
+          uint constant CONVERTED_ADDRESS = uint160(MY_ADDRESS);␊
+          ␊
+          interface IDummy {␊
+          }␊
+          ␊
+          abstract contract UsesAddress {␊
+            IDummy public constant MY_CONTRACT = IDummy(MY_ADDRESS);␊
+          }␊
+          ␊
+          abstract contract HasFunctionWithRequiredReturn {␊
+              struct S { uint x; }␊
+              function foo(S calldata s) internal pure returns (bool) {}␊
+          }␊
+          ␊
+          ␊
+          function hasMultipleReturns() pure returns (bool, bool) {}␊
+          ␊
+          ␊
+          function hasMultipleNamedReturns() pure returns (bool a, bool b) {}␊
+          ␊
+          ␊
+          function hasReturnsDocumentedAsParams() pure returns (bool a, bool b) {}␊
+          ␊
+          abstract contract HasNatSpecWithMultipleReturns {␊
+              ␊
+              function hasMultipleReturnsInContract() public pure returns (bool, bool) {}␊
+          ␊
+              ␊
+              function hasMultipleNamedReturnsInContract() public pure returns (bool a, bool b) {}␊
+          ␊
+              ␊
+              function hasReturnsDocumentedAsParamsInContract() public pure returns (bool a, bool b) {}␊
+          }␊
+          ␊
+          interface IHasExternalViewFunction {␊
+              function foo() external view returns (bool);␊
+          }␊
+          ␊
+          abstract contract HasExternalViewFunction is IHasExternalViewFunction {␊
+              // This generates a getter function that conforms to the interface␊
+              enum $astId_id_random { dummy }␊
+          ␊
+              // References a selector in an interface␊
+              bytes4 constant USING_INTERFACE_FUNCTION_SELECTOR = IHasExternalViewFunction.foo.selector;␊
+          ␊
+              // References a getter generated for a public variable␊
+              enum $astId_id_random { dummy }␊
+          }␊
+          ␊
+          abstract contract DeploysContractToImmutable {␊
+              enum $astId_id_random { 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";␊
+          ␊
+          abstract contract Example {}␊
+          `,
+        },
+      },
+    }
+
+## make namespaced input - keep all natspec
+
+> Snapshot 1
+
+    {
+      language: 'Solidity',
+      settings: {
+        evmVersion: 'paris',
+        optimizer: {
+          enabled: true,
+          runs: 200,
+        },
+        outputSelection: {
+          '*': {
+            '': [
+              'ast',
+            ],
+            '*': [
+              'storageLayout',
+            ],
+          },
+        },
+      },
+      sources: {
+        'contracts/test/NamespacedToModify.sol': {
+          content: `// SPDX-License-Identifier: MIT␊
+          pragma solidity ^0.8.20;␊
+          ␊
+          abstract contract Example {␊
+              /// @custom:storage-location erc7201:example.main␊
+              struct MainStorage {␊
+                  uint256 x;␊
+                  uint256 y;␊
+              } MainStorage $MainStorage_random;␊
+          ␊
+              /// @custom:storage-location erc7201:example.secondary␊
+              struct SecondaryStorage {␊
+                  uint256 a;␊
+                  uint256 b;␊
+              } SecondaryStorage $SecondaryStorage_random;␊
+          ␊
+              /// @custom:storage-location erc7201:example.with.following.comment␊
+              // some comment␊
+              struct StorageWithComment {␊
+                  uint256 a;␊
+                  uint256 b;␊
+              } StorageWithComment $StorageWithComment_random;␊
           ␊
               /// @notice some natspec␊
               function foo() public {}␊