juanfranblanco / vscode-solidity

Visual Studio Code language support extension for Solidity smart contracts in Ethereum https://marketplace.visualstudio.com/items?itemName=JuanBlanco.solidity
MIT License
904 stars 194 forks source link

Remappings example for brownie #288

Open PatrickAlphaC opened 3 years ago

PatrickAlphaC commented 3 years ago

Hi Juan!

Thank you so much for the remapping addition, this is huge!

So for a brownie project, in my contracts I have something like:

import "@openzeppelin/contracts/access/Ownable.sol";

And the package is located at:

/Users/patrick/.brownie/packages/OpenZeppelin/openzeppelin-contracts@4.3.2

I can't seem to get the settings write for Source "@chainlink/contracts/src/v0.8/ChainlinkClient.sol" not found: File import callback not supported to go away. Any thoughts?

The following doesn't seem to be working:

  "solidity.remappings": [
    "@openzeppelin/=/Users/patrick/.brownie/packages/OpenZeppelin/openzeppelin-contracts@4.3.2"
  ]
juanfranblanco commented 3 years ago

Ah.. @PatrickAlphaC all the mappings are setup to be relative to the folder. I need to change that when prefixed by "/" or "C:\", thanks for pointing that out.

juanfranblanco commented 3 years ago

@PatrickAlphaC check the latest release and let me know how it goes, added the absolute support.

Brodan commented 3 years ago

@juanfranblanco don't want to speak for Patrick, but I was running into this same issue and going insane trying to google it and figure out how to make this extension work better with brownie. finally came across this issue. the remappings setting you listed above worked for me and the infuriating "File import callback not supported" highlight has gone away. thank you very much.

juanfranblanco commented 3 years ago

@Brodan thank you very much for confirming this. I wanted to validate it before updating the Readme. Note that you can also add now a remappings.txt at the root of your workspace file with each remapping included in a new line.

PatrickAlphaC commented 3 years ago

I was being a dunce, got it :D

Just needed to give ol-vscode a reboot after installing the update.

Example:

"solidity.remappings": [
    "@chainlink/=/Users/patrick/.brownie/packages/smartcontractkit/chainlink-brownie-contracts@0.2.2",
    "@openzeppelin/=/Users/patrick/.brownie/packages/OpenZeppelin/openzeppelin-contracts@4.3.2"
  ]

Thank you so much for this!!!

juanfranblanco commented 3 years ago

That is awesome @PatrickAlphaC thanks for confirming!!

PatrickAlphaC commented 3 years ago

Hi @juanfranblanco, so found out another edge case though :(

"solidity.remappings": [
    "@openzeppelin/=/Users/patrick/.brownie/packages/OpenZeppelin/openzeppelin-contracts@4.3.2",
    "@openzeppelin/contracts-upgradeable/=/Users/patrick/.brownie/packages/OpenZeppelin/@openzeppelin/contracts-upgradeable@4.3.2"
  ]

If I have 2 repos from the same org, the remappings settings file doesn't know how to handle this. Is this something we could fix too?

PatrickAlphaC commented 3 years ago

Additionally.... it would be great to add home directory functionality, ie:

~/

instead of absolute... Thanks again for taking a look!

cameel commented 3 years ago

If I have 2 repos from the same org, the remappings settings file doesn't know how to handle this. Is this something we could fix too?

What is the actual error? I see that your remapping targets do not end with / and that will mangle your paths so maybe that's the problem. E.g. @x/=y will remap @x/a.sol to ya.sol, not y/a.sol as you might expect. Remappings are just simple pattern replacements and the final / is not treated specially as it would be in paths. The proper form is @x/=y/. See the docs on Import Remapping in the compiler.

Additionally.... it would be great to add home directory functionality, ie:

I think that translating ~ might be a bit misleading to users because it looks like your home dir name is not embedded in the contract metadata while it in fact would be. That's because remappings at the compiler level do not support this and the translation would have to be performed by vscode-solidity. I mean, if you run solc abc/=~/abc/ contract.sol the ~ will be replaced - but by shell, not the compiler (and this is equally misleading but not much can be done about it because it's standard shell behavior).

