Revert "Simplify peer project mode by renaming imported peer symbols"

frangio · frangio · commit f0d12a6cc5a3 · 2023-10-02T10:44:23.000-03:00
This reverts commit 89e891d.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,8 +1,12 @@
 # Changelog
 
+## 0.3.32 (2023-09-30)
+
+- Revert 0.3.31 changes.
+
 ## 0.3.31 (2023-09-30)
 
-- Simplify peer project mode by renaming imported peer symbols.
+- (Reverted in 0.3.32) ~Simplify peer project mode by renaming imported peer symbols.~
 
 ## 0.3.30 (2023-09-27)
 
diff --git a/src/generate-with-init.ts b/src/generate-with-init.ts
@@ -23,11 +23,7 @@ export function generateWithInit(
 
   for (const sourceUnit of transform.asts()) {
     for (const contract of findAll('ContractDefinition', sourceUnit)) {
-      if (
-        contract.contractKind !== 'contract' ||
-        !contract.fullyImplemented ||
-        transform.getData(contract).importFromPeer !== undefined
-      ) {
+      if (contract.contractKind !== 'contract' || !contract.fullyImplemented) {
         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);
+  transform.apply(fixImportDirectives(options.peerProject !== undefined));
   transform.apply(appendInitializableImport(outputPaths.initializable));
   transform.apply(fixNewStatement);
   transform.apply(transformConstructor(namespaceInclude));
@@ -110,12 +110,13 @@ 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[] = [];
@@ -168,7 +169,7 @@ function transpileInitializable(
   transform.apply(function* (ast, tools) {
     if (ast.absolutePath === options.initializablePath) {
       yield* renameIdentifiers(ast, tools);
-      yield* fixImportDirectives(ast, tools);
+      yield* fixImportDirectives(options.peerProject !== undefined)(ast, tools);
     }
   });
 
diff --git a/src/prepare-peer-project.test.ts b/src/prepare-peer-project.test.ts
@@ -29,13 +29,17 @@ test.serial.before('compile', async t => {
   });
 });
 
-for (const fileName of [
-  'ISomeInterface.sol',
-  'SomeLibrary.sol',
-  'SomeStatelessContract.sol',
-  'SomeContract.sol',
-  'SomeOtherContract.sol',
-]) {
+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']) {
   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,50 +4,6 @@ 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
@@ -58,27 +14,27 @@ Generated by [AVA](https://avajs.dev).
       source: `// SPDX-License-Identifier: UNLICENSED␊
       pragma solidity ^0.8.0;␊
       ␊
-      import {ISomeInterfaceUpgradeable} from "./ISomeInterfaceUpgradeable.sol";␊
-      import {SomeLibraryUpgradeable} from "./SomeLibraryUpgradeable.sol";␊
-      import {SomeStatelessContractUpgradeable} from "./SomeStatelessContractUpgradeable.sol";␊
+      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 {Initializable} from "../Initializable.sol";␊
       ␊
-      import {ISomeContract as ISomeContractUpgradeable} from "@openzeppelin/contracts/project/SomeContract.sol";␊
+      import { ISomeContract } from "@openzeppelin/contracts/project/SomeContract.sol";␊
       ␊
-      import {Error1 as Error1Upgradeable} from "@openzeppelin/contracts/project/SomeContract.sol";␊
+      import { Error1 } from "@openzeppelin/contracts/project/SomeContract.sol";␊
       ␊
-      import {freeFn_1 as freeFn_1Upgradeable} from "@openzeppelin/contracts/project/SomeContract.sol";␊
+      import { freeFn_1 } from "@openzeppelin/contracts/project/SomeContract.sol";␊
       ␊
-      import {SomeStruct as SomeStructUpgradeable} from "@openzeppelin/contracts/project/SomeContract.sol";␊
+      import { SomeStruct } from "@openzeppelin/contracts/project/SomeContract.sol";␊
       ␊
       contract SomeBaseContractUpgradeable is Initializable {␊
           function __SomeBaseContract_init() internal onlyInitializing {␊
           }␊
       ␊
           function __SomeBaseContract_init_unchained() internal onlyInitializing {␊
           }␊
-          function test(ISomeInterfaceUpgradeable other) public virtual returns (bool) {␊
-              return SomeLibraryUpgradeable.bothFunctions(other);␊
+          function test(ISomeInterface other) public virtual returns (bool) {␊
+              return SomeLibrary.bothFunctions(other);␊
           }␊
       ␊
           /**␊
@@ -89,27 +45,27 @@ Generated by [AVA](https://avajs.dev).
           uint256[50] private __gap;␊
       }␊
       ␊
-      contract SomeContractUpgradeable is Initializable, ISomeContractUpgradeable, SomeBaseContractUpgradeable {␊
+      contract SomeContractUpgradeable is Initializable, ISomeContract, SomeBaseContractUpgradeable {␊
           SomeStruct s;␊
       ␊
           function __SomeContract_init() internal onlyInitializing {␊
           }␊
       ␊
           function __SomeContract_init_unchained() internal onlyInitializing {␊
           }␊
-          /// @inheritdoc ISomeInterfaceUpgradeable␊
+          /// @inheritdoc ISomeInterface␊
           function someFunction() public pure override returns (bool) {␊
               return false;␊
           }␊
       ␊
-          /// @inheritdoc ISomeInterfaceUpgradeable␊
+          /// @inheritdoc ISomeInterface␊
           function someOtherFunction() public pure override returns (bool) {␊
               return true;␊
           }␊
       ␊
           /// @inheritdoc SomeBaseContractUpgradeable␊
-          function test(ISomeInterfaceUpgradeable other) public override returns (bool) {␊
-              return SomeLibraryUpgradeable.bothFunctions(this) && super.test(other);␊
+          function test(ISomeInterface other) public override returns (bool) {␊
+              return SomeLibrary.bothFunctions(this) && super.test(other);␊
           }␊
       ␊
           /**␊
@@ -132,9 +88,10 @@ Generated by [AVA](https://avajs.dev).
       source: `// SPDX-License-Identifier: UNLICENSED␊
       pragma solidity ^0.8.0;␊
       ␊
-      import {ISomeInterfaceUpgradeable} from "./ISomeInterfaceUpgradeable.sol";␊
-      import {SomeLibraryUpgradeable} from "./SomeLibraryUpgradeable.sol";␊
-      import {ISomeContractUpgradeable, SomeContractUpgradeable} from "./SomeContractUpgradeable.sol";␊
+      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 {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,12 +6,14 @@ 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)) {
-              continue;
+              shouldExclude = false;
+              break;
             }
             if (extractContractStorageSize(node) !== undefined) {
               throw transform.error(
@@ -56,5 +58,8 @@ 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);
+  t.context.transform.apply(fixImportDirectives(false));
   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);
+  t.context.transform.apply(fixImportDirectives(false));
   t.snapshot(t.context.transform.results()[file]);
 });
 
diff --git a/src/transform.ts b/src/transform.ts
@@ -186,6 +186,10 @@ 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,17 +7,11 @@ 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,
-    { getData }: TransformerTools,
-  ): Generator<Transformation> {
+  return function* (sourceUnit: SourceUnit): Generator<Transformation> {
     const contracts = [...findAll('ContractDefinition', sourceUnit)];
-    if (
-      !contracts.some(c => c.contractKind === 'contract' && getData(c).importFromPeer === undefined)
-    ) {
+    if (!contracts.some(c => c.contractKind === 'contract')) {
       return;
     }
 
diff --git a/src/transformations/fix-import-directives.ts b/src/transformations/fix-import-directives.ts
@@ -5,41 +5,60 @@ import { Transformation } from './type';
 import { renameContract, renamePath } from '../rename';
 import { TransformerTools } from '../transform';
 
-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}`);
+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(''),
+        );
       }
-      let alias = '';
-      const contract = resolver.resolveContract(id);
-      if (contract === undefined) {
-        alias += a.foreign.name;
-      } else {
-        alias += renameContract(a.foreign.name);
+
+      const statement = [];
+      for (const [path, aliases] of Object.entries(imports)) {
+        statement.push(`import {${aliases.join(', ')}} from "${path}";`);
       }
-      if (a.local != null && a.local !== a.foreign.name) {
-        alias += ` as ${a.local}`;
+      if (imp.symbolAliases.length == 0) {
+        statement.push(`import "${renamePath(imp.file)}";`);
       }
-      return alias;
-    });
 
-    const statement = ['import'];
-    if (aliases.length > 0) {
-      statement.push(`{${aliases.join(', ')}} from`);
+      yield {
+        kind: 'fix-import-directives',
+        text: statement.join('\n'),
+        ...getNodeBounds(imp),
+      };
     }
-    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,12 +6,14 @@ 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;`
`9`	`10`	`for (const node of ast.nodes) {`
`10`	`11`	`switch (node.nodeType) {`
`11`	`12`	`case 'ContractDefinition': {`
`12`	`13`	`if (node.contractKind === 'contract') {`
`13`	`14`	`if (!extractContractStateless(node)) {`
`14`		`- continue;`
	`15`	`+ shouldExclude = false;`
	`16`	`+ break;`
`15`	`17`	`}`
`16`	`18`	`if (extractContractStorageSize(node) !== undefined) {`
`17`	`19`	`throw transform.error(`
`@@ -56,5 +58,8 @@ export function preparePeerProject(transform: Transform, peerProject: string) {`
`56`	`58`	`}`
`57`	`59`	`}`
`58`	`60`	`}`
	`61`	`+ if (shouldExclude) {`
	`62`	`+ transform.exclude(ast.absolutePath);`
	`63`	`+ }`
`59`	`64`	`}`
`60`	`65`	`}`
Original file line number	Diff line number	Diff line change
`@@ -186,6 +186,10 @@ 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`	`+ }`
`189`	`193`	`}`
`190`	`194`
`191`	`195`	`function insertSortedAndValidate(`