Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/analysis/passes/initflagparse: add check for avoid flag.Parse at init #206

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jespino
Copy link

@jespino jespino commented Feb 9, 2020

I made the changes to fix the naif approach, but I would like to know how much we want to complicate the things here.

For example, now we have a false positive if you use other package called flag what have a function Parse and it is used in the init.

Other problem can be a function creation during init what contains the flag.Parse or functions called from init that calls the flag.Parse.

I guess we want to fix the main straig forward case (what is already solved with this PR) and maybe get rid of the case where the flag package is not the built-in flag package.

The other options are more complicated and I'm not sure if provides much value.

I'm also not very sure about the naming of things is correct.

This PR fixes the golang/go#33190 ticket

@gopherbot
Copy link
Contributor

This PR (HEAD: b3bcc7c) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/218757 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Agniva De Sarker:

Patch Set 2:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 2d1136b) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/218757 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 4: Run-TryBot+1 Code-Review+1

(3 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 4:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=9b4bbe24


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 4:

Build is still in progress...
This change failed on linux-amd64 (Go 1.12.x):
See https://storage.googleapis.com/go-build-log/249c282c/linux-amd64_e727300a.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 26c541c) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/218757 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Agniva De Sarker:

Patch Set 5: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 5:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=98e5ab4b


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 5:

Build is still in progress...
This change failed on linux-amd64 (Go 1.12.x):
See https://storage.googleapis.com/go-build-log/46cb0161/linux-amd64_f3dbed63.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 5: TryBot-Result-1

10 of 10 TryBots failed:
Failed on linux-amd64 (Go 1.12.x): https://storage.googleapis.com/go-build-log/46cb0161/linux-amd64_f3dbed63.log
Failed on linux-amd64 (Go 1.13.x): https://storage.googleapis.com/go-build-log/a7acf9af/linux-amd64_1fe6ecca.log
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/6917529c/freebsd-amd64-12_0_54d0d25f.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/6917529c/linux-amd64_1eb13db8.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/6917529c/linux-386_bba726a3.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/6917529c/windows-amd64-2016_59274b8a.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/6917529c/openbsd-amd64-64_bb57661a.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/6917529c/windows-386-2008_49fa622b.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/6917529c/linux-amd64-race_78ba76d5.log
Failed on android-amd64-emu: https://storage.googleapis.com/go-build-log/6917529c/android-amd64-emu_e6c2d608.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Alan Donovan:

Patch Set 5:

(3 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Ian Cottrell:

Patch Set 5:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 9863617) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/218757 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Jesús Espino:

Patch Set 6:

(3 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 4:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=9b4bbe24


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 4:

Build is still in progress...
This change failed on linux-amd64 (Go 1.12.x):
See https://storage.googleapis.com/go-build-log/249c282c/linux-amd64_e727300a.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 5:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=98e5ab4b


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 5:

Build is still in progress...
This change failed on linux-amd64 (Go 1.12.x):
See https://storage.googleapis.com/go-build-log/46cb0161/linux-amd64_f3dbed63.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 5: TryBot-Result-1

10 of 10 TryBots failed:
Failed on linux-amd64 (Go 1.12.x): https://storage.googleapis.com/go-build-log/46cb0161/linux-amd64_f3dbed63.log
Failed on linux-amd64 (Go 1.13.x): https://storage.googleapis.com/go-build-log/a7acf9af/linux-amd64_1fe6ecca.log
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/6917529c/freebsd-amd64-12_0_54d0d25f.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/6917529c/linux-amd64_1eb13db8.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/6917529c/linux-386_bba726a3.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/6917529c/windows-amd64-2016_59274b8a.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/6917529c/openbsd-amd64-64_bb57661a.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/6917529c/windows-386-2008_49fa622b.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/6917529c/linux-amd64-race_78ba76d5.log
Failed on android-amd64-emu: https://storage.googleapis.com/go-build-log/6917529c/android-amd64-emu_e6c2d608.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/218757.
After addressing review feedback, remember to publish your drafts!

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

Successfully merging this pull request may close these issues.

3 participants