protofire / solhint Goto Github PK

i want to add some file into the ignore, but i don't know how to do.

Example: https://github.com/trufflesuite/truffle/blob/develop/CONTRIBUTING.md

Hi, I wish to calculate the cyclomatic complexity score for the contracts I'll be using. Is it possible to get that value?

As I understand, here we are just checking with a "max" number?

Running solhint over this contract

pragma solidity ^0.4.18;


contract TestThing {

    modifier validSender() {
        require(msg.sender != 0x0);
        _;
    }

    function statusIsOkay() external view returns(bool) {
        return true;
    }

    function TestThing() public validSender() {
        // does something
    }
}

gives error

11:5  error    Function order is incorrect, external constant function can not go after  function  func-order

Thanks for creating this. Very helpful.

I couldn't find this in the style guide:

two-lines-top-level-separator | Definition must be surrounded with two blank line indent

What does this mean?

I would like to be able to lint a set of contracts for a specific rule.

That might look like:

solhint --rule no-empty-blocks contracts/**/*.sol

Does this fit with the vision?

Evaluate the idea of separating core library from from CLI app:

• Maintenance
• Scalability
• Enable/ease web version development

It would be nice if I could specify an option that enforced explicit declaration of storage type.

For example

function something(string foo) public { ... }

should specify a storage type of memory or storage for the parameter foo.

0.1.7

The byte keyword is not recognized as a keyword in inline assembly.

    function toByte(uint addr, uint8 index) internal pure returns (byte b) {
        require(index < 32);
        uint8 n;
        assembly {
            n := byte(index, mload(addr))
        }
        b = byte(n);
    }

line 102:17 mismatched input 'byte' expecting {'return', 'address', DecimalNumber, HexNumber, HexLiteral, Identifier, 'byte', StringLiteral}

Additionally, I know this was the problem because when commenting the asm section out, the linting worked (in fact i got a warning that 'addr' was never used).

When a default-function is used, its common to not do anything in the function itself.

    function() public payable {
        // got money
    }

Solhint currently gives the warning Code contains empty block no-empty-blocks.

Is there any way to access all existing rules of solhint in a programatic way?

For instance the method through which the documentation files are created, if they are made automatically?

Running solhint -f json Contract.sol throws an error:

TypeError: Converting circular structure to JSON

I have a particular line in our contract that is unable to pass the linters indentation rules:

(bytes32 r, bytes32 s, uint8 v) = SomeLibrary.splitSignature(signature);

Error:

Expected indentation of 12 spaces, but found 13 [indent]

If you remove a space from before the first bracket, it replaces 13 with 11 in the error message. Seems to be an issue with any function that has multiple return values.

solhint ./contracts/Test.sol for next contract

pragma solidity ^0.4.18;


contract Test {
    uint256 constant public MAX_POSSIBLE_AMOUNT = 1e9 ether;
}


//Works
contract Test2 {
    uint256 constant public MAX_POSSIBLE_AMOUNT = 1000000000 ether;
}

returns:

line 5:51 missing ';' at 'e9'
line 5:54 mismatched input 'ether' expecting {'from', 'constant', 'public', 'internal', 'private', Identifier}

./contracts/Test.sol
1:17 warning Compiler version must be fixed compiler-fixed
5:52 error Expected indentation of 4 spaces but found 51 indent
5:52 warning Explicitly mark visibility of state state-visibility

✖ 3 problems (1 error, 2 warnings)

Expected:
No errors for Test contract

It'd be nice if it was made explicit somewhere that solhint just delegates to ESLint for formatters (and to add all the formatters to the README and CLI's help text).

