Bug 289492 - Invalid Warning Parameter Type Bound by Final Type
Summary: Invalid Warning Parameter Type Bound by Final Type
Status: VERIFIED INVALID
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: PC Windows Vista
: P3 minor (vote)
Target Milestone: 3.6 M3   Edit
Assignee: Kent Johnson CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-15 12:17 EDT by Michael CLA
Modified: 2009-10-27 02:12 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael CLA 2009-09-15 12:17:26 EDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3 (.NET CLR 3.5.30729)
Build Identifier: M20090211-1700

Under Preferences->Java->Compiler->Errors/Warnings->GenericTypes is an option named "Generic type parameter declared with a final type bound:" and it is set to warning by default. 

There is no reason this should be a warning much less should it be on by default. Java generic bounds specifically include the type of the bound itself as well as all sub-types. There is no reason for a programmer to be warned when they are using the language properly and as it was intended. 

Reproducible: Always

Steps to Reproduce:
1. Create a final class. 
2. Create a bounded parameter type. 
3. Bind the parameter to the final class. 



Of course the user can disable the warning but it can take some time to find where these settings are since the average user will never have to mess with them. Also there is no reason for this warning to exist since the JLS specifically allows this type of behavior. This can be especially confusing for a new Java programmer and may push them away from using generic types properly.
Comment 1 Stephan Herrmann CLA 2009-09-15 18:09:13 EDT
This get's me puzzled. 
Are you indeed thinking of programs like this?

public final class C1 {
  // details omitted
}

public class C2<T extends C1> {
  T someField;
  // more details omitted
}

Let me ask you: what is the advantage of making C2 parameterized
and why should one use T?

Given that "generics" were invented to make classes more generic,
that is, work with various types (and abstract over those types
using a type parameter), how does T make C2 more generic?
The _only_ legal way of using C2 is to write "C2<C1>",
no other parameter will ever be allowed.
Also, the declaration of "T someField" has _no_ other effect than
if it were "C1 someField", T will always be exactly C1.

So, the warning simply tells you: this is overly complicated
with no actual benefit. It's not an error, because as you mention
the code does not violate the JLS, but the type parameter is 
plain useless. Am I missing something?
Comment 2 Michael CLA 2009-09-16 13:40:17 EDT
I hope that isn't what I meant so let me provide my example code, apologies for not doing this with the original post. 

My example is of a generic observer type. 

public interface Observable<T> {
    void addObserver(Observer<? super T> observer);
    void deleteObserver(Observer<? super T> observer);
}

public interface Observer<T> {
    <S extends T> void update(Observable<S> obs, S arg);
}

Now say we have an observable event type: 

public final class ImmutableEvent() {
    public ImmutableEvent(Detail details);
}

So if I want to observe this event I might make an observer as such: 

public class ImmutableEventObserver implements Observer<ImmutableEvent> {
    @Override
    public <S extends ImmutableEvent> void update(Observable<S> obs, S arg) {
        // do stuff here
    }
}

This last declaration of ImmutableEventObserver will bring up this warning and I do believe it is a valid use of generic types.
Comment 3 Kent Johnson CLA 2009-09-16 14:12:09 EDT
> This last declaration of ImmutableEventObserver will bring up this warning and
> I do believe it is a valid use of generic types.

I disagree that is. As Stephan already mentioned, any type variable T that extends a final class is completely redundant.

Instead of declaring :

public interface Observer<T> {
    <S extends T> void update(Observable<S> obs, S arg);
}
public class ImmutableEventObserver implements Observer<ImmutableEvent> {
    public <S extends ImmutableEvent> void update(Observable<S> obs, S arg) {}
}

why not just define it as :

public interface Observer<T> {
    void update(Observable<T> obs, T arg);
}
public class ImmutableEventObserver implements Observer<ImmutableEvent> {
    public void update(Observable<ImmutableEvent> obs, ImmutableEvent arg) {}
}


Since S1 is just T, and S2 cannot possibly be anything but ImmutableEvent, why define more type variables ?

