Skip to content

Commit

Permalink
fixed no-empty-blocks to not report inherited constructors
Browse files Browse the repository at this point in the history
  • Loading branch information
duaraghav8 committed Sep 14, 2019
1 parent 95726f6 commit d929b9e
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 16 deletions.
80 changes: 70 additions & 10 deletions lib/rules/no-empty-blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,17 @@ module.exports = {
return isFunc(node) && node.name === null;
}

function isPayableFuncOrCons(node) {
function isPayable(n) {
for (let m of (n.modifiers || [])) {
if (m.name === "payable") { return true; }
}
return false;
function isPayable(funcNode) {
for (let m of (funcNode.modifiers || [])) {
if (m.name === "payable") { return true; }
}
return false;
}

function isPayableFuncOrCons(node) {
return isFuncOrConstructor(node) && isPayable(node);
}

const similarNodeTypes = ["ContractStatement", "LibraryStatement", "InterfaceStatement"];

function inspectCLI(emitted) {
let node = emitted.node,
sourceCode = context.getSourceCode(), text = sourceCode.getText(node);
Expand All @@ -77,13 +75,75 @@ module.exports = {
}

if (Array.isArray(body) && body.length === 0) {
if (isConstructor(parent)) {
// If an empty block is a constructor func's body, then it shouldn't
// be reported if the constructor is calling a base constructor.
// The cons. must have a modifier with a name that is also present
// in its enclosing contract's inheritance declaration. Eg-
//
// contract Foo is Bar {
// constructor() Bar("hello") {}
// }
//
// See issue #264
//
// Since we can't fetch enclosing contract from the BlockStatement
// node or its parent (due to current limitations), we use a
// workaround to not report such a constructor.
constructorsToCheckForBaseCall.push(parent.start);
return;
}

report(node);
}
}

function inspectConstructorDeclaration(emitted) {
const { node } = emitted;

if (!emitted.exit) {
// Because parent of a node is not accessible during exit phase,
// cache the parents of all constructors during entry so they
// can be used during exit.
enclosingContractsOfConstructors[node.start] = context.getSourceCode().getParent(node);
return;
}

// No need to check the constructor currently being exited if it
// isn't even flagged for empty block.
if (!constructorsToCheckForBaseCall.includes(node.start)) {
return;
}

// Run constructor inspection while exiting nodes.
// By this time, the constructorsToCheckForBaseCall list has been
// populated.
const enclosingContract = enclosingContractsOfConstructors[node.start];

// If node.modifiers is null, it means no modifiers exist for this
// constructor and it should therefore be reported.
for (let i = 0; i < (node.modifiers || []).length; i++) {
const functionModif = node.modifiers[i].name;

for (let j = 0; j < enclosingContract.is.length; j++) {
// The constructor is calling a base cons, no need
// to report it.
if (enclosingContract.is[j].name === functionModif) {
return;
}
}
}

report(node.body);
}


let constructorsToCheckForBaseCall = [], enclosingContractsOfConstructors = {};
const similarNodeTypes = ["ContractStatement", "LibraryStatement", "InterfaceStatement"];

let response = {
BlockStatement: inspectBlockStatement
const response = {
BlockStatement: inspectBlockStatement,
ConstructorDeclaration: inspectConstructorDeclaration
};

similarNodeTypes.forEach(function(nodeName) {
Expand Down
36 changes: 30 additions & 6 deletions test/lib/rules/no-empty-blocks/no-empty-blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@

"use strict";

let Solium = require("../../../../lib/solium");
let wrappers = require("../../../utils/wrappers");
let toContract = wrappers.toContract;
let toFunction = wrappers.toFunction;
let addPragma = wrappers.addPragma;
const Solium = require("../../../../lib/solium");
const wrappers = require("../../../utils/wrappers");
const toContract = wrappers.toContract;
const toFunction = wrappers.toFunction;
const addPragma = wrappers.addPragma;

let userConfig = {
"custom-rules-filename": null,
Expand Down Expand Up @@ -54,7 +54,7 @@ describe("[RULE] no-empty-blocks: Acceptances", function() {
done();
});

it("should allow fallback and payable functions & payable constructors", done => {
it("should allow fallback and payable functions & payable constructors to have empty bodies", done => {
let snippets = [
"function(string address) {}",
"function foo(string address) payable external {}",
Expand All @@ -76,6 +76,30 @@ describe("[RULE] no-empty-blocks: Acceptances", function() {
done();
});

it("should allow constructors calling base constructors to have empty bodies", done => {
const code = `
contract Foo is Bar {
constructor(uint _y) Bar(_y * _y) public {}
}
contract Jax is Base(10) {
constructor() public blah Base foo bar(100) {}
}
// This should be accepted because payable constructor
contract Ipsum is Foo {
constructor() payable public {}
}
`;
const errors = Solium.lint(code, userConfig);

errors.should.be.Array();
errors.should.be.empty();

Solium.reset();
done();
});

it("should accept all non-empty if-else declarations", function(done) {
let code = "if (true) { foo (); } else { bar (); }",
errors = Solium.lint(toFunction(code), userConfig);
Expand Down

0 comments on commit d929b9e

Please sign in to comment.