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: | @NamedServiceDefinition | ||
|---|---|---|---|
| Product: | platform | Reporter: | Jaroslav Tulach <jtulach> |
| Component: | Lookup | Assignee: | Jaroslav Tulach <jtulach> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | apireviews, jglick |
| Priority: | P3 | Keywords: | API_REVIEW_FAST |
| Version: | 7.2 | ||
| Hardware: | All | ||
| OS: | All | ||
| Issue Type: | DEFECT | Exception Reporter: | |
| Bug Depends on: | |||
| Bug Blocks: | 200636 | ||
| Attachments: |
API, test and usage
Accepting most of the comments |
||
|
Description
Jaroslav Tulach
2012-03-20 10:00:42 UTC
Y01 wrong issue number, should be 209780 [JG01] searchAnnotations, findPath, etc. seem much too magical. While this issue is an interesting idea, I would recommend just exposing something like AbstractServiceProviderProcessor as an SPI, permitting subclasses to interpret various annotation attributes (and perform domain-specific validation) however they like, just as we do for LayerGeneratingProcessor. Other comments follow under the assumption you will reject this advice and choose the more complicated and less flexible approach. [JG02] Javadoc for ServiceProvider.path should recommend the new API as a replacement. [JG03] Javadoc is unclear whether @location or @location() is used. [JG04] Javadoc is unclear how a String[] in the path is substituted (all variants registered?), or what a non-singleton Class[] for the serviceType would mean (disjunction?). [JG05] AbstractServiceProviderProcessor should be moved into a (truly) private package, in which case the subclass checks are unnecessary. [JG06] Stray System.err.println. [JG07] Default value for position() attr may be unfriendly to Javadoc etc. Just use "-" for example (it cannot be an identifier). Or simply make "" be the default; I think it is no burden to specify position="position" if you have such an attribute and want to use it. [JG08] Do not throw ISE from a processor. Use Messager. [JG09] Do not use e.getAnnotation(c) & nsd.serviceType() as the types involved might not be loadable from the processing environment. 269 has more general APIs for inspecting Class-valued annotation members. Created attachment 117186 [details]
Accepting most of the comments
Re. JG01 - yes, I'd rather prefer declarative approach to Turing completeness.
Implemented: JG02, JG03, JG04, JG06, JG07. As for the JG08, I've added more checks during processing of @NamedServiceDeclaration usage and I report errors using messager. If the same errors occur later during processing of the annotated annotations, I consider that (internal) error and still throw ISE.
Re. JG05 - that would be an incompatible change for openide.util. People might face it when using new version of openide.util.lookup and old openide.util. I'd rather keep the class where it is at least for one version.
Re. JG09 - I know there is AnnotationMirror, but I am not sure why it would be beneficial to use it in this case. If you can show me a sample that fails and requires advanced usage of annotations, I'll use them.
I'd like to integrate tomorrow. JG09 - not sure if it could cause a compilation failure, but might cause problems with in-IDE annotation processing. jlahoda would be able to tell you more precisely. FWIW, known places where @SP.path is currently used but @NSD could be used instead: ant.grammar/build/classes/META-INF/namedservices/Plugins/XML/GrammarQueryManagers/org.netbeans.modules.xml.api.model.GrammarQueryManager autoupdate.pluginimporter/build/classes/META-INF/namedservices/WarmUp/java.lang.Runnable core.ui/build/classes/META-INF/namedservices/WarmUp/java.lang.Runnable hudson.maven/build/classes/META-INF/namedservices/Plugins/XML/UserCatalogs/org.netbeans.modules.xml.catalog.spi.CatalogReader hudson.tasklist/build/classes/META-INF/namedservices/TaskList/Scanners/org.netbeans.spi.tasklist.PushTaskScanner java.editor/build/classes/META-INF/namedservices/WarmUp/java.lang.Runnable Not clear whether bug #200636 will cover the WarmUp/java.lang.Runnable cases. GrammarQueryManagers, UserCatalogs, and TaskList/Scanners are odd cases because there is no apparent reason why these are not just in Lookup.getDefault (i.e. null path); probably for historical reasons. Integrated into 'main-golden', will be available in build *201203271056* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/c78ef1210520 User: Jaroslav Tulach <jtulach@netbeans.org> Log: #209780: Initial implementation including comments JG02, JG03, JG04, JG06, JG07 Integrated into 'main-golden', will be available in build *201204040400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/ec301ca4a477 User: Jaroslav Tulach <jtulach@netbeans.org> Log: #209780: Remind (well currently enforce) people to use @Retention and @Target |