Bug 359721 - [options] add command line option for new warning token "resource"
Summary: [options] add command line option for new warning token "resource"
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.8   Edit
Hardware: PC Linux
: P3 enhancement (vote)
Target Milestone: 3.8 M3   Edit
Assignee: Stephan Herrmann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-03 11:31 EDT by Stephan Herrmann CLA
Modified: 2011-10-25 04:10 EDT (History)
3 users (show)

See Also:
amj87.iitr: review+


Attachments
proposed fix (6.21 KB, patch)
2011-10-03 12:12 EDT, Stephan Herrmann CLA
no flags Details | Diff
proposed doc update (5.13 KB, patch)
2011-10-18 11:58 EDT, Stephan Herrmann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Herrmann CLA 2011-10-03 11:31:34 EDT
The new warnings from bug 349326 can be suppressed in the code using
  @SuppressWarnings("resource")
It would be good if these warnings can also be configured from the
command line, thus I propose to support "resource" in:
  -warn:resource
  -warn:-resource
  -warn:+resource
Comment 1 Ayushman Jain CLA 2011-10-03 11:44:37 EDT
Please also update the batch compiler documentation when the options are added. This is located in org.eclipse.jdt.doc.user/tasks/task-using_batch_compiler.htm (still in CVS). Thanks!
Comment 2 Stephan Herrmann CLA 2011-10-03 12:12:44 EDT
Created attachment 204459 [details]
proposed fix

Could someone please comment on the wording:

ecj -help:warn:

....
resource           + (potential) leaks of resources of type Closeable
Comment 3 Stephan Herrmann CLA 2011-10-03 12:16:17 EDT
someone ate part of my comment, here's the rest:

We could also say "(potentially) unsafe usage of resources of type Closeable"
which would better capture also the warning "should be managed with t-w-r"

Other suggestions?

Another observation: message.properties does not mention
"redundantSuperinterface", what happended there?
Comment 4 Ayushman Jain CLA 2011-10-03 12:28:40 EDT
(In reply to comment #3)
> someone ate part of my comment, here's the rest:
> 
> We could also say "(potentially) unsafe usage of resources of type Closeable"
> which would better capture also the warning "should be managed with t-w-r"

How about "(potential) leakage/unsafe usage of resource of type Closeable"

> 
> Another observation: message.properties does not mention
> "redundantSuperinterface", what happended there?

It seems like "intfRedundant" now handles that warning.
Comment 5 Stephan Herrmann CLA 2011-10-03 13:05:13 EDT
(In reply to comment #4)
> (In reply to comment #3)
> > someone ate part of my comment, here's the rest:
> > 
> > We could also say "(potentially) unsafe usage of resources of type Closeable"
> > which would better capture also the warning "should be managed with t-w-r"
> 
> How about "(potential) leakage/unsafe usage of resource of type Closeable"

that'd cover it all but it sounds a bit complicated to me :)

What's the precedence? Is it
  ( potential (leakage/unsafe usage) )
or
  ((potential) leakage) / unsafe usage
:-/

> > Another observation: message.properties does not mention
> > "redundantSuperinterface", what happended there?
> 
> It seems like "intfRedundant" now handles that warning.

I see, so redundantSuperinterface is just handled for compatibility,
no longer documented because it shouldn't be used anymore?
Makes sense.

(In reply to comment #1)
> Please also update the batch compiler documentation when the options are added.
> This is located in org.eclipse.jdt.doc.user/tasks/task-using_batch_compiler.htm
> (still in CVS). Thanks!

For the records: also "Excluding warnings" needs an update.
Comment 6 Stephan Herrmann CLA 2011-10-18 08:13:12 EDT
(In reply to comment #1)
> Please also update the batch compiler documentation when the options are added.
> This is located in org.eclipse.jdt.doc.user/tasks/task-using_batch_compiler.htm

That page wants a "-" or "+" to indicate the default, however, the new
token "resource" is an umbrella for 3 warnings. By default one of them
is "warning" the other two are "ignore". Now, how do I document the
default for "resource"?
Comment 7 Stephan Herrmann CLA 2011-10-18 11:58:06 EDT
Created attachment 205436 [details]
proposed doc update

Here's a proposed doc update which marks several option defaults as "+/-"
and gives according explanation.

Also, all-static-method was missing from the list. Fixed.

What do you think?
Comment 8 Ayushman Jain CLA 2011-10-19 04:09:29 EDT
Looks good. Thanks for fixing the other missing pieces in the doc as well. :)

Btw, this patch looks like its coming from the CVS repo. Isn't this doc project now in o.e.platform.common.git?
Comment 9 Ayushman Jain CLA 2011-10-20 01:54:07 EDT
Stephan please go ahead and release the change. Thanks!
Comment 10 Stephan Herrmann CLA 2011-10-20 05:03:32 EDT
Released for 3.8 M3:
JDT/Core commit cb924aa1e344454d0d95de687a37524ed808452e
jdt.doc.user commit b938d41e8feb948d7cb2afc5b3027f939688a119
Comment 11 Ayushman Jain CLA 2011-10-25 04:10:58 EDT
Verified batch compiler and doc for 3.8M3 using build N20111022-2000.