From e74ee2e15146d5fb1fd266ec352c8392f4301cee Mon Sep 17 00:00:00 2001 From: Klaus Date: Tue, 5 Dec 2023 19:58:54 -0300 Subject: [PATCH] Removing forced separation of functions and contracts --- src/common/backward-compatibility.js | 12 ---- src/common/printer-helpers.js | 59 +++---------------- .../__snapshots__/jsfmt.spec.js.snap | 1 - .../__snapshots__/jsfmt.spec.js.snap | 24 -------- tests/format/Comments/Comments.sol | 20 +++++++ .../Comments/__snapshots__/jsfmt.spec.js.snap | 32 ++++++++++ .../__snapshots__/jsfmt.spec.js.snap | 2 - .../__snapshots__/jsfmt.spec.js.snap | 36 ----------- .../Inbox/__snapshots__/jsfmt.spec.js.snap | 1 - .../__snapshots__/jsfmt.spec.js.snap | 1 - .../Issues/__snapshots__/jsfmt.spec.js.snap | 1 - .../__snapshots__/jsfmt.spec.js.snap | 2 - .../__snapshots__/jsfmt.spec.js.snap | 2 - .../TryCatch/__snapshots__/jsfmt.spec.js.snap | 1 - .../Tupples/__snapshots__/jsfmt.spec.js.snap | 2 - tests/unit/comments/printer.test.js | 8 ++- 16 files changed, 67 insertions(+), 137 deletions(-) diff --git a/src/common/backward-compatibility.js b/src/common/backward-compatibility.js index a1349b31f..a8ae3d512 100644 --- a/src/common/backward-compatibility.js +++ b/src/common/backward-compatibility.js @@ -27,20 +27,8 @@ export function getNextNonSpaceNonCommentCharacter(text, node, locEnd) { : util.getNextNonSpaceNonCommentCharacter(text, locEnd(node)); // V3 exposes this function directly } -export function isFirst(path, _, index) { - return isPrettier2 ? index === 0 : path.isFirst; -} - export function isLast(path, key, index) { return isPrettier2 ? index === path.getParentNode()[key].length - 1 : path.isLast; } - -export function previous(path, key, index) { - return isPrettier2 ? path.getParentNode()[key][index - 1] : path.previous; -} - -export function next(path, key, index) { - return isPrettier2 ? path.getParentNode()[key][index + 1] : path.next; -} diff --git a/src/common/printer-helpers.js b/src/common/printer-helpers.js index d63128c67..1401e1aac 100644 --- a/src/common/printer-helpers.js +++ b/src/common/printer-helpers.js @@ -1,11 +1,8 @@ import { doc } from 'prettier'; import { - isFirst, isLast, isNextLineEmpty, - isPrettier2, - next, - previous + isPrettier2 } from './backward-compatibility.js'; const { group, indent, join, line, softline, hardline } = doc.builders; @@ -24,7 +21,7 @@ export const printComments = (node, path, options, filter = () => true) => { return null; } comment.printed = true; - return options.printer.printComment(commentPath); + return options.printer.printComment(commentPath, options); }, 'comments') .filter(Boolean) ); @@ -39,22 +36,6 @@ export const printComments = (node, path, options, filter = () => true) => { /* c8 ignore stop */ }; -const shouldHaveEmptyLine = (node, checkForLeading) => - Boolean( - // if node is not FunctionDefinition, it should have an empty line - node.type !== 'FunctionDefinition' || - // if FunctionDefinition is not abstract, it should have an empty line - node.body || - // if FunctionDefinition has the comment we are looking for (trailing or - // leading), it should have an empty line - node.comments?.some((comment) => checkForLeading && comment.leading) - ); - -const separatingLine = (firstNode, secondNode) => - shouldHaveEmptyLine(firstNode, false) || shouldHaveEmptyLine(secondNode, true) - ? hardline - : ''; - export function printPreservingEmptyLines(path, key, options, print) { const parts = []; path.each((childPath, index) => { @@ -71,38 +52,16 @@ export function printPreservingEmptyLines(path, key, options, print) { parts.push(hardline); } - // Only attempt to prepend an empty line if `node` is not the first item - // and an empty line hasn't already been appended after the previous `node` - if ( - !isFirst(childPath, key, index) && - parts[parts.length - 2] !== hardline - ) { - if (nodeType === 'FunctionDefinition') { - // Prepend FunctionDefinition with an empty line if there should be a - // separation with the previous `node` - parts.push(separatingLine(previous(childPath, key, index), node)); - } else if (nodeType === 'ContractDefinition') { - // Prepend ContractDefinition with an empty line - parts.push(hardline); - } - } - parts.push(print(childPath)); // Only attempt to append an empty line if `node` is not the last item - if (!isLast(childPath, key, index)) { - if (isNextLineEmpty(options.originalText, options.locEnd(node) + 1)) { - // Append an empty line if the original text already had an one after - // the current `node` - parts.push(hardline); - } else if (nodeType === 'FunctionDefinition') { - // Append FunctionDefinition with an empty line if there should be a - // separation with the next `node` - parts.push(separatingLine(node, next(childPath, key, index))); - } else if (nodeType === 'ContractDefinition') { - // Append ContractDefinition with an empty line - parts.push(hardline); - } + if ( + !isLast(childPath, key, index) && + isNextLineEmpty(options.originalText, options.locEnd(node) + 1) + ) { + // Append an empty line if the original text already had an one after + // the current `node` + parts.push(hardline); } }, key); diff --git a/tests/format/AddressPayable/__snapshots__/jsfmt.spec.js.snap b/tests/format/AddressPayable/__snapshots__/jsfmt.spec.js.snap index 93a398341..df021991a 100644 --- a/tests/format/AddressPayable/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/AddressPayable/__snapshots__/jsfmt.spec.js.snap @@ -23,7 +23,6 @@ pragma solidity ^0.5.2; contract AddressPayable { using Address for address payable; address payable[] hello; - function sendSomeEth( address payable to, address payable[] memory world diff --git a/tests/format/AllSolidityFeatures/__snapshots__/jsfmt.spec.js.snap b/tests/format/AllSolidityFeatures/__snapshots__/jsfmt.spec.js.snap index 1be3c95bd..5955de940 100644 --- a/tests/format/AllSolidityFeatures/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/AllSolidityFeatures/__snapshots__/jsfmt.spec.js.snap @@ -568,7 +568,6 @@ import {symbol1 as alias, symbol2} from "File.sol"; interface i { event ForeignEvent(); - function f(); } @@ -582,7 +581,6 @@ contract c { val3 = 1 finney; // 1 * 10 ** 15 val4 = 1 ether; // 1 * 10 ** 18 } - uint256 val1; uint256 val2; uint256 val3; @@ -600,11 +598,9 @@ contract test { function test() { choices = ActionChoices.GoStraight; } - function getChoice() returns (uint d) { d = uint256(choices); } - ActionChoices choices; } @@ -612,10 +608,8 @@ contract Base { function Base(uint i) { m_i = i; } - uint public m_i; } - contract Derived is Base(0) { function Derived(uint i) Base(i) {} } @@ -653,20 +647,16 @@ contract c { function() returns (uint) { return g(8); } - function g(uint pos) internal returns (uint) { setData(pos, 8); return getData(pos); } - function setData(uint pos, uint value) internal { data[pos] = value; } - function getData(uint pos) internal { return data[pos]; } - mapping(uint => uint) data; mapping(uint id => uint) data; mapping(address owner => mapping(address spender => uint amount)) allowances; @@ -689,7 +679,6 @@ library IntegerSet { /// Number of stored items. uint size; } - function insert( data storage self, uint value @@ -705,7 +694,6 @@ library IntegerSet { return false; } } - function remove(data storage self, uint value) returns (bool success) { uint index = self.index[value]; if (index == 0) return false; @@ -713,19 +701,15 @@ library IntegerSet { delete self.items[index]; self.size--; } - function contains(data storage self, uint value) returns (bool) { return self.index[value] > 0; } - function iterate_start(data storage self) returns (uint index) { return iterate_advance(self, 0); } - function iterate_valid(data storage self, uint index) returns (bool) { return index < self.items.length; } - function iterate_advance( data storage self, uint index @@ -736,7 +720,6 @@ library IntegerSet { ) index++; return index; } - function iterate_get(data storage self, uint index) returns (uint value) { return self.items[index]; } @@ -746,7 +729,6 @@ library IntegerSet { contract User { /// Just a struct holding our data. IntegerSet.data data; - /// Insert something function insert(uint v) returns (uint size) { /// Sends \`data\` via reference, so IntegerSet can modify it. @@ -754,7 +736,6 @@ contract User { /// We can access members of the struct - but we should take care not to mess with them. return data.size; } - /// Computes the sum of all stored data. function sum() returns (uint s) { for ( @@ -974,7 +955,6 @@ contract Ballot { owner(myPrice) returns (uint[], address myAdd, string[] names) {} - function foobar() payable owner(myPrice) @@ -1071,13 +1051,9 @@ contract Overrides { require(msg.sender == owner(), "Ownable: caller is not the owner"); _; } - function foo() public override {} - function bar() public override(Foo) {} - function baz() public override(Foo, Bar) {} - function long() public override( diff --git a/tests/format/Comments/Comments.sol b/tests/format/Comments/Comments.sol index ccc9340d3..9cc06b612 100644 --- a/tests/format/Comments/Comments.sol +++ b/tests/format/Comments/Comments.sol @@ -2,6 +2,14 @@ pragma solidity ^0.4.24; contract Comments1 { + /* solhint-disable var-name-mixedcase */ +IEIP712DomainSeparator private EIP712domainSeparator; +bytes32 private _CACHED_DOMAIN_SEPARATOR; + + +/* solhint-enable var-name-mixedcase */ + + function() { // solhint-disable-previous-line no-empty-blocks } @@ -39,6 +47,18 @@ contract Comments6 /*why the name `Comments6`*/ is Interface1/*why we used Inter } contract Comments7 { + + + // 1 comment before first function + + + + // 2 comment before first function + + + + // 3 comment before first function + function someFunction( uint a, // the first value uint b, // the second value diff --git a/tests/format/Comments/__snapshots__/jsfmt.spec.js.snap b/tests/format/Comments/__snapshots__/jsfmt.spec.js.snap index c3001fecd..8592e9fbb 100644 --- a/tests/format/Comments/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/Comments/__snapshots__/jsfmt.spec.js.snap @@ -10,6 +10,14 @@ pragma solidity ^0.4.24; contract Comments1 { + /* solhint-disable var-name-mixedcase */ +IEIP712DomainSeparator private EIP712domainSeparator; +bytes32 private _CACHED_DOMAIN_SEPARATOR; + + +/* solhint-enable var-name-mixedcase */ + + function() { // solhint-disable-previous-line no-empty-blocks } @@ -47,6 +55,18 @@ contract Comments6 /*why the name \`Comments6\`*/ is Interface1/*why we used Int } contract Comments7 { + + + // 1 comment before first function + + + + // 2 comment before first function + + + + // 3 comment before first function + function someFunction( uint a, // the first value uint b, // the second value @@ -149,6 +169,12 @@ contract Comments13 { pragma solidity ^0.4.24; contract Comments1 { + /* solhint-disable var-name-mixedcase */ + IEIP712DomainSeparator private EIP712domainSeparator; + bytes32 private _CACHED_DOMAIN_SEPARATOR; + + /* solhint-enable var-name-mixedcase */ + function() { // solhint-disable-previous-line no-empty-blocks } @@ -212,6 +238,12 @@ contract Comments4 is } contract Comments7 { + // 1 comment before first function + + // 2 comment before first function + + // 3 comment before first function + function someFunction( uint a, // the first value uint b, // the second value diff --git a/tests/format/FunctionCalls/__snapshots__/jsfmt.spec.js.snap b/tests/format/FunctionCalls/__snapshots__/jsfmt.spec.js.snap index 5f024eaed..3a1ad79d8 100644 --- a/tests/format/FunctionCalls/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/FunctionCalls/__snapshots__/jsfmt.spec.js.snap @@ -62,7 +62,6 @@ contract FunctionCalls { Voter airbnb = Voter({ weight: 2, voted: true }); } - function verify( ForwardRequest calldata req, bytes calldata signature @@ -215,7 +214,6 @@ contract FunctionCalls { Voter airbnb = Voter({weight: 2, voted: true}); } - function verify( ForwardRequest calldata req, bytes calldata signature diff --git a/tests/format/FunctionDefinitions/__snapshots__/jsfmt.spec.js.snap b/tests/format/FunctionDefinitions/__snapshots__/jsfmt.spec.js.snap index e1537fe26..7dcdd2002 100644 --- a/tests/format/FunctionDefinitions/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/FunctionDefinitions/__snapshots__/jsfmt.spec.js.snap @@ -400,13 +400,10 @@ interface FunctionInterfaces { function functionDefinition0(); function functionDefinition1(); function functionDefinition2(); - // Leading Comment function functionDefinition3(); function functionDefinition4(); - function functionDefinition5() public {} - function functionDefinition6(); function functionDefinition7(); // Tailing Comment function functionDefinition8(); @@ -415,31 +412,22 @@ interface FunctionInterfaces { contract FunctionDefinitions { function() external {} - fallback() external {} - function() external payable {} - fallback() external payable {} - receive() external payable {} - function noParamsNoModifiersNoReturns() { a = 1; } - function oneParam(uint x) { a = 1; } - function oneModifier() modifier1 { a = 1; } - function oneReturn() returns (uint y1) { a = 1; } - function manyParams( uint x1, uint x2, @@ -454,7 +442,6 @@ contract FunctionDefinitions { ) { a = 1; } - function manyModifiers() modifier1 modifier2 @@ -469,7 +456,6 @@ contract FunctionDefinitions { { a = 1; } - function manyReturns() returns ( uint y1, @@ -486,7 +472,6 @@ contract FunctionDefinitions { { a = 1; } - function someParamsSomeModifiers( uint x1, uint x2, @@ -494,7 +479,6 @@ contract FunctionDefinitions { ) modifier1 modifier2 modifier3 { a = 1; } - function someParamsSomeReturns( uint x1, uint x2, @@ -502,7 +486,6 @@ contract FunctionDefinitions { ) returns (uint y1, uint y2, uint y3) { a = 1; } - function someModifiersSomeReturns() modifier1 modifier2 @@ -511,7 +494,6 @@ contract FunctionDefinitions { { a = 1; } - function someParamSomeModifiersSomeReturns( uint x1, uint x2, @@ -519,7 +501,6 @@ contract FunctionDefinitions { ) modifier1 modifier2 modifier3 returns (uint y1, uint y2, uint y3) { a = 1; } - function someParamsManyModifiers( uint x1, uint x2, @@ -538,7 +519,6 @@ contract FunctionDefinitions { { a = 1; } - function someParamsManyReturns( uint x1, uint x2, @@ -559,7 +539,6 @@ contract FunctionDefinitions { { a = 1; } - function manyParamsSomeModifiers( uint x1, uint x2, @@ -574,7 +553,6 @@ contract FunctionDefinitions { ) modifier1 modifier2 modifier3 { a = 1; } - function manyParamsSomeReturns( uint x1, uint x2, @@ -589,7 +567,6 @@ contract FunctionDefinitions { ) returns (uint y1, uint y2, uint y3) { a = 1; } - function manyParamsManyModifiers( uint x1, uint x2, @@ -616,7 +593,6 @@ contract FunctionDefinitions { { a = 1; } - function manyParamsManyReturns( uint x1, uint x2, @@ -644,7 +620,6 @@ contract FunctionDefinitions { { a = 1; } - function manyParamsManyModifiersManyReturns( uint x1, uint x2, @@ -682,7 +657,6 @@ contract FunctionDefinitions { { a = 1; } - function modifierOrderCorrect01() public view @@ -694,7 +668,6 @@ contract FunctionDefinitions { { a = 1; } - function modifierOrderCorrect02() private pure @@ -705,7 +678,6 @@ contract FunctionDefinitions { { a = 1; } - function modifierOrderCorrect03() external payable @@ -716,7 +688,6 @@ contract FunctionDefinitions { { a = 1; } - function modifierOrderCorrect04() internal virtual @@ -727,7 +698,6 @@ contract FunctionDefinitions { { a = 1; } - function modifierOrderIncorrect01() public view @@ -739,7 +709,6 @@ contract FunctionDefinitions { { a = 1; } - function modifierOrderIncorrect02() external virtual @@ -750,7 +719,6 @@ contract FunctionDefinitions { { a = 1; } - function modifierOrderIncorrect03() internal pure @@ -761,7 +729,6 @@ contract FunctionDefinitions { { a = 1; } - function modifierOrderIncorrect04() external payable @@ -772,11 +739,8 @@ contract FunctionDefinitions { { a = 1; } - fallback() external payable virtual {} - fallback(bytes calldata _input) external {} - receive() external payable virtual {} } diff --git a/tests/format/Inbox/__snapshots__/jsfmt.spec.js.snap b/tests/format/Inbox/__snapshots__/jsfmt.spec.js.snap index 00049880d..7ecbcae24 100644 --- a/tests/format/Inbox/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/Inbox/__snapshots__/jsfmt.spec.js.snap @@ -52,7 +52,6 @@ lines. */ pragma solidity ^0.4.23; - contract Inbox { string public message; diff --git a/tests/format/InheritanceSpecifier/__snapshots__/jsfmt.spec.js.snap b/tests/format/InheritanceSpecifier/__snapshots__/jsfmt.spec.js.snap index 060fd5015..c95f10839 100644 --- a/tests/format/InheritanceSpecifier/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/InheritanceSpecifier/__snapshots__/jsfmt.spec.js.snap @@ -11,7 +11,6 @@ contract LongInheritanceSpecifier is SomeOtherContract(123467890,false,0xCA35b7d =====================================output===================================== contract InheritanceSpecifier is SomeOtherContract(1234, false) {} - contract LongInheritanceSpecifier is SomeOtherContract( 123467890, diff --git a/tests/format/Issues/__snapshots__/jsfmt.spec.js.snap b/tests/format/Issues/__snapshots__/jsfmt.spec.js.snap index 4bb8283b5..145090c30 100644 --- a/tests/format/Issues/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/Issues/__snapshots__/jsfmt.spec.js.snap @@ -28,7 +28,6 @@ contract Example { } mapping(address => mapping(address => BalanceState)) private balanceStates; - function example(address token, uint amount) public { balanceStates[msg.sender][token].balance = balanceStates[msg.sender][ token diff --git a/tests/format/NameValueExpression/__snapshots__/jsfmt.spec.js.snap b/tests/format/NameValueExpression/__snapshots__/jsfmt.spec.js.snap index 821524aa1..7bb12c9a8 100644 --- a/tests/format/NameValueExpression/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/NameValueExpression/__snapshots__/jsfmt.spec.js.snap @@ -27,7 +27,6 @@ contract NameValueExpression { forSplits: true }(); } - D newD = new D{ value: amount }(arg); } @@ -60,7 +59,6 @@ contract NameValueExpression { forSplits: true }(); } - D newD = new D{value: amount}(arg); } diff --git a/tests/format/StyleGuide/__snapshots__/jsfmt.spec.js.snap b/tests/format/StyleGuide/__snapshots__/jsfmt.spec.js.snap index de6763df7..b84c1da48 100644 --- a/tests/format/StyleGuide/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/StyleGuide/__snapshots__/jsfmt.spec.js.snap @@ -30,7 +30,6 @@ contract A { pragma solidity >=0.4.0 <0.7.0; contract A {} - contract B {} contract C {} @@ -39,7 +38,6 @@ contract A { function spam() public pure { // ... } - function ham() public pure { // ... } diff --git a/tests/format/TryCatch/__snapshots__/jsfmt.spec.js.snap b/tests/format/TryCatch/__snapshots__/jsfmt.spec.js.snap index e331e35ef..72355b966 100644 --- a/tests/format/TryCatch/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/TryCatch/__snapshots__/jsfmt.spec.js.snap @@ -120,7 +120,6 @@ interface DataFeed { contract FeedConsumer { DataFeed feed; uint errorCount; - function rate(address token) public returns (uint value, bool success) { // Permanently disable the mechanism if there are // more than 10 errors. diff --git a/tests/format/Tupples/__snapshots__/jsfmt.spec.js.snap b/tests/format/Tupples/__snapshots__/jsfmt.spec.js.snap index 3e008a812..4ddda35fc 100644 --- a/tests/format/Tupples/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/Tupples/__snapshots__/jsfmt.spec.js.snap @@ -37,9 +37,7 @@ pragma solidity ^0.5.0; contract demo { function hello() public view returns (bool, bool) {} - function hello2() public view returns (bool) {} - function hello3() public view returns (bool, bool, bool) {} } diff --git a/tests/unit/comments/printer.test.js b/tests/unit/comments/printer.test.js index c28a03f15..b4a28d747 100644 --- a/tests/unit/comments/printer.test.js +++ b/tests/unit/comments/printer.test.js @@ -1,9 +1,13 @@ import { printComment } from '../../../src/comments/printer.js'; +import loc from '../../../src/loc.js'; test('given an unknown comment type then printComment function should throw', () => { - const mockCommentPath = { getValue: () => ({ type: 'UnknownComment' }) }; + const mockCommentPath = { + getValue: () => ({ type: 'UnknownComment', range: [0, 1] }) + }; + const mockOptions = { ...loc, originalText: 'foo' }; expect(() => { - printComment(mockCommentPath); + printComment(mockCommentPath, mockOptions); }).toThrow(); });