duaraghav8 / ethlint Goto Github PK

Import statement blocks should be succeeded by a 2-line gap but not each import statement as the rules currently enforce.

For example

import "A.sol";
import "B.sol";
import "C.sol";


contract D { [...] }

generate the warnings:

FILE:   /D.sol
--------------------------------------------------------------------------------------------

[WARNING] At line: 1, column: 0 -> ImportStatement must be succeeded by a 2-line gap.

[WARNING] At line: 2, column: 0 -> ImportStatement must be succeeded by a 2-line gap.

[WARNING] At line: 3, column: 0 -> ImportStatement must be succeeded by a 2-line gap.

This is useful for integrating with text editors and IDEs, as it allows linting from the latest buffer contents instead of the data on disk.

It could be implemented with an option named --stdin. For example:

cat MyContract.sol | solium --stdin

I have a following file:

contract Test {
    function Test() {
        var (x, y) = doTest();
    }

    function doTest() returns (uint, uint) {
        return (1, 2);
    }
}

which causes an error:

An error occured while running the linter on contracts/test.sol:
Error: An error occured while parsing the source code:
SyntaxError: Expected ")" but "," found. Line: 3, Column: 15

I guess this error is internally in the solparse library, but I'm not sure.

Since Solidity 4 we are able to implement an empty payable fallback function

  function ()
  payable {}

Functionality defined in
http://solidity.readthedocs.io/en/develop/frequently-asked-questions.html?highlight=payable#what-is-the-deal-with-function-inside-solidity-contracts-how-can-a-function-not-have-a-name

This however throws a warning in solium
[WARNING] At line: x, column: y -> Use of empty block statement {}

start & and offset for *, /, % are same (which is wrong)
try:

solparse.parse ('1 / 8 % 5;').body [0].expression

Same problem with + and -. Problem exists in the way we've defined Operators in solparse & solidity-parser

This issue is directly related to #33

3rd party

• Truffle
• Grunt
• Gulp
• Embark
• Zeppelin
• Yeoman
• Webpack
• Github
• JetBrains IDEA plugin
• Atom
• Sublime
• Visual Studio

If you're working on any of these (even if they're already done), let us know. We'd love to give you a shout out on our Community Page

node.end, in some cases, doesn't include the semicolon, so sourceCodeUtils.getText (node) returns code that doesn't contain the last semicolon.

if (
    lorem &&
    ipsum > 100 &&
    dolor === 'hello'
) {
    foo ();
}

Above code is (wrongly) flagged by the brace-on-same-line rule. This style is perfectly fine and shouldn't be flagged because we may have conditions too big to fit in a single line.

what kind of a pathetic soul does one have to be to be incapable of thinking of edge cases before they start writing code :'(

duaraghav8/solparse#12

Need to start making PRs to SP to even it out with solparse and once AST construction is same, should update package.json to use SP instead of solparse

uint[] x is parsed, but space between uint & sq brackets throws "unexpected token" error
to fix in solparse

• operator-whitespace: currently doesn't support 0 space on either side (this is useful when we wish to denote precedence). For eg- it allows 4 * 3 + 19 but doesn't allow 4*3 + 19. Fix this.
• Column information is not accurate. See issue on solparse

for eg- ObjectExpression or StructDeclaration doesn't get linted for indentation:

struct Foo ({
    hello: 'world'
});

same for ArrayExpression that span over multiple lines:

uint[] ages = [
    19,
    20
];

same for CallExpression

foo ({
    'hello': 'world'
});

something like this shouldn't bypass the wrath of solium:

foo (
    "hello", "world", "mofo"
);

1 arg per line, if you're spreading your function call over multiple line. Need to code logic for this too

if (true)
    hello();

Also read #122

without block statement, indentation raises error saying no indent before "hello()" which shouldn't happen

indentation is a bitch o.O

When run solium:
An error occurRed while running the linter on...

This wasn't happening before v1.6 but now it is. The code below will give me a FunctionDeclaration must be succeeded by 1 blank line

