Merge solc output selection config

[ItsNickBarry]

closes #6551

[ItsNickBarry]

I suppose the object keys also need to be sorted, but the default is not sorted. "" should come before "*".

[ItsNickBarry]

This would be useful as an exported helper function so that plugins can make additions to the output selection without overwriting.

[zoeyTM]

Will approve after a couple of minor changes, but this looks great overall. Thanks!

[changeset-bot]

⚠️ No Changeset found

Latest commit: 1590d4c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[ItsNickBarry]

@zoeyTM I think this should be exported as a helper function. Plugins often need to add output selection. They could copy/paste this code, but it's ugly and should be hidden behind a function. Otherwise I think they might do it incorrectly and potentially clash with each other. However, I don't know where to put such a function.

[ItsNickBarry]

I don't think sorting the keys is strictly necessary because they order of keys isn't expected to change, but it's better to keep the object in a normalized state.

[ItsNickBarry]

Maybe not, actually. I can't quite contrive a situation in which a plugin would reasonably change key order without adding or removing keys.

[ItsNickBarry]

@zoeyTM What do you think of this? Everything is sorted and empty entries are ignored.

const configOutputSelection = settings.outputSelection ?? {};
const outputSelection: typeof defaultOutputSelection = {};

const fileKeys = Array.from(
  new Set([
    ...Object.keys(defaultOutputSelection),
    ...Object.keys(configOutputSelection),
  ]),
).sort();

for (const fileKey of fileKeys) {
  const contractKeys = Array.from(
    new Set([
      ...Object.keys(defaultOutputSelection[fileKey] ?? {}),
      ...Object.keys(configOutputSelection[fileKey] ?? {}),
    ]),
  ).sort();

  for (const contractKey of contractKeys) {
    const values = Array.from(
      new Set([
        ...(defaultOutputSelection[fileKey]?.[contractKey] ?? []),
        ...(configOutputSelection[fileKey]?.[contractKey] ?? []),
      ]),
    ).sort();

    if (values.length > 0) {
      outputSelection[fileKey] = {};
      outputSelection[fileKey][contractKey] = values;
    }
  }
}

[ItsNickBarry]

Shouldn't all of this go in the config hook?

[alcuadrado]

I think this can be simplified, and @zoeyTM should have some time to do it now.

The reason that I think this could be simpler that all we need from Hardhat's point of view is ensure that these are set, as both Hardhat and EDR need them:

"*": {
        "": ["ast"],
        "*": [
          "abi",
          "evm.bytecode",
          "evm.deployedBytecode",
          "evm.methodIdentifiers",
          "metadata",
        ],
      },

So we can just clone whatever the user provides, and add those selectors if not present.

If that's the case, do you think you'd need a helper? can you explain the use case? Wouldn't just getting the build project from hre.solidity be enough?

[ItsNickBarry]

I was thinking that both users and plugin developers would expect to be able to pass in an output selection object with the same standard structure. The object isn't even documented (argotorg/solidity#15744), so it's uncomfortable to manipulate directly.

The HH solidity module is also a plugin, and doesn't need to behave any differently. Isn't this a config resolution problem that should be done in the resolveConfig hook?

This is what the code looks like without a helper:

const settings = compiler.settings;
settings.outputSelection ??= {};
settings.outputSelection['*'] ??= {};
settings.outputSelection['*']['*'] ??= [];

if (!settings.outputSelection['*']['*'].includes('storageLayout')) {
  settings.outputSelection['*']['*'].push('storageLayout');
}

It's not that bad, but needs to be duplicated in at least 4 places:

• builtin:solidity
• hardhat-docgen
• hardhat-storage-layout-diff (in two places)

I'd rather do something like this:

const STORAGE_LAYOUT_OUTPUT_SELECTION = { '*': { '*': ['storageLayout'] } };

settings.outputSelection = mergeOutputSelection(settings.outputSelection, STORAGE_LAYOUT_OUTPUT_SELECTION);

Then, nobody has to think about any of these issues:

• overwriting other plugins' entries
• nonexistent entries (especially if Hardhat waits until just before compilation to add the "*" defaults)
• duplicate entries
• changes in sort order leading to unnecessary recompilation

[kanej]

I am closing this PR, thanks @ItsNickBarry, the approach was encoded in these two PRs:

• Merge user's output selection with our defaults #6668
• Dedupe and sort merged solc outputSelection #6678

The discussion around exposing the helper function to plugin authors I am going to capture in a separate issue: #6704.