Skip to content

Commit

Permalink
fix: double import issue (#58)
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xteddybear
0xteddybear authored Nov 27, 2024
1 parent 3df3ab4 commit beb99c0
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 50 deletions.
4 changes: 4 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,7 @@ The scripts and documentation in this project are released under the [MIT Licens
# Contributors

Maintained with love by [Wonderland](https://defi.sucks). Made possible by viewers like you.

# Known issues

This project relies on regular expressions to locate import statements, which is sub-optimal and has known false positives and negatives. See issue #62 for details and progress on migrating to using a proper parser
22 changes: 5 additions & 17 deletions dist/index.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map
View file

Large diffs are not rendered by default.

25 changes: 5 additions & 20 deletions src/transformRemappings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,13 @@ export const transformRemappings = (file: string): string => {

const remapping = remappings.find(([find]) => line.match(find));

if (!remapping) {
// Transform:
// import '../../../node_modules/some-file.sol';
// into:
// import 'some-file.sol';
const modulesKey = 'node_modules/';

if (line.includes(modulesKey)) {
line = `import '` + line.substring(line.indexOf(modulesKey) + modulesKey.length);
} else {
return line;
}

return line;
if (remapping) {
const remappingOrigin = remapping[0];
const remappingDestination = remapping[1];
line = line.replace(remappingOrigin, remappingDestination);
}

const remappingOrigin = remapping[0];
const remappingDestination = remapping[1];

const dependencyDirectory = line.replace(remappingOrigin, remappingDestination);

line = `import '` + dependencyDirectory;
line = line.replace(/(['""]).*node_modules\//, `$1`);

return line;
})
Expand Down
180 changes: 168 additions & 12 deletions test/unit/transformRemappings.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,191 @@ import mock from 'mock-fs';
describe('transformRemappings', () => {
before(() => {
mock({
'remappings.txt': 'interfaces=../../interfaces',
'remappings.txt': 'foo=bar',
});
});

after(() => {
mock.restore();
});

it('should transform imports with remappings', () => {
describe('with remapping for local file only', function () {
before(function () {
mock({
'remappings.txt': 'interfaces=../../interfaces',
});
});

// Foundry produces a warning when doing this, but compiles the project doing
// the substitution regardless. We do the same to be compatible, but foundry's
// behaviour might change in the future.
it('should replace remapped path if its not at the beggining of string', function() {
const fileContent = `import '../../interfaces/ITest.sol';`
const transformedContent = transformRemappings(fileContent);

expect(transformedContent).to.include(`import '../../../../interfaces/ITest.sol';`);
})

it('should replace path as instructed by remapping with all import syntaxes', function () {
const fileContent = `
import 'interfaces/ITest.sol';
import {ITest} from 'interfaces/ITest.sol';
import {ITest, ITest2} from 'interfaces/ITest.sol';
import {IBar as IFoo, ITest} from 'interfaces/ITest.sol';
import * as Interfaces 'interfaces/ITest.sol';
import "interfaces/ITest.sol";
import {ITest} from "interfaces/ITest.sol";
import {ITest, ITest2} from "interfaces/ITest.sol";
import {IBar as IFoo, ITest} from "interfaces/ITest.sol";
import * as Interfaces "interfaces/ITest.sol";
`;

const transformedContent = transformRemappings(fileContent);

expect(transformedContent).to.include(`import '../../interfaces/ITest.sol';`);
expect(transformedContent).to.include(`import {ITest} from '../../interfaces/ITest.sol';`);
expect(transformedContent).to.include(`import {ITest, ITest2} from '../../interfaces/ITest.sol';`);
expect(transformedContent).to.include(`import {IBar as IFoo, ITest} from '../../interfaces/ITest.sol';`);
expect(transformedContent).to.include(`import * as Interfaces '../../interfaces/ITest.sol';`);
expect(transformedContent).to.include(`import "../../interfaces/ITest.sol";`);
expect(transformedContent).to.include(`import {ITest} from "../../interfaces/ITest.sol";`);
expect(transformedContent).to.include(`import {ITest, ITest2} from "../../interfaces/ITest.sol";`);
expect(transformedContent).to.include(`import {IBar as IFoo, ITest} from "../../interfaces/ITest.sol";`);
expect(transformedContent).to.include(`import * as Interfaces "../../interfaces/ITest.sol";`);
});

it('should remove node_modules from import paths', function () {
const fileContent = `
import '../../../node_modules/some-package/Contract.sol';
import 'node_modules/another-package/Token.sol';
import {FixedPointMathLib} from 'node_modules/solmate/src/utils/FixedPointMathLib.sol';
import {FixedPointMathLib} from "node_modules/solmate/src/utils/FixedPointMathLib.sol";
import {FixedPointMathLib as FPL} from "node_modules/solmate/src/utils/FixedPointMathLib.sol";
import * as FPL from "../node_modules/solmate/src/utils/FixedPointMathLib.sol";
`;

const transformedContent = transformRemappings(fileContent);

expect(transformedContent).to.include(`import 'some-package/Contract.sol';`);
expect(transformedContent).to.include(`import 'another-package/Token.sol';`);
expect(transformedContent).to.include(
`import {FixedPointMathLib} from 'solmate/src/utils/FixedPointMathLib.sol';`,
);
expect(transformedContent).to.include(
`import {FixedPointMathLib} from "solmate/src/utils/FixedPointMathLib.sol";`,
);
expect(transformedContent).to.include(
`import {FixedPointMathLib as FPL} from "solmate/src/utils/FixedPointMathLib.sol";`,
);
expect(transformedContent).to.include(`import * as FPL from "solmate/src/utils/FixedPointMathLib.sol";`);
});
});

describe('with remapping for path inside node_modules', function () {
before(() => {
mock({
'remappings.txt': `
solmate/=node_modules/solmate/src/`,
});
});

it('should apply remapping and then remove node_modules from path', () => {
const fileContent = `
import {FixedPointMathLib} from 'solmate/utils/FixedPointMathLib.sol';
import 'solmate/utils/FixedPointMathLib.sol';
import "solmate/utils/FixedPointMathLib.sol";
import {FixedPointMathLib as FPL} from 'solmate/utils/FixedPointMathLib.sol';
import * as FPL from "solmate/utils/FixedPointMathLib.sol";
`;

const transformedContent = transformRemappings(fileContent);

expect(transformedContent).to.include(
`import {FixedPointMathLib} from 'solmate/src/utils/FixedPointMathLib.sol';`,
);
expect(transformedContent).to.include(`import 'solmate/src/utils/FixedPointMathLib.sol';`);
expect(transformedContent).to.include(`import "solmate/src/utils/FixedPointMathLib.sol";`);
expect(transformedContent).to.include(
`import {FixedPointMathLib as FPL} from 'solmate/src/utils/FixedPointMathLib.sol';`,
);
expect(transformedContent).to.include(`import * as FPL from "solmate/src/utils/FixedPointMathLib.sol";`);
});
});

it('should leave unrelated imports alone', () => {
const fileContent = `
import '../../../interfaces/ITest.sol';
import './someFile.sol';
`;
import './Contract.sol';
import '../contracts/Contract.sol';
import {Contract} from "../contracts/Contract.sol";
import "foo/Foo.sol";
import {Contract as MyName} from'../contracts/Contract.sol';
import * as MyName from'../contracts/Contract.sol';
`;

const transformedContent = transformRemappings(fileContent);

expect(transformedContent).to.include(`import '../../../../../interfaces/ITest.sol';`);
expect(transformedContent).to.include(`import './someFile.sol';`);
expect(transformedContent).to.include(`import './Contract.sol';`);
expect(transformedContent).to.include(`import '../contracts/Contract.sol';`);
expect(transformedContent).to.include(`import {Contract} from "../contracts/Contract.sol";`);
expect(transformedContent).to.include(`import "bar/Foo.sol";`);
expect(transformedContent).to.include(`import {Contract as MyName} from'../contracts/Contract.sol';`);
expect(transformedContent).to.include(`import * as MyName from'../contracts/Contract.sol';`);
});

it('should handle imports from node_modules correctly', () => {
it('should leave unrelated statements alone', () => {
const fileContent = `
// import './Contract.sol';
// important comment
contract C {}
`;

const transformedContent = transformRemappings(fileContent);

expect(transformedContent).to.include(`// import './Contract.sol';`);
expect(transformedContent).to.include(`// important comment`);
expect(transformedContent).to.include(`contract C {}`);
});

// see https://github.com/defi-wonderland/solidity-exporter-action/issues/62 for tracking and details
it.skip('should not have false positives choosing imports', () => {
const fileContent = `
/*
import '../../../node_modules/some-package/Contract.sol';
import 'node_modules/another-package/Token.sol';
*/
`;

const transformedContent = transformRemappings(fileContent);

expect(transformedContent).not.to.include(`import 'some-package/Contract.sol';`);
expect(transformedContent).to.include(`import '../../../node_modules/some-package/Contract.sol';`);
});

// see https://github.com/defi-wonderland/solidity-exporter-action/issues/62 for tracking and details
it.skip('should process two imports present in a single line', () => {
const fileContent = `
import 'foo/Foo.sol'; import 'foo/Bar.sol';
`;

const transformedContent = transformRemappings(fileContent);
expect(transformedContent).to.include(`import 'bar/Foo.sol'; import 'bar/Bar.sol';`);
});

expect(transformedContent).to.include(`import 'some-package/Contract.sol';`);
expect(transformedContent).to.include(`import 'another-package/Token.sol';`);
// see https://github.com/defi-wonderland/solidity-exporter-action/issues/62 for tracking and details
it.skip('should process a multi-line import', () => {
const fileContent = `
import {
Foo,
Bar,
Baz
} from 'foo/Foo.sol';
`;

const transformedContent = transformRemappings(fileContent);
expect(transformedContent).to.include(`
import {
Foo,
Bar,
Baz
} from 'bar/Foo.sol';
`);
});
});

0 comments on commit beb99c0

Please sign in to comment.