/// @notice this function returns something important
/// @return description
function someFunction() constant returns(uint256) {
    return 1;
}

  /// @notice gets something else important
  function anotherFunction() constant returns(uint256)
  {
    return 2;
  }

I have no reason to think that names like MyToken2 are invalid according to the style guide.

We should expand the camel case regex to accept this and other variations.

- seems to have higher precedence than /.
Fix.
and also ensure other precedences are correctly followed

When I ran the following command:

$ solium --file BlindAuction.sol

The result is:

An error occurred while running the linter on BlindAuction.sol:
TypeError: Cannot read property '0' of undefined
    at EventEmitter.<anonymous> (/usr/local/lib/node_modules/solium/lib/rules/operator-whitespace.js:24:84)
    at emitOne (events.js:96:13)
    at EventEmitter.emit (events.js:188:7)
    at EventGenerator.enterNode (/usr/local/lib/node_modules/solium/lib/utils/node-event-generator.js:25:16)
    at Controller.enter (/usr/local/lib/node_modules/solium/lib/solium.js:101:24)
    at Controller.exec (/usr/local/lib/node_modules/solium/node_modules/sol-explore/lib/traverse.js:99:21)
    at Controller.traverse (/usr/local/lib/node_modules/solium/node_modules/sol-explore/lib/traverse.js:123:17)
    at /usr/local/lib/node_modules/solium/node_modules/sol-explore/lib/traverse.js:137:17
    at Array.forEach (native)
    at Controller.traverse (/usr/local/lib/node_modules/solium/node_modules/sol-explore/lib/traverse.js:133:22)

The source code of BlindAuction.sol is the same as that in the official tutorial here:

pragma solidity ^0.4.0;

contract BlindAuction {
    struct Bid {
        bytes32 blindedBid;
        uint deposit;
    }

    address public beneficiary;
    uint public auctionStart;
    uint public biddingEnd;
    uint public revealEnd;
    bool public ended;

    mapping(address => Bid[]) public bids;

    address public highestBidder;
    uint public highestBid;

    // Allowed withdrawals of previous bids
    mapping(address => uint) pendingReturns;

    event AuctionEnded(address winner, uint highestBid);

    /// Modifiers are a convenient way to validate inputs to
    /// functions. `onlyBefore` is applied to `bid` below:
    /// The new function body is the modifier's body where
    /// `_` is replaced by the old function body.
    modifier onlyBefore(uint _time) { if (now >= _time) throw; _; }
    modifier onlyAfter(uint _time) { if (now <= _time) throw; _; }

    function BlindAuction(
        uint _biddingTime,
        uint _revealTime,
        address _beneficiary
    ) {
        beneficiary = _beneficiary;
        auctionStart = now;
        biddingEnd = now + _biddingTime;
        revealEnd = biddingEnd + _revealTime;
    }

    /// Place a blinded bid with `_blindedBid` = keccak256(value,
    /// fake, secret).
    /// The sent ether is only refunded if the bid is correctly
    /// revealed in the revealing phase. The bid is valid if the
    /// ether sent together with the bid is at least "value" and
    /// "fake" is not true. Setting "fake" to true and sending
    /// not the exact amount are ways to hide the real bid but
    /// still make the required deposit. The same address can
    /// place multiple bids.
    function bid(bytes32 _blindedBid)
        payable
        onlyBefore(biddingEnd)
    {
        bids[msg.sender].push(Bid({
            blindedBid: _blindedBid,
            deposit: msg.value
        }));
    }

    /// Reveal your blinded bids. You will get a refund for all
    /// correctly blinded invalid bids and for all bids except for
    /// the totally highest.
    function reveal(
        uint[] _values,
        bool[] _fake,
        bytes32[] _secret
    )
        onlyAfter(biddingEnd)
        onlyBefore(revealEnd)
    {
        uint length = bids[msg.sender].length;
        if (
            _values.length != length ||
            _fake.length != length ||
            _secret.length != length
        ) {
            throw;
        }

        uint refund;
        for (uint i = 0; i < length; i++) {
            var bid = bids[msg.sender][i];
            var (value, fake, secret) =
                    (_values[i], _fake[i], _secret[i]);
            if (bid.blindedBid != keccak256(value, fake, secret)) {
                // Bid was not actually revealed.
                // Do not refund deposit.
                continue;
            }
            refund += bid.deposit;
            if (!fake && bid.deposit >= value) {
                if (placeBid(msg.sender, value))
                    refund -= value;
            }
            // Make it impossible for the sender to re-claim
            // the same deposit.
            bid.blindedBid = 0;
        }
        if (!msg.sender.send(refund))
            throw;
    }

    // This is an "internal" function which means that it
    // can only be called from the contract itself (or from
    // derived contracts).
    function placeBid(address bidder, uint value) internal
            returns (bool success)
    {
        if (value <= highestBid) {
            return false;
        }
        if (highestBidder != 0) {
            // Refund the previously highest bidder.
            pendingReturns[highestBidder] += highestBid;
        }
        highestBid = value;
        highestBidder = bidder;
        return true;
    }

    /// Withdraw a bid that was overbid.
    function withdraw() returns (bool) {
        var amount = pendingReturns[msg.sender];
        if (amount > 0) {
            // It is important to set this to zero because the recipient
            // can call this function again as part of the receiving call
            // before `send` returns (see the remark above about
            // conditions -> effects -> interaction).
            pendingReturns[msg.sender] = 0;

            if (!msg.sender.send(amount)){
                // No need to call throw here, just reset the amount owing
                pendingReturns[msg.sender] = amount;
                return false;
            }
        }
        return true;
    }

    /// End the auction and send the highest bid
    /// to the beneficiary.
    function auctionEnd()
        onlyAfter(revealEnd)
    {
        if (ended)
            throw;
        AuctionEnded(highestBidder, highestBid);
        ended = true;
        // We send all the money we have, because some
        // of the refunds might have failed.
        if (!beneficiary.send(this.balance))
            throw;
    }
}

