Simplify peer project mode by renaming imported peer symbols

Amxx · frangio · web-flow · commit 89e891da1ab7 · 2023-09-30T02:25:29.000-03:00
Co-authored-by: Francisco Giordano &lt;fg@frang.io&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.3.31 (2023-09-30)
+
+- Simplify peer project mode by renaming imported peer symbols.
+
 ## 0.3.30 (2023-09-27)
 
 - Fix `@inheritdoc` in peer project mode.
diff --git a/src/generate-with-init.ts b/src/generate-with-init.ts
@@ -23,7 +23,11 @@ export function generateWithInit(
 
   for (const sourceUnit of transform.asts()) {
     for (const contract of findAll('ContractDefinition', sourceUnit)) {
-      if (contract.contractKind !== 'contract' || !contract.fullyImplemented) {
+      if (
+        contract.contractKind !== 'contract' ||
+        !contract.fullyImplemented ||
+        transform.getData(contract).importFromPeer !== undefined
+      ) {
         continue;
       }
 
diff --git a/src/index.ts b/src/index.ts
@@ -101,7 +101,7 @@ export async function transpile(
   transform.apply(renameContractDefinition);
   transform.apply(renameInheritdoc);
   transform.apply(prependInitializableBase);
-  transform.apply(fixImportDirectives(options.peerProject !== undefined));
+  transform.apply(fixImportDirectives);
   transform.apply(appendInitializableImport(outputPaths.initializable));
   transform.apply(fixNewStatement);
   transform.apply(transformConstructor(namespaceInclude));
@@ -110,13 +110,12 @@ export async function transpile(
   transform.apply(removeInheritanceListArguments);
   transform.apply(removeStateVarInits);
   transform.apply(removeImmutable);
-  transform.apply(peerImport);
-
   if (options.namespaced) {
     transform.apply(addNamespaceStruct(namespaceInclude));
   } else {
     transform.apply(addStorageGaps);
   }
+  transform.apply(peerImport);
 
   // build a final array of files to return
   const outputFiles: OutputFile[] = [];
@@ -169,7 +168,7 @@ function transpileInitializable(
   transform.apply(function* (ast, tools) {
     if (ast.absolutePath === options.initializablePath) {
       yield* renameIdentifiers(ast, tools);
-      yield* fixImportDirectives(options.peerProject !== undefined)(ast, tools);
+      yield* fixImportDirectives(ast, tools);
     }
   });
 
diff --git a/src/prepare-peer-project.test.ts b/src/prepare-peer-project.test.ts
@@ -29,17 +29,13 @@ test.serial.before('compile', async t => {
   });
 });
 
-for (const fileName of ['ISomeInterface.sol', 'SomeLibrary.sol', 'SomeStatelessContract.sol']) {
-  test(`do not transpile ${fileName}`, t => {
-    const file = t.context.files.find(f => f.fileName === fileName);
-    // source file exists
-    t.true(t.context.inputs.includes(path.join(projectDir, fileName)));
-    // transpiled file does not exist
-    t.is(file, undefined, 'file should not be transpiled');
-  });
-}
-
-for (const fileName of ['SomeContract.sol', 'SomeOtherContract.sol']) {
+for (const fileName of [
+  'ISomeInterface.sol',
+  'SomeLibrary.sol',
+  'SomeStatelessContract.sol',
+  'SomeContract.sol',
+  'SomeOtherContract.sol',
+]) {
   test(`transpile ${fileName}`, t => {
     const file = t.context.files.find(f => f.fileName === fileName);
     // source file exists
diff --git a/src/prepare-peer-project.test.ts.md b/src/prepare-peer-project.test.ts.md
@@ -4,6 +4,50 @@ The actual snapshot is saved in `prepare-peer-project.test.ts.snap`.
 
 Generated by [AVA](https://avajs.dev).
 
+## transpile ISomeInterface.sol
+
+> Snapshot 1
+
+    {
+      fileName: 'ISomeInterface.sol',
+      path: 'contracts/project/ISomeInterfaceUpgradeable.sol',
+      source: `// SPDX-License-Identifier: UNLICENSED␊
+      pragma solidity ^0.8.0;␊
+      ␊
+      import {ISomeInterface as ISomeInterfaceUpgradeable} from "@openzeppelin/contracts/project/ISomeInterface.sol";␊
+      `,
+    }
+
+## transpile SomeLibrary.sol
+
+> Snapshot 1
+
+    {
+      fileName: 'SomeLibrary.sol',
+      path: 'contracts/project/SomeLibraryUpgradeable.sol',
+      source: `// SPDX-License-Identifier: UNLICENSED␊
+      pragma solidity ^0.8.0;␊
+      ␊
+      import {ISomeInterfaceUpgradeable} from "./ISomeInterfaceUpgradeable.sol";␊
+      ␊
+      import {SomeLibrary as SomeLibraryUpgradeable} from "@openzeppelin/contracts/project/SomeLibrary.sol";␊
+      `,
+    }
+
+## transpile SomeStatelessContract.sol
+
+> Snapshot 1
+
+    {
+      fileName: 'SomeStatelessContract.sol',
+      path: 'contracts/project/SomeStatelessContractUpgradeable.sol',
+      source: `// SPDX-License-Identifier: UNLICENSED␊
+      pragma solidity ^0.8.0;␊
+      ␊
+      import {SomeStatelessContract as SomeStatelessContractUpgradeable} from "@openzeppelin/contracts/project/SomeStatelessContract.sol";␊
+      `,
+    }
+
 ## transpile SomeContract.sol
 
 > Snapshot 1
@@ -14,27 +58,27 @@ Generated by [AVA](https://avajs.dev).
       source: `// SPDX-License-Identifier: UNLICENSED␊
       pragma solidity ^0.8.0;␊
       ␊
-      import {ISomeInterface} from "@openzeppelin/contracts/project/ISomeInterface.sol";␊
-      import {SomeLibrary} from "@openzeppelin/contracts/project/SomeLibrary.sol";␊
-      import {SomeStatelessContract} from "@openzeppelin/contracts/project/SomeStatelessContract.sol";␊
+      import {ISomeInterfaceUpgradeable} from "./ISomeInterfaceUpgradeable.sol";␊
+      import {SomeLibraryUpgradeable} from "./SomeLibraryUpgradeable.sol";␊
+      import {SomeStatelessContractUpgradeable} from "./SomeStatelessContractUpgradeable.sol";␊
       import {Initializable} from "../Initializable.sol";␊
       ␊
-      import { ISomeContract } from "@openzeppelin/contracts/project/SomeContract.sol";␊
+      import {ISomeContract as ISomeContractUpgradeable} from "@openzeppelin/contracts/project/SomeContract.sol";␊
       ␊
-      import { Error1 } from "@openzeppelin/contracts/project/SomeContract.sol";␊
+      import {Error1 as Error1Upgradeable} from "@openzeppelin/contracts/project/SomeContract.sol";␊
       ␊
-      import { freeFn_1 } from "@openzeppelin/contracts/project/SomeContract.sol";␊
+      import {freeFn_1 as freeFn_1Upgradeable} from "@openzeppelin/contracts/project/SomeContract.sol";␊
       ␊
-      import { SomeStruct } from "@openzeppelin/contracts/project/SomeContract.sol";␊
+      import {SomeStruct as SomeStructUpgradeable} from "@openzeppelin/contracts/project/SomeContract.sol";␊
       ␊
       contract SomeBaseContractUpgradeable is Initializable {␊
           function __SomeBaseContract_init() internal onlyInitializing {␊
           }␊
       ␊
           function __SomeBaseContract_init_unchained() internal onlyInitializing {␊
           }␊
-          function test(ISomeInterface other) public virtual returns (bool) {␊
-              return SomeLibrary.bothFunctions(other);␊
+          function test(ISomeInterfaceUpgradeable other) public virtual returns (bool) {␊
+              return SomeLibraryUpgradeable.bothFunctions(other);␊
           }␊
       ␊
           /**␊
@@ -45,27 +89,27 @@ Generated by [AVA](https://avajs.dev).
           uint256[50] private __gap;␊
       }␊
       ␊
-      contract SomeContractUpgradeable is Initializable, ISomeContract, SomeBaseContractUpgradeable {␊
+      contract SomeContractUpgradeable is Initializable, ISomeContractUpgradeable, SomeBaseContractUpgradeable {␊
           SomeStruct s;␊
       ␊
           function __SomeContract_init() internal onlyInitializing {␊
           }␊
       ␊
           function __SomeContract_init_unchained() internal onlyInitializing {␊
           }␊
-          /// @inheritdoc ISomeInterface␊
+          /// @inheritdoc ISomeInterfaceUpgradeable␊
           function someFunction() public pure override returns (bool) {␊
               return false;␊
           }␊
       ␊
-          /// @inheritdoc ISomeInterface␊
+          /// @inheritdoc ISomeInterfaceUpgradeable␊
           function someOtherFunction() public pure override returns (bool) {␊
               return true;␊
           }␊
       ␊
           /// @inheritdoc SomeBaseContractUpgradeable␊
-          function test(ISomeInterface other) public override returns (bool) {␊
-              return SomeLibrary.bothFunctions(this) && super.test(other);␊
+          function test(ISomeInterfaceUpgradeable other) public override returns (bool) {␊
+              return SomeLibraryUpgradeable.bothFunctions(this) && super.test(other);␊
           }␊
       ␊
           /**␊
@@ -88,10 +132,9 @@ Generated by [AVA](https://avajs.dev).
       source: `// SPDX-License-Identifier: UNLICENSED␊
       pragma solidity ^0.8.0;␊
       ␊
-      import {ISomeInterface} from "@openzeppelin/contracts/project/ISomeInterface.sol";␊
-      import {SomeLibrary} from "@openzeppelin/contracts/project/SomeLibrary.sol";␊
-      import {ISomeContract} from "@openzeppelin/contracts/project/SomeContract.sol";␊
-      import {SomeContractUpgradeable} from "./SomeContractUpgradeable.sol";␊
+      import {ISomeInterfaceUpgradeable} from "./ISomeInterfaceUpgradeable.sol";␊
+      import {SomeLibraryUpgradeable} from "./SomeLibraryUpgradeable.sol";␊
+      import {ISomeContractUpgradeable, SomeContractUpgradeable} from "./SomeContractUpgradeable.sol";␊
       import {Initializable} from "../Initializable.sol";␊
       ␊
       contract SomeOtherContractUpgradeable is Initializable, SomeContractUpgradeable {␊
diff --git a/src/prepare-peer-project.test.ts.snap b/src/prepare-peer-project.test.ts.snap
diff --git a/src/prepare-peer-project.ts b/src/prepare-peer-project.ts
@@ -6,14 +6,12 @@ import { isStorageVariable } from './transformations/utils/is-storage-variable';
 
 export function preparePeerProject(transform: Transform, peerProject: string) {
   for (const ast of transform.asts()) {
-    let shouldExclude = true;
     for (const node of ast.nodes) {
       switch (node.nodeType) {
         case 'ContractDefinition': {
           if (node.contractKind === 'contract') {
             if (!extractContractStateless(node)) {
-              shouldExclude = false;
-              break;
+              continue;
             }
             if (extractContractStorageSize(node) !== undefined) {
               throw transform.error(
@@ -58,8 +56,5 @@ export function preparePeerProject(transform: Transform, peerProject: string) {
         }
       }
     }
-    if (shouldExclude) {
-      transform.exclude(ast.absolutePath);
-    }
   }
 }
diff --git a/src/transform.test.ts b/src/transform.test.ts
@@ -121,14 +121,14 @@ test('skip contract rename when Upgradeable suffix', t => {
 
 test('fix import directives', t => {
   const file = 'contracts/solc-0.6/Local.sol';
-  t.context.transform.apply(fixImportDirectives(false));
+  t.context.transform.apply(fixImportDirectives);
   t.snapshot(t.context.transform.results()[file]);
 });
 
 test('fix import directives complex', t => {
   const file = 'contracts/TransformImport2.sol';
   t.context.transform.apply(renameIdentifiers);
-  t.context.transform.apply(fixImportDirectives(false));
+  t.context.transform.apply(fixImportDirectives);
   t.snapshot(t.context.transform.results()[file]);
 });
 
diff --git a/src/transform.ts b/src/transform.ts
@@ -186,10 +186,6 @@ export class Transform {
   asts(): SourceUnit[] {
     return Object.values(this.state).map(s => s.ast);
   }
-
-  exclude(source: string) {
-    delete this.state[source];
-  }
 }
 
 function insertSortedAndValidate(
diff --git a/src/transformations/append-initializable-import.ts b/src/transformations/append-initializable-import.ts
@@ -7,11 +7,17 @@ import { relativePath } from '../utils/relative-path';
 
 import { getNodeBounds } from '../solc/ast-utils';
 import { Transformation } from './type';
+import { TransformerTools } from '../transform';
 
 export function appendInitializableImport(initializablePath: string) {
-  return function* (sourceUnit: SourceUnit): Generator<Transformation> {
+  return function* (
+    sourceUnit: SourceUnit,
+    { getData }: TransformerTools,
+  ): Generator<Transformation> {
     const contracts = [...findAll('ContractDefinition', sourceUnit)];
-    if (!contracts.some(c => c.contractKind === 'contract')) {
+    if (
+      !contracts.some(c => c.contractKind === 'contract' && getData(c).importFromPeer === undefined)
+    ) {
       return;
     }
 
diff --git a/src/transformations/fix-import-directives.ts b/src/transformations/fix-import-directives.ts
@@ -5,60 +5,41 @@ import { Transformation } from './type';
 import { renameContract, renamePath } from '../rename';
 import { TransformerTools } from '../transform';
 
-declare module '../transform' {
-  interface TransformData {
-    importFromPeer: string;
-  }
-}
-
-export function fixImportDirectives(withPeerProject?: boolean) {
-  return function* (
-    ast: SourceUnit,
-    { resolver, getData }: TransformerTools,
-  ): Generator<Transformation> {
-    for (const imp of findAll('ImportDirective', ast)) {
-      const referencedSourceUnit = resolver.resolveNode('SourceUnit', imp.sourceUnit);
-
-      if (withPeerProject && imp.symbolAliases.length == 0) {
-        throw new Error(
-          `Transpile with peer doesn't support import without aliases in ${imp.absolutePath}`,
-        );
-      }
-
-      const imports: Record<string, string[]> = {};
-
-      for (const a of imp.symbolAliases) {
-        const id = referencedSourceUnit.exportedSymbols[a.foreign.name]?.[0];
-        if (id === undefined) {
-          throw new Error(`Can't find symbol imported in ${ast.absolutePath}`);
-        }
-
-        const node = resolver.resolveNode('*', id);
-        const importFromPeer = getData(node).importFromPeer;
-        const importPath = importFromPeer ?? renamePath(imp.file);
-
-        imports[importPath] ??= [];
-        imports[importPath].push(
-          [
-            importFromPeer === undefined ? renameContract(a.foreign.name) : a.foreign.name,
-            [null, undefined, a.foreign.name].includes(a.local) ? '' : ` as ${a.local}`,
-          ].join(''),
-        );
+export function* fixImportDirectives(
+  ast: SourceUnit,
+  { resolver }: TransformerTools,
+): Generator<Transformation> {
+  for (const imp of findAll('ImportDirective', ast)) {
+    const referencedSourceUnit = resolver.resolveNode('SourceUnit', imp.sourceUnit);
+
+    const aliases = imp.symbolAliases.map(a => {
+      const id = referencedSourceUnit.exportedSymbols[a.foreign.name]?.[0];
+      if (id === undefined) {
+        throw new Error(`Can't find symbol imported in ${ast.absolutePath}`);
       }
-
-      const statement = [];
-      for (const [path, aliases] of Object.entries(imports)) {
-        statement.push(`import {${aliases.join(', ')}} from "${path}";`);
+      let alias = '';
+      const contract = resolver.resolveContract(id);
+      if (contract === undefined) {
+        alias += a.foreign.name;
+      } else {
+        alias += renameContract(a.foreign.name);
       }
-      if (imp.symbolAliases.length == 0) {
-        statement.push(`import "${renamePath(imp.file)}";`);
+      if (a.local != null && a.local !== a.foreign.name) {
+        alias += ` as ${a.local}`;
       }
+      return alias;
+    });
 
-      yield {
-        kind: 'fix-import-directives',
-        text: statement.join('\n'),
-        ...getNodeBounds(imp),
-      };
+    const statement = ['import'];
+    if (aliases.length > 0) {
+      statement.push(`{${aliases.join(', ')}} from`);
     }
-  };
+    statement.push(`"${renamePath(imp.file)}";`);
+
+    yield {
+      kind: 'fix-import-directives',
+      text: statement.join(' '),
+      ...getNodeBounds(imp),
+    };
+  }
 }
diff --git a/src/transformations/peer-import.ts b/src/transformations/peer-import.ts
diff --git a/src/transformations/rename-identifiers.ts b/src/transformations/rename-identifiers.ts
diff --git a/src/transformations/rename-inheritdoc.ts b/src/transformations/rename-inheritdoc.ts

Original file line number	Diff line number	Diff line change
`@@ -6,14 +6,12 @@ import { isStorageVariable } from './transformations/utils/is-storage-variable';`
`6`	`6`
`7`	`7`	`export function preparePeerProject(transform: Transform, peerProject: string) {`
`8`	`8`	`for (const ast of transform.asts()) {`
`9`		`- let shouldExclude = true;`
`10`	`9`	`for (const node of ast.nodes) {`
`11`	`10`	`switch (node.nodeType) {`
`12`	`11`	`case 'ContractDefinition': {`
`13`	`12`	`if (node.contractKind === 'contract') {`
`14`	`13`	`if (!extractContractStateless(node)) {`
`15`		`- shouldExclude = false;`
`16`		`- break;`
	`14`	`+ continue;`
`17`	`15`	`}`
`18`	`16`	`if (extractContractStorageSize(node) !== undefined) {`
`19`	`17`	`throw transform.error(`
`@@ -58,8 +56,5 @@ export function preparePeerProject(transform: Transform, peerProject: string) {`
`58`	`56`	`}`
`59`	`57`	`}`
`60`	`58`	`}`
`61`		`- if (shouldExclude) {`
`62`		`- transform.exclude(ast.absolutePath);`
`63`		`- }`
`64`	`59`	`}`
`65`	`60`	`}`
Original file line number	Diff line number	Diff line change
`@@ -186,10 +186,6 @@ export class Transform {`
`186`	`186`	`asts(): SourceUnit[] {`
`187`	`187`	`return Object.values(this.state).map(s => s.ast);`
`188`	`188`	`}`
`189`		`-`
`190`		`- exclude(source: string) {`
`191`		`- delete this.state[source];`
`192`		`- }`
`193`	`189`	`}`
`194`	`190`
`195`	`191`	`function insertSortedAndValidate(`