Querz / NBT

A java implementation of the NBT protocol, including a way to implement custom tags.
MIT License
177 stars 47 forks source link

Excessive usage of raw types #20

Closed Marcono1234 closed 5 years ago

Marcono1234 commented 5 years ago

Often generic types such as Tag do not have the type parameters specified (i.e. raw type), even in recent commits.

Raw types can become pretty dangerous, see this StackOverflow answer for a good explanation.

Querz commented 5 years ago

I see your point and i'm working on it.

There is one Problem creating a ListTag of ListTags. Based on how you create ListTags using the constructor that takes the type class, it would be intuitive to do it this way:

ListTag<ListTag<?>> l = new ListTag<>(ListTag.class);

But obviously this doesn't compile because ListTag.class is not of type ListTag<?>.

And something like the following would be really ugly and having users of this library do this would be unreasonable:

ListTag<ListTag<?>> l = new ListTag<>((Class<ListTag<?>>)((Class<?>)ListTag.class));

Edit: Nevermind, i'm stupid. I can just modify the Constructor to accept a class of any type extending T

public ListTag(Class<? super T> typeClass)
Marcono1234 commented 5 years ago

public ListTag(Class<? super T> typeClass)

That should be ? extends T, otherwise it would allow stuff like new ListTag(Object.class).

Querz commented 5 years ago

I am allowing the type of the class to be a superclass of T, which in turn must be a subclass of Tag<?> (Tag of any type), therefore the type of the class can't be a supertype of the ListTag's type.

If i restrict the type of the class to be a subclass of T instead, i have trouble creating a ListTag of ListTags:

ListTag<ListTag<?>> l = new ListTag<>(ListTag.class);
java: incompatible types: cannot infer type arguments for net.querz.nbt.ListTag<>
    reason: cannot infer type-variable(s) T
      (argument mismatch; java.lang.Class<net.querz.nbt.ListTag> cannot be converted to java.lang.Class<? extends net.querz.nbt.ListTag<?>>)

Also, this restricts the type of the class to be exactly the type of the ListTag, even though the definition is kinda verbose.

Marcono1234 commented 5 years ago

which in turn must be a subclass of Tag<?> Where is this restriction?


I am not getting any compilation problems when changing the constructor parameter.

Querz commented 5 years ago

Where is this restriction?

In the class definition:

public class ListTag<T extends Tag<?>> extends Tag<List<T>> implements Iterable<T> {

I am not getting any compilation problems when changing the constructor parameter.

Weird... I'm using Intellij IDEA 2018.2 to compile with JDK 1.8.0_181 and the unit tests won't compile with this error when i change the constructor parameter to Class<? extends T>:

[...]\NBT\src\test\java\net\querz\nbt\ListTagTest.java:242: error: incompatible types: cannot infer type arguments for ListTag<>
        ListTag<ListTag<?>> recursive = new ListTag<>(ListTag.class);
                                                   ^
    reason: cannot infer type-variable(s) T
      (argument mismatch; Class<ListTag> cannot be converted to Class<? extends ListTag<?>>)
  where T is a type-variable:
    T extends Tag<?> declared in class ListTag
Querz commented 5 years ago

Removed all raw types. The constructor of ListTag now takes a class of type ? super T while T extends Tag<?>, which restricts the class to be the exact same type as T but still allows a ListTag of type ListTag<?>:

ListTag<ListTag<?>> l = new ListTag<>(ListTag.class);
Marcono1234 commented 5 years ago

Sorry that I have not said it before, but thanks for putting all this work into this project!


To the compiling problem again, it looks like it is working for me because I use Eclipse which uses its own compiler. There have been other cases where Eclipse compiled code just fine, but javac did not.

Here is the minimal test code, which should fail compiling for you:

public class ClassTest<T> {
    public ClassTest(Class<? extends T> classArg) { }

    public static void testNested() {
        // Compiles with Eclipse, but not with standard JDK
        ClassTest<ClassTest<?>> nested = new ClassTest<>(ClassTest.class);
    }
}

Hopefully I find the respective Eclipse bug report or get around to reporting it at some point.