Hi, Currently CheckStyle raises an error due to an "unused import" if a class imports types which are only referenced in JavaDoc comments. This issue occurs for instance when referring to a super type in the comments while only sub-types are used in the actual code: /** * This factory creates {@link Service} objects. */ public class ServiceFactory { FooService getFooService() { ... } } Another example is "high-level" documentation on a central type of an API (e.g. its entry point) which refers to types actually used by specific parts of the API but not the entry point itself. In that case it can still make sense to mention these types in the high-level docs. To work around the issue one could use the FQN in the JavaDoc or just format it as {@code}, but both makes up for less readable documentation IMO. Personally I don't see a problem with this kind of import and thus suggest to loosen that CS rule accordingly (it can be configured to take JavaDocs into account). WDYT? --Gunnar _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

Hi, Personally I prefer to include a class via fully qualified name if it is only used in the javadocs. I think the readability does not suffer too much and adding an actual import has actually runtime consequences. We already had cases where a javadoc import caused a hard link between code which is otherwise decoupled.

Even the check stye documentation recommends against it in the configuration of Unused Imports - http://checkstyle.sourceforge.net/config_imports.html#UnusedImports. --Hardy On 31 Jan 2013, at 9:33 AM, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: > Hi, > > Currently CheckStyle raises an error due to an "unused import" if a class > imports types which are only referenced in JavaDoc comments. This issue > occurs for instance when referring to a super type in the comments while > only sub-types are used in the actual code: > > /** > * This factory creates {@link Service} objects. > */ > public class ServiceFactory { > > FooService getFooService() { ... } > } > > Another example is "high-level" documentation on a central type of an API > (e.g. its entry point) which refers to types actually used by specific > parts of the API but not the entry point itself. In that case it can still > make sense to mention these types in the high-level docs. > > To work around the issue one could use the FQN in the JavaDoc or just > format it as {@code}, but both makes up for less readable documentation IMO. > > Personally I don't see a problem with this kind of import and thus suggest > to loosen that CS rule accordingly (it can be configured to take JavaDocs > into account). WDYT? > > --Gunnar > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev

On 31 July 2013 09:41, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: > 2013/7/31 Hardy Ferentschik <hardy(a)hibernate.org&gt; > >> Hi, >> >> Personally I prefer to include a class via fully qualified name if it is >> only used in the javadocs. >> I think the readability does not suffer too much and adding an actual >> import has actually >> runtime consequences. We already had cases where a javadoc import caused a >> hard link >> between code which is otherwise decoupled. >> > > WDYM exactly by "hard link" in this context? Is it about referencing a type > from an optional dependency which might not be present at runtime? > > I just tried out this scenario (adding an import statement just for JavaDoc > to a type which is not present at runtime) and still could execute the > importing class without problems. Only when accessing the imported type in > the actual code I'm getting a CNFE. But this might be specific to the VM in > use, not sure. It is VM specific indeed, and sensitive to situation as to prevent circularity reference loading it never loads all imports aggressively, but some might depending on the moon phase :-) AFAIR IBM's JVM eagerly verifies class definition availability (but does not necessarily initialize the class), and it wouldn't surprise me even hotspot's behaviour could depend on specific situations and/or obscure flags.. I don't really know how to write a comprehensive test to showcase this but let's admit a simple test isn't enough to cover all situations.

I'd expect more recent compilers to remove such dead imports, but given it's not a big deal to remove these in the sources I'd rather be safe: we had problems with such imports in the past. I guess ideally we'd want IDEs to warn about it, I haven't seen an option for this (other than checkstyle integration plugins).

Sanne

