Add tests, fix checking duplicates

packages/core/contracts/test/Namespaced.sol

@@ -24,18 +24,6 @@ contract Example {
     }
 }
 
-contract DuplicateNamespace {
-    /// @custom:storage-location erc7201:conflicting
-    struct Conflicting1 {
-        uint256 b;
-    }
-
-    /// @custom:storage-location erc7201:conflicting
-    struct Conflicting2 {
-        uint256 c;
-    }
-}
-
 contract MultipleNamespaces {
     /// @custom:storage-location erc7201:one
     struct S1 {

packages/core/contracts/test/NamespacedConflicts.sol

@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+contract DuplicateNamespace {
+    /// @custom:storage-location erc7201:conflicting
+    struct Conflicting1 {
+        uint256 b;
+    }
+
+    /// @custom:storage-location erc7201:conflicting
+    struct Conflicting2 {
+        uint256 c;
+    }
+}
+
+contract ConflictsWithParent is DuplicateNamespace {
+    /// @custom:storage-location erc7201:conflicting
+    struct Conflicting {
+        uint256 a;
+    }
+}
+
+contract ConflictsInBothParents is DuplicateNamespace, ConflictsWithParent {
+}

packages/core/hardhat.config.js

@@ -40,6 +40,7 @@ module.exports = {
     ],
     overrides: {
       'contracts/test/Namespaced.sol': { version: '0.8.20', settings },
+      'contracts/test/NamespacedConflicts.sol': { version: '0.8.20', settings },
     },
   },
   etherscan: {

packages/core/src/storage-namespaced-conflicts.test.ts

@@ -0,0 +1,49 @@
+import _test, { TestFn } from 'ava';
+import { ContractDefinition } from 'solidity-ast';
+import { findAll, astDereferencer } from 'solidity-ast/utils';
+import { artifacts } from 'hardhat';
+
+import { SolcOutput } from './solc-api';
+import { StorageLayout } from './storage/layout';
+import { extractStorageLayout } from './storage/extract';
+import { solcInputOutputDecoder } from './src-decoder';
+
+interface Context {
+  extractStorageLayout: (contract: string) => ReturnType<typeof extractStorageLayout>;
+}
+
+const test = _test as TestFn<Context>;
+
+test.before(async t => {
+  const buildInfo = await artifacts.getBuildInfo('contracts/test/NamespacedConflicts.sol:DuplicateNamespace');
+  if (buildInfo === undefined) {
+    throw new Error('Build info not found');
+  }
+  const solcOutput: SolcOutput = buildInfo.output;
+  const contracts: Record<string, ContractDefinition> = {};
+  const storageLayouts: Record<string, StorageLayout> = {};
+  for (const def of findAll('ContractDefinition', solcOutput.sources['contracts/test/NamespacedConflicts.sol'].ast)) {
+    contracts[def.name] = def;
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    storageLayouts[def.name] = solcOutput.contracts['contracts/test/NamespacedConflicts.sol'][def.name].storageLayout!;
+  }
+  const deref = astDereferencer(solcOutput);
+  const decodeSrc = solcInputOutputDecoder(buildInfo.input, solcOutput);
+  t.context.extractStorageLayout = name =>
+    extractStorageLayout(contracts[name], decodeSrc, deref, storageLayouts[name]);
+});
+
+test('duplicate namespace', t => {
+  const error = t.throws(() => t.context.extractStorageLayout('DuplicateNamespace'));
+  t.snapshot(error?.message);
+});
+
+test('conflicts with parent', t => {
+  const error = t.throws(() => t.context.extractStorageLayout('ConflictsWithParent'));
+  t.snapshot(error?.message);
+});
+
+test('conflicts in both parents', t => {
+  const error = t.throws(() => t.context.extractStorageLayout('ConflictsInBothParents'));
+  t.snapshot(error?.message);
+});

packages/core/src/storage-namespaced-conflicts.test.ts.md

@@ -0,0 +1,43 @@
+# Snapshot report for `src/storage-namespaced-conflicts.test.ts`
+
+The actual snapshot is saved in `storage-namespaced-conflicts.test.ts.snap`.
+
+Generated by [AVA](https://avajs.dev).
+
+## duplicate namespace
+
+> Snapshot 1
+
+    `Namespace erc7201:conflicting is defined multiple times for contract DuplicateNamespace␊
+    ␊
+    The namespace erc7201:conflicting was found in structs at the following locations:␊
+    - contracts/test/NamespacedConflicts.sol:6␊
+    - contracts/test/NamespacedConflicts.sol:11␊
+    ␊
+    Use a unique namespace id for each struct annotated with '@custom:storage-location erc7201:<NAMESPACE_ID>' in your contract and its inherited contracts.`
+
+## conflicts with parent
+
+> Snapshot 1
+
+    `Namespace erc7201:conflicting is defined multiple times for contract ConflictsWithParent␊
+    ␊
+    The namespace erc7201:conflicting was found in structs at the following locations:␊
+    - contracts/test/NamespacedConflicts.sol:18␊
+    - contracts/test/NamespacedConflicts.sol:6␊
+    - contracts/test/NamespacedConflicts.sol:11␊
+    ␊
+    Use a unique namespace id for each struct annotated with '@custom:storage-location erc7201:<NAMESPACE_ID>' in your contract and its inherited contracts.`
+
+## conflicts in both parents
+
+> Snapshot 1
+
+    `Namespace erc7201:conflicting is defined multiple times for contract ConflictsInBothParents␊
+    ␊
+    The namespace erc7201:conflicting was found in structs at the following locations:␊
+    - contracts/test/NamespacedConflicts.sol:18␊
+    - contracts/test/NamespacedConflicts.sol:6␊
+    - contracts/test/NamespacedConflicts.sol:11␊
+    ␊
+    Use a unique namespace id for each struct annotated with '@custom:storage-location erc7201:<NAMESPACE_ID>' in your contract and its inherited contracts.`

packages/core/src/storage-namespaced.test.ts

@@ -40,11 +40,6 @@ test('layout', t => {
   t.snapshot(stabilizeStorageLayout(layout));
 });
 
-test('duplicate namespace', t => {
-  const layout = t.context.extractStorageLayout('DuplicateNamespace');
-  t.snapshot(stabilizeStorageLayout(layout));
-});
-
 test('multiple namespaces', t => {
   const layout = t.context.extractStorageLayout('MultipleNamespaces');
   t.snapshot(stabilizeStorageLayout(layout));

packages/core/src/storage/namespace.ts

@@ -61,7 +61,7 @@ class DuplicateNamespaceError extends UpgradesError {
     super(
       `Namespace ${id} is defined multiple times for contract ${contractName}`,
       () => `\
-The namespace ${id} was found in:
+The namespace ${id} was found in structs at the following locations:
 - ${srcs.join('\n- ')}
 
 Use a unique namespace id for each struct annotated with '@custom:storage-location erc7201:<NAMESPACE_ID>' in your contract and its inherited contracts.`,
@@ -111,7 +111,7 @@ function pushInheritedNamespaces(
   if (namespacedContext === undefined) {
     for (let i = 0; i < origInheritIds.length; i++) {
       const origInherit = origContext.deref(['ContractDefinition'], origInheritIds[i]);
-      pushDirectNamespaces(namespaces, decodeSrc, layout, origContext, origInherit);
+      pushDirectNamespaces(namespaces, decodeSrc, layout, { ...origContext, contractDef: origInherit }, origInherit);
     }
   } else {
     const namespacedInheritIds = namespacedContext.contractDef.linearizedBaseContracts.slice(1);
@@ -215,7 +215,7 @@ function getOriginalStruct(structCanonicalName: string, origContractDef: Contrac
       }
     }
   }
-  throw new Error(`Could not find original source location for namespace struct with name ${structCanonicalName}`);
+  throw new Error(`Could not find original source location for namespace struct with name ${structCanonicalName} from contract ${origContractDef.name}`);
 }
 
 function getOriginalMemberSrc(structCanonicalName: string, memberLabel: string, origContractDef: ContractDefinition) {