hardhat icon indicating copy to clipboard operation
hardhat copied to clipboard

Bump solhint dependency to 3.0.0

Open gionn opened this issue 1 year ago • 8 comments

Fix #1362 Fix #654

gionn avatar Aug 19 '22 15:08 gionn

🦋 Changeset detected

Latest commit: 605d8c3f7fec6be9bbf47ef27c594e0aa261b9d2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@nomiclabs/hardhat-solhint Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Aug 19 '22 15:08 changeset-bot[bot]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
hardhat ✅ Ready (Inspect) Visit Preview Dec 13, 2022 at 9:19AM (UTC)
hardhat-storybook ✅ Ready (Inspect) Visit Preview Dec 13, 2022 at 9:19AM (UTC)

vercel[bot] avatar Aug 19 '22 15:08 vercel[bot]

This issue is also being tracked on Linear.

We use Linear to manage our development process, but we keep the conversations on Github.

LINEAR-ID: 902d7700-bfd8-4475-ba25-b1b031bda764

github-actions[bot] avatar Aug 19 '22 15:08 github-actions[bot]

Looks like all we need is a changeset added here, bumping the major version, per Franco's comment here. Sound right to you @fvictorio ?

feuGeneA avatar Aug 24 '22 18:08 feuGeneA

Yes. This also needs manual testing. Maybe it just works with that change, but I would be a bit surprised if it does.

fvictorio avatar Aug 24 '22 19:08 fvictorio

@gionn feel free to share your manual test plan :)

feuGeneA avatar Aug 29 '22 15:08 feuGeneA

Hello, for testing this I was trying to use my branch as a package source in my personal hardhat project with the help of gitpkg (I am a casual node developer and I have really no idea what I am doing):

npm install -D 'https://gitpkg.now.sh/gionn/hardhat/packages/hardhat-solhint?bump-solhint'

then after adding require("@nomiclabs/hardhat-solhint"); to my hardhat.config.js I get:

$ npx hardhat check
An unexpected error occurred:

Error: Cannot find module '/home/gionn/src/myproject/node_modules/@nomiclabs/hardhat-solhint/dist/src/index.js'. Please verify that the package.json has a valid "main" entry
    at tryPackage (node:internal/modules/cjs/loader:364:19)
    at Function.Module._findPath (node:internal/modules/cjs/loader:577:18)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:942:27)
    at Function.Module._load (node:internal/modules/cjs/loader:804:27)
    at Module.require (node:internal/modules/cjs/loader:1022:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/home/gionn/src/myproject/hardhat.config.js:5:1)
    at Module._compile (node:internal/modules/cjs/loader:1120:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1174:10)
    at Module.load (node:internal/modules/cjs/loader:998:32) {
  code: 'MODULE_NOT_FOUND',
  path: '/home/gionn/src/myproject/node_modules/@nomiclabs/hardhat-solhint/package.json',
  requestPath: '@nomiclabs/hardhat-solhint'
}

In that package.json I have:

  "main": "dist/src/index.js",
  "types": "dist/src/index.d.ts",

and those files doesn't exists.

Any help?

gionn avatar Sep 10 '22 08:09 gionn

@gion that happens because the project is in typescript and the pushed code is not built. I guess gitpkg only works for javascript projects.

Read this to learn how to try changes like this one locally.

fvictorio avatar Sep 20 '22 19:09 fvictorio

@fvictorio thanks!

So with yarn I built hardhat locally with my branch checked out , then cd packages/hardhat-solhint and npm link.

At this point on my personal hardhat project I run npm link @nomiclabs/hardhat-solhint and then npx hardhat check worked as expected!

~/s/e/Metaverse-Graffiti ❯❯❯ npm link @nomiclabs/hardhat-solhint

removed 97 packages, changed 1 package, and audited 526 packages in 2s

106 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
~/s/e/Metaverse-Graffiti ❯❯❯ npx hardhat check
~/s/e/Metaverse-Graffiti ❯❯❯      

Changing my .solhint.json indeed change the behavior:

~/s/e/Metaverse-Graffiti ❯❯❯ npx hardhat check

/home/gionn/s/e/Metaverse-Graffiti/c/MetaverseGraffiti.sol
   21:1   error    Compiler version ^0.8.0 does not satisfy the ^0.5.8 semver requirement                             compiler-version
  118:5   warning  Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)  func-visibility
  261:21  warning  Avoid to make time-based decisions in your business logic                                          not-rely-on-time
  269:28  warning  Avoid to make time-based decisions in your business logic                                          not-rely-on-time
  302:43  warning  Avoid to make time-based decisions in your business logic                                          not-rely-on-time
  310:38  warning  Avoid to make time-based decisions in your business logic                                          not-rely-on-time
  467:32  warning  Code contains empty blocks                                                                         no-empty-blocks

✖ 7 problems (1 error, 6 warnings)

~/s/e/Metaverse-Graffiti ❯❯❯

Is there anything else I can do to move this forward?

gionn avatar Sep 24 '22 15:09 gionn

Hey @gionn, sorry, I will take a look into this as soon as I have some time.

fvictorio avatar Oct 18 '22 14:10 fvictorio

Hi @fvictorio, anything I can do to move this forward?

gionn avatar Dec 06 '22 16:12 gionn

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2022 Hardhat Contributor:

GitPOAP: 2022 Hardhat Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

gitpoap-bot[bot] avatar Dec 14 '22 09:12 gitpoap-bot[bot]

Thank you @gionn, and sorry for taking so long to merge this!

fvictorio avatar Dec 14 '22 09:12 fvictorio