Fix Hardhat compile error when public variables are used to implement interface functions (#1055)

ericglau · web-flow · commit 13b6c1252564 · 2024-07-31T09:40:16.000-04:00
diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- Fix Hardhat compile error when public variables are used to implement interface functions. ([#1055](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1055))
+
 ## 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))
diff --git a/packages/core/contracts/test/NamespacedToModify.sol b/packages/core/contracts/test/NamespacedToModify.sol
@@ -260,3 +260,22 @@ contract HasNatSpecWithMultipleReturns {
     function hasReturnsDocumentedAsParamsInContract() public pure returns (uint a, uint b) {
     }
 }
+
+interface IHasExternalViewFunction {
+    function foo() external view returns (uint256);
+}
+
+contract HasExternalViewFunction is IHasExternalViewFunction {
+    // This generates a getter function that conforms to the interface
+    uint256 public foo;
+
+    // References a selector in an interface
+    bytes4 constant USING_INTERFACE_FUNCTION_SELECTOR = IHasExternalViewFunction.foo.selector;
+
+    // References a getter generated for a public variable
+    bytes4 immutable IMMUTABLE_USING_GETTER = this.foo.selector;
+}
+
+contract DeploysContractToImmutable {
+    HasFunction public immutable example = new HasFunction(1);
+}
diff --git a/packages/core/src/utils/make-namespaced.test.ts.md b/packages/core/src/utils/make-namespaced.test.ts.md
@@ -32,7 +32,7 @@ Generated by [AVA](https://avajs.dev).
           content: `// SPDX-License-Identifier: MIT␊
           pragma solidity ^0.8.20;␊
           ␊
-          contract Example {␊
+          abstract contract Example {␊
               /// @custom:storage-location erc7201:example.main␊
               struct MainStorage {␊
                   uint256 x;␊
@@ -98,12 +98,12 @@ Generated by [AVA](https://avajs.dev).
               enum $astId_id_random { dummy }␊
           }␊
           ␊
-          contract HasFunction {␊
+          abstract contract HasFunction {␊
             ␊
             function foo() pure public returns (bool) {}␊
           }␊
           ␊
-          contract UsingFunction is HasFunction {␊
+          abstract contract UsingFunction is HasFunction {␊
             enum $astId_id_random { dummy }␊
           }␊
           ␊
@@ -117,7 +117,7 @@ Generated by [AVA](https://avajs.dev).
             function plusOne(uint x) pure public returns (bool) {}␊
           }␊
           ␊
-          contract Consumer {␊
+          abstract contract Consumer {␊
             enum $astId_id_random { dummy }␊
           ␊
             function usingFreeFunction() pure public returns (bool) {}␊
@@ -127,7 +127,7 @@ Generated by [AVA](https://avajs.dev).
             function usingLibraryFunction() pure public returns (bool) {}␊
           }␊
           ␊
-          contract HasConstantWithSelector {␊
+          abstract contract HasConstantWithSelector {␊
             bytes4 constant CONTRACT_CONSTANT_USING_SELECTOR = HasFunction.foo.selector;␊
           }␊
           ␊
@@ -159,7 +159,7 @@ Generated by [AVA](https://avajs.dev).
            */␊
           enum $astId_id_random { dummy }␊
           ␊
-          contract UsingForDirectives {␊
+          abstract contract UsingForDirectives {␊
             enum $astId_id_random { dummy }␊
           ␊
             function usingFor(uint x) pure public returns (bool) {}␊
@@ -185,7 +185,7 @@ Generated by [AVA](https://avajs.dev).
           ␊
           int8 constant MAX_SIZE_C = 2;␊
           ␊
-          contract StructArrayUsesConstant {␊
+          abstract contract StructArrayUsesConstant {␊
             uint16 private constant MAX_SIZE = 10;␊
           ␊
             struct NotNamespaced {␊
@@ -208,11 +208,11 @@ Generated by [AVA](https://avajs.dev).
           interface IDummy {␊
           }␊
           ␊
-          contract UsesAddress {␊
+          abstract contract UsesAddress {␊
             IDummy public constant MY_CONTRACT = IDummy(MY_ADDRESS);␊
           }␊
           ␊
-          contract HasFunctionWithRequiredReturn {␊
+          abstract contract HasFunctionWithRequiredReturn {␊
               struct S { uint x; }␊
               function foo(S calldata s) internal pure returns (bool) {}␊
           }␊
@@ -235,7 +235,7 @@ Generated by [AVA](https://avajs.dev).
            */␊
           function hasReturnsDocumentedAsParams() pure returns (bool a, bool b) {}␊
           ␊
-          contract HasNatSpecWithMultipleReturns {␊
+          abstract contract HasNatSpecWithMultipleReturns {␊
               /**␊
                * @return uint 1␊
                * @return uint 2␊
@@ -254,15 +254,33 @@ Generated by [AVA](https://avajs.dev).
                */␊
               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";␊
           ␊
-          contract Example {}␊
+          abstract contract Example {}␊
           `,
         },
       },
@@ -295,12 +313,12 @@ Generated by [AVA](https://avajs.dev).
           content: `// SPDX-License-Identifier: MIT␊
           pragma solidity 0.7.6;␊
            ␊
-          contract HasFunction {␊
+          abstract contract HasFunction {␊
             ␊
             function foo() pure public returns (bool) {}␊
           }␊
           ␊
-          contract UsingFunction is HasFunction {␊
+          abstract 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
diff --git a/packages/core/src/utils/make-namespaced.ts b/packages/core/src/utils/make-namespaced.ts
@@ -48,6 +48,12 @@ export function makeNamespacedInput(input: SolcInput, output: SolcOutput): SolcI
         case 'ContractDefinition': {
           const contractDef = node;
 
+          // Mark contracts as abstract, since state variables are converted to dummy enums
+          // which would not generate public getters for inherited interface functions
+          if (contractDef.contractKind === 'contract' && !contractDef.abstract) {
+            modifications.push(makeInsertBefore(contractDef, 'abstract '));
+          }
+
           // Remove any calls to parent constructors from the inheritance list
           const inherits = contractDef.baseContracts;
           for (const inherit of inherits) {
@@ -210,6 +216,11 @@ function makeReplace(node: Node, orig: Buffer, text: string): Modification {
   return { start, end, text };
 }
 
+function makeInsertBefore(node: Node, text: string): Modification {
+  const { start } = getPositions(node);
+  return { start: start, end: start, text };
+}
+
 function makeInsertAfter(node: Node, text: string): Modification {
   const { end } = getPositions(node);
   return { start: end, end, text };
