<AWT Dev> JDK 9 RFR of JDK-8039109 : Fix unchecked and raw lint warnings in java.awt

[Joe Darcy]

Hello,

Any further comments on the latest version

      http://cr.openjdk.java.net/~darcy/8039109.3/

Thanks,

-Joe

On 4/30/2014 4:04 PM, Joe Darcy wrote:
> Hi Rémi,
>
>> Hi Joe,
>>
>> in AWTKeyStroke, instead of
>>
>> Class<AWTKeyStroke> clazz = 
>> (Class<AWTKeyStroke>)AppContext.getAppContext().get(AWTKeyStroke.class);
>>
>> I should have the right type Class<? extends ...> (the class is a
>> subclass of AWTKeyStroke) and
>> do a classcheck at runtime when the class is extracted instead of later
>>
>> Class<? extends AWTKeyStroke> clazz = 
>> ((Class<?>)AppContext.getAppContext().get(AWTKeyStroke.class)).asSubClass(AWTKeyStroke.class);
>
> As I'm not overly familiar with this code and the app context 
> mechanism, in the context of this lint removal exercise, I'd prefer to 
> leave the timing of the checks as they are today.
>
>>
>> and I think the second @SuppressWarnings in getCachedStroke() is
>> unnecessary.
>
> Right you are; I've removed the second @SuppressWarnings in the next 
> iteration of the patch.
>
>>
>>
>> in GraphicsEnvironment, the last line of your diff can be simplified,
>>
>> ge = GraphicsEnvironment.class.cast(geCls.newInstance());
>>
>> can be written
>>
>> ge = (GraphicsEnvironment)geCls.newInstance();
>>
>> which is more readable IMO but will also generate exactly the same
>> bytecode as the current implementation.
>
> Yes, since the type being cast to is unchanging, it is not necessary 
> to do the cast using core reflection; updated in the next iteration.
>
>>
>>
>> in KeyboardFocusManager,
>>
>> private Set<AWTKeyStroke>[] defaultFocusTraversalKeys = new Set[4];
>>
>> should be
>>
>> private Set<AWTKeyStroke>[] defaultFocusTraversalKeys = new Set<?>[4];
>>
>> so @SuppressWarnings("rawtypes") is not needed.
>
> Actually, javac objects to the code you've proposed:
>
> src/share/classes/java/awt/KeyboardFocusManager.java:352: error: 
> incompatible types: Set<?>[] cannot be converted to Set<AWTKeyStroke>[]
>     private Set<AWTKeyStroke>[] defaultFocusTraversalKeys = new 
> Set<?>[4];
>
> (IIRC, I ran into this when I was first putting the changeset together.)
>
>>
>>
>> in DragGestureEvent, the newEvents should be a List<? extends 
>> InputEvent>,
>> and @SuppressWarnings("rawtypes") should be a 
>> @SuppressWarnings("unchecked")
>
> For the purposes of this clean up effort, I'm not eager to take on 
> more extensive updates to DragGestureEvent than adding the one 
> @SuppressWarnings. I would encourage the awt team to consider the 
> update you've suggested.
>
>>
>> in RenderableImageOp,
>> getRenderableSources() should return a Vector of RenderableImage,
>>
>>        public Vector<RenderableImage> getSources() {
>>           getRenderableSources();
>>        }
>>           private Vector<RenderableImage> getRenderableSources() {
>>           Vector<RenderableImage sources = null;
>
> Petr made the same observation and I've addressed that in the second 
> iteration of the patch:
>
>     http://cr.openjdk.java.net/~darcy/8039109.2/
>
>>
>> all other modifications are OK for me.
>
> Newest iteration at:
>
>     http://cr.openjdk.java.net/~darcy/8039109.3/
>
> Thanks for the review,
>
> -Joe
>