Bug 201254

Created attachment 110162 [details]
Proposed patch

Missing Javadoc for additions in CodeStyle.

Why would you use addImports rather than the existing fixImports, which seems more convenient? What is the use case?

Created attachment 110176 [details]
Patch with the Javadoc added

GeneratorUtilities.addImports is not introduced to replace the FixImports or any other higher-level feature managing imports, but rather to fix the current situation where the FixImports and Hints use the non-API JavaFixAllImports.addImports method to add necessary import statements to a compilation unit which is the exact copy of the non-API SourceUtils.addImport method used by the Code Completion and Code Templates.

GeneratorUtilities.addImports is introduced as a low-level API method that should be used by ALL higher-level editor features (FixImports, Hints, Code Completion, Code Templates, etc) to add necessary import statements into a compilation unit. In contrast to the original non-API methods, it should follow the rules specified in CodeStyle (like whether to use single class imports or package imports, whether and how to group imports, how to order the import groups, etc).

Perhaps my question was not clear, since I gave the wrong method name originally.

There is already a public API GeneratorUtilities.importFQNs which (as far as I can tell) works fine to add import statements to the source file, usable from various features (editor hint fixes, etc.) which insert code blocks defined using TreeMaker with fully-qualified identifiers.

So is there something wrong with importFQNs that requires a new method? (addImports looks like it would be more work to use correctly - you not only need to enumerate everything you want imported, which importFQNs would handle automatically, but you need to rewrite the entire CompilationUnitTree, presumably using TreeUtilities.translate.)

Or are the proposed uses for addImports of a different nature than the existing uses of importFQNs?

There is nothing wrong with TreeMaker.QualIdent/Type and GeneratorUtilities.importFQNs (all internally based on the QualIdent). In fact, these methods methods are preferred over SourceUtils.resolveImport or the new GeneratorUtilities.addImports and should be used by vast majority of features. The QualIdent-based methods should always do the "right thing", so that the correct class is referred in the source. There are, however, some features that cannot use the QualIdent-based methods, like the add import hint (where the user explicitly said that a particular import should be added). The SourceUtils.resolveImport and GeneratorUtilities.addImports are intended only for these special cases.

I think this should be clarified in the javadoc of these methods. What about:
<p><strong>Use TreeMaker.QualIdent, TreeMaker.Type or GeneratorUtilities.importFQNs instead of this method if possible. These methods will correctly resolve imports according to the user's preferences.</strong></p>

I also suggest to deprecate the TreeMaker.{add|insert}CompUnitImport, as I see no valid use case for these methods, even though there are several uses of them across the codebase.

(In reply to comment #6)
> <p><strong>Use TreeMaker.QualIdent, TreeMaker.Type or
> GeneratorUtilities.importFQNs instead of this method if possible. These methods
> will correctly resolve imports according to the user's
> preferences.</strong></p>

Thanks, that would certainly make the situation clearer.

> deprecate the TreeMaker.{add|insert}CompUnitImport

Perhaps this should be left alone as a low-level structure manipulation API, with a clear warning in the Javadoc that typical code should be using addImports or importFQNs instead.

Integrated into in jet-main.

http://hg.netbeans.org/jet-main/rev/c604329f2888