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.
| Summary: | InstantiatingIterator takes a type parameter but it can only be WizardDescriptor | ||
|---|---|---|---|
| Product: | platform | Reporter: | Jesse Glick <jglick> |
| Component: | Dialogs&Wizards | Assignee: | Jesse Glick <jglick> |
| Status: | RESOLVED FIXED | ||
| Severity: | blocker | CC: | jtulach |
| Priority: | P4 | Keywords: | API |
| Version: | 6.x | ||
| Hardware: | All | ||
| OS: | All | ||
| Issue Type: | DEFECT | Exception Reporter: | |
| Bug Depends on: | |||
| Bug Blocks: | 92762 | ||
| Attachments: |
Suggested patch
Usage of InstantiatingIterator<StringBuilder> 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 |
||
Problem introduced in 6.0. Created attachment 74619 [details]
Suggested patch
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. Created attachment 77962 [details]
Usage of InstantiatingIterator<StringBuilder>
I think the types are not bad. I can write InstantiatingIterator<StringBuilder> and everything compiles OK and seems to work. I suggest "won'tfix". 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. 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
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. "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. Improving Javadoc at least, not sure what else can be done now: core-main #6343aff04895 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. 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. It's not just for file and project templates, but also for Java platforms, etc. |
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.)