Improve compiler selection errors#7988
Conversation
🦋 Changeset detectedLatest commit: df14a31 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
5a72e71 to
654c2e1
Compare
There was a problem hiding this comment.
Pull request overview
This pull request improves compiler selection error messages and API usability in the Hardhat Solidity build system. It addresses issue #7748 by providing more detailed, specific error messages when a Solidity file cannot be compiled due to version pragma incompatibilities, and it uses file system paths instead of input source names for better clarity. Additionally, it refactors the API to use discriminated unions with explicit success flags instead of relying on "reason" in ... checks, making the code safer and more maintainable.
Changes:
- Introduced more granular error types with specific reasons (e.g.,
NO_COMPATIBLE_SOLC_VERSION_WITH_DEPENDENCY,OVERRIDDEN_SOLC_VERSION_INCOMPATIBLE_WITH_DEPENDENCY,NO_COMPATIBLE_SOLC_VERSION_FOR_TRANSITIVE_IMPORT_PATH) - Refactored return types to use discriminated unions with explicit
successfields - Added
isSuccessfulBuildResult()helper method to theSolidityBuildSysteminterface for type-safe result checking - Updated error messages to display file system paths instead of input source names, improving readability for users
- Enhanced error detection algorithm to identify the specific source of incompatibility in dependency trees
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
v-next/hardhat/src/types/solidity/build-system.ts |
Added new error reason enums, refactored error interfaces to use discriminated unions with success: false, added success: true to GetCompilationJobsResult, and defined isSuccessfulBuildResult method signature |
v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/solc-config-selection.ts |
Refactored return types to include success discriminant, enhanced error detection logic to identify specific incompatibility scenarios, updated error messages to use file system paths via shortenPath() |
v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/build-system.ts |
Implemented isSuccessfulBuildResult() method, updated result checks from "reason" in ... to !result.success, added success: true to returned compilation job results |
v-next/hardhat/src/internal/builtin-plugins/solidity/hook-handlers/hre.ts |
Implemented isSuccessfulBuildResult() in lazy build system wrapper using instanceof Map check |
v-next/hardhat/src/internal/builtin-plugins/solidity/build-results.ts |
Updated throwIfSolidityBuildFailed() to accept SolidityBuildSystem parameter and use isSuccessfulBuildResult() helper |
v-next/hardhat/src/internal/builtin-plugins/solidity/tasks/build.ts |
Updated throwIfSolidityBuildFailed() call to pass solidity parameter |
v-next/hardhat-verify/src/internal/artifacts.ts |
Updated error check from "reason" in ... to result.success |
v-next/hardhat-typechain/src/internal/hook-handlers/solidity.ts |
Updated build result check to use isSuccessfulBuildResult() helper |
v-next/hardhat/test/internal/builtin-plugins/solidity/build-system/solc-config-selection.ts |
Added comprehensive test cases for new error scenarios, updated existing tests to expect discriminated union results with success field |
v-next/hardhat/test/internal/builtin-plugins/solidity/build-system/partial-compilation/*.ts |
Updated tests to use isSuccessfulBuildResult() and result.success checks |
v-next/hardhat/test/internal/builtin-plugins/solidity/build-system/integration/custom-compiler.ts |
Updated assertion helper to use isSuccessfulBuildResult() |
v-next/hardhat/test/internal/builtin-plugins/solidity/build-system/build-system.ts |
Updated test to use result.success check |
.peer-bumps.json |
Added peer dependency bumps for hardhat-verify and hardhat-typechain due to API changes |
.changeset/wet-lines-design.md |
Documented user-facing improvement to error messages |
.changeset/flat-birds-start.md |
Documented API improvement for SolidityBuildSystem |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ChristopherDedominici
left a comment
There was a problem hiding this comment.
LGTM, only 2 minor comments
There was a problem hiding this comment.
The docs needs to be updated according to the changes
|
|
||
| return { compilationJobsPerFile, indexedIndividualJobs, cacheHits }; | ||
| return { | ||
| success: true as const, |
There was a problem hiding this comment.
minor: I think as const can be removed
It now has a better algorithm that detects more cases, explaining them to the users. It also uses file-paths everywhere, instead of shoing the internal inputSourceNames.
…stem/solc-config-selection.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…tem/solc-config-selection.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…tem/solc-config-selection.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
cc7152c to
df14a31
Compare
Fixes #7748 by:
inputSourceNames to display the filesIt also makes working with results of
SolidityBuildSystem#buildandSolidityBuildSystem#getCompilationJobssimpler. /cc @kanej no more"reason" in ...Replaces #7748