Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/flat-birds-start.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@nomicfoundation/hardhat-typechain": patch
"@nomicfoundation/hardhat-verify": patch
"hardhat": patch
---

Make SolidityBuildSystem easier to work with
5 changes: 5 additions & 0 deletions .changeset/wet-lines-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"hardhat": patch
---

Show fs paths and better error messages when a Solidity file can't be compiled with any configured compiler [#7988](https://github.com/NomicFoundation/hardhat/pull/7988)
10 changes: 10 additions & 0 deletions .peer-bumps.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@
"package": "@nomicfoundation/hardhat-node-test-runner",
"peer": "hardhat",
"reason": "It depends on the new TestHooks#onTestRunStart, TestHooks#onTestWorkerDone, and TestHooks#onTestRunDone hooks"
},
{
"package": "@nomicfoundation/hardhat-verify",
"peer": "hardhat",
"reason": "Uses the new api to detect errors in SolidityBuildSystem#getCompilationJobs"
},
{
"package": "@nomicfoundation/hardhat-typechain",
"peer": "hardhat",
"reason": "Uses the new api to detect errors in SolidityBuildSystem#build"
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export default async (): Promise<Partial<SolidityHooks>> => {
const result = await next(context, rootFilePaths, options);

// Skip if build failed (returned an error)
if ("reason" in result) {
if (!context.solidity.isSuccessfulBuildResult(result)) {
return result;
}

Expand Down
2 changes: 1 addition & 1 deletion v-next/hardhat-verify/src/internal/artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export async function getCompilerInput(
);

assertHardhatInvariant(
!("reason" in getCompilationJobsResult),
getCompilationJobsResult.success,
"getCompilationJobs should not error",
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type {
CompilationJobCreationError,
FailedFileBuildResult,
FileBuildResult,
SolidityBuildSystem,
} from "../../../types/solidity.js";

import { HardhatError } from "@nomicfoundation/hardhat-errors";
Expand All @@ -22,9 +23,10 @@ type SuccessfulSolidityBuildResults = Map<
* job failed.
*/
export function throwIfSolidityBuildFailed(
solidity: SolidityBuildSystem,
results: SolidityBuildResults,
): asserts results is SuccessfulSolidityBuildResults {
if ("reason" in results) {
if (!solidity.isSuccessfulBuildResult(results)) {
throw new HardhatError(
HardhatError.ERRORS.CORE.SOLIDITY.COMPILATION_JOB_CREATION_ERROR,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem {
}
}

public isSuccessfulBuildResult(
buildResult: CompilationJobCreationError | Map<string, FileBuildResult>,
): buildResult is Map<string, FileBuildResult> {
return buildResult instanceof Map;
}

public async build(
rootFilePaths: string[],
_options?: BuildOptions,
Expand Down Expand Up @@ -244,7 +250,7 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem {
options,
);

if ("reason" in compilationJobsResult) {
if (!compilationJobsResult.success) {
return compilationJobsResult;
}

Expand Down Expand Up @@ -430,7 +436,6 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem {
const solcConfigSelector = new SolcConfigSelector(
buildProfileName,
buildProfile,
dependencyGraph,
);

let subgraphsWithConfig: Array<
Expand All @@ -446,11 +451,11 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem {
const configOrError =
solcConfigSelector.selectBestSolcConfigForSingleRootGraph(subgraph);

if ("reason" in configOrError) {
if (!configOrError.success) {
return configOrError;
}

subgraphsWithConfig.push([configOrError, subgraph]);
subgraphsWithConfig.push([configOrError.config, subgraph]);
}

// get longVersion and isWasm from the compiler for each version
Expand Down Expand Up @@ -632,7 +637,12 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem {
}
}

return { compilationJobsPerFile, indexedIndividualJobs, cacheHits };
return {
success: true,
compilationJobsPerFile,
indexedIndividualJobs,
cacheHits,
};
}

#getBuildProfile(buildProfileName: string = DEFAULT_BUILD_PROFILE) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,17 @@

/**
* Creates a new SolcConfigSelector that can be used to select the best solc
* configuration for subgraphs of the given dependency graph.
* configuration for single-root subgraphs to create their resepective

Check warning on line 21 in v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/solc-config-selection.ts

View workflow job for this annotation

GitHub Actions / spellcheck

Misspelled word (resepective) Suggestions: (respective*)
* individual compilation jobs.
*
* All the queries are done in the context of the given dependency graph, and
* using the same build profile.
* All the queries use the same build profile.
*
* @param buildProfileName The name of the build profile to use.
* @param buildProfile The build profile config.
* @param _dependencyGraph The entire dependency graph of the project.
*/
constructor(
buildProfileName: string,
buildProfile: SolidityBuildProfileConfig,
_dependencyGraph: DependencyGraph,
) {
this.#buildProfileName = buildProfileName;
this.#buildProfile = buildProfile;
Expand All @@ -46,7 +44,7 @@
*/
public selectBestSolcConfigForSingleRootGraph(
subgraph: DependencyGraph,
): SolcConfig | CompilationJobCreationError {
): { success: true; config: SolcConfig } | CompilationJobCreationError {
const roots = subgraph.getRoots();

assertHardhatInvariant(
Expand Down Expand Up @@ -75,7 +73,7 @@
);
}

return overriddenCompiler;
return { success: true, config: overriddenCompiler };
}

// if there's no override, we find a compiler that matches the version range
Expand All @@ -100,37 +98,92 @@
`Matching config not found for version '${matchingVersion.toString()}'`,
);

return matchingConfig;
return { success: true, config: matchingConfig };
}

/**
* Returns a description of why we couldn't get a compiler configuration for
* the given root file and dependency subgraph.
*
* @param root The root file that created the single-root dependency subgraph
* @param dependencyGraph The dependency subgraph we couldn't get a compiler
* configuration for
* @param compilerVersions The compiler versions that are configured for the
* selected build profile. For overridden roots, it's a single one.
* @param overridden True if the root has an overridden config.
* @returns The error why we couldn't get a compiler configuration.
*/
#getCompilationJobCreationError(
root: ResolvedFile,
dependencyGraph: DependencyGraph,
compilerVersions: string[],
overridden: boolean,
): CompilationJobCreationError {
const rootVersionRange = root.content.versionPragmas.join(" ");
if (maxSatisfying(compilerVersions, rootVersionRange) === null) {
let reason: CompilationJobCreationErrorReason;
let formattedReason: string;
if (overridden) {
reason =
CompilationJobCreationErrorReason.INCOMPATIBLE_OVERRIDDEN_SOLC_VERSION;
formattedReason = `An override with incompatible solc version was found for this file.`;
} else {
reason =
CompilationJobCreationErrorReason.NO_COMPATIBLE_SOLC_VERSION_WITH_ROOT;
formattedReason = `No solc version enabled in this profile is compatible with this file.`;

// This logic is pretty different depending if we are dealing with a config
// override or not. If we are, we have a single compiler option, so things
// are simpler.

if (overridden) {
// The root may not be compatible with the override version
if (maxSatisfying(compilerVersions, rootVersionRange) === null) {
return {
success: false,
reason:
CompilationJobCreationErrorReason.INCOMPATIBLE_OVERRIDDEN_SOLC_VERSION,
rootFilePath: root.fsPath,
buildProfile: this.#buildProfileName,
formattedReason: `An override with incompatible solc version was found for this file.`,
};
}

// A transitive dependency can have a pragma that's incompatible with
// the overridden version.
for (const transitiveDependency of this.#getTransitiveDependencies(
root,
dependencyGraph,
)) {
const depOwnRange =
transitiveDependency.dependency.content.versionPragmas.join(" ");

if (maxSatisfying(compilerVersions, depOwnRange) === null) {
return {
success: false,
reason:
CompilationJobCreationErrorReason.OVERRIDDEN_SOLC_VERSION_INCOMPATIBLE_WITH_DEPENDENCY,
rootFilePath: root.fsPath,
buildProfile: this.#buildProfileName,
incompatibleImportPath: transitiveDependency.fsPath,
formattedReason: `The compiler version override is incompatible with a dependency of this file:\n * ${shortenPath(root.fsPath)}\n * ${transitiveDependency.fsPath.map((s) => shortenPath(s)).join("\n * ")}`,
};
}
}

// There's no other case. If the root and all the dependencies are
// compatible, and we still can choose a version, we have a bug.
/* c8 ignore next 5 */
assertHardhatInvariant(
false,
"Trying to get the error for an overridden solidity file that has no compatible config, but failed to detect it, as the root and all the dependencies are compatible with the overridden compiler config.",
);
}

// Non-overridden case: we first check if the root is compatible with any
// configured compiler
if (maxSatisfying(compilerVersions, rootVersionRange) === null) {
return {
reason,
success: false,
reason:
CompilationJobCreationErrorReason.NO_COMPATIBLE_SOLC_VERSION_WITH_ROOT,
rootFilePath: root.fsPath,
buildProfile: this.#buildProfileName,
formattedReason,
formattedReason: `No solc version enabled in this profile is compatible with this file.`,
};
}

// We check all the transitive dependencies of the root to try to return
// the most specific error that we can.
for (const transitiveDependency of this.#getTransitiveDependencies(
root,
dependencyGraph,
Expand All @@ -140,21 +193,59 @@
.map((pragmas) => pragmas.join(" "))
.join(" ");

const depOwnRange =
transitiveDependency.dependency.content.versionPragmas.join(" ");

// A transitive dependency can have a pragma that's incompatible with
// all the configured compilers
if (maxSatisfying(compilerVersions, depOwnRange) === null) {
return {
success: false,
reason:
CompilationJobCreationErrorReason.NO_COMPATIBLE_SOLC_VERSION_WITH_DEPENDENCY,
rootFilePath: root.fsPath,
buildProfile: this.#buildProfileName,
incompatibleImportPath: transitiveDependency.fsPath,
formattedReason: `No solc version enabled in this profile is compatible with a dependency of this file:\n * ${shortenPath(root.fsPath)}\n * ${transitiveDependency.fsPath.map((s) => shortenPath(s)).join("\n * ")}`,
};
}

// The root and the version ranges to get to this transitive dependency
// may be contradictory, so no version ever can satisfy them.
if (!intersects(rootVersionRange, transitiveDependencyVersionRange)) {
return {
success: false,
reason: CompilationJobCreationErrorReason.IMPORT_OF_INCOMPATIBLE_FILE,
rootFilePath: root.fsPath,
buildProfile: this.#buildProfileName,
incompatibleImportPath: transitiveDependency.fsPath,
formattedReason: `Following these imports leads to an incompatible solc version pragma that no version can satisfy:
* ${shortenPath(root.fsPath)}
* ${transitiveDependency.fsPath.map((s) => shortenPath(s)).join("\n * ")}
`,
formattedReason: `Following these imports leads to an incompatible solc version pragma that no version can satisfy:\n * ${shortenPath(root.fsPath)}\n * ${transitiveDependency.fsPath.map((s) => shortenPath(s)).join("\n * ")}`,
};
}

// The root and the version ranges to get to this transitive dependency
// may not be compatible with any configured compiler.
const combinedRange = `${rootVersionRange} ${transitiveDependencyVersionRange}`;
if (maxSatisfying(compilerVersions, combinedRange) === null) {
return {
success: false,
reason:
CompilationJobCreationErrorReason.NO_COMPATIBLE_SOLC_VERSION_FOR_TRANSITIVE_IMPORT_PATH,
rootFilePath: root.fsPath,
buildProfile: this.#buildProfileName,
incompatibleImportPath: transitiveDependency.fsPath,
formattedReason: `No solc version enabled in this profile is compatible with this file and this import path:\n * ${shortenPath(root.fsPath)}\n * ${transitiveDependency.fsPath.map((s) => shortenPath(s)).join("\n * ")}`,
};
}
}

// This is a generic case that can happen when the incompatibilities exist
// but we can't detect them with the above algorithm. For example, if a
// root imports two compatible dependencies that are incompatible with each
// other. We could try and improve this error message, but it's
// computationally expensive and hard to express to the users.
return {
success: false,
reason:
CompilationJobCreationErrorReason.NO_COMPATIBLE_SOLC_VERSION_FOUND,
rootFilePath: root.fsPath,
Expand All @@ -163,6 +254,12 @@
};
}

/**
* Returns a generator of all the transitive dependencies of a root file. For each
* dependency, it yields the sequence of fsPaths from the root to that dependency,
* along with the corresponding version pragma paths for each file in the import chain.
* The paths don't include the root itself.
*/
*#getTransitiveDependencies(
root: ResolvedFile,
dependencyGraph: DependencyGraph,
Expand All @@ -179,7 +276,7 @@
continue;
}

visited.add(file);
visited = new Set([...visited, file]);
Comment thread
alcuadrado marked this conversation as resolved.

yield {
fsPath: [file.fsPath],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ class LazySolidityBuildSystem implements SolidityBuildSystem {
return buildSystem.getScope(fsPath);
}

public isSuccessfulBuildResult(
buildResult: CompilationJobCreationError | Map<string, FileBuildResult>,
): buildResult is Map<string, FileBuildResult> {
// Note: This duplicates the logic of the actual implementation because it's
// a synchronous method, so we can't import the implementation.
return buildResult instanceof Map;
}

public async build(
rootFiles: string[],
options?: BuildOptions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ async function buildForScope(
scope,
});

throwIfSolidityBuildFailed(results);
throwIfSolidityBuildFailed(solidity, results);

// If we recompiled the entire project we cleanup the artifacts
if (isFullCompilation) {
Expand Down
Loading
Loading