diff --git a/.changeset/nine-plants-carry.md b/.changeset/nine-plants-carry.md new file mode 100644 index 0000000000..827086f06d --- /dev/null +++ b/.changeset/nine-plants-carry.md @@ -0,0 +1,5 @@ +--- +"@latticexyz/common": patch +--- + +Prevented errors not included in the contract (but present in the file) from being included in the interface by `contractToInterface` diff --git a/e2e/packages/contracts/src/systems/CustomErrorsSystem.sol b/e2e/packages/contracts/src/systems/CustomErrorsSystem.sol index 7eab852b31..9a43670985 100644 --- a/e2e/packages/contracts/src/systems/CustomErrorsSystem.sol +++ b/e2e/packages/contracts/src/systems/CustomErrorsSystem.sol @@ -4,6 +4,8 @@ pragma solidity >=0.8.21; import { System } from "@latticexyz/world/src/System.sol"; import { Position } from "../CustomTypes.sol"; +error ThisErrorShouldBeAbsentFromTheGeneratedSystemInterface(); + contract CustomErrorsSystem is System { error TestError1(); error TestError2(Position position, uint256 value, string name, bool flag); diff --git a/packages/common/src/codegen/utils/contractToInterface.ts b/packages/common/src/codegen/utils/contractToInterface.ts index 6ec50f0ea3..13c347290a 100644 --- a/packages/common/src/codegen/utils/contractToInterface.ts +++ b/packages/common/src/codegen/utils/contractToInterface.ts @@ -1,5 +1,10 @@ import { parse, visit } from "@solidity-parser/parser"; -import type { SourceUnit, TypeName, VariableDeclaration } from "@solidity-parser/parser/dist/src/ast-types"; +import type { + ContractDefinition, + SourceUnit, + TypeName, + VariableDeclaration, +} from "@solidity-parser/parser/dist/src/ast-types"; import { MUDError } from "../../errors"; export interface ContractInterfaceFunction { @@ -36,52 +41,55 @@ export function contractToInterface( } { const ast = parse(data); - let withContract = false; + const contractNode = findContractNode(parse(data), contractName); let symbolImports: SymbolImport[] = []; const functions: ContractInterfaceFunction[] = []; const errors: ContractInterfaceError[] = []; - visit(ast, { - ContractDefinition({ name }) { - if (name === contractName) { - withContract = true; - } - }, - FunctionDefinition( - { name, visibility, parameters, stateMutability, returnParameters, isConstructor, isFallback, isReceiveEther }, - parent - ) { - if (parent !== undefined && parent.type === "ContractDefinition" && parent.name === contractName) { - try { - // skip constructor and fallbacks - if (isConstructor || isFallback || isReceiveEther) return; - // forbid default visibility (this check might be unnecessary, modern solidity already disallows this) - if (visibility === "default") throw new MUDError(`Visibility is not specified`); - - if (visibility === "external" || visibility === "public") { - functions.push({ - name: name === null ? "" : name, - parameters: parameters.map(parseParameter), - stateMutability: stateMutability || "", - returnParameters: returnParameters === null ? [] : returnParameters.map(parseParameter), - }); - - for (const { typeName } of parameters.concat(returnParameters ?? [])) { - const symbols = typeNameToSymbols(typeName); - symbolImports = symbolImports.concat(symbolsToImports(ast, symbols)); - } - } - } catch (error: unknown) { - if (error instanceof MUDError) { - error.message = `Function "${name}" in contract "${contractName}": ${error.message}`; + if (!contractNode) { + throw new MUDError(`Contract not found: ${contractName}`); + } + + visit(contractNode, { + FunctionDefinition({ + name, + visibility, + parameters, + stateMutability, + returnParameters, + isConstructor, + isFallback, + isReceiveEther, + }) { + try { + // skip constructor and fallbacks + if (isConstructor || isFallback || isReceiveEther) return; + // forbid default visibility (this check might be unnecessary, modern solidity already disallows this) + if (visibility === "default") throw new MUDError(`Visibility is not specified`); + + if (visibility === "external" || visibility === "public") { + functions.push({ + name: name === null ? "" : name, + parameters: parameters.map(parseParameter), + stateMutability: stateMutability || "", + returnParameters: returnParameters === null ? [] : returnParameters.map(parseParameter), + }); + + for (const { typeName } of parameters.concat(returnParameters ?? [])) { + const symbols = typeNameToSymbols(typeName); + symbolImports = symbolImports.concat(symbolsToImports(ast, symbols)); } - throw error; } + } catch (error: unknown) { + if (error instanceof MUDError) { + error.message = `Function "${name}" in contract "${contractName}": ${error.message}`; + } + throw error; } }, CustomErrorDefinition({ name, parameters }) { errors.push({ - name: name === null ? "" : name, + name, parameters: parameters.map(parseParameter), }); @@ -92,10 +100,6 @@ export function contractToInterface( }, }); - if (!withContract) { - throw new MUDError(`Contract not found: ${contractName}`); - } - return { functions, errors, @@ -103,6 +107,20 @@ export function contractToInterface( }; } +function findContractNode(ast: SourceUnit, contractName: string): ContractDefinition | undefined { + let contract = undefined; + + visit(ast, { + ContractDefinition(node) { + if (node.name === contractName) { + contract = node; + } + }, + }); + + return contract; +} + function parseParameter({ name, typeName, storageLocation }: VariableDeclaration): string { let typedNameWithLocation = "";