-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Avoid unchecked casts/assignments/calls #3626
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove the no-arg constructor from ActivatableViewAndModel, which sets a dummy Activatable singleton as the model. (Since the model type param can't be checked at runtime, improper use of the constructor could cause heap pollution.) Instead, extend 'ActivatableView<R, Void>' consistently, as other views without a model currently do.
Refactor all the unchecked casts from Overlay<T> to T into a single private cast() method. Also add a runtime type check to the constructor to prevent creation of window objects of the form "A extends Overlay<B>" for unrelated A & B, as such casts would then subvert the type system.
Add a Class<T> parameter to the method, in order to avoid an unchecked cast to the Message type T. The cast was wrapped in a try-catch block, which is useless due to erasure, so use Class.cast(..) instead.
Add missing ILoggingEvent type arg to local variable declarations.
Add missing 'Number' coord type args to some XYChart.(Data|Series) & AreaChart declarations, and avoid passing them as generic varargs, in order to eliminate some more unchecked cast warnings. Also simplify OfferBookChartView.updateChartData() by unboxing the x- coordinate of each (buy & sell) datapoint.
Make sure the generic classes MutableOfferView & AgentRegistrationView don't use raw bounds for their associated view models, as that leads to unchecked assignments from the model fields further down.
(This still leaves a few more which are hard to avoid.)
freimair
approved these changes
Nov 18, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack. Thanks for cleaning that up!
ripcurlx
approved these changes
Nov 18, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Improve type safety by avoiding most of the current unchecked operations, instead of simply suppressing the warnings (as is being done for almost all of them now). This mostly involves adding some missing arguments to raw types, using wildcards in a few extra places and making some minor changes to the logic (such as passing an explicit Class<T> argument or replacing a generic vararg list with List.of(..)).
(In the case of OfferBookChartView.updateChartData(), I made a somewhat larger change to the method body. I also replaced some ActivatableViewAndModel superclasses with ActivatableView<R, Void> where this made sense.)
The test classes and some of the harder cases of potentially type-unsafe unchecked operations have been skipped for now, such as bisq.common.storage.FileManager.read(File).