Skip to content

Commit 2351319

Browse files
JantherJanther
authored
Reviewing how and when we call cleanModifierInvocationArguments (#1444)
1 parent 9fb29e3 commit 2351319

File tree

12 files changed

+213
-31
lines changed

12 files changed

+213
-31
lines changed

‎src/slang-nodes/ContractDefinition.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1+
import { NonterminalKind } from '@nomicfoundation/slang/cst';
12
import { doc } from 'prettier';
23
import { satisfies } from 'semver';
3-
import { NonterminalKind } from '@nomicfoundation/slang/cst';
44
import { SlangNode } from './SlangNode.js';
55
import { TerminalNode } from './TerminalNode.js';
66
import { ContractSpecifiers } from './ContractSpecifiers.js';
@@ -42,12 +42,10 @@ export class ContractDefinition extends SlangNode {
4242

4343
this.updateMetadata(this.specifiers, this.members);
4444

45-
this.cleanModifierInvocationArguments(options);
46-
}
47-
48-
cleanModifierInvocationArguments(options: ParserOptions<AstNode>): void {
4945
// Older versions of Solidity defined a constructor as a function having
5046
// the same name as the contract.
47+
// So we delegate to the parents the responsibility of cleaning the
48+
// arguments of modifier invocations.
5149
if (!satisfies(options.compiler, '>=0.5.0')) {
5250
for (const member of this.members.items) {
5351
if (

‎src/slang-nodes/FallbackFunctionDefinition.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,8 @@ export class FallbackFunctionDefinition extends SlangNode {
5252
this.body
5353
);
5454

55-
this.cleanModifierInvocationArguments();
56-
}
57-
58-
cleanModifierInvocationArguments(): void {
5955
for (const attribute of this.attributes.items) {
60-
if (
61-
typeof attribute !== 'string' &&
62-
attribute.kind === NonterminalKind.ModifierInvocation
63-
) {
56+
if (attribute.kind === NonterminalKind.ModifierInvocation) {
6457
attribute.cleanModifierInvocationArguments();
6558
}
6659
}

‎src/slang-nodes/FunctionDefinition.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { satisfies } from 'semver';
21
import { NonterminalKind } from '@nomicfoundation/slang/cst';
2+
import { satisfies } from 'semver';
33
import { printFunctionWithBody } from '../slang-printers/print-function.js';
44
import { extractVariant } from '../slang-utils/extract-variant.js';
55
import { SlangNode } from './SlangNode.js';
@@ -60,17 +60,16 @@ export class FunctionDefinition extends SlangNode {
6060

6161
// Older versions of Solidity defined a constructor as a function having
6262
// the same name as the contract.
63+
// So we delegate to the parents the responsibility of cleaning the
64+
// arguments of modifier invocations.
6365
if (satisfies(options.compiler, '>=0.5.0')) {
6466
this.cleanModifierInvocationArguments();
6567
}
6668
}
6769

6870
cleanModifierInvocationArguments(): void {
6971
for (const attribute of this.attributes.items) {
70-
if (
71-
typeof attribute !== 'string' &&
72-
attribute.kind === NonterminalKind.ModifierInvocation
73-
) {
72+
if (attribute.kind === NonterminalKind.ModifierInvocation) {
7473
attribute.cleanModifierInvocationArguments();
7574
}
7675
}

‎src/slang-nodes/ImportDeconstructionSymbols.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1+
import { NonterminalKind } from '@nomicfoundation/slang/cst';
12
import { doc } from 'prettier';
23
import { satisfies } from 'semver';
3-
import { NonterminalKind } from '@nomicfoundation/slang/cst';
44
import { printSeparatedList } from '../slang-printers/print-separated-list.js';
55
import { SlangNode } from './SlangNode.js';
66
import { ImportDeconstructionSymbol } from './ImportDeconstructionSymbol.js';

‎src/slang-nodes/LibraryDefinition.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { NonterminalKind } from '@nomicfoundation/slang/cst';
22
import { doc } from 'prettier';
3+
import { satisfies } from 'semver';
34
import { SlangNode } from './SlangNode.js';
45
import { TerminalNode } from './TerminalNode.js';
56
import { LibraryMembers } from './LibraryMembers.js';
@@ -29,6 +30,18 @@ export class LibraryDefinition extends SlangNode {
2930
this.members = new LibraryMembers(ast.members, collected, options);
3031

3132
this.updateMetadata(this.members);
33+
34+
// Older versions of Solidity defined a constructor as a function having
35+
// the same name as the contract.
36+
// So we delegate to the parents the responsibility of cleaning the
37+
// arguments of modifier invocations.
38+
if (!satisfies(options.compiler, '>=0.5.0')) {
39+
for (const member of this.members.items) {
40+
if (member.kind === NonterminalKind.FunctionDefinition) {
41+
member.cleanModifierInvocationArguments();
42+
}
43+
}
44+
}
3245
}
3346

3447
print(path: AstPath<LibraryDefinition>, print: PrintFunction): Doc {

‎src/slang-nodes/ReceiveFunctionDefinition.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,8 @@ export class ReceiveFunctionDefinition extends SlangNode {
4141

4242
this.updateMetadata(this.parameters, this.attributes, this.body);
4343

44-
this.cleanModifierInvocationArguments();
45-
}
46-
47-
cleanModifierInvocationArguments(): void {
4844
for (const attribute of this.attributes.items) {
49-
if (
50-
typeof attribute !== 'string' &&
51-
attribute.kind === NonterminalKind.ModifierInvocation
52-
) {
45+
if (attribute.kind === NonterminalKind.ModifierInvocation) {
5346
attribute.cleanModifierInvocationArguments();
5447
}
5548
}

‎src/slang-nodes/UnnamedFunctionDefinition.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,6 @@ export class UnnamedFunctionDefinition extends SlangNode {
4141

4242
this.updateMetadata(this.parameters, this.attributes, this.body);
4343

44-
this.cleanModifierInvocationArguments();
45-
}
46-
47-
cleanModifierInvocationArguments(): void {
4844
for (const attribute of this.attributes.items) {
4945
if (attribute.kind === NonterminalKind.ModifierInvocation) {
5046
attribute.cleanModifierInvocationArguments();

‎tests/format/ModifierInvocations/ModifierInvocations.sol‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,21 @@ contract ModifierInvocations is ModifierDefinitions {
1515
// We remove parentheses in modifiers without arguments.
1616
function test() public emptyParams noParams() {}
1717
}
18+
19+
library ModifierInvocationsLibrary {
20+
// We enforce the use of parentheses in modifiers without parameters.
21+
modifier emptyParams {_;}
22+
modifier noParams() {_;}
23+
24+
modifier nonZero (uint x) {
25+
require (x != 0);
26+
_;
27+
}
28+
29+
function isPrime (uint x) public nonZero (x) returns (bool) {
30+
// Complicated logic here
31+
}
32+
33+
// We remove parentheses in modifiers without arguments.
34+
function test() public emptyParams noParams() {}
35+
}

‎tests/format/ModifierInvocations/__snapshots__/format.test.js.snap‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,24 @@ contract ModifierInvocations is ModifierDefinitions {
2424
function test() public emptyParams noParams() {}
2525
}
2626
27+
library ModifierInvocationsLibrary {
28+
// We enforce the use of parentheses in modifiers without parameters.
29+
modifier emptyParams {_;}
30+
modifier noParams() {_;}
31+
32+
modifier nonZero (uint x) {
33+
require (x != 0);
34+
_;
35+
}
36+
37+
function isPrime (uint x) public nonZero (x) returns (bool) {
38+
// Complicated logic here
39+
}
40+
41+
// We remove parentheses in modifiers without arguments.
42+
function test() public emptyParams noParams() {}
43+
}
44+
2745
=====================================output=====================================
2846
// SPDX-License-Identifier: MIT
2947
pragma solidity 0.8.34;
@@ -47,5 +65,27 @@ contract ModifierInvocations is ModifierDefinitions {
4765
function test() public emptyParams noParams {}
4866
}
4967
68+
library ModifierInvocationsLibrary {
69+
// We enforce the use of parentheses in modifiers without parameters.
70+
modifier emptyParams() {
71+
_;
72+
}
73+
modifier noParams() {
74+
_;
75+
}
76+
77+
modifier nonZero(uint x) {
78+
require(x != 0);
79+
_;
80+
}
81+
82+
function isPrime(uint x) public nonZero(x) returns (bool) {
83+
// Complicated logic here
84+
}
85+
86+
// We remove parentheses in modifiers without arguments.
87+
function test() public emptyParams noParams {}
88+
}
89+
5090
================================================================================
5191
`;

‎tests/format/ModifierInvocationsV0.4.26/ModifierInvocations.sol‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity 0.4.26;
3+
4+
contract ModifierDefinitions {
5+
// We enforce the use of parentheses in modifiers without parameters.
6+
modifier emptyParams {_;}
7+
modifier noParams() {_;}
8+
}
9+
10+
contract ModifierInvocations is ModifierDefinitions {
11+
// We can't differentiate between constructor calls or modifiers, so we keep
12+
// parentheses untouched in constructors.
13+
function ModifierInvocations () emptyParams noParams() ModifierDefinitions() {}
14+
15+
// We remove parentheses in modifiers without arguments.
16+
function test() public emptyParams noParams() {}
17+
}
18+
19+
library ModifierInvocationsLibrary {
20+
// We enforce the use of parentheses in modifiers without parameters.
21+
modifier emptyParams {_;}
22+
modifier noParams() {_;}
23+
24+
modifier nonZero (uint x) {
25+
require (x != 0);
26+
_;
27+
}
28+
29+
function isPrime (uint x) public nonZero (x) returns (bool) {
30+
// Complicated logic here
31+
}
32+
33+
// We remove parentheses in modifiers without arguments.
34+
function test() public emptyParams noParams() {}
35+
}

0 commit comments

Comments
 (0)