zeppelinsolutions / solium-plugin-zeppelin Goto Github PK

In missing-natspec-comments functions are added to the list of nodes to be evaluated and it checks that they have a title.

However, according to the format specification @title applies only to contract and interface, not function.

In fact, adding a @title to a function natspec will result in solc reporting:

DocstringParsingError: Doc tag @title not valid for functions.

The rule no-state-variable-shadowing currently only analyzes the contracts of the same file. It should also analyze the contracts coming from other imported files.

Everything works like a charm except for that rule. Haven't looked into it, letting you guys know.

Thanks :)

✖ [Fatal error] An error occurred while linting over contract.sol: "zeppelin/all-state-variables-private" - No such rule exists.

If I take it out, the others work well.

I tried running the no-state-variable-shadowing rule on the zeppelin-solidity project, and it failed with:

✖ [Fatal error] An error occurred while linting over /home/ubuntu/zeppelin-solidity/contracts/Bounty.sol: Cannot read property 'stateVariables' of undefined

I tried running solium using this plugin within zeppelin-solidity and I get:

✖ [Fatal error] An error occurred while linting over .../zeppelin-solidity/contracts/Bounty.sol: ENOENT: no such file or directory, open './payment/PullPayment.sol'

Steps to reproduce

1. git clone [email protected]:OpenZeppelin/zeppelin-solidity.git
2. git clone [email protected]:elopio/solium-plugin-zeppelin.git
3. npm install -g solium
4. cd solium-plugin-zeppelin
5. npm install --dev
6. npm link
7. npm link solium-plugin-zeppelin
8. cd ../zeppelin-solidity
9. vim .soliumrc.json (replace file's content with the one bellow)
10. solium -d contracts

Configuration file

{
  "extends": "solium:all",
  "plugins": ["security","zeppelin"],
  "rules": {
    "quotes": ["error", "double"],
    "indentation": ["error", 2],
    "arg-overflow": ["warning", 3],
    "security/enforce-explicit-visibility": ["error"],
    "security/no-block-members": ["warning"],
    "security/no-inline-assembly": ["warning"],
    "zeppelin/constant-candidates": ["warning"],
    "zeppelin/highlight-comments": ["warning"],
    "zeppelin/missing-natspec-comments": ["warning"],
    "zeppelin/no-arithmetic-operations": ["warning"],
    "zeppelin/no-state-variable-shadowing": ["warning"],
    "zeppelin/no-unchecked-send": ["warning"],
    "zeppelin/no-unused-imports": ["warning"]
  }
}

On the tests for the rules, when we expect an error we check that the return value is an array and that it contains one element. Some times we check the error message, and some other times we don't. But this is not really following any rules.

For each possible error message, we should check the string once. Some other tests variations are ok just checking that there is an error, but the current way to check for that is ugly and repeated many times. It could be a common helper.

I tried running the constant-candidates rule on the zeppelin-solidity project, and it failed with:

✖ [Fatal error] An error occurred while linting over /home/ubuntu/zeppelin-solidity/contracts/ownership/Heritable.sol: Cannot read property 'type' of null

I tried running the no-unchecked-send rule on the zeppelin-solidity project, and it reported:

(node:8223) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 CallExpression listeners added. Use emitter.setMaxListeners() to increase limit

On #17 we found that the no-unused-imports rule tries to read the imported path relative the the directory where the solium command is being executed, not relative to the file currently being analyzed.

That means that if we run solium on a directory root, and there is a file in root/contracts/TestImport.sol with a statement like import './TestImported.sol', we will try to find this file in root/TestImported.sol instead of root/contracts/TestImported.sol.

To fix this, we need a change in solium because the rule currently has no access to the path of the file being analyzed.

On the highlight-comments rule, we define a list of comments that will report a warning.

It seems that solium allows to define configuration options for plugins, so maybe we can turn this list into something we can easily configure. Look at this rule for example:
https://github.com/duaraghav8/Solium/blob/master/lib/rules/quotes.js#L38

We have the rule no-state-variable-shadowing, that warns when a child contract redefines a variable from the parent contract.

@facuspagnuolo suggest to extend it to warn also when function are overwritten, which would require just a little more code, reusing the same rule.

When I don't add the comment /// @title ... for any of my contract functions the linter complains. However, with it added tests fail due to compile errors:

DocstringParsingError: Doc tag @title not valid for functions.

What i'm doing wrong?