no-empty-blocks false positives in inherited constructor

[quezak]

Description
In an inherited contract, the constructor often just calls the base constructor, and its block is empty. This should be an exception from the no-empty-block rule, but now it causes a warning Code contains empty block.

Steps to reproduce

• The command you used to lint: npx solium --file test.sol

• The solidity code over which you ran the linter
  Copied from the official example.

contract Base {
    uint x;
    constructor(uint _x) public { x = _x; }
}

// or through a "modifier" of the derived constructor.
contract Derived2 is Base {
    constructor(uint _y) Base(_y * _y) public {}
}

• Contents of your .soliumrc.json file

{
  "extends": "solium:recommended",
  "plugins": [
    "security"
  ]
}

• Contents of your .soliumignore: node_modules

Expected behavior
The example constructor should not cause linter warnings.

Operating System
Linux

Linter version
1.2.4

[duaraghav8]

Thanks for reporting, I'll be investigating this

[quezak]

@duaraghav8 I just discovered another false-positive case: empty contract body in an inherited contract.

An example (simplified from my real-world use case): I have an abstract contract inheriting an abstract mixin. I then make a concrete contract just by inheriting a concrete mixin contract. The contract body is empty, but still required, while solium complains that warning Code contains empty block.

Example code:

contract AbstractMixin {
    function mixinFn() internal pure returns (uint256);
}

contract AbstractTest is AbstractMixin {
    function test() external pure returns (uint256) { return mixinFn(); }
}

contract Mixin1 {
    function mixinFn() internal pure returns (uint256) { return 1; }
}

contract Test1 is AbstractTest, Mixin1 {}

[duaraghav8]

Agreed, I'll be adding inheritance as an exception to this rule.

[duaraghav8]

Behaviour of this rule for constructors after fix:

If a base contract is called Foo, no-empty-block will ignore the constructor function of any contract which is Foo provided that the constructor also has a modifier named Foo.

eg-

contract Foo {}

contract Bar is Foo {
  // This cons. will not be flagged
  constructor() Foo() {}
}

contract Lorem {
  // This cons. WILL be flagged
  constructor() public {}
}

contract Ipsum is Foo {
  // This cons. WILL be flagged
  constructor() public {}
}

contract Derived1 is Base(7) {
  // This cons. WILL be flagged
    constructor() public {}
}

As for the case of contract, I'm still not sure I fully understand the thing, I'll have to read up on mixins, will update here then.

[duaraghav8]

The issue you originally filed has been fixed in v1.2.5.
The subsequent issue you filed has been moved to and will be tracked at #273
Thanks for reporting this!

The latest release is available on both solium & ethlint NPM packages, but I highly recommend you to switch to downloading from ethlint if you're currently not doing so.

[quezak]

Thanks!

[emilbayes]

This also shows up for contracts with an internal constructor, which are "abstract" contracts according to the solidity documentation

[duaraghav8]

@emilbayes can you point me to the docs on internal?

[emilbayes]

@duaraghav8 https://solidity.readthedocs.io/en/v0.5.12/contracts.html#constructor

A constructor set as internal causes the contract to be marked as abstract.

Edit: wrong link