uber-go / nilaway

Static analysis tool to detect potential nil panics in Go code
Apache License 2.0
3.17k stars 65 forks source link

Handle assignments to global variables from the init function #253

Open k4n4ry opened 5 months ago

k4n4ry commented 5 months ago

fixes #56

Please provide comments if this implementation differs from nilaway's coding practices or design principles. Thank you.

CLAassistant commented 5 months ago

CLA assistant check
All committers have signed the CLA.

sonalmahajan15 commented 5 months ago

@k4n4ry, thank you for submitting the PR! We'll take a look soon and follow up with you.

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 96.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 87.70%. Comparing base (c91e71c) to head (9c61f48).

Files with missing lines Patch % Lines
assertion/global/analyzer.go 92.85% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #253 +/- ## ========================================== + Coverage 87.60% 87.70% +0.09% ========================================== Files 63 63 Lines 7916 7961 +45 ========================================== + Hits 6935 6982 +47 + Misses 799 798 -1 + Partials 182 181 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

SongShawn commented 5 months ago
package sxn01

var (
    b *int
)

func init() {
    init_b_val()
}

func init_b_val() {
    b = new(int)
}

func use_global_var() {
    *b = 20
}

The above code triggers a false positive image

k4n4ry commented 5 months ago

@SongShawn Thanks for the review. I've updated the function to retrieve not only the init functions but also all functions called directly or indirectly from them. https://github.com/uber-go/nilaway/pull/253/commits/dce532a2509d6ebfb1452310abf809847b289370

k4n4ry commented 3 months ago

@sonalmahajan15 Hello, could you please rerun the CI and review this code? Thank you.

yuxincs commented 3 months ago

I'm updating your branch since we recently merged CI fixes, which are required for the CI to pass on forked repositories. I'll review the code this week :)

github-actions[bot] commented 3 months ago

Golden Test

[!WARNING]
❌ NilAway errors reported on stdlib are different 📉.

3271 errors on base branch (main, c91e71c) 2968 errors on test branch (69d920e)

