search

NomicFoundation / hardhat

Hardhat is a development environment to compile, deploy, test, and debug your Ethereum software.
https://hardhat.org
Other
7.29k stars 1.41k forks source link

Detect top-level `settings` in hardhat config #2213

Open fvictorio opened 2 years ago

fvictorio commented 2 years ago

A very common mistake is doing this:

solidity: "0.8.4",
settings: { ... }

instead of

solidity: {
  version: "0.8.4",
  settings: { ... }
}

I think it's worth detecting these scenarios and showing a warning when there is a settings value in the root of the config but the userConfig.solidity value is a string. We could also throw an error, which would be annoying if some plugin uses config.settings, but that could be worked around by changing solidity: "0.8.4" to solidity: { version: "0.8.4" }

alcuadrado commented 2 years ago

I'm not sold on this tbh. It starts polluting the core of hardhat with the functionality built on top. I know we've already done this a few times though

fvictorio commented 2 years ago

How can we do it in a way that doesn't cause that?

One idea: add the concept of warnings to the config validation, that way plugins can also emit warnings for certain patterns. But that's the first thing that comes to mind, there surely are other options.

In any case, I really think we need to do something about this issue. It happens a lot.

alcuadrado commented 2 years ago

Well, your comment made me realize that maybe what we need is a better way for plugins to deal with the situation. We don't need it to be implemented before adding this warning.

github-actions[bot] commented 2 years ago

This issue was marked as stale because it didn't have any activity in the last 30 days. If you think it's still relevant, please leave a comment indicating so. Otherwise, it will be closed in 7 days.

frangio commented 1 year ago

Another error that I've seen is:

solidity: {
  compilers: [{ version: "0.8.4" }],
  settings: { ... },
},

The settings object is ignored.

IMO this should definitely be caught somehow.

If it's not producing an error, the misplaced settings object could be merged with the settings in the compilers array.