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

Findbugs reports lots of issues #25

Closed
GlenKPeterson opened this issue Oct 29, 2017 · 2 comments
Closed

Findbugs reports lots of issues #25

GlenKPeterson opened this issue Oct 29, 2017 · 2 comments
Labels

Comments

@GlenKPeterson
Copy link
Owner

GlenKPeterson commented Oct 29, 2017

@cprice404 says (in issue #24):

Our build system runs findbugs automatically. With (as far as I know) a pretty basic findbugs setup, there are 4 or 5 "high priority" warnings on the project, and a few dozen "medium priority" ones. Based on your unit test coverage i somewhat doubt that the things findbugs is calling out are critical bugs, so that won't prevent me from evaluating the lib or anything... But I thought you might be interested to know. The kind of stuff that findbugs picks up in general are spiritually similar to what you are doing with equality tests in your testutils lib, so you might find them interesting.

Thanks for reporting this @cprice404! One quick thought is that these collections come from Clojure and are designed to be fast. They break many rules about type-safety internally, but are tested very carefully to enforce all relevant rules externally. So without seeing the report, I'm concerned that there may be some issues I just can't fix.

On the other hand, actual bugs are not allowed and I hope to be able to fix any soon!

@GlenKPeterson
Copy link
Owner Author

I fixed everything that seemed to have a clear solution. Thanks for letting me know about this! At least one was a real bug - an exposed mutable array. It had a note like, "Fix this before a release" next to it. Oops! There was a "possible circular instantiation" issue that I've eliminated, though it never caused problems. I also was able to delete a very small amount of duplicated or unnecessary code to get rid of a few other issues. I think this is a significant improvement!

There are a few things I don't know what to do about. The Comparator issues are complicated and appear to work as is. I actually agree with the error, but don't want to fix it because I think the behavior might be useful (you might want to sort Comparables that contains nulls).

I think the serialization issues are false positives. Write a unit test that proves me wrong and I'll fix it!

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 participant