diff --git a/src/slang-nodes/ContractDefinition.ts b/src/slang-nodes/ContractDefinition.ts index 55e0c479f..e9366a68b 100644 --- a/src/slang-nodes/ContractDefinition.ts +++ b/src/slang-nodes/ContractDefinition.ts @@ -1,6 +1,6 @@ +import { NonterminalKind } from '@nomicfoundation/slang/cst'; import { doc } from 'prettier'; import { satisfies } from 'semver'; -import { NonterminalKind } from '@nomicfoundation/slang/cst'; import { SlangNode } from './SlangNode.js'; import { TerminalNode } from './TerminalNode.js'; import { ContractSpecifiers } from './ContractSpecifiers.js'; @@ -42,12 +42,10 @@ export class ContractDefinition extends SlangNode { this.updateMetadata(this.specifiers, this.members); - this.cleanModifierInvocationArguments(options); - } - - cleanModifierInvocationArguments(options: ParserOptions): void { // Older versions of Solidity defined a constructor as a function having // the same name as the contract. + // So we delegate to the parents the responsibility of cleaning the + // arguments of modifier invocations. if (!satisfies(options.compiler, '>=0.5.0')) { for (const member of this.members.items) { if ( diff --git a/src/slang-nodes/FallbackFunctionDefinition.ts b/src/slang-nodes/FallbackFunctionDefinition.ts index 537a16a8b..29abc5a25 100644 --- a/src/slang-nodes/FallbackFunctionDefinition.ts +++ b/src/slang-nodes/FallbackFunctionDefinition.ts @@ -52,15 +52,8 @@ export class FallbackFunctionDefinition extends SlangNode { this.body ); - this.cleanModifierInvocationArguments(); - } - - cleanModifierInvocationArguments(): void { for (const attribute of this.attributes.items) { - if ( - typeof attribute !== 'string' && - attribute.kind === NonterminalKind.ModifierInvocation - ) { + if (attribute.kind === NonterminalKind.ModifierInvocation) { attribute.cleanModifierInvocationArguments(); } } diff --git a/src/slang-nodes/FunctionDefinition.ts b/src/slang-nodes/FunctionDefinition.ts index d30e270bf..44c8cb547 100644 --- a/src/slang-nodes/FunctionDefinition.ts +++ b/src/slang-nodes/FunctionDefinition.ts @@ -1,5 +1,5 @@ -import { satisfies } from 'semver'; import { NonterminalKind } from '@nomicfoundation/slang/cst'; +import { satisfies } from 'semver'; import { printFunctionWithBody } from '../slang-printers/print-function.js'; import { extractVariant } from '../slang-utils/extract-variant.js'; import { SlangNode } from './SlangNode.js'; @@ -60,6 +60,8 @@ export class FunctionDefinition extends SlangNode { // Older versions of Solidity defined a constructor as a function having // the same name as the contract. + // So we delegate to the parents the responsibility of cleaning the + // arguments of modifier invocations. if (satisfies(options.compiler, '>=0.5.0')) { this.cleanModifierInvocationArguments(); } @@ -67,10 +69,7 @@ export class FunctionDefinition extends SlangNode { cleanModifierInvocationArguments(): void { for (const attribute of this.attributes.items) { - if ( - typeof attribute !== 'string' && - attribute.kind === NonterminalKind.ModifierInvocation - ) { + if (attribute.kind === NonterminalKind.ModifierInvocation) { attribute.cleanModifierInvocationArguments(); } } diff --git a/src/slang-nodes/ImportDeconstructionSymbols.ts b/src/slang-nodes/ImportDeconstructionSymbols.ts index a8de1f972..9ee7274b2 100644 --- a/src/slang-nodes/ImportDeconstructionSymbols.ts +++ b/src/slang-nodes/ImportDeconstructionSymbols.ts @@ -1,6 +1,6 @@ +import { NonterminalKind } from '@nomicfoundation/slang/cst'; import { doc } from 'prettier'; import { satisfies } from 'semver'; -import { NonterminalKind } from '@nomicfoundation/slang/cst'; import { printSeparatedList } from '../slang-printers/print-separated-list.js'; import { SlangNode } from './SlangNode.js'; import { ImportDeconstructionSymbol } from './ImportDeconstructionSymbol.js'; diff --git a/src/slang-nodes/LibraryDefinition.ts b/src/slang-nodes/LibraryDefinition.ts index d563d3fe6..56056c56f 100644 --- a/src/slang-nodes/LibraryDefinition.ts +++ b/src/slang-nodes/LibraryDefinition.ts @@ -1,5 +1,6 @@ import { NonterminalKind } from '@nomicfoundation/slang/cst'; import { doc } from 'prettier'; +import { satisfies } from 'semver'; import { SlangNode } from './SlangNode.js'; import { TerminalNode } from './TerminalNode.js'; import { LibraryMembers } from './LibraryMembers.js'; @@ -29,6 +30,18 @@ export class LibraryDefinition extends SlangNode { this.members = new LibraryMembers(ast.members, collected, options); this.updateMetadata(this.members); + + // Older versions of Solidity defined a constructor as a function having + // the same name as the contract. + // So we delegate to the parents the responsibility of cleaning the + // arguments of modifier invocations. + if (!satisfies(options.compiler, '>=0.5.0')) { + for (const member of this.members.items) { + if (member.kind === NonterminalKind.FunctionDefinition) { + member.cleanModifierInvocationArguments(); + } + } + } } print(path: AstPath, print: PrintFunction): Doc { diff --git a/src/slang-nodes/ReceiveFunctionDefinition.ts b/src/slang-nodes/ReceiveFunctionDefinition.ts index ddc72c6db..f0c064398 100644 --- a/src/slang-nodes/ReceiveFunctionDefinition.ts +++ b/src/slang-nodes/ReceiveFunctionDefinition.ts @@ -41,15 +41,8 @@ export class ReceiveFunctionDefinition extends SlangNode { this.updateMetadata(this.parameters, this.attributes, this.body); - this.cleanModifierInvocationArguments(); - } - - cleanModifierInvocationArguments(): void { for (const attribute of this.attributes.items) { - if ( - typeof attribute !== 'string' && - attribute.kind === NonterminalKind.ModifierInvocation - ) { + if (attribute.kind === NonterminalKind.ModifierInvocation) { attribute.cleanModifierInvocationArguments(); } } diff --git a/src/slang-nodes/UnnamedFunctionDefinition.ts b/src/slang-nodes/UnnamedFunctionDefinition.ts index 53d937854..65be02b5e 100644 --- a/src/slang-nodes/UnnamedFunctionDefinition.ts +++ b/src/slang-nodes/UnnamedFunctionDefinition.ts @@ -41,10 +41,6 @@ export class UnnamedFunctionDefinition extends SlangNode { this.updateMetadata(this.parameters, this.attributes, this.body); - this.cleanModifierInvocationArguments(); - } - - cleanModifierInvocationArguments(): void { for (const attribute of this.attributes.items) { if (attribute.kind === NonterminalKind.ModifierInvocation) { attribute.cleanModifierInvocationArguments(); diff --git a/tests/format/ModifierInvocations/ModifierInvocations.sol b/tests/format/ModifierInvocations/ModifierInvocations.sol index 2b446316e..0953a3891 100644 --- a/tests/format/ModifierInvocations/ModifierInvocations.sol +++ b/tests/format/ModifierInvocations/ModifierInvocations.sol @@ -15,3 +15,21 @@ contract ModifierInvocations is ModifierDefinitions { // We remove parentheses in modifiers without arguments. function test() public emptyParams noParams() {} } + +library ModifierInvocationsLibrary { + // We enforce the use of parentheses in modifiers without parameters. + modifier emptyParams {_;} + modifier noParams() {_;} + + modifier nonZero (uint x) { + require (x != 0); + _; + } + + function isPrime (uint x) public nonZero (x) returns (bool) { + // Complicated logic here + } + + // We remove parentheses in modifiers without arguments. + function test() public emptyParams noParams() {} +} diff --git a/tests/format/ModifierInvocations/__snapshots__/format.test.js.snap b/tests/format/ModifierInvocations/__snapshots__/format.test.js.snap index 51e69dcf6..b83141fd4 100644 --- a/tests/format/ModifierInvocations/__snapshots__/format.test.js.snap +++ b/tests/format/ModifierInvocations/__snapshots__/format.test.js.snap @@ -24,6 +24,24 @@ contract ModifierInvocations is ModifierDefinitions { function test() public emptyParams noParams() {} } +library ModifierInvocationsLibrary { + // We enforce the use of parentheses in modifiers without parameters. + modifier emptyParams {_;} + modifier noParams() {_;} + + modifier nonZero (uint x) { + require (x != 0); + _; + } + + function isPrime (uint x) public nonZero (x) returns (bool) { + // Complicated logic here + } + + // We remove parentheses in modifiers without arguments. + function test() public emptyParams noParams() {} +} + =====================================output===================================== // SPDX-License-Identifier: MIT pragma solidity 0.8.34; @@ -47,5 +65,27 @@ contract ModifierInvocations is ModifierDefinitions { function test() public emptyParams noParams {} } +library ModifierInvocationsLibrary { + // We enforce the use of parentheses in modifiers without parameters. + modifier emptyParams() { + _; + } + modifier noParams() { + _; + } + + modifier nonZero(uint x) { + require(x != 0); + _; + } + + function isPrime(uint x) public nonZero(x) returns (bool) { + // Complicated logic here + } + + // We remove parentheses in modifiers without arguments. + function test() public emptyParams noParams {} +} + ================================================================================ `; diff --git a/tests/format/ModifierInvocationsV0.4.26/ModifierInvocations.sol b/tests/format/ModifierInvocationsV0.4.26/ModifierInvocations.sol new file mode 100644 index 000000000..c7da85e2e --- /dev/null +++ b/tests/format/ModifierInvocationsV0.4.26/ModifierInvocations.sol @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.4.26; + +contract ModifierDefinitions { + // We enforce the use of parentheses in modifiers without parameters. + modifier emptyParams {_;} + modifier noParams() {_;} +} + +contract ModifierInvocations is ModifierDefinitions { + // We can't differentiate between constructor calls or modifiers, so we keep + // parentheses untouched in constructors. + function ModifierInvocations () emptyParams noParams() ModifierDefinitions() {} + + // We remove parentheses in modifiers without arguments. + function test() public emptyParams noParams() {} +} + +library ModifierInvocationsLibrary { + // We enforce the use of parentheses in modifiers without parameters. + modifier emptyParams {_;} + modifier noParams() {_;} + + modifier nonZero (uint x) { + require (x != 0); + _; + } + + function isPrime (uint x) public nonZero (x) returns (bool) { + // Complicated logic here + } + + // We remove parentheses in modifiers without arguments. + function test() public emptyParams noParams() {} +} diff --git a/tests/format/ModifierInvocationsV0.4.26/__snapshots__/format.test.js.snap b/tests/format/ModifierInvocationsV0.4.26/__snapshots__/format.test.js.snap new file mode 100644 index 000000000..0eec60d9b --- /dev/null +++ b/tests/format/ModifierInvocationsV0.4.26/__snapshots__/format.test.js.snap @@ -0,0 +1,96 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`ModifierInvocations.sol - {"compiler":"0.4.26"} format 1`] = ` +====================================options===================================== +compiler: "0.4.26" +parsers: ["slang"] +printWidth: 80 + | printWidth +=====================================input====================================== +// SPDX-License-Identifier: MIT +pragma solidity 0.4.26; + +contract ModifierDefinitions { + // We enforce the use of parentheses in modifiers without parameters. + modifier emptyParams {_;} + modifier noParams() {_;} +} + +contract ModifierInvocations is ModifierDefinitions { + // We can't differentiate between constructor calls or modifiers, so we keep + // parentheses untouched in constructors. + function ModifierInvocations () emptyParams noParams() ModifierDefinitions() {} + + // We remove parentheses in modifiers without arguments. + function test() public emptyParams noParams() {} +} + +library ModifierInvocationsLibrary { + // We enforce the use of parentheses in modifiers without parameters. + modifier emptyParams {_;} + modifier noParams() {_;} + + modifier nonZero (uint x) { + require (x != 0); + _; + } + + function isPrime (uint x) public nonZero (x) returns (bool) { + // Complicated logic here + } + + // We remove parentheses in modifiers without arguments. + function test() public emptyParams noParams() {} +} + +=====================================output===================================== +// SPDX-License-Identifier: MIT +pragma solidity 0.4.26; + +contract ModifierDefinitions { + // We enforce the use of parentheses in modifiers without parameters. + modifier emptyParams() { + _; + } + modifier noParams() { + _; + } +} + +contract ModifierInvocations is ModifierDefinitions { + // We can't differentiate between constructor calls or modifiers, so we keep + // parentheses untouched in constructors. + function ModifierInvocations() + emptyParams + noParams() + ModifierDefinitions() + {} + + // We remove parentheses in modifiers without arguments. + function test() public emptyParams noParams {} +} + +library ModifierInvocationsLibrary { + // We enforce the use of parentheses in modifiers without parameters. + modifier emptyParams() { + _; + } + modifier noParams() { + _; + } + + modifier nonZero(uint x) { + require(x != 0); + _; + } + + function isPrime(uint x) public nonZero(x) returns (bool) { + // Complicated logic here + } + + // We remove parentheses in modifiers without arguments. + function test() public emptyParams noParams {} +} + +================================================================================ +`; diff --git a/tests/format/ModifierInvocationsV0.4.26/format.test.js b/tests/format/ModifierInvocationsV0.4.26/format.test.js new file mode 100644 index 000000000..85c6f9a33 --- /dev/null +++ b/tests/format/ModifierInvocationsV0.4.26/format.test.js @@ -0,0 +1 @@ +runFormatTest(import.meta, ['slang'], { compiler: '0.4.26' });