2020-12-02 03:57

Proposal: Add semgrep-go checks into validation step

Discovered recently this amazing semgrep-go rules collection put together by here https://github.com/dgryski/semgrep-go allowing to avoid common issue patterns in Go.

The check shouldn't by blocking for the time being, but once all errors / warnings will be solved, maybe it should become blocking in case something is detected.

I will be pushing a PR to introduce this one in the workflow so we can discuss and let me know what you think about it.

Cheers ! :)


  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答


  • weixin_39682511 weixin_39682511 5月前

    I agree with that this won't replace the linter as the linter checks more for style related rules vs this seems to check for potential bugs and such.

    ~~Also, I did find a few other static analysis tools for Go like goreporter, go-critic (This has it's own set of rules and plus also supports a ruleguard integration) and staticcheck which look interesting and useful too and so wanted to throw them in the mix here as well so that we can consider them all and pick the one suited best for Prebid-Server.~~

    ~~I personally found goreporter the most interesting as it supports a bunch of different linters and normalizes their output but I am not sure if that project is maintained anymore :(~~

    Looks like golangci-lint is one of the popular open-source linters for Go that supports a bunch of linters (one of them being go-critic which has a ruleguard integration as well)

    My question would be, are there any semgrep-go rules that aren't covered by the myriad of linters supported by golangci-lint? And if there are, how important they are to this project?

    If they are quite important then we can consider adding semgrep-go but as a starting point golangci-lint should be the most helpful IMHO.

    点赞 评论 复制链接分享
  • weixin_39682511 weixin_39682511 5月前

    out of curiosity, do you consider integrating semgrep-go with golangci-lint in the future?

    点赞 评论 复制链接分享
  • weixin_39765290 weixin_39765290 5月前

    My suggestion is to start by integrating tools with little to no false positives: go vet, staticcheck, ineffassign, and nilness. Most of the other liners catch nits that are either more style issues and just generate code churn.

    点赞 评论 复制链接分享
  • weixin_39883194 weixin_39883194 5月前

    Hey ! Semgrep maintainer here. 's rules are also published to the Semgrep registry if you'd like to skip the curl:

    semgrep --config http://semgrep.dev/r/dgryski.semgrep-go

    This is the equivalent and lets you skip the .gitignore. There is also a Semgrep GitHub Action that only displays and fails on new results, so it's easy to introduce new checks without having to fix all previous findings: https://github.com/returntocorp/semgrep-action/#readme

    Would love to hear your feedback!

    点赞 评论 复制链接分享
  • weixin_39669075 weixin_39669075 5月前

    Hello !

    Thanks a lot for your help here, really appreciate :) I will try to use action directly, but first wanted to make this check part of the validation script so it's also available locally to contributors.

    Let's see if I can make it happen !

    Thanks !

    点赞 评论 复制链接分享
  • weixin_39883194 weixin_39883194 5月前

    Let me know if you run into any issues, the maintainer group loves supporting use cases like yours! 🙂

    点赞 评论 复制链接分享
  • weixin_39669075 weixin_39669075 5月前

    Hey !

    is working on adding a linter. I'm curious if you recommend this as a complimentary static analysis or a replacement to a traditional linter?

    I think it's a complimentary static analysis tool along the linter, it won't replace a linter IMO as it won't serve the same purpose.

    点赞 评论 复制链接分享