Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing forced separation of functions and contracts #960

Merged
merged 1 commit into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions src/common/backward-compatibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
59 changes: 9 additions & 50 deletions src/common/printer-helpers.js
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)
);
Expand All @@ -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) => {
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 0 additions & 24 deletions tests/format/AllSolidityFeatures/__snapshots__/jsfmt.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,6 @@ import {symbol1 as alias, symbol2} from "File.sol";

interface i {
event ForeignEvent();

function f();
}

Expand All @@ -582,7 +581,6 @@ contract c {
val3 = 1 finney; // 1 * 10 ** 15
val4 = 1 ether; // 1 * 10 ** 18
}

uint256 val1;
uint256 val2;
uint256 val3;
Expand All @@ -600,22 +598,18 @@ contract test {
function test() {
choices = ActionChoices.GoStraight;
}

function getChoice() returns (uint d) {
d = uint256(choices);
}

ActionChoices choices;
}

contract Base {
function Base(uint i) {
m_i = i;
}

uint public m_i;
}

contract Derived is Base(0) {
function Derived(uint i) Base(i) {}
}
Expand Down Expand Up @@ -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;
Expand All @@ -689,7 +679,6 @@ library IntegerSet {
/// Number of stored items.
uint size;
}

function insert(
data storage self,
uint value
Expand All @@ -705,27 +694,22 @@ library IntegerSet {
return false;
}
}

function remove(data storage self, uint value) returns (bool success) {
uint index = self.index[value];
if (index == 0) return false;
delete self.index[value];
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
Expand All @@ -736,7 +720,6 @@ library IntegerSet {
) index++;
return index;
}

function iterate_get(data storage self, uint index) returns (uint value) {
return self.items[index];
}
Expand All @@ -746,15 +729,13 @@ 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.
IntegerSet.insert(data, v);
/// 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 (
Expand Down Expand Up @@ -974,7 +955,6 @@ contract Ballot {
owner(myPrice)
returns (uint[], address myAdd, string[] names)
{}

function foobar()
payable
owner(myPrice)
Expand Down Expand Up @@ -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(
Expand Down
20 changes: 20 additions & 0 deletions tests/format/Comments/Comments.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions tests/format/Comments/__snapshots__/jsfmt.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions tests/format/FunctionCalls/__snapshots__/jsfmt.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ contract FunctionCalls {

Voter airbnb = Voter({ weight: 2, voted: true });
}

function verify(
ForwardRequest calldata req,
bytes calldata signature
Expand Down Expand Up @@ -215,7 +214,6 @@ contract FunctionCalls {

Voter airbnb = Voter({weight: 2, voted: true});
}

function verify(
ForwardRequest calldata req,
bytes calldata signature
Expand Down
Loading