Bug 323078 - Potential JDT Core bugs with associated (failed) JUnit test!
Summary: Potential JDT Core bugs with associated (failed) JUnit test!
Status: VERIFIED INVALID
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 3.7 M2   Edit
Assignee: Srikanth Sankaran CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-18 14:47 EDT by Sai CLA
Modified: 2010-09-14 10:15 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sai CLA 2010-08-18 14:47:39 EDT
Build Identifier: ecj 3.4.2

Hi all:

I am now writing an automated test generation tool for Java, and use jdt core 3.4.2 (latest release) as one of the experimental benchmarks.


According to the tool design and its internal checking mechanism, it generates many failing tests.
I attach 1 (executable failing tests in JUnit form) in attachment, and am wondering could you
please have a look at that, to check whether they are real bugs or not?

If it is regarded as a bug, I have many similar one over different methods in jdt core, and would be happy to send these tests to you.

The  test presented in the attachments violates one of the common Java code practices (at least in my viewpoint):
if all the inputs are not null, there should not be null pointer exception thrown. 

The Junit tests attached are fully automatically generated, thus, the readability might not be so good. sorry...
But they are generally relatively short.

Thanks very much for your time! And any suggestion to the generated tests (including format and other aspects)
are welcome!

Best


-Sai


Reproducible: Always

Steps to Reproduce:
Please run the JUnit tests below:


  public void test285() throws Throwable {
    char[] var0 = new char[] { };
    org.eclipse.jdt.internal.compiler.batch.CompilationUnit var4 = new org.eclipse.jdt.internal.compiler.batch.CompilationUnit(var0, "hi!", "hi!", "hi!");
    char[] var8 = var4.getMainTypeName();
    org.eclipse.jdt.internal.compiler.CompilationResult var12 = new org.eclipse.jdt.internal.compiler.CompilationResult(var8, 0, 0, 0);
    org.eclipse.jdt.internal.compiler.ast.TypeDeclaration var13 = new org.eclipse.jdt.internal.compiler.ast.TypeDeclaration(var12);
    char[] var14 = new char[] { };
    org.eclipse.jdt.internal.compiler.batch.CompilationUnit var18 = new org.eclipse.jdt.internal.compiler.batch.CompilationUnit(var14, "hi!", "hi!", "hi!");
    org.eclipse.jdt.internal.compiler.CompilationResult var28 = new org.eclipse.jdt.internal.compiler.CompilationResult((org.eclipse.jdt.internal.compiler.env.ICompilationUnit)var18, 0, 1, 100);
    try {
      org.eclipse.jdt.internal.compiler.ClassFile.createProblemType(var13, var28);
    } catch (java.lang.NullPointerException e) {
      fail("Statement throw java.lang.NullPointerException.");
    }
  }
Comment 1 Srikanth Sankaran CLA 2010-08-18 20:37:34 EDT
(In reply to comment #0)
> Build Identifier: ecj 3.4.2
> 
> Hi all:
> 
> I am now writing an automated test generation tool for Java, and use jdt core
> 3.4.2 (latest release) as one of the experimental benchmarks.

The latest release is 3.6.

> wondering could you
> please have a look at that, to check whether they are real bugs or not?

No, this is not a real bug.

> The  test presented in the attachments violates one of the common Java code
> practices (at least in my viewpoint):
> if all the inputs are not null, there should not be null pointer exception
> thrown. 

While the input parameters themselves are not null, most of the object
state is null.

See that most of the methods and types you are using are internal and
not API. They are being used in ways completely unintended. 

There is no reasonable reaction to an object's internal state containing
numerous null references - we cannot check for an NPE on every instance
field of every object touched by a method and even if we were to, we
could at best throw an NPE which the VM would do for free.

What we would be interested in are reports about (a) bugs where eclipse is
used as a black box (as IDE or command line compiler) (b) bugs where a
published API behaves contrary to its documented behavior.
Comment 2 Sai CLA 2010-08-19 15:44:28 EDT
Thanks srikanth for your quick reply.


> 
> > The  test presented in the attachments violates one of the common Java code
> > practices (at least in my viewpoint):
> > if all the inputs are not null, there should not be null pointer exception
> > thrown. 
> 
> While the input parameters themselves are not null, most of the object
> state is null.


yes. I agree that most of the object state is null, which leads to the test crash. However, isn't it the case that the code should check its validity? I notice that in many places of jdt core, there are null pointer checks to prevent illegal state (such null) to propogate through the program.

Personally, I think the illegal state should be identified and stopped as soon as possible (e.g. in the constructor), instead of handling that "lazily" up to the crash point. I admit that may lead to more efforts to write checking code. What do you think about on this point?


> 
> See that most of the methods and types you are using are internal and
> not API. They are being used in ways completely unintended. 

In the tool design, we only call public methods on public classes. Thus, we treat any visible methods as valid program entry points. 

I suggest that, if the method or types are used internally, their visibility should reflect this. e.g. making them package visible or private.

Sorry that I am not quite understand what an API mean here? is not it represent a client visible public method? or the method explicitly documented? I think the first may make more sense. In another presepctive, if it is too hard to check every points of the program for validity, the best way might be preventing clients to do the potentially illegal call (like the tests). Many system software does in this way. 




> 
> There is no reasonable reaction to an object's internal state containing
> numerous null references - we cannot check for an NPE on every instance



> field of every object touched by a method and even if we were to, we
> could at best throw an NPE which the VM would do for free.
> 
> What we would be interested in are reports about (a) bugs where eclipse is
> used as a black box (as IDE or command line compiler) (b) bugs where a
> published API behaves contrary to its documented behavior.

Thanks for your comments. It will help us to improve the tool.
Comment 3 Srikanth Sankaran CLA 2010-08-19 18:35:06 EDT
(In reply to comment #2)
> Thanks srikanth for your quick reply.

> Sorry that I am not quite understand what an API mean here? is not it represent
> a client visible public method? or the method explicitly documented? I think
> the first may make more sense. In another presepctive, if it is too hard to
> check every points of the program for validity, the best way might be
> preventing clients to do the potentially illegal call (like the tests). Many
> system software does in this way. 

In eclipse terminology, any type/method whose fully qualified name includes
the component name "internal" is not public API. There is nothing at the
Java language level to guarantee "limited public access" at this point, so
a type that is public is public to the whole world and hence the evolved
convention that a client isn't to call/use "internal" types and methods.
Comment 4 Olivier Thomann CLA 2010-09-14 10:15:10 EDT
Verified for 3.7M2 using I20100914-0100