Option to exclude test code from the analysis
Closed this issue · 2 comments
Howdy!
Right now there is no way to exclude test code from the analysis and, I believe, this is something that could be useful. As often in tests we don't really mind a panic, since it will already result in a test failure.
In NullAway it is easily achieved by excluding test packages, however, in Go excluding packages is often not a viable solutions, since, more often than not, production code shares the same package with the test code.
With that said, I think it might be a good idea to have some option, like, include_tests with a default value of true(or ignore_tests with default false). Then we can go filter test files as *_test.
@sonalmahajan15 @yuxincs what are your thoughts? If you think this is something that could be useful, I, perhaps, can try to cook up a PR for it :)
Hey @philipuvarov, thanks for the suggestion!
Are you trying to propose adding a flag for (1) suppressing the NilAway errors in test code (but the test code is still analyzed), or simply (2) not analyzing test code (perhaps for performance benefits)?
For (1), depending on which driver you're using to run NilAway, they should have error suppression mechanisms to just suppress NilAway errors reported in *_test.go (see wiki/Configuration). IMO, error suppressions should really be a feature of the linter driver, where the linter should just faithfully report the errors it finds.
Even then, admitting that NilAway still has a number of false positives, I think reporting issues in test code is valuable and advocates test clarity (since, usually the fix is to add require.NotNil(t, ptr)). But I definitely understand that it may not always be the case, and hence devs can use the driver's suppression mechanism to just suppress them altogether.
For (2), that may make sense in large repositores, but I'm a bit hesitant to do so because usually test code is the "first client" of the library, such that analyzing the test code (the errors can be suppressed) still provides valuable information to the analysis.
Thoughts?
I'm gonna go ahead and close the issue since I think it's actually supported already, but feel free to re-open if it doesn't actually address the issue :)