This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 154776 - InstantiatingIterator takes a type parameter but it can only be WizardDescriptor
Summary: InstantiatingIterator takes a type parameter but it can only be WizardDescriptor
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Dialogs&Wizards (show other bugs)
Version: 6.x
Hardware: All All
: P4 blocker (vote)
Assignee: Jesse Glick
URL:
Keywords: API
Depends on:
Blocks: 92762
  Show dependency tree
 
Reported: 2008-12-05 15:41 UTC by Jesse Glick
Modified: 2009-03-12 16:34 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Suggested patch (3.78 KB, patch)
2008-12-05 15:46 UTC, Jesse Glick
Details | Diff
Usage of InstantiatingIterator<StringBuilder> (3.53 KB, patch)
2009-03-10 11:04 UTC, Jaroslav Tulach
Details | Diff
java.lang.ClassCastException: org.netbeans.modules.project.ui.NewProjectWizard cannot be cast to java.util.concurrent.atomic.AtomicReference at test154776.brokenWizardPanel.readSettings(brokenWizardPanel.java:12) at org.openide.WizardDesc (6.85 KB, application/x-compressed)
2009-03-10 15:24 UTC, Jesse Glick
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2008-12-05 15:41:35 UTC
WD.II was declared in issue #92762 to take an arbitrary <Data> type parameter. In fact it is expected to be
<WizardDescriptor>, since the real WD.Iterator is predetermined and will be <WD> (using the WD itself as the settings
object).

You can see the mistake by the fact that TemplateWizard.getIterator needs @SuppressWarnings("unchecked") in order to compile

new InstantiatingIteratorBridge((WizardDescriptor.InstantiatingIterator<WizardDescriptor>) unknownIterator)

I have not tried creating an II with some other type parameter, but I assume it would just result in a runtime CCE in a
Panel.read/storeSettings implementation.

Clearly it is too late to remove the type parameter now, since implementations correctly using <WizardDescriptor> would
no longer compile. It would be nice to change its declaration to something like

public interface InstantiatingIterator<Data super WizardDescriptor> extends Iterator<Data> {...}

(which would ensure that you could not pass in some other type which would result in a CCE) but javac seems to reject
this syntax for whatever reason.

We could put a <strong> warning in the Javadoc that the type parameter, if specified, must in fact be WizardDescriptor.
The warning would of course need to be repeated in AsynchronousInstantiatingIterator and ProgressInstantiatingIterator.

A better solution might simply be:

public interface InstantiatingIterator<Ignored> extends Iterator<WizardDescriptor> {...}

i.e. permit 6.x module code to specify a type parameter, but ignore it and require current() to return
Panel<WizardDescriptor>.

On a related note, TW.I has no type param and extends WD.I<WD>. This might also be a mistake, I am not sure - probably
it should extend WD.I<TW>? (So that a panel could use TW methods without a cast in read/storeSettings.)
Comment 1 Jesse Glick 2008-12-05 15:42:32 UTC
Problem introduced in 6.0.
Comment 2 Jesse Glick 2008-12-05 15:46:38 UTC
Created attachment 74619 [details]
Suggested patch
Comment 3 Jesse Glick 2008-12-05 15:48:13 UTC
BTW assigning to you simply because you introduced the original generification and may want to comment on this. Feel
free to reassign to default owner.
Comment 4 Jaroslav Tulach 2009-03-10 11:04:34 UTC
Created attachment 77962 [details]
Usage of InstantiatingIterator<StringBuilder>
Comment 5 Jaroslav Tulach 2009-03-10 11:06:09 UTC
I think the types are not bad. I can write InstantiatingIterator<StringBuilder> and everything compiles OK and seems 
to work. I suggest "won'tfix".
Comment 6 Jesse Glick 2009-03-10 15:19:42 UTC
Maybe you did not read the opening sentences of the original description. Yes your test compiles OK, but it is not
"working" because it is not actually running a wizard. If you tried to connect this WD.II to an actual New wizard, which
is the only purpose of a WD.II, it would throw a CCE.
Comment 7 Jesse Glick 2009-03-10 15:24:37 UTC
Created attachment 77987 [details]
java.lang.ClassCastException: org.netbeans.modules.project.ui.NewProjectWizard cannot be cast to java.util.concurrent.atomic.AtomicReference         at test154776.brokenWizardPanel.readSettings(brokenWizardPanel.java:12)         at org.openide.WizardDesc
Comment 8 Jaroslav Tulach 2009-03-10 16:24:27 UTC
Oh, New Wizard. Yes, TemplateWizard requires Iterator<WizardIterator>, but it reads it from disk, and as such there is 
nothing to type check in this situation.

But it is not reason for restricting InstantiatingIterator, I am quite sure my test is OK - one can use 
InstantiatingIterator<Anything> with regular WizardDescriptor without any problems.

If you can think of a way to early detect that Iterator<?> loaded from "templateWizardIterator" is not of the right 
type, feel free to add the check there. Otherwise, I do not think there is anything to fix in the signatures.
Comment 9 Jesse Glick 2009-03-10 20:04:21 UTC
"one can use InstantiatingIterator<Anything> with regular WizardDescriptor without any problems" - you could, though
this seems to be very rare. I think I found an example in php.rt. There seems to be no purpose in doing so; while
instantiate() will be called, php.rt does not call getInstantiatedObjects, it just expects the side-effect of
instantiate() to be its purpose and the return value is irrelevant. You could more simply just do this action when the
wizard returns FINISH, without using II at all.

The whole point of creating II (or, originally, TW.I) was to serve as a means of communication by which an iterator
implemented in one module could return some objects back to the wizard invoker implemented in another module. For this
case it is senseless to let the II freely choose its Data type, because the iterator is not in control of the creation
of the wizard - it is passively bound to the wizard by foreign code which must choose a generic data type, which in
practice is the WD itself. That is why I still think the signature of II is a mistake.

The Javadoc at least needs to clarify that the type parameter must be WizardDescriptor for the expected use case.
Type-checking the iterator at load time is impossible because we neglected to include RTTI in the interface, i.e. an
abstract Class<Data> type() method.
Comment 10 Jesse Glick 2009-03-10 20:16:57 UTC
Improving Javadoc at least, not sure what else can be done now: core-main #6343aff04895
Comment 11 Quality Engineering 2009-03-11 22:51:53 UTC
Integrated into 'main-golden', will be available in build *200903111543* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/6343aff04895
User: Jesse Glick <jglick@netbeans.org>
Log: #154776: documenting that Data = WizardDescriptor on an instantiating iterator.
Comment 12 Jaroslav Tulach 2009-03-12 09:03:32 UTC
Well, I am not fully comfortable with the new javadoc. Maybe instead of "in practice" you should say that "if used 
from layer as template attribute". Or something like that.
Comment 13 Jesse Glick 2009-03-12 16:34:26 UTC
It's not just for file and project templates, but also for Java platforms, etc.