Diffs ```diff + /opt/hostedtoolcache/go/1.22.6/x64/src/archive/tar/writer.go:536:7: Potential nil panic detected. Observed nil flow from source to dereference point: + - fmt/print.go:170:11: literal `nil` assigned into field `buf` + - fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()` + - tar/writer.go:533:26: passed as parameter `b` to `regFileWriter.Write()` (implementing `Writer.Write()`) + - tar/writer.go:536:7: function parameter `b` sliced into + /opt/hostedtoolcache/go/1.22.6/x64/src/archive/tar/writer.go:576:7: Potential nil panic detected. Observed nil flow from source to dereference point: + - fmt/print.go:170:11: literal `nil` assigned into field `buf` + - fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()` + - tar/writer.go:573:29: passed as parameter `b` to `sparseFileWriter.Write()` (implementing `Writer.Write()`) + - tar/writer.go:576:7: function parameter `b` sliced into + /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/md5/md5.go:128:7: Potential nil panic detected. Observed nil flow from source to dereference point: + - fmt/print.go:170:11: literal `nil` assigned into field `buf` + - fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()` + - io/multi.go:83:23: passed as parameter `p` to `multiWriter.Write()` (implementing `Writer.Write()`) + - io/multi.go:85:20: function parameter `p` passed as arg `p` to `Write()` + - md5/md5.go:111:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`) + - md5/md5.go:128:7: function parameter `p` sliced into + /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/md5/md5.go:128:7: Potential nil panic detected. Observed nil flow from source to dereference point: + - fmt/print.go:170:11: literal `nil` assigned into field `buf` + - fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()` + - md5/md5.go:111:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`) + - md5/md5.go:128:7: function parameter `p` sliced into + /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha1/sha1.go:134:7: Potential nil panic detected. Observed nil flow from source to dereference point: + - fmt/print.go:170:11: literal `nil` assigned into field `buf` + - fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()` + - io/multi.go:83:23: passed as parameter `p` to `multiWriter.Write()` (implementing `Writer.Write()`) + - io/multi.go:85:20: function parameter `p` passed as arg `p` to `Write()` + - sha1/sha1.go:123:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`) + - sha1/sha1.go:134:7: function parameter `p` sliced into + /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha1/sha1.go:134:7: Potential nil panic detected. Observed nil flow from source to dereference point: + - fmt/print.go:170:11: literal `nil` assigned into field `buf` + - fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()` + - sha1/sha1.go:123:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`) + - sha1/sha1.go:134:7: function parameter `p` sliced into + /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha256/sha256.go:190:7: Potential nil panic detected. Observed nil flow from source to dereference point: + - fmt/print.go:170:11: literal `nil` assigned into field `buf` + - fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()` + - io/multi.go:83:23: passed as parameter `p` to `multiWriter.Write()` (implementing `Writer.Write()`) + - io/multi.go:85:20: function parameter `p` passed as arg `p` to `Write()` + - sha256/sha256.go:179:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`) + - sha256/sha256.go:190:7: function parameter `p` sliced into + /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha256/sha256.go:190:7: Potential nil panic detected. Observed nil flow from source to dereference point: + - fmt/print.go:170:11: literal `nil` assigned into field `buf` + - fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()` + - sha256/sha256.go:179:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`) + - sha256/sha256.go:190:7: function parameter `p` sliced into + /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha512/sha512.go:269:7: Potential nil panic detected. Observed nil flow from source to dereference point: + - fmt/print.go:170:11: literal `nil` assigned into field `buf` + - fmt/print.go:225:19: field `buf` passed as arg `p` to `Write()` + - io/multi.go:83:23: passed as parameter `p` to `multiWriter.Write()` (implementing `Writer.Write()`) + - io/multi.go:85:20: function parameter `p` passed as arg `p` to `Write()` + - sha512/sha512.go:256:18: passed as parameter `p` to `digest.Write()` (implementing `Writer.Write()`) + - sha512/sha512.go:269:7: function parameter `p` sliced into + /opt/hostedtoolcache/go/1.22.6/x64/src/crypto/sha512/sha512.go:269:7: Potential nil panic detected. Observed nil flow from source to dereference point ...(truncated)... l panic detected. Observed nil flow from source to dereference point: - - os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0 - - os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout` - - rand/example_test.go:53:27: global variable `Stdout` passed as arg `output` to `NewWriter()` - - tabwriter/tabwriter.go:599:26: function parameter `output` passed as arg `output` to `Init()` - - tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output` - - tabwriter/tabwriter.go:251:12: field `output` called `Write()` - /opt/hostedtoolcache/go/1.22.6/x64/src/text/tabwriter/tabwriter.go:251:12: Potential nil panic detected. Observed nil flow from source to dereference point: - - os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0 - - os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout` - - tabwriter/example_test.go:17:9: global variable `Stdout` passed as arg `output` to `Init()` - - tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output` - - tabwriter/tabwriter.go:251:12: field `output` called `Write()` - /opt/hostedtoolcache/go/1.22.6/x64/src/text/tabwriter/tabwriter.go:251:12: Potential nil panic detected. Observed nil flow from source to dereference point: - - os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0 - - os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout` - - tabwriter/example_test.go:26:9: global variable `Stdout` passed as arg `output` to `Init()` - - tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output` - - tabwriter/tabwriter.go:251:12: field `output` called `Write()` - /opt/hostedtoolcache/go/1.22.6/x64/src/text/tabwriter/tabwriter.go:251:12: Potential nil panic detected. Observed nil flow from source to dereference point: - - os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0 - - os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout` - - tabwriter/example_test.go:43:27: global variable `Stdout` passed as arg `output` to `NewWriter()` - - tabwriter/tabwriter.go:599:26: function parameter `output` passed as arg `output` to `Init()` - - tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output` - - tabwriter/tabwriter.go:251:12: field `output` called `Write()` - /opt/hostedtoolcache/go/1.22.6/x64/src/text/tabwriter/tabwriter.go:251:12: Potential nil panic detected. Observed nil flow from source to dereference point: - - os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0 - - os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout` - - tabwriter/example_test.go:61:27: global variable `Stdout` passed as arg `output` to `NewWriter()` - - tabwriter/tabwriter.go:599:26: function parameter `output` passed as arg `output` to `Init()` - - tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output` - - tabwriter/tabwriter.go:251:12: field `output` called `Write()` - /opt/hostedtoolcache/go/1.22.6/x64/src/text/tabwriter/tabwriter.go:251:12: Potential nil panic detected. Observed nil flow from source to dereference point: - - os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0 - - os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout` - - v2/example_test.go:54:27: global variable `Stdout` passed as arg `output` to `NewWriter()` - - tabwriter/tabwriter.go:599:26: function parameter `output` passed as arg `output` to `Init()` - - tabwriter/tabwriter.go:212:13: function parameter `output` assigned into field `output` - - tabwriter/tabwriter.go:251:12: field `output` called `Write()` - /opt/hostedtoolcache/go/1.22.6/x64/src/text/template/funcs.go:635:2: Potential nil panic detected. Observed nil flow from source to dereference point: - - os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0 - - os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout` - - template/example_test.go:104:22: global variable `Stdout` passed as arg `w` to `HTMLEscape()` - - template/escape.go:970:22: function parameter `w` passed as arg `w` to `HTMLEscape()` - - template/funcs.go:635:2: function parameter `w` called `Write()` - /opt/hostedtoolcache/go/1.22.6/x64/src/text/template/funcs.go:718:2: Potential nil panic detected. Observed nil flow from source to dereference point: - - os/file_unix.go:108:10: literal `nil` returned from `NewFile()` in position 0 - - os/file.go:66:2: result 0 of `NewFile()` assigned into global variable `Stdout` - - template/example_test.go:109:20: global variable `Stdout` passed as arg `w` to `JSEscape()` - - template/escape.go:986:20: function parameter `w` passed as arg `w` to `JSEscape()` - - template/funcs.go:718:2: function parameter `w` called `Write()` ```