unlock-protocol / unlock

Ʉnlock is a protocol for memberships built on a blockchain.
https://unlock-protocol.com
MIT License
835 stars 244 forks source link

solhint is not running in Travis CI correctly (Solidity linter) #1104

Closed HardlyDifficult closed 5 years ago

HardlyDifficult commented 5 years ago

Describe the bug solhint is executed with each build. But somehow errors are not appearing! I just checked, this is the current state on the master branch:

contracts/PublicLock.sol
   46:3   error  Definitions inside contract / library must be separated by one line                     separate-by-one-line-in-contract
  282:3   error  Function order is incorrect, external function can not go after private function        func-order
  299:3   error  Function order is incorrect, external function can not go after private function        func-order
  314:3   error  Function order is incorrect, external_const function can not go after private function  func-order
  329:3   error  Function order is incorrect, external_const function can not go after private function  func-order
  344:3   error  Function order is incorrect, external_const function can not go after private function  func-order
  359:3   error  Function order is incorrect, external_const function can not go after private function  func-order
  373:3   error  Function order is incorrect, public function can not go after private function          func-order
  385:3   error  Function order is incorrect, public function can not go after private function          func-order
  398:3   error  Function order is incorrect, public function can not go after private function          func-order
  410:17  error  Expression indentation is incorrect. Required space after =                             expression-indent
  431:3   error  Function order is incorrect, public function can not go after private function          func-order
  445:3   error  Function order is incorrect, public function can not go after private function          func-order
  460:3   error  Function order is incorrect, public function can not go after private function          func-order
  485:3   error  Function order is incorrect, public function can not go after private function          func-order
  508:3   error  Function order is incorrect, internal function can not go after private function        func-order
  515:3   error  Function body contains 53 lines but allowed no more than 50 lines                       function-max-lines
  540:7   error  Statement indentation is incorrect. Required space after if                             statement-indent
  541:7   error  Open bracket must be on same line. It must be indented by other constructions by space  bracket-align
  576:3   error  Function order is incorrect, internal function can not go after private function        func-order

contracts/Unlock.sol
  34:2  error  Line length must be no more than 120 but current length is 219  max-line-length
  35:1  error  Definition must be surrounded with two blank line indent        two-lines-top-level-separator

✖ 22 problems (22 errors, 0 warnings)

To Reproduce Steps to reproduce the behavior:

  1. Go to https://travis-ci.com/hardlydifficult/unlock/builds/97759769#L1172 and see that solhint was called
  2. Compare output (which is none in Travis) to running locally unlock\smart-contracts> solhint contracts\**\*.sol

Expected behavior Errors detected locally should appear in Travis.

Additional context Here is a link to an old build of Unlock which called solhint and caught my errors: https://travis-ci.com/hardlydifficult/unlock/builds/85219093#L4362 It seems to be called the same way. I'm not sure what changed.

julien51 commented 5 years ago

This is pretty serious. Looking into that.

julien51 commented 5 years ago

When running things locally I also do not get anything. Could this be a different version?

$ solhint --version
1.1.10
julien51 commented 5 years ago

Wait, I know what is going on, we're running $ solhint contracts/**/*.sol from /unlock/smart-contracts so it is just running against the contracts inside contracts/interfaces/ which are good :/.

2 steps:

  1. Fixing the contracts
  2. Updating the solhint script.

Do you think you or @nfurfaro can work on the first part?

julien51 commented 5 years ago

Even weirder: $ solhint "contracts/**/*.sol" does show the issues when $ solhint contracts/**/*.sol does not :/

HardlyDifficult commented 5 years ago

1) I already fixed the lint issues: https://github.com/unlock-protocol/unlock/pull/1105 2) The docs seem to say we are doing it correctly... but maybe we can try this to cover all files: solhint contracts/*.sol contracts/**/*.sol (https://protofire.github.io/solhint/)

julien51 commented 5 years ago
  1. Cool! We're all set then ;)
  2. I think the doc show the glob wrapped in " which we are not doing. Adding a PR to test for that which should hopefully fail and then pass once #1015 is merged! Thanks
julien51 commented 5 years ago

Thanks for the quick turnaround on this @hardlydifficult !