Closing as invalid.
Comment 4 Stephan Herrmann CLA 2009-09-16 17:15:13 EDT
(In reply to comment #2)
> My example is of a generic observer type. 

OK, the examples gives me an idea of what you want to do.

Disclaimer: I'm still convinced the warning is generally a very good idea,
since "in almost all cases" the warning will hint at a bug or stupid code.

Your code may just be the one single exception to this rule, were _ignoring_
the warning may be a good option.

> public interface Observable<T> {
>     void addObserver(Observer<? super T> observer);
>     void deleteObserver(Observer<? super T> observer);
> }
> 
> public interface Observer<T> {
>     <S extends T> void update(Observable<S> obs, S arg);
> }

OK, assuming that these classes might be part of a library they don't
look suspicios.

> Now say we have an observable event type: 
> 
> public final class ImmutableEvent() {
>     public ImmutableEvent(Detail details);
> }

OK, final classes exist in real life, too. Still nothing suspicous.

> So if I want to observe this event I might make an observer as such: 
> 
> public class ImmutableEventObserver implements Observer<ImmutableEvent> {
>     @Override
>     public <S extends ImmutableEvent> void update(Observable<S> obs, S arg) {
>         // do stuff here
>     }
> }

OK, this can be interpreted as freezing the genericity of a super class.
Interesting situation I'd say. Now, taking Kent's suggestion a little further,
we might try to get rid of S since we know exactly what it stands for.

It _might_ be interesting to find out where exactly the JLS forbids this:

public class ImmutableEventObserver implements Observer<ImmutableEvent> {
    @Override
    public void update(Observable<ImmutableEvent> obs, ImmutableEvent arg) {
        // do stuff here
    }
}

Since we don't need the type parameter S any more, it would be neat if
we could just drop it. But both ecj and javac complain that this variant
of update() does _not_ override its abstract super.


> I do believe it is a valid use of generic types.

Nobody ever claimed that this is invalid. Otherwise the compiler
would have reported an error, not a warning.

Still: the warning is not to blame here, my comments are just after-thoughts.
Comment 5 Michael CLA 2009-09-16 19:40:35 EDT
(In reply to comment #3)
> Instead of declaring :
> 
> public interface Observer<T> {
>     <S extends T> void update(Observable<S> obs, S arg);
> }
> public class ImmutableEventObserver implements Observer<ImmutableEvent> {
>     public <S extends ImmutableEvent> void update(Observable<S> obs, S arg) {}
> }
> 
> why not just define it as :
> 
> public interface Observer<T> {
>     void update(Observable<T> obs, T arg);
> }
> public class ImmutableEventObserver implements Observer<ImmutableEvent> {
>     public void update(Observable<ImmutableEvent> obs, ImmutableEvent arg) {}
> }
> 
> 
> Since S1 is just T, and S2 cannot possibly be anything but ImmutableEvent, why
> define more type variables ?
> 
> Closing as invalid.

Well if I defined

public class DbEventObserver implements Observer<DbEvent> {
     public <S extends DbEvent> void update(Observable<S> obs, S arg) {}
}

and I did not use a bounded generic type then I could not do the following: 

public class MoreSpecificDbEvent extends DbEvent { }

MoreSpecificDbEvent reallySpecificEvent = new MoreSpecificDbEvent();
Observable<SpecificDbEvent> specificObservable = new ObservableImpl<SpecificDbEvent>();
DbEventObserver.update(specificObservable, reallySpecificEvent);

and thus my interface would become not as useful. 

I can't imagine a situation where this warning does anything other than correct a user that does now know how to use Java... The example in comment #2 is a bit absurd and a small warning will not help enough because if a developer is lost enough to write such code... well the rest of their code must not be very good.

It's just that I've been using generics a lot in my libraries and my programs and I've never seen this warning (I've seen lots of useful Java warnings) until now and I can't imagine this warning being anything other than a "bad design" warning and does not suggest any runtime issues (like a raw type warning would).
Comment 6 Kent Johnson CLA 2009-09-17 14:08:25 EDT
Sorry Michael but can you please provide a more complete example of the code in comment 5.


From comment #2, I see no reason for the type variable S :

public interface Observer<T> {
    <S extends T> void update(Observable<S> obs, S arg);
}


So if you have a better complete example that would help.
Comment 7 Srikanth Sankaran CLA 2009-10-27 02:12:23 EDT
Verified for 3.6M3