Skip to content
This repository has been archived by the owner on Apr 10, 2019. It is now read-only.

Rename gas security checker to gosec #505

Merged
merged 6 commits into from
Jul 24, 2018

Conversation

ccojocar
Copy link
Contributor

@ccojocar ccojocar commented Jul 22, 2018

@alecthomas
Copy link
Owner

Tests are not working.

@ccojocar
Copy link
Contributor Author

@alecthomas I had to rework a bit the test. gosec expects now the files to be in the GOPATH, because it analyses the entire package. It doesn't work any more with temporary files.

cc @gcmurphy

@kolyshkin kolyshkin mentioned this pull request Jul 24, 2018
@kolyshkin
Copy link

This should also fix issue reported in #499 (comment)


func TestGosec(t *testing.T) {
t.Parallel()
gopath := os.Getenv("GOPATH")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please change this to create a new GOPATH in a temporary directory? You can use gotest.tools/fs to do this fairly easily.

You might need to have a GOPATH with two entries - the existing one and the temporary one.

.goreleaser.yml Outdated
goos: *goos
goarch: *goarch
env: *env
main: ./_linters/src/github.com/GoASTScanner/gas
main: ./_linters/src/github.com/securego/gosec
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the main command target?

@alecthomas
Copy link
Owner

Thanks for this, I appreciate it.

@ccojocar
Copy link
Contributor Author

@alecthomas Thanks for your suggestion. I fixed the test code accordantly.

@alecthomas
Copy link
Owner

Brilliant, thanks!

@alecthomas alecthomas merged commit 445bce7 into alecthomas:master Jul 24, 2018
@@ -84,7 +84,7 @@ There are two options for installing gometalinter.
- [interfacer](https://github.com/mvdan/interfacer) - Suggest narrower interfaces that can be used.
- [unconvert](https://github.com/mdempsky/unconvert) - Detect redundant type conversions.
- [goconst](https://github.com/jgautheron/goconst) - Finds repeated strings that could be replaced by a constant.
- [gas](https://github.com/GoASTScanner/gas) - Inspects source code for security problems by scanning the Go AST.
- [goesc](https://github.com/securego/gosec) - Inspects source code for security problems by scanning the Go AST.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here.

Copy link
Contributor Author

@ccojocar ccojocar Jul 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I fixed it in #506

@JeanMertz
Copy link

Could it be that this change breaks usage of // nolint: gosec?

I can still disable using // nolint, but not when I only want to disable gosec.

@HaraldNordgren
Copy link
Contributor

HaraldNordgren commented Jul 28, 2018

@alecthomas @ccojocar

This is a breaking change. We were running the linter with --disable=gosec, which then started giving issues when the rule was renamed.

Could you please bump the major version to v3.0.0?

Tools like http://labix.org/gopkg.in are designed so that breaking changes have to go into major versions.

@dnephin
Copy link
Collaborator

dnephin commented Jul 28, 2018

This change is not yet part of a release. If you're concerned about breaking changes use a tag. v2.0.6 is the latest.

@alecthomas
Copy link
Owner

Hello @ccojocar, this change appears to be having some issues. See #510 for example. Mind taking a look?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants