SpongePowered / Sponge

The SpongeAPI implementation targeting vanilla Minecraft and 3rd party platforms.
MIT License
389 stars 211 forks source link

Nullability and Checks #1931

Open gabizou opened 6 years ago

gabizou commented 6 years ago

While working on clearing up all the compiler warnings and overall code auditing of the project, I'm finding lots of areas where we have nullable and a lack of nullable checks. Some interface method contracts in the implementation are either lacking nullability or intending non-null values where the implementation is not following that contract.

An example is AbstractInventoryAdapter

I'll be working on cleaning this up and pending some discussion whether it's worth our while for long term maintenance of 1.12 and 1.13.

A lot of our mixin interfaces have methods that are unused or not documented for their "intended" use.

gabizou commented 6 years ago

Pinging @Faithcaio:

For AbstractInventoryAdapter, it looks like there's a corner case where if the parent is this, you could get a recursive loop for getting the plugin instance. This should be fixed imo.

Faithcaio commented 6 years ago

How? If parent == this I dont have a plugin.

gabizou commented 6 years ago

Right, but you set it to this in the constructor if the plugin parent was null here: https://github.com/SpongePowered/SpongeCommon/blob/stable-7/src/main/java/org/spongepowered/common/item/inventory/adapter/impl/AbstractInventoryAdapter.java#L87-L90