I suspect this is the reason Solium reports a warning for a function named: throwIfIsEmptyBytes32

[WARNING] At line: 32, column: 2 -> 'throwIfIsEmptyBytes32' doesn't follow the mixedCase notation

contract Abc {
}

/*foo bar*/
contract Bcd {
}

This should pass without lint errors from the blank-lines rule because there's a 2-line gap between both contracts.
it should ignore the comment /*foo bar*/

bar ()

gives no error, but

foo.bar ()

elicits:
A call without arguments should have brackets without any whitespace between them, like 'functionName ()'.

(NOTE: this rule is not in production right now)

but x * (3**2) works. This is because x * 3**2 gives higher precedence to ** and the AST Node create has positions because of which the rule breaks.

The reason is because of the way AST is organized (MemberExpression).
Needs fixing.
Fix, then add tests

• Write Test for lib/cli.js
• Enable ignoring of specific lines in code file which are accompanied by comment //solium-ignore or something like that
• Lint rules should comply with official Solidity Style guide
• whitespace rule: add check for space before smicolon and comma in ImportStatement & UsingStatement
• add contract IUpgradable { function upgrade(address newAddress); } in accepted in lbrace tests
• For fuck's sake document & test rules as well
• Update the developer guide to list the newly added functions accessible through context
• raise issue on solidity-parser: ObjectExpression properties missing type field
• re-implement operator-whitespace rule
• Write tests for astUtils.getEndingColumn ()
• whitespace rule should lint FunctionDeclaration params the same way as CallExpression.
• If rule programmer sets column: 0, column info gets fucked because we're using condition like location.column || alternative, fix this
• When exceptions occur, send messages with e.stack instead of e (for stack info)
• Possibly introduce getEndingLine (node) in SourceCode object - return the line no. where node's code ends
• reporter displays file name even if that file has no lint errors
• Update unit test for astUtils.getColumn () once column information logic is added to solparse/solidity-parser
• Write Unit Tests (should.js maybe?)
• CLI should Solium.report () all caught exceptions instead of console.log()ing them. Solium.lint () should be called with noReset = true (so it doesn't wipe out the error messages) the first time its called
• Make column numbers in rule implementations more accurate (currently most of them simply point to the start of line i.e., are 0)
• Rule refinement: modifier names should follow mixedCase, not CamelCase. Make changes.
• add --sync option, so every time a new rule is added, user doesn't have to manually add this rule into .soliumrc.json
• SELF ACTUALIZATION: Use node.end for previous node in rules like blank-lines & indentation
• --dir option to select particular directory to lint files of
• Improve error reporting with ^ pointing at the starting character of rule violation
• Improve docs - add variable name in the @returns field & document the CLI engine
• Provide fix Interface to rule implementer so they can define a fix for the lint error
• Stop using console.log in the CLI engine. use a centralized Log object for printing purposes.
• Rules need to be more descriptive
• Re-design user configuration to enable easy plug-in of custom lint rules
• Hot loading --hot in CLI
• Docs - Usage & Developer Guide
• Provide ast-utils' getParent() to rule via context object
• Provide example of getSourceCode () functionality in sample rules
• SourceCode object - add getNextChars (), getNextChar (), getPrevChars (), getPrevChar ()
• Add option to ignore particular files while linting the project (this feature has now been removed)
• Shareable configs
• Style choices for user (eg- tabs vs. space)

The regex here is failing to detect the new line
https://github.com/duaraghav8/Solium/blob/master/lib/rules/lbrace.js#L418
When outputting the charsBeforeLBrace this is correctly a new line " "

Perhaps a possible alternative is to compare the declarationEndingLine and bodyStartingLine and check bodyStartingLine = declarationEndingLine + 1?

'nough said

contract Slap is Crap {...}

'Crap' is flagged as not following mixedCase, fix this

Error summary is not reset when running with --hot, so error count increases continually.

If/else with one-line body can omit braces according to the style guide, but I get the following error:

Expected '{' after if clause

with the code:

modifier positive(uint256 amount) {
    if (amount <= 0)
       throw;
    _
}

Contract names should be CamelCase, and that's what Solium applies for the contract definition, however it appears to not recognise Contract names used outside of the contract X syntax, as for the following

contract MyContract is IMyContract, IMyOtherContract {... }

throws a warning

 'IMyContract' doesn't follow the mixedCase notation
 'IMyOtherContract' doesn't follow the mixedCase notation

This is my wishlist for additional rules. Let me know if you are against any of them getting implemented in the core. I will work on some of them but anything else is up for grabs.

• Suggest send instead of dangerous call.value() (WIP @federicobond)
• Suggest avoiding block.timestamp as it can be manipulated by miners (only in functions) (WIP @federicobond)
• Ensure any function that uses msg.value has payable modifier (WIP @federicobond)
• Ensure fallback function has payable modifier (WIP @federicobond)
• Ensure send return value is checked.
• Extract magic number to constant
• Suggest using selfdestruct instead of deprecated suicide (WIP @federicobond)
• Suggest better storage packing (see Warning here)
• Find duplicate code (this one is hard, perhaps we can write a generic implementation and then integrate it into solium)

When a contract has a constructor, a warning informs the user, that it should follow the mixedCase convention:

contract Test {
    function Test() {
        /* my awesome constructor */
    }
}

and the warning:

[WARNING] At line: 2, column: 4 -> 'Test' doesn't follow the mixedCase notation

'function foo () { bar (); }'

AST Node:

{
  "type": "FunctionDeclaration",
  "name": "foo",
  "params": null,
  "modifiers": null,
  "body": {
    "type": "BlockStatement",
    "body": [
      {
        "type": "ExpressionStatement",
        "expression": {
          "type": "CallExpression",
          "callee": {
            "type": "Identifier",
            "name": "bar",
            "start": 18,
            "end": 21
          },
          "arguments": [],
          "start": 18,
          "end": 24
        },
        "start": 18,
        "end": 25
      }
    ],
    "start": 18,
    "end": 25
  },
  "is_abstract": false,
  "start": 0,
  "end": 27
}

node.body.start is 18, should've been 16. Same problem with node.body.end.
The braces '{' & '}' are not included in FunctionDeclaration's BlockStatement

Using 0x0 for an address type variable results in

String Literals must be quoted with double quotes only.

Code sample

function myFunction(address x)
  {
    if(x == 0x0) { throw; }
  }

x [0] = fooBar ().baz;

In the code above, .baz is a MemberExpression which causes node.right.start to be incorrect.
The AST for above code is:

{
  "type": "ExpressionStatement",
  "expression": {
    "type": "AssignmentExpression",
    "operator": "=",
    "left": {
      "type": "MemberExpression",
      "object": {
        "type": "Identifier",
        "name": "x",
        "start": 0,
        "end": 1
      },
      "property": {
        "type": "Literal",
        "value": 0,
        "start": 3,
        "end": 4
      },
      "computed": true,
      "start": 0,
      "end": 5
    },
    "right": {
      "type": "MemberExpression",
      "property": {
        "type": "Identifier",
        "name": "baz",
        "start": 18,
        "end": 21
      },
      "computed": false,
      "start": 17,
      "end": 21,
      "object": {
        "type": "CallExpression",
        "callee": {
          "type": "Identifier",
          "name": "fooBar",
          "start": 8,
          "end": 14
        },
        "arguments": [],
        "start": 8,
        "end": 17
      }
    },
    "start": 0,
    "end": 21
  },
  "start": 0,
  "end": 22
}

node.right is the MemberExpression instead of CallExpression (because of fooBar ()).

Having a

using LibraryContract **for** someDataType;

As described here http://solidity.readthedocs.io/en/latest/contracts.html?#using-for

causes a SyntaxError:

Error: An error occured while parsing the source code:
SyntaxError: Expected "!=", "!==", "%", "%=", "&", "&&", "&=", "*", "*=", "+", "++", "+=", ",", "-", "--", "-=", "/", "/*", "//", "/=", ";", "<", "<<", "<<=", "<=", "=", "==", "===", ">", ">=", ">>", ">>=", ">>>", ">>>=", "?", "^", "^=", "in", "instanceof", "|", "|=", "||", comment, end of line or whitespace but "f" found. Line: 25, Column: 21
An error occured while running the linter on /MyContract.sol:

remove rule operator-whitespace and merge its logic into whitespace

whitespace rule must comply with all tests written (currently not uploaded)

array-declarations rule's check for whitespace uint[ ] x must be shifted to whitespace

It should say something like:

No issues found.

Or:

2 errors, 3 warnings found.

This is probably a matter of opinion, but most tools use the word "watch", as in "watching files" for this functionality. Hot sounds more like code auto-reloading to me.

If you want to keep --hot for backward compatibility, we could configure it as an alias.

What do you think?

https://github.com/OpenZeppelin/zeppelin-solidity

Start integration once API is modified

The commandline displays simple text.
Maybe coloured text would be more helpful.

Red: Fatal Errors
Blue: Lint/Style errors

newNumber = (10 * 78 + 982 % 6**2);

says '=' must be succeeded by exactly a single space.

when we remove the opening bracket '(' (and the closing obvio), then its all fine.

It treats '(' as whitespace, modify rule!!

The style guide doesn't specify whether all uppercase acronyms are acceptable in contract names (e.g. IOUToken). Personally I find nothing wrong with it.

What do you think?

I get an [ERROR] At line: x, column: y -> Variable 'someAddress' is declared but never used. in the scenario below, in which the variable is used in contracts inheriting from the base but not in the base contract itself.

contract IMyContract {

  address public someAddress;

  function myFunction(address someVariable);
}

import 'IMyContract.sol';
contract MyContract is IMyContract {
  function someOtherFunction(address X)
  {
    someAddress = X;
  }
}

Custom rules should export an object with a verify key. This would make it easy to eventually expand the API so that custom rules provide their own automated fixing code and other features planned for core rules.

We could support passing a function for backwards compatibility but deprecate that usage slowly.

pragma solidity ^0.4.0
pragma solidity 0.4.0