Fix: double import issue #58
Conversation
| }); | ||
| it('should replace path as instructed by remapping with all import syntaxes', function () { | ||
| const fileContent = ` | ||
| import '../../../interfaces/ITest.sol'; |
There was a problem hiding this comment.
I might be wrong here, but I don't think remappings work this way. If your remapping key is interfaces, ../../../interfaces should not be affected, only when it starts with interfaces/
| import '../../../interfaces/ITest.sol'; | ||
| import './someFile.sol'; | ||
| `; | ||
| import './Contract.sol'; |
There was a problem hiding this comment.
what do you think about adding something that should be automatically modified in between these lines? Just to make sure our code is not skipping all the imports after seeing that the first one does not need any modification
No need to add a new test, we could do it in this one
There was a problem hiding this comment.
sure!
now that I think about it (related to your other comment) having multiple imports in a single line will probably break as well (but we could actually make that work without properly parsing the code). Will add a test for that too!
| expect(transformedContent).to.include(`contract C {}`); | ||
| }); | ||
|
|
||
| it.skip('should not have false positives choosing imports', () => { |
There was a problem hiding this comment.
why is this test being skipped? we are not able to know if we are talking about code or comment?
There was a problem hiding this comment.
we are not, since we don't reuse an existing lexer+parser, and only do regular-style modifications to the code. For most cases it is sufficient though, and single-line comments are skipped since we match \s*import to detect imports
Also, its worth noting that the impact of modifying comments when we shouldn't is very low considering multi-line natspec is required to have a leading * which would not match the regex above
No description provided.