Summary: | Potential JDT Core bugs with associated (failed) JUnit test! | ||
---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Sai <szhang> |
Component: | Core | Assignee: | Srikanth Sankaran <srikanth_sankaran> |
Status: | VERIFIED INVALID | QA Contact: | |
Severity: | normal | ||
Priority: | P3 | CC: | Olivier_Thomann, srikanth_sankaran |
Version: | 3.7 | ||
Target Milestone: | 3.7 M2 | ||
Hardware: | PC | ||
OS: | Linux | ||
Whiteboard: |
Description
Sai
2010-08-18 14:47:39 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. 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. (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. Verified for 3.7M2 using I20100914-0100 |