@fvictorio and I have missed this before while adding vim plugins :) (e.g. #16 (comment)).

I tried to integrate solhint with Ale (a lint engine for vim that takes advantage of vim 8 async features), but I wasn't able to do it because none of the formatters is supported by Ale, at least right now. There is support for the unix format, but it can't tell warnings from errors.

Adding a gcc formatter to solhint seems to me like the easiest way to do it.

I receive space-after-comma error for:

var (x, , ,y,) = myContract.get(); // space-after-comma error
var (x,  ,  ,y , ) = myContract.get(); // space-after-comma error

Move Security Rules, Style Guide Rules and Best Practices Rules sections put from readme file and simply link /docs/rules.md sections

Known high severity security vulnerability detected in cryptiles < 4.1.2 defined in package-lock.json.

package-lock.json update suggested: cryptiles ~> 4.1.2.

I'm trying to use import "zeppelin-solidity/contracts/token/ERC20/MintableToken.sol";
but solhint is giving me an error because it can't find the file. How can I indicate a folder for libraries ?
Using a fully relative path works but is quite ugly

Blacklisting is quite a tricky and tedious process. Requires many iterations and isn't straightforward at all.
Proposal is to evaluate switching this approach in favor of Rulesets + Whitelisting. Guess it that this will make Solhint setup simpler and more intuitive.

Actually solhint doesnt support:

• event emit : emit eventName()
• constructor keyword in place of function name equal to contract name

Version 1.1.6, (according to package.json file, but solhint --version says 1.1.5)

Using pure or other state mutability keywords is currently not supported by the linter, but it is supported by the compiler.

Using pure will result in a line like the following:

line 15:55 extraneous input 'pure' expecting {';', '{', 'returns'}

Eventually, the line numbers in errors/warnings and other things will be jumbled and will not work properly. Replacing pure with constant will remove the error.

This may require a lot of changes, but allowing Solhint to be pluggable will bring nothing but great potential to the table.

Just a few things, off the top of my head, to be defined:

1. Configuration. How Solhint will be configured to run plugins.
2. Execution model. Standard API to be implemented by each plugin.

May I suggest ignoring comments when counting the number of lines in a function to check whether it exceeds function-max-lines?

i.e. the following should only count as 2 lines of code:

uint foo = 0; // comment
// comment
Event(bar);

This will prevent solhint from giving function-max-lines errors for functions which exceed 50 lines but are mainly comprised of comments. IMO, it's good practice to comment smart contract code and this shouldn't be penalised by the tool. Thanks!

In order to make solhint more compatible with eslint, we should allow disabling rules using the value "off" instead of using false. We can make this change backward-compatible and, ideally, show a deprecated message for configurations that use false.

I always get Function order is incorrect, external constant function can not go after function. (func-order) solhint error for external functions.

(using solhint in Atom)

pragma solidity 0.4.18;


contract SolhinTest {
    function get() external view returns (uint ct) { // solhint error: func-order on this line
        return 1;
    }
}

Add "Used by", "Projects using solhint", etc section to README.md and link/list projects that use solhint.

Solidity 0.4.20+ now recommends using 'emit' keyword to creating an event. e.g.
emit Transfer(msg.sender, _to, _tokens);

This creates 2 issues in solhint:

• Parsing produces something like line 26:21 mismatched input '(' expecting {';', '='}
• Indentation message 26:22 error Expected indentation of 8 spaces but found 21 indent

Not sure if there was a licensing issue (solidity-parser-antlr is GPL, which would force this project to be GPL?) or something, but a quick look over a diff of the antlr grammars in this package and solidity-parser-antlr make it seem like the differences are trivial, and hence unnecessary to maintain two copies.

The only non-trivial differences seem to be a few extra rules here for some assembly items, namely subAssembly, datasize, and linkerSymbol and one potentially wrong rule for expression (AFAIK you can only use an identifier after a .).

Add 'How to Contribute' section to README file, covering:

• Contributing process: Fork, branch, commit, PR and so on.
• Adding new rules.
• (optional?) Supporting new output formats.

Hello, It would be nice to have the option "--watch" or a kind of watch mode, where you can see errors and problems in real time.

Thanks in advance

common mistakes I make when copy-paste:

/* Calling set() works fine on javascriptVM, fails on testrpc/ganache-cli 6.0.3 */
pragma solidity ^0.4.18;

contract Test {

  uint public counter = 1;
  
  function set() public  {
      counter = 0 // missing a semicolon
  }; // having an unnecessary semicolon
  
}

would be great to show warnings/errors

contract Token {
    string constant name = " Token";
    string constant symbol = "TOK";
    uint8 constant decimals = 18;

Each of the constant declarations has the constant name must be in SNAKE_CASE error.

Capitalizing the names removes the error. But snake case doesn't say anything about being capitalized.

Also, I don't see anything about it in the style guide: https://solidity.readthedocs.io/en/develop/style-guide.html

Exception
I am getting the following exception if anyone can help me out.
"Solhint is not installed: For correct run of Solidity Solhint Plugin you need to install Solhint."

Installed
I have the latest NodeJs installed.
I also have Solidity and Solhint (1.1.8) installed via Browse Repository

OS
Operating System: Windows 10 x64
IDE: IntelliJ

In addition, how do you get started once you have all package installed? What is the run command?

ERC 20 is a standard. Unfortunately, they named their Transfer event the same as their transfer function.

You're correct in marking this as a warning if no-simple-event-func-name is being checked, but I think it's a bad idea to do so.

A nieve programmer may change that event name to something else, only to silence the linter, but this will make the token contract non-compliant. In order to remove the warning, the programmer must create an incompatible client.

I would make an exception and not report a warning if the smart contract is an ERC20 token (which may be impossible, but in that case simply don't warn for the Transfer(address indexed, address indexed, uint256) event.

I am not sure if this is a bug or a feature but the no-simple-event-func-name rule does not see a difference between a function called claimSatisfied and an event ClaimSatisfied.

Is that rule supposed to be case insensitive?

I'm getting an error message when editing files that are in certain directories:

[Linter] Error running Solhint
See console for more info

...(console)...

C:\Users\patri\.atom\packages\linter\lib\linter-registry.js:177 [Linter] Error running Solhint Error: Invalid or no `directory` provided
    at Object.validateFind (C:\Users\patri\.atom\packages\atom-solidity-linter\node_modules\atom-linter\lib\helpers.js:92:11)
    at Object.<anonymous> (C:\Users\patri\.atom\packages\atom-solidity-linter\node_modules\atom-linter\lib\index.js:67:13)
    at Generator.next (<anonymous>)
    at step (C:\Users\patri\.atom\packages\atom-solidity-linter\node_modules\atom-linter\lib\index.js:145:191)
    at C:\Users\patri\.atom\packages\atom-solidity-linter\node_modules\atom-linter\lib\index.js:145:437
    at Object.<anonymous> (C:\Users\patri\.atom\packages\atom-solidity-linter\node_modules\atom-linter\lib\index.js:145:99)
    at Object.findCachedAsync (C:\Users\patri\.atom\packages\atom-solidity-linter\node_modules\atom-linter\lib\index.js:86:18)
    at file:///C:/Users/patri/.atom/packages/atom-solidity-linter/lib/helpers.js:15:45
    at Generator.next (<anonymous>)
    at step (file:///C:/Users/patri/.atom/packages/atom-solidity-linter/lib/helpers.js:49:2)

For example, by default, remote-atom creates files in Windows 10 at AppData/Local/Temp. If I move the exact same file to these directories:

AppData/Local
AppData
OneDrive/Documents
(or my home dir)

it doesn't work. However, if I move it to $HOME/OneDrive/Documents/solidity I no longer get those error message. File permissions are exact same, moving the exact same file. I thought maybe it was remote-atom but it seems to happen regardless of whether remote-atom creates a file or I move one.

Add --max-warnings [int] option in formatter option: Number of warnings to trigger nonzero exit code.

Number of warnings to trigger nonzero exit code - default: -1

The motivation comes from an option that EsLint CLI offers, and continue tha idea of issue #5.

Hello, It would be interesting to have the option "--fix" to solve some existing warnings, for example with spaces, quotes, tabs, etc.

This feature would be very helpful

Do I need to npm install solhint too?

Just to get the discussion rolling, I was wondering if it might make sense to conform to some of ESLint's CLI and options given how this project uses a lot of the great ideas that have come out of ESLint.

Some ideas (that may be better as separate issues if agreed upon):

• Change solhint.json to solhintrc.json (we could use cosmiconfig for this)
• Change init-config to --init
• Add support for using 0, 1, and 2 as severity levels (or state that you can only use "off", "warn", "error") (see also #43)
• Add --quiet to only report errors
• Add support for a .ignores-type file
• Add support for custom rulesets, and inheriting from them
• Add support for custom plugins, and moving the security rules into a solhint-plugin-security-type package

A lot of these features are either present or coming via solium@1, but I don't see much of a point in having both when solhint seems like a cleaner implementation already using antlr4.

If you have a function like this

function doThing(uint someTimeInDays) public {
    expiry[msg.sender] = now + timelockPeriod days;
}

you get a warning

warning  Avoid to make time-based decisions in your business logic  not-rely-on-time

But if you add // solhint-disable-line not-rely-on-time, i.e.

function doThing(uint someTimeInDays) public {
    expiry[msg.sender] = now + timelockPeriod days; // solhint-disable-line not-rely-on-time
}

you get

extraneous input 'days' expecting ';'

Likewise if you use // solhint-disable-next-line not-rely-on-time, or even if you start the file with

/* solhint-disable not-rely-on-time */

The same error appears.

Similarly if you add

"not-rely-on-time": false,

to the rules in .solhint.json you get this error as well.

This appears to be a bug in comment-directive-parser.js but I am not familiar enough with the code to see where.