How does Brownie handle ~? If it does translate ~ then it might be good for compatibility but otherwise I'd recommend keeping it as is.

The real solution to this problem is to use relative paths as remapping targets. Absolute paths end up in your metadata and should be avoided in contracts in general. Unfortunately until recently the compiler did not have good support for this use case on the CLI (and Brownie uses the CLI). Relative targets were relative to base path (current work dir by default) which works for package managers like npm which install libs in a subdir of your project but not for Brownie which installs them under a fixed global path.

solc 0.8.8 finally has the --include-path option. Hopefully Brownie starts using it at some point. If it invoked the compiler with solc --base-path <project dir> --include-path ~/.brownie/packages/ ..., you'd be able to use remappings looking like this:

@openzeppelin/=OpenZeppelin/openzeppelin-contracts@4.3.2/
@openzeppelin/contracts-upgradeable/=OpenZeppelin/@openzeppelin/contracts-upgradeable@4.3.2/
PatrickAlphaC commented 3 years ago

There isn't an error, but if I try to do something like:

"solidity.remappings": [
    "@openzeppelin/=/Users/patrick/.brownie/packages/OpenZeppelin/openzeppelin-contracts@4.3.2",
    "@openzeppelin/contracts-upgradeable/=/Users/patrick/.brownie/packages/OpenZeppelin/@openzeppelin/contracts-upgradeable@4.3.2"
  ]

Only the openzeppelin is remapped and @openzeppelin/contracts-upgradeable is ignored. So I go back to getting File import callback not supported on the @openzeppelin/contracts-upgradeable files.


So brownie stores imports in a special location at ~/.brownie, so all packages are actually remapped from ~/.brownie.

cameel commented 3 years ago

Maybe you're hitting this bug: https://github.com/eth-brownie/brownie/issues/893?

EDIT: Ah, sorry, the bug is in Brownie and your problem is in the extension so it's probably not it.

juanfranblanco commented 3 years ago

@PatrickAlphaC let me check that, yes I can see the issue with matching the first element.

Also. yes the extension does not translate ~, only works with absolute / relative.

Note that there are two remappings, one for the compiler and one for the extension to find imports, autocomplete etc.

juanfranblanco commented 3 years ago

Check the latest release, although the ~ path @cameel can you confirm that is ok?

cameel commented 3 years ago

What ~ path? There's no ~ in these remappings. I don't understand the question.

juanfranblanco commented 3 years ago

@cameel oh I meant, if the ~ should be supported, and if it is currently supported.

cameel commented 3 years ago

Oh. No it's not supported directly by the compiler and I don't think it should be. Using absolute paths is something that should be discouraged (and relative paths should eventually get better support).

You can use ~ on the command line on Linux/mac but it's Bash making the substitution then, not solc. In Standard JSON that won't work.

juanfranblanco commented 3 years ago

Ah thanks, then I won't do anything about that. And I totally understand, as it is not going to be very portable anyway with windows machines.

purpledeerz commented 2 years ago

Hi, is there any solution regarding this solidity.remappings path issue when developing on different platforms? I cannot 'dupe' the remapping like so, because it doesn't support fallback behavior (or at least on Mac it works, and on Windows it does not):

"solidity.remappings": [
    "@openzeppelin/=/Users/<USERNAME>/.brownie/packages/OpenZeppelin/openzeppelin-contracts@4.0.0",
    "@openzeppelin/=C:/Users/<USERNAME>/.brownie/packages/OpenZeppelin/openzeppelin-contracts@4.0.0"
]

I also cannot use VS Code environment variables:

"solidity.remappings": [
    "@openzeppelin/=${env:BROWNIE_INSTALL}/packages/OpenZeppelin/openzeppelin-contracts@4.0.0"
]

And as far as I know, VS Code does not support platform specific settings.

juanfranblanco commented 2 years ago

Hey, that is an interesting thought! Having those env values.

juanfranblanco commented 2 years ago

Do you have a node example to quickly test it?