Bug 319626

Summary: Preferences->Java Compiler-> Errors/Warnings -> Undocumented Empty Block
Product: [Eclipse Project] JDT Reporter: Daniel Holmes <dholmes>
Component: CoreAssignee: Srikanth Sankaran <srikanth_sankaran>
Status: VERIFIED FIXED QA Contact:
Severity: enhancement    
Priority: P3 CC: amj87.iitr, jarthana, markus.kell.r, Olivier_Thomann, srikanth_sankaran
Version: 3.7   
Target Milestone: 3.7 M2   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Possible patch none

Description Daniel Holmes CLA 2010-07-12 15:02:35 EDT
Build Identifier: 20100218-1602

Would be nice for this check to be able to ignore default constructors which are generally empty blocks which really require no comment.

Reproducible: Always
Comment 1 Srikanth Sankaran CLA 2010-07-12 22:39:47 EDT
I'll investigate this.
Comment 2 Srikanth Sankaran CLA 2010-07-14 01:02:16 EDT
Olivier, this seems like a reasonable request and I would like
to fix this, let me know if I am overlooking something here.
Comment 3 Srikanth Sankaran CLA 2010-07-14 01:04:54 EDT
Created attachment 174258 [details]
Possible patch
Comment 4 Srikanth Sankaran CLA 2010-07-14 01:10:43 EDT
Jay, please review, TIA.
Comment 5 Jay Arthanareeswaran CLA 2010-07-14 02:31:31 EDT
Patch looks good.
Comment 6 Olivier Thomann CLA 2010-07-14 09:37:55 EDT
This would always ignore this warning for constructors.
I would prefer a flag to do this so that it can be enabled for the users who always want it to be reported.
So +1 for the idea with an option to configure it.

Why do you also ignore the warning for method with no parameters ?
Comment 7 Srikanth Sankaran CLA 2010-07-14 23:35:29 EDT
(In reply to comment #6)
> This would always ignore this warning for constructors.
> I would prefer a flag to do this so that it can be enabled for the users who
> always want it to be reported.
> So +1 for the idea with an option to configure it.

Markus, will JDT/UI be willing & interested in supporting this ? 

> Why do you also ignore the warning for method with no parameters ?

Not sure if you meant "why do you ... " or "why don't you ..."

The patch does not alter the behavior w.r.t to empty methods
with no parameters - i.e we will continue to flag them as having
undocumented empty block. In fact the new regression test 
org.eclipse.jdt.core.tests.compiler.regression.NonFatalErrorTest.test006()
tests for this.

We are making this special allowance only for programmer supplied
no-arg empty constructors because the language forces you to supply
one in some cases (viz if you have some other constructor in your class
but wish to allow a subclass to have a no-arg constructor.)  We don't
want to extend the allowance to other methods where the programmer is
not similarly constrained to supply a (dummy/stub) method.
Comment 8 Srikanth Sankaran CLA 2010-08-04 09:01:08 EDT
(In reply to comment #7)
> (In reply to comment #6)
> > This would always ignore this warning for constructors.
> > I would prefer a flag to do this so that it can be enabled for the users who
> > always want it to be reported.
> > So +1 for the idea with an option to configure it.
> 
> Markus, will JDT/UI be willing & interested in supporting this ? 

Markus, have you had a chance to look into this ? TIA.
Comment 9 Markus Keller CLA 2010-08-04 09:47:30 EDT
I agree with Srikanth. This is so minor that I don't want to add yet another preference. We could rename the option to "Unforced undocumented empty block", but I think you should just release the patch.

BTW: I find (!(this.isConstructor() && this.arguments == null)) easier to understand than (!this.isConstructor() || this.arguments != null)
Comment 10 Srikanth Sankaran CLA 2010-08-04 11:02:26 EDT
(In reply to comment #9)
> I agree with Srikanth. This is so minor that I don't want to add yet another
> preference. We could rename the option to "Unforced undocumented empty block",
> but I think you should just release the patch.
> 

OK, thanks for weighing in Markus, I'll release it next week
after M1 is out of the door.
Comment 11 Srikanth Sankaran CLA 2010-08-08 23:55:41 EDT
Released in HEAD for 3.7 M2.
Comment 12 Ayushman Jain CLA 2010-09-14 06:59:33 EDT
Verified for 3.7M2 using build I20100909-1700.