> > So if you guys are concerned about this case then let's better keep it as > is. > > > >> >> Even the check stye documentation recommends against it in the >> configuration >> of Unused Imports - >> http://checkstyle.sourceforge.net/config_imports.html#UnusedImports. >> >> --Hardy >> >> >> On 31 Jan 2013, at 9:33 AM, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: >> >> > Hi, >> > >> > Currently CheckStyle raises an error due to an "unused import" if a class >> > imports types which are only referenced in JavaDoc comments. This issue >> > occurs for instance when referring to a super type in the comments while >> > only sub-types are used in the actual code: >> > >> > /** >> > * This factory creates {@link Service} objects. >> > */ >> > public class ServiceFactory { >> > >> > FooService getFooService() { ... } >> > } >> > >> > Another example is "high-level" documentation on a central type of an API >> > (e.g. its entry point) which refers to types actually used by specific >> > parts of the API but not the entry point itself. In that case it can >> still >> > make sense to mention these types in the high-level docs. >> > >> > To work around the issue one could use the FQN in the JavaDoc or just >> > format it as {@code}, but both makes up for less readable documentation >> IMO. >> > >> > Personally I don't see a problem with this kind of import and thus >> suggest >> > to loosen that CS rule accordingly (it can be configured to take JavaDocs >> > into account). WDYT? >> > >> > --Gunnar >> > _______________________________________________ >> > hibernate-dev mailing list >> > hibernate-dev(a)lists.jboss.org >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> >> > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

On 31 Jan 2013, at 10:41 AM, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: > Personally I prefer to include a class via fully qualified name if it is only used in the javadocs. > I think the readability does not suffer too much and adding an actual import has actually > runtime consequences. We already had cases where a javadoc import caused a hard link > between code which is otherwise decoupled. > > WDYM exactly by "hard link" in this context? Is it about referencing a type from an optional dependency which might not be present at runtime? Right, optional dependencies. Take JPA and Search. In the engine module we avoid importing it directly and do processing of @Id reflectively. Having an import of javax.persitence.Id would create a runtime dependency. > I just tried out this scenario (adding an import statement just for JavaDoc to a type which is not present at runtime) and still could execute the importing class > without problems. Only when accessing the imported type in the actual code I'm getting a CNFE. But this might be specific to the VM in use, not sure. We had this discussion now several times. I remember when we wrote this reflection code the first time (many years back ;-)) all referenced classes were eagerly loaded. In this case just having the import statement required the class being available at runtime. Obviously this must have changed a bit which also is proven by your experiment.

Does the concept of imports apply at runtime at all, or is this concept not only present at compile time, making life easier by allowing to refer to types using their simple name? In [1] it says about type names in class files: "Class and interface names that appear in class file structures are always represented in a fully qualified form known as binary names". I also can't see any part of the class file structure which would hold information about imports. So when an imported type is not used in the actual code, I don't think it will be referenced in the resulting class file and thus can't cause any issues when not being present. [1] http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.2.1 2013/8/1 Emmanuel Bernard <emmanuel(a)hibernate.org&gt; > > On 31 juil. 2013, at 13:14, Hardy Ferentschik wrote: > > > > > On 31 Jan 2013, at 10:41 AM, Gunnar Morling <gunnar(a)hibernate.org&gt; > wrote: > > > >> Personally I prefer to include a class via fully qualified name if it > is only used in the javadocs. > >> I think the readability does not suffer too much and adding an actual > import has actually > >> runtime consequences. We already had cases where a javadoc import > caused a hard link > >> between code which is otherwise decoupled. > >> > >> WDYM exactly by "hard link" in this context? Is it about referencing a > type from an optional dependency which might not be present at runtime? > > > > Right, optional dependencies. Take JPA and Search. In the engine module > we avoid importing it directly and do processing of > > @Id reflectively. Having an import of javax.persitence.Id would create > a runtime dependency. > > > >> I just tried out this scenario (adding an import statement just for > JavaDoc to a type which is not present at runtime) and still could execute > the importing class > >> without problems. Only when accessing the imported type in the actual > code I'm getting a CNFE. But this might be specific to the VM in use, not > sure. > > > > We had this discussion now several times. I remember when we wrote this > reflection code the first time (many years back ;-)) > > all referenced classes were eagerly loaded. In this case just having the > import statement required the class being > > available at runtime. Obviously this must have changed a bit which also > is proven by your experiment. > > > > Of course my memory might be flawed but the problems I recall around > isolating a piece of code for hv, search and later bean validation from > core did not involve unused imports. Come to think of it, I have never seen > an *unused* import eagerly loading a class even though in practice it's > left unspecified by the JLS. > > > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

So why not be safe than sorry? --Hardy

Of course my memory might be flawed but the problems I recall around isolating a piece of code for hv, search and later bean validation from core did not involve unused imports. Come to think of it, I have never seen an *unused* import eagerly loading a class even though in practice it's left unspecified by the JLS.