From 6567a8d69d717daea1fa764b8d04a1bb2fa644e5 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 28 Jun 2023 21:10:18 -0300 Subject: [PATCH] don't use fully qualified names for types if not needed --- CHANGELOG.md | 4 +++ contracts/Test.sol | 16 +++++++++ src/core.test.ts.md | 58 +++++++++++++++++++++--------- src/core.test.ts.snap | Bin 1983 -> 2034 bytes src/core.ts | 82 ++++++++++++++++++++++++------------------ 5 files changed, 110 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94ef6e0..7c57c4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 0.3.9 + +- Do not use fully qualified names for types in generated code unless necessary. + ## 0.3.8 - Support custom `paths.root` in Hardhat config. diff --git a/contracts/Test.sol b/contracts/Test.sol index 1bb16ac..df5f2e7 100644 --- a/contracts/Test.sol +++ b/contracts/Test.sol @@ -239,3 +239,19 @@ contract HasReceiveFunction { import { Imported } from './Imported.sol'; contract ImportedChild is Imported {} + +contract HasEnum { + enum InheritedEnum { + X + } +} + +contract ParentHasEnum is HasEnum { + function _getValue(InheritedEnum foo) internal pure returns (uint8) { + return uint8(foo); + } + + function _getY(Types.Enum e) internal pure returns (uint8) { + return uint8(e); + } +} diff --git a/src/core.test.ts.md b/src/core.test.ts.md index 58d6d37..7b115c6 100644 --- a/src/core.test.ts.md +++ b/src/core.test.ts.md @@ -377,7 +377,7 @@ Generated by [AVA](https://avajs.dev). ␊ mapping(uint256 => S) internal $v_S;␊ ␊ - mapping(uint256 => Foo.Z) internal $v_Foo_Z;␊ + mapping(uint256 => Z) internal $v_Z;␊ ␊ constructor() payable {␊ }␊ @@ -406,7 +406,7 @@ Generated by [AVA](https://avajs.dev). return _x4;␊ }␊ ␊ - function $z() external view returns (Foo.Integer) {␊ + function $z() external view returns (Integer) {␊ return z;␊ }␊ ␊ @@ -434,11 +434,11 @@ Generated by [AVA](https://avajs.dev). super._testClash($v_S[t]);␊ }␊ ␊ - function $_testClash_Foo_Z(uint256 t) external {␊ - super._testClash($v_Foo_Z[t]);␊ + function $_testClash_Z(uint256 t) external {␊ + super._testClash($v_Z[t]);␊ }␊ ␊ - function $_testUDVT(Foo.Integer i) external {␊ + function $_testUDVT(Integer i) external {␊ super._testUDVT(i);␊ }␊ ␊ @@ -450,7 +450,7 @@ Generated by [AVA](https://avajs.dev). ␊ mapping(uint256 => S) internal $v_S;␊ ␊ - mapping(uint256 => Foo.Z) internal $v_Foo_Z;␊ + mapping(uint256 => Z) internal $v_Z;␊ ␊ constructor() payable {␊ }␊ @@ -479,7 +479,7 @@ Generated by [AVA](https://avajs.dev). return _x4;␊ }␊ ␊ - function $z() external view returns (Foo.Integer) {␊ + function $z() external view returns (Integer) {␊ return z;␊ }␊ ␊ @@ -511,11 +511,11 @@ Generated by [AVA](https://avajs.dev). super._testClash($v_S[t]);␊ }␊ ␊ - function $_testClash_Foo_Z(uint256 t) external {␊ - super._testClash($v_Foo_Z[t]);␊ + function $_testClash_Z(uint256 t) external {␊ + super._testClash($v_Z[t]);␊ }␊ ␊ - function $_testUDVT(Foo.Integer i) external {␊ + function $_testUDVT(Integer i) external {␊ super._testUDVT(i);␊ }␊ ␊ @@ -604,7 +604,7 @@ Generated by [AVA](https://avajs.dev). constructor() payable {␊ }␊ ␊ - function $_testEnumType(Types.Enum e) external {␊ + function $_testEnumType(Enum e) external {␊ super._testEnumType(e);␊ }␊ ␊ @@ -694,23 +694,23 @@ Generated by [AVA](https://avajs.dev). return var3[arg0];␊ }␊ ␊ - function $var4() external view returns (WithVars.Struct memory) {␊ + function $var4() external view returns (Struct memory) {␊ return var4;␊ }␊ ␊ - function $var5() external view returns (WithVars.Struct[] memory) {␊ + function $var5() external view returns (Struct[] memory) {␊ return var5;␊ }␊ ␊ - function $var6(uint256 arg0) external view returns (WithVars.Struct memory) {␊ + function $var6(uint256 arg0) external view returns (Struct memory) {␊ return var6[arg0];␊ }␊ ␊ - function $var7(uint256 arg0, bool arg1) external view returns (WithVars.Struct memory) {␊ + function $var7(uint256 arg0, bool arg1) external view returns (Struct memory) {␊ return var7[arg0][arg1];␊ }␊ ␊ - function $var8(uint256 arg0, bool arg1) external view returns (WithVars.Struct[] memory) {␊ + function $var8(uint256 arg0, bool arg1) external view returns (Struct[] memory) {␊ return var8[arg0][arg1];␊ }␊ ␊ @@ -780,6 +780,32 @@ Generated by [AVA](https://avajs.dev). ␊ receive() external payable {}␊ }␊ + ␊ + contract $HasEnum is HasEnum {␊ + bytes32 public constant __hh_exposed_bytecode_marker = "hardhat-exposed";␊ + ␊ + constructor() payable {␊ + }␊ + ␊ + receive() external payable {}␊ + }␊ + ␊ + contract $ParentHasEnum is ParentHasEnum {␊ + bytes32 public constant __hh_exposed_bytecode_marker = "hardhat-exposed";␊ + ␊ + constructor() payable {␊ + }␊ + ␊ + function $_getValue(InheritedEnum foo) external pure returns (uint8 ret0) {␊ + (ret0) = super._getValue(foo);␊ + }␊ + ␊ + function $_getY(Types.Enum e) external pure returns (uint8 ret0) {␊ + (ret0) = super._getY(e);␊ + }␊ + ␊ + receive() external payable {}␊ + }␊ ` ## snapshot initializers diff --git a/src/core.test.ts.snap b/src/core.test.ts.snap index 365b87d7cc24cf1d91102a616069cea9ae58b2d0..ae3e4613cba8bfe10daede57623370dab21c080c 100644 GIT binary patch literal 2034 zcmVMeM=Z0S z%b?{L*dL1s00000000B+T+MD9HxN!z6ajol&=)8cfe#X3%eLgWMhyqB6{j`eI)P<7 zmJLIoReckA`V5>wLydXsW6K&alq9Ioc%0De>Q>(p@=3(Y|0VPu~=YyUwyYd`F)W};0waZ~QBeVMb;Fcz96hId{^l21K1Y4(ICsOC&pI`7_Q1D;O1^D}!z~7fz_af}9exgk|abNDN z3z@UhFczBiGdrtJomGF3&Z?&vsp_hcsy^086{Y?^LFqli()+2Ne-L>o&OUN*Eg-n~ z4z>6xv_t*9IH4WtDcMS=dCwNzy}q$+5BlD}i9p{3oer{Y1GDeG(rQJGd|^gg?lo$1 z%F8h2WtOqol}AraGofiagP8DfC4e%AixTH{JwsfYS6;(w8D9GvS%RH3lQJxaFxqus zI)-H_!%8-W6)D5bY+c@zFuaqqJ0xJbg-#5+JDGOgAr%P@WuLu@b&+v_RqTqVU6fIh zq#Aa-5ReLU>2nU-R3d$j3aKQRSM>wZe<q`~uv)AXRewb;>7rvBw%on}l zQ5bOOSs3}3&s|zRcja?eG$2WEe4e?BGRkMke3r~-$;Fx_k5F^g(m}95#IT#e`2p2?kA33tl6r5%AjysGy1BA@4(d zoM;A3mqoL1?cfIJ?79Ih%I-{>`o0!W&WB(up4-yCoNVSs=~JqUH;6 z#rbs5ffOUGFsubR)Xi&8z2*_db5{4U;k}^}O zS21E6oXGwMW(7*=o6J=47aJv5^2)rt!mPe~)nc95{4R49c_bd=EI8PW`-glBtC}Dl1uDUCYluJqB{#mGq~e+wsgR ziN~OXAmLDu?N<_Ymz@ogM=w>36=r!UXT6oqEGhKgQkmzQkf;@{Y1@WZ+_;a_ZW)-6lh6OzjMo zP@|hOdNYfrSBVMTtYY>nyczyvK-i!3WP~}978hWz%Cg%8-NO}g;M`k?&%N{L0#9g% zEvng#Z704Q*jB#N3mAK>WQRzEsLsx%N|AG~j6&<9M`9{>@t>LL`oAutAtBpu_?t67 z^BbMd{|?>g{Oft1bC@YKrk8UlWRVE29qEAgQRk@S*@^R$ohYO;sobhcmdf)OA07~= zm&ceLh^$hPahPX8=ArXYvmk|yrz&(*nLc^G}?^5ptc*keX?|0_U zKKuEyt(oda`{zF)Mp%Pg+W&=z=dVOVM zZVu@^)56lTdVN_n4Qv4w+snJIsyQ9lsgn-R2vKlHnh)+s$d=bLy9#uBrVTraN2Hn^sOZ4@2rX$-de#M2rwi~BBk|;U>}Y?>QL$;& z7Nj2NgSG~ls82qL1yzWyA-N4*3@rm_1G-OhpXQj?9yt-OT|OF<*Y;>*=(SWDIqup6 z69;!a@A6-#)xn(hyy~Uved!n~My4UnD?~KrTv_AYY9P)qpMm$WogTF6v7Ur_qX_y= zBIx^e+~`Y3mOKP4d6uvx^WZ8^^L*%%Cj+m?|3={T&*O2y%jiN25zneH4zPE?B0^TZ z@C27*4~~NjEWcQn+=W19&CM6EfIKTYA>BxEiMd4{xMc#E4cyWX+#=Wdx6eSIRbyaD zV{(?<1eP>vH@CN>*+p81B9&cLKxIymL#Ip-rtYE?LoD@A7I;f_RU>rO1oL1vU9}dv zYONes4K8m6m#WBt+(_dqT~nvQt8B7=@_S^m|Mkh(CVPYt(9jLE?D%XS-lh6^OR`AA z3@j+Ojy<)>-}0A`^MF_Q8o?ikA=t4DTMFR3z#+-MC(VNm0Tz`l1(9%ssQ}Ud84!bo z40Z}>=>lYsb?PXLxo)8FFtSa9wS6yxwH*ysJJqL*xGfIWwkTM67<1iHMBF=gH7UHB z_Yz)B-~Xwu3}`Z#aK6sLj;F4`KfT~TrQkoE3h=vy!0!sZdm0Uvo9a_W?23bRDhgH} z##}eIaIns3u+Hvfu+DOfRA*Tu)!9fRRh;_&1f};rmflbF{Da8LarRMxYX-r^cBs$) zi0n{*dp@Qe>JiyW=XuT+-@U#uO%M9szp+4H2fZFLu0pqOzLr`G68YjgZT?zmi7AOz z^$$^M(xiMSGDh3tENV$Q)J?0OkVduuSo&abVqL8i$WQ&;>v$o;t-D$z=}9*sLpg#~ z$AbA7%0h;XVhkHXhHJ%4UK2395wr7fwMRg81+4^@HzEza11r)s({8U}g=bvz8#eiq zFUlxQQVUyNItYc?^ge@bE|CGp45=ic#8O)$3Z%@k1-3{IgDd#f!953OXX*&jhdAd} zWzX>FgNWiWsN_rVM!<7?k*Ziz19n$wC2Es+tV%nKz?zP#1=!PmSt5>iPLBALlQJI! z(;g!}0OpU#fJM*9$iHe%tLC&A!)i{8CMZdiyP4A{qiXi7X3uK&oZZ=TAGOP#HsLkd z6*G_oZ{8e?5U-Ozmq4394_kZTf-&`8Ty{b5BrBLnU#9{#$k7 zml@Jr<~5&*SE7&lJxDR)UrcKHN@xmJ_0frq5i0C%_`mR5*1LwId$!i7!FBJe1hWlcOBJs(>o)r-SJ3 z*z+U0S22K2A}!XZ<`djOl>xB>VNMXMH;6UQzyQ(oALFd}uV1q3@*f;4d7hpnKQkNM z_*KsM^5&zGjF&m%r=nJ7BF4W*xO)ODIyHvZuNIp>wOax43wZdWN3BEcqlNPRQ&})G_L5(MT{j{|X}@)^vy^WtFU|02?Fd z8^ov-eFIpg=^F=SlA0qu+DKrTtZtkzO;b@axLKeb%9LTn7g=+l{;(WZp#3l%$E)aA z`W>(jed;%X*<>h9Xf0vAHN2CfFMrVrF1}{mZzz8`SmLh|upb$5g5MUA5LfFp_{EW* z*!z4>|Bk%R_t(>^;;c~aOs?XLt0CY*J7j?O$*xeSYSX5tHjOK0$qH*$sZ3SHcK?7l zxhl5eK=d0k>dmSexGHt-YYiN?C&)^jtU8aXT5fu(<%);ZZ_Fq?s>-2e9f)Pt+GQcK R0^j`u-+$o6*BDY%003~%)!YC8 diff --git a/src/core.ts b/src/core.ts index 9a0280d..ab9d0bd 100644 --- a/src/core.ts +++ b/src/core.ts @@ -5,6 +5,7 @@ import { formatLines, Lines, spaceBetween } from './utils/format-lines'; import type { Visibility, SourceUnit, ContractDefinition, FunctionDefinition, VariableDeclaration, StorageLocation, TypeDescriptions, TypeName, InheritanceSpecifier, ModifierInvocation, FunctionCall } from 'solidity-ast'; import type { FileContent, ProjectPathsConfig, ResolvedFile } from 'hardhat/types'; import type { ExposedConfig } from './config'; +import assert from 'assert'; export interface SolcOutput { sources: { @@ -105,7 +106,7 @@ function getExposedContent(ast: SourceUnit, relativizePath: (p: string) => strin const clashingFunctions: Record = {}; for (const fn of externalizableFunctions) { - const id = getFunctionId(fn); + const id = getFunctionId(fn, c, deref); clashingFunctions[id] ??= 0; clashingFunctions[id] += 1; } @@ -121,13 +122,13 @@ function getExposedContent(ast: SourceUnit, relativizePath: (p: string) => strin [`bytes32 public constant __hh_exposed_bytecode_marker = "hardhat-exposed";\n`], spaceBetween( // slots for storage function parameters - ...getAllStorageArguments(externalizableFunctions).map(a => [ + ...getAllStorageArguments(externalizableFunctions, c, deref).map(a => [ `mapping(uint256 => ${a.storageType}) internal ${prefix}${a.storageVar};`, ]), // events for internal returns ...returnedEventFunctions.map(fn => { - const evName = clashingEvents[fn.name] === 1 ? fn.name : getFunctionNameQualified(fn, false); - const params = getFunctionReturnParameters(fn, null); + const evName = clashingEvents[fn.name] === 1 ? fn.name : getFunctionNameQualified(fn, c, deref, false); + const params = getFunctionReturnParameters(fn, c, deref, null); return [ `event return${prefix}${evName}(${params.map(printArgument).join(', ')});` ] @@ -138,24 +139,24 @@ function getExposedContent(ast: SourceUnit, relativizePath: (p: string) => strin ...externalizableVariables.map(v => [ [ 'function', - `${prefix}${v.name}(${getVarGetterArgs(v).map(printArgument).join(', ')})`, + `${prefix}${v.name}(${getVarGetterArgs(v, c, deref).map(printArgument).join(', ')})`, 'external', v.mutability === 'mutable' || (v.mutability === 'immutable' && !v.value) ? 'view' : 'pure', 'returns', - `(${getVarGetterReturnType(v)})`, + `(${getVarGetterReturnType(v, c, deref)})`, '{' ].join(' '), [ - `return ${isLibrary ? c.name + '.' : ''}${v.name}${getVarGetterArgs(v).map(a => `[${a.name}]`).join('')};`, + `return ${isLibrary ? c.name + '.' : ''}${v.name}${getVarGetterArgs(v, c, deref).map(a => `[${a.name}]`).join('')};`, ], '}', ]), // external functions ...externalizableFunctions.map(fn => { - const fnName = clashingFunctions[getFunctionId(fn)] === 1 ? fn.name : getFunctionNameQualified(fn); - const fnArgs = getFunctionArguments(fn); - const fnRets = getFunctionReturnParameters(fn); - const evName = isNonViewWithReturns(fn) && (clashingEvents[fn.name] === 1 ? fn.name : getFunctionNameQualified(fn, false)); + const fnName = clashingFunctions[getFunctionId(fn, c, deref)] === 1 ? fn.name : getFunctionNameQualified(fn, c, deref); + const fnArgs = getFunctionArguments(fn, c, deref); + const fnRets = getFunctionReturnParameters(fn, c, deref); + const evName = isNonViewWithReturns(fn) && (clashingEvents[fn.name] === 1 ? fn.name : getFunctionNameQualified(fn, c, deref, false)); // function header const header = [ @@ -228,14 +229,14 @@ function areFunctionsFullyImplemented(contract: ContractDefinition, deref: ASTDe return abstractFunctionIds.size === 0; } -function getFunctionId(fn: FunctionDefinition): string { - const storageArgs = new Set(getStorageArguments(fn)); - const nonStorageArgs = getFunctionArguments(fn).filter(a => !storageArgs.has(a)); +function getFunctionId(fn: FunctionDefinition, context: ContractDefinition, deref: ASTDereferencer): string { + const storageArgs = new Set(getStorageArguments(fn, context, deref)); + const nonStorageArgs = getFunctionArguments(fn, context, deref).filter(a => !storageArgs.has(a)); return fn.name + nonStorageArgs.map(a => a.type).join(''); } -function getFunctionNameQualified(fn: FunctionDefinition, onlyStorage: boolean = true): string { - return fn.name + (onlyStorage ? getStorageArguments(fn) : getFunctionArguments(fn)) +function getFunctionNameQualified(fn: FunctionDefinition, context: ContractDefinition, deref: ASTDereferencer, onlyStorage: boolean = true): string { + return fn.name + (onlyStorage ? getStorageArguments(fn, context, deref) : getFunctionArguments(fn, context, deref)) .map(arg => arg.storageType ?? arg.type) .map(type => type.replace(/ .*/,'').replace(/[^0-9a-zA-Z$_]+/g, '_')) // sanitize .join('_') @@ -289,7 +290,7 @@ function makeConstructor(contract: ContractDefinition, deref: ASTDereferencer, i const args = []; for (const a of constructors.get(c.id)?.parameters.parameters ?? []) { const name = missingArguments.has(a.name) ? `${c.name}_${a.name}` : a.name; - const type = getVarType(a, 'memory'); + const type = getVarType(a, c, deref, 'memory'); missingArguments.set(name, type); args.push(name); } @@ -390,55 +391,68 @@ interface Argument { const printArgument = (arg: Argument) => `${arg.type} ${arg.name}`; -function getFunctionArguments(fnDef: FunctionDefinition): Argument[] { +function getFunctionArguments(fnDef: FunctionDefinition, context: ContractDefinition, deref: ASTDereferencer): Argument[] { return fnDef.parameters.parameters.map((p, i) => { const name = p.name || `arg${i}`; if (p.storageLocation === 'storage') { - const storageType = getVarType(p, null); + const storageType = getVarType(p, context, deref, null); const storageVar = 'v_' + storageType.replace(/[^0-9a-zA-Z$_]+/g, '_'); // The argument is an index to an array in storage. return { name, type: 'uint256', storageVar, storageType }; } else { - const type = getVarType(p, 'calldata'); + const type = getVarType(p, context, deref, 'calldata'); return { name, type }; } }); } -function getStorageArguments(fn: FunctionDefinition): Required[] { - return getFunctionArguments(fn) +function getStorageArguments(fn: FunctionDefinition, context: ContractDefinition, deref: ASTDereferencer): Required[] { + return getFunctionArguments(fn, context, deref) .filter((a): a is Required => !!(a.storageVar && a.storageType)); } -function getAllStorageArguments(fns: FunctionDefinition[]): Required[] { +function getAllStorageArguments(fns: FunctionDefinition[], context: ContractDefinition, deref: ASTDereferencer): Required[] { return [ ...new Map( - fns.flatMap(getStorageArguments).map(a => [a.storageVar, a]), + fns.flatMap(fn => getStorageArguments(fn, context, deref)).map(a => [a.storageVar, a]), ).values(), ]; } -function getFunctionReturnParameters(fnDef: FunctionDefinition, location: StorageLocation | null = 'memory'): Argument[] { +function getFunctionReturnParameters(fnDef: FunctionDefinition, context: ContractDefinition, deref: ASTDereferencer, location: StorageLocation | null = 'memory'): Argument[] { return fnDef.returnParameters.parameters.map((p, i) => { const name = p.name || `ret${i}`; - const type = getVarType(p, location); + const type = getVarType(p, context, deref, location); return { name, type }; }); } -function getVarType(varDecl: VariableDeclaration, location: StorageLocation | null = varDecl.storageLocation): string { +function getVarType(varDecl: VariableDeclaration, context: ContractDefinition, deref: ASTDereferencer, location: StorageLocation | null = varDecl.storageLocation): string { if (!varDecl.typeName) { throw new Error('Missing type information'); } - return getType(varDecl.typeName, location); + return getType(varDecl.typeName, context, deref, location); } -function getType(typeName: TypeName, location: StorageLocation | null): string { +function getType(typeName: TypeName, context: ContractDefinition, deref: ASTDereferencer, location: StorageLocation | null): string { const { typeString, typeIdentifier } = typeName.typeDescriptions; if (typeof typeString !== 'string' || typeof typeIdentifier !== 'string') { throw new Error('Missing type information'); } - const type = typeString.replace(/^(struct|enum|contract) /, '') + (typeIdentifier.endsWith('_ptr') && location ? ` ${location}` : ''); + + let type = typeString.replace(/^(struct|enum|contract) /, '') + (typeIdentifier.endsWith('_ptr') && location ? ` ${location}` : ''); + + const typeScopeMatch = type.match(/^([a-zA-Z0-9_$]+)\./); + if (typeScopeMatch) { + const [, typeScope] = typeScopeMatch; + + const isScopeImplicit = context.linearizedBaseContracts.some(c => deref('ContractDefinition', c).name === typeScope); + + if (isScopeImplicit) { + type = type.replace(`${typeScope}.`, ''); + } + } + return type; } @@ -458,18 +472,18 @@ function getVariables(contract: ContractDefinition, deref: ASTDereferencer, subs return res; } -function getVarGetterArgs(v: VariableDeclaration): Argument[] { +function getVarGetterArgs(v: VariableDeclaration, context: ContractDefinition, deref: ASTDereferencer): Argument[] { if (!v.typeName) { throw new Error('missing typenName'); } const types = []; for (let t = v.typeName; t.nodeType === 'Mapping'; t = t.valueType) { - types.push({ name: `arg${types.length}`, type: getType(t.keyType, 'memory') }) + types.push({ name: `arg${types.length}`, type: getType(t.keyType, context, deref, 'memory') }) } return types; } -function getVarGetterReturnType(v: VariableDeclaration): string { +function getVarGetterReturnType(v: VariableDeclaration, context: ContractDefinition, deref: ASTDereferencer): string { if (!v.typeName) { throw new Error('missing typenName'); } @@ -477,7 +491,7 @@ function getVarGetterReturnType(v: VariableDeclaration): string { while (t.nodeType === 'Mapping') { t = t.valueType; } - return getType(t, 'memory'); + return getType(t, context, deref, 'memory'); } function getFunctions(contract: ContractDefinition, deref: ASTDereferencer, subset?: Visibility[]): FunctionDefinition[] {