Repository: NomicFoundation/hardhat-ignition
Pull request #563 (2023-10-16 21:38:51)

Support fully qualified contract names (PR 3)
This adds support for Fully Qualified Names (FQN) to be used as contract names. This is important, as Hardhat uses FQNs (e.g. `contracts/Foo.sol:Foo`) whenever a bare name (e.g. `Foo`) is duplicated.

To achieve this, I changed two things:

* If a future id is derived from a contract name, and the contract name is a FQN, we only use the contract name (e.g. `Foo` above) part of the FQN.
* If a FQN is used as a contract name, we only validate its contract name part.


Patch:
diff --git a/packages/core/src/internal/module-builder.ts b/packages/core/src/internal/module-builder.ts
index 99e49fb5..17ca37ce 100644
--- a/packages/core/src/internal/module-builder.ts
+++ b/packages/core/src/internal/module-builder.ts
@@ -67,11 +67,12 @@ import { resolveArgsToFutures } from "./utils";
 import { assertIgnitionInvariant } from "./utils/assertions";
 import {
   toCallFutureId,
-  toDeploymentFutureId,
+  toContractFutureId,
   toReadEventArgumentFutureId,
   toSendDataFutureId,
 } from "./utils/future-id-builders";
 import {
+  isValidContractName,
   isValidFunctionOrEventName,
   isValidIgnitionIdentifier,
   isValidSolidityIdentifier,
@@ -254,7 +255,7 @@ class IgnitionModuleBuilderImplementation<
     args: ArgumentType[] = [],
     options: ContractOptions = {}
   ): NamedArtifactContractDeploymentFuture<ContractNameT> {
-    const futureId = toDeploymentFutureId(
+    const futureId = toContractFutureId(
       this._module.id,
       options.id,
       contractName
@@ -309,7 +310,7 @@ class IgnitionModuleBuilderImplementation<
     args: ArgumentType[] = [],
     options: ContractOptions = {}
   ): ContractDeploymentFuture {
-    const futureId = toDeploymentFutureId(
+    const futureId = toContractFutureId(
       this._module.id,
       options.id,
       contractName
@@ -391,7 +392,7 @@ class IgnitionModuleBuilderImplementation<
     libraryName: LibraryNameT,
     options: LibraryOptions = {}
   ): NamedArtifactLibraryDeploymentFuture<LibraryNameT> {
-    const futureId = toDeploymentFutureId(
+    const futureId = toContractFutureId(
       this._module.id,
       options.id,
       libraryName
@@ -433,7 +434,7 @@ class IgnitionModuleBuilderImplementation<
     artifact: Artifact,
     options: LibraryOptions = {}
   ): LibraryDeploymentFuture {
-    const futureId = toDeploymentFutureId(
+    const futureId = toContractFutureId(
       this._module.id,
       options.id,
       libraryName
@@ -677,7 +678,7 @@ class IgnitionModuleBuilderImplementation<
       | ModuleParameterRuntimeValue<string>,
     options: ContractAtOptions = {}
   ): NamedArtifactContractAtFuture<ContractNameT> {
-    const futureId = toDeploymentFutureId(
+    const futureId = toContractFutureId(
       this._module.id,
       options.id,
       contractName
@@ -719,7 +720,7 @@ class IgnitionModuleBuilderImplementation<
       | ModuleParameterRuntimeValue<string>,
     options: ContractAtOptions = {}
   ): ContractAtFuture {
-    const futureId = toDeploymentFutureId(
+    const futureId = toContractFutureId(
       this._module.id,
       options.id,
       contractName
@@ -932,13 +933,12 @@ class IgnitionModuleBuilderImplementation<
     contractName: string,
     func: (...[]: any[]) => any
   ) {
-    // TODO: This doesn't support FQNs
-    if (isValidSolidityIdentifier(contractName)) {
+    if (isValidContractName(contractName)) {
       return;
     }
 
     this._throwErrorWithStackTrace(
-      `The contract name "${contractName}" is invalid, make sure you use a valid identifier.`,
+      `The contract name "${contractName}" is invalid.`,
       func
     );
   }
diff --git a/packages/core/src/internal/utils/future-id-builders.ts b/packages/core/src/internal/utils/future-id-builders.ts
index be8ffe31..a3d7f4f1 100644
--- a/packages/core/src/internal/utils/future-id-builders.ts
+++ b/packages/core/src/internal/utils/future-id-builders.ts
@@ -16,23 +16,40 @@ const SUBMODULE_SEPARATOR = "~";
 const SUBKEY_SEPERATOR = ".";
 
 /**
- * Construct the future id for a contract or library deployment, namespaced by the
+ * Construct the future id for a contract, contractAt or library, namespaced by the
  * moduleId.
  *
+ * This method supports both bare contract names (e.g. `MyContract`) and fully
+ * qualified names (e.g. `contracts/MyModule.sol:MyContract`).
+ *
+ * If a fully qualified name is used, the id is only direvied from its contract
+ * name, ignoring its source name part. The reason is that ids need to be
+ * compatible with most common file systems (including Windows!), and the source
+ * name may have incompatible characters.
+ *
  * @param moduleId - the id of the module the future is part of
  * @param userProvidedId - the overriding id provided by the user (it will still
  * be namespaced)
- * @param contractOrLibraryName - the contract or library name as a fallback
+ * @param contractOrLibraryName - the contract or library name, either a bare name
+ * or a fully qualified name.
  * @returns the future id
  */
-export function toDeploymentFutureId(
+export function toContractFutureId(
   moduleId: string,
   userProvidedId: string | undefined,
   contractOrLibraryName: string
 ) {
-  return `${moduleId}${MODULE_SEPERATOR}${
-    userProvidedId ?? contractOrLibraryName
-  }`;
+  // IMPORTANT: Keep in sync with src/internal/utils/identifier-validators.ts#isValidContractName
+
+  if (userProvidedId !== undefined) {
+    return `${moduleId}${MODULE_SEPERATOR}${userProvidedId}`;
+  }
+
+  const contractName = contractOrLibraryName.includes(":")
+    ? contractOrLibraryName.split(":").at(-1)!
+    : contractOrLibraryName;
+
+  return `${moduleId}${MODULE_SEPERATOR}${contractName}`;
 }
 
 /**
diff --git a/packages/core/src/internal/utils/identifier-validators.ts b/packages/core/src/internal/utils/identifier-validators.ts
index f7c81272..28399c78 100644
--- a/packages/core/src/internal/utils/identifier-validators.ts
+++ b/packages/core/src/internal/utils/identifier-validators.ts
@@ -49,3 +49,30 @@ export function isValidSolidityIdentifier(identifier: string): boolean {
 export function isValidFunctionOrEventName(functionName: string): boolean {
   return functionNameRegex.test(functionName);
 }
+
+/**
+ * Returns true if a contract name (either bare - e.g. `MyContract` - or fully
+ * qualified - e.g. `contracts/MyContract.sol:MyContract`) is valid.
+ *
+ * In the case of FQNs, we only validate the contract name part.
+ *
+ * The reason to validate the contract name is that we want to use them in
+ * future ids, and those need to be compatible with most common file systems
+ * (including Windows!).
+ *
+ * We don't validate the entire FQN, as we'll only use its bare name to
+ * derive ids.
+ *
+ * @param contractName A bare or FQN contract name to validate.
+ * @returns true if the contract name is valid.
+ */
+export function isValidContractName(contractName: string): boolean {
+  // IMPORTANT: Keep in sync with src/internal/utils/future-id-builders.ts#toContractFutureId
+
+  // Is it an FQN?
+  if (contractName.includes(":")) {
+    contractName = contractName.split(":").at(-1)!;
+  }
+
+  return isValidSolidityIdentifier(contractName);
+}


Test patch:
diff --git a/packages/core/test/utils/future-id-builders.ts b/packages/core/test/utils/future-id-builders.ts
index 5673afa0..8f123935 100644
--- a/packages/core/test/utils/future-id-builders.ts
+++ b/packages/core/test/utils/future-id-builders.ts
@@ -2,7 +2,7 @@ import { assert } from "chai";
 
 import {
   toCallFutureId,
-  toDeploymentFutureId,
+  toContractFutureId,
   toReadEventArgumentFutureId,
   toSendDataFutureId,
 } from "../../src/internal/utils/future-id-builders";
@@ -11,14 +11,14 @@ describe("future id rules", () => {
   describe("contract, library, contractAt ids", () => {
     it("the fallback id should be built based on the contract or library name", () => {
       assert.equal(
-        toDeploymentFutureId("MyModule", undefined, "MyContract"),
+        toContractFutureId("MyModule", undefined, "MyContract"),
         "MyModule#MyContract"
       );
     });
 
     it("namespaces to the module a user provided id", () => {
       assert.equal(
-        toDeploymentFutureId("MyModule", "MyId", "MyContract"),
+        toContractFutureId("MyModule", "MyId", "MyContract"),
         "MyModule#MyId"
       );
     });
diff --git a/packages/core/test/validations/id-rules.ts b/packages/core/test/validations/id-rules.ts
index 6516ff36..c7917a7a 100644
--- a/packages/core/test/validations/id-rules.ts
+++ b/packages/core/test/validations/id-rules.ts
@@ -163,68 +163,168 @@ describe("id rules", () => {
     it("should not allow non-alphanumerics in contract name", () => {
       assert.throws(() => {
         buildModule("MyModule", (m) => {
-          const myContract = m.contract("MyContract:v2");
+          const myContract = m.contract("MyContract#v2");
 
           return { myContract };
         });
-      }, /IGN702: Module validation failed with reason: The contract name "MyContract:v2" is invalid, make sure you use a valid identifier./);
+      }, /The contract name "MyContract#v2" is invalid/);
     });
 
     it("should not allow non-alphanumerics in contractFromArtifact contract name", () => {
       assert.throws(() => {
         buildModule("MyModule", (m) => {
-          const myContract = m.contract("MyContract:v2", fakeArtifact);
+          const myContract = m.contract("MyContract#v2", fakeArtifact);
 
           return { myContract };
         });
-      }, /IGN702: Module validation failed with reason: The contract name "MyContract:v2" is invalid, make sure you use a valid identifier./);
+      }, /The contract name "MyContract#v2" is invalid/);
     });
 
     it("should not allow non-alphanumerics in library contract names", () => {
       assert.throws(() => {
         buildModule("MyModule", (m) => {
-          const library = m.library("MyLibrary:v2");
+          const library = m.library("MyLibrary#v2");
 
           return { library };
         });
-      }, /IGN702: Module validation failed with reason: The contract name "MyLibrary:v2" is invalid, make sure you use a valid identifier./);
+      }, /The contract name "MyLibrary#v2" is invalid/);
     });
 
     it("should not allow non-alphanumerics in libraryFromArtifact contract names", () => {
       assert.throws(() => {
         buildModule("MyModule", (m) => {
           const myLibraryFromArtifact = m.library(
-            "MyLibraryFromArtifact:v2",
+            "MyLibraryFromArtifact#v2",
             fakeArtifact
           );
 
           return { myLibraryFromArtifact };
         });
-      }, /IGN702: Module validation failed with reason: The contract name "MyLibraryFromArtifact:v2" is invalid, make sure you use a valid identifier./);
+      }, /The contract name "MyLibraryFromArtifact#v2" is invalid/);
     });
 
     it("should not allow non-alphanumerics in contractAt contract names", () => {
       assert.throws(() => {
         buildModule("MyModule", (m) => {
-          const myContractAt = m.contractAt("MyContract:v2", exampleAddress);
+          const myContractAt = m.contractAt("MyContract#v2", exampleAddress);
 
           return { myContractAt };
         });
-      }, /IGN702: Module validation failed with reason: The contract name "MyContract:v2" is invalid, make sure you use a valid identifier./);
+      }, /The contract name "MyContract#v2" is invalid/);
     });
 
     it("should not allow non-alphanumerics in contractAtFromArtifact contract names", () => {
       assert.throws(() => {
         buildModule("MyModule", (m) => {
           const myContractAt = m.contractAt(
-            "MyContractAt:v2",
+            "MyContractAt#v2",
             fakeArtifact,
             exampleAddress
           );
 
           return { myContractAt };
         });
-      }, /IGN702: Module validation failed with reason: The contract name "MyContractAt:v2" is invalid, make sure you use a valid identifier./);
+      }, /The contract name "MyContractAt#v2" is invalid/);
+    });
+
+    describe("With Fully Qualified Names", () => {
+      it("should allow non-alphanumerics in the source name", () => {
+        assert.doesNotThrow(() => {
+          buildModule("MyModule", (m) => {
+            m.contract("sourceName.sol:MyContract");
+            m.contract("asd/sourceName.sol:MyContract2");
+            m.contractAt("sourceName.sol:MyContractAt", "0x1234");
+            m.library("sourceName.sol:MyLibrary");
+
+            return {};
+          });
+        });
+      });
+
+      it("should throw if the FQN has no contract name", () => {
+        assert.throws(() => {
+          buildModule("MyModule", (m) => {
+            const myContract = m.contract("sourceName.sol:");
+
+            return { myContract };
+          });
+        }, /The contract name "sourceName.sol:" is invalid/);
+      });
+
+      describe("All the same validations should apply, but to the contract name part of the FQN", () => {
+        it("should not allow non-alphanumerics in contract name", () => {
+          assert.throws(() => {
+            buildModule("MyModule", (m) => {
+              const myContract = m.contract("sourceName.sol:MyContract#v2");
+
+              return { myContract };
+            });
+          }, /The contract name "sourceName.sol:MyContract#v2" is invalid/);
+        });
+
+        it("should not allow non-alphanumerics in contractFromArtifact contract name", () => {
+          assert.throws(() => {
+            buildModule("MyModule", (m) => {
+              const myContract = m.contract(
+                "sourceName.sol:MyContract#v2",
+                fakeArtifact
+              );
+
+              return { myContract };
+            });
+          }, /The contract name "sourceName.sol:MyContract#v2" is invalid/);
+        });
+
+        it("should not allow non-alphanumerics in library contract names", () => {
+          assert.throws(() => {
+            buildModule("MyModule", (m) => {
+              const library = m.library("sourceName.sol:MyLibrary#v2");
+
+              return { library };
+            });
+          }, /The contract name "sourceName.sol:MyLibrary#v2" is invalid/);
+        });
+
+        it("should not allow non-alphanumerics in libraryFromArtifact contract names", () => {
+          assert.throws(() => {
+            buildModule("MyModule", (m) => {
+              const myLibraryFromArtifact = m.library(
+                "sourceName.sol:MyLibraryFromArtifact#v2",
+                fakeArtifact
+              );
+
+              return { myLibraryFromArtifact };
+            });
+          }, /The contract name "sourceName.sol:MyLibraryFromArtifact#v2" is invalid/);
+        });
+
+        it("should not allow non-alphanumerics in contractAt contract names", () => {
+          assert.throws(() => {
+            buildModule("MyModule", (m) => {
+              const myContractAt = m.contractAt(
+                "sourceName.sol:MyContract#v2",
+                exampleAddress
+              );
+
+              return { myContractAt };
+            });
+          }, /The contract name "sourceName.sol:MyContract#v2" is invalid/);
+        });
+
+        it("should not allow non-alphanumerics in contractAtFromArtifact contract names", () => {
+          assert.throws(() => {
+            buildModule("MyModule", (m) => {
+              const myContractAt = m.contractAt(
+                "sourceName.sol:MyContractAt#v2",
+                fakeArtifact,
+                exampleAddress
+              );
+
+              return { myContractAt };
+            });
+          }, /The contract name "sourceName.sol:MyContractAt#v2" is invalid/);
+        });
+      });
     });
   });