Bug 201912 - [compiler] Unused public members of private classes not flagged
Summary: [compiler] Unused public members of private classes not flagged
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.5 M6   Edit
Assignee: Srikanth Sankaran CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-08-31 10:13 EDT by Markus Keller CLA
Modified: 2009-03-10 07:38 EDT (History)
4 users (show)

See Also:


Attachments
Proposed fix and tests (23.80 KB, patch)
2009-02-06 05:57 EST, Srikanth Sankaran CLA
no flags Details | Diff
Proposed fix and tests (29.46 KB, patch)
2009-02-09 04:00 EST, Srikanth Sankaran CLA
no flags Details | Diff
Proposed patch and testcases (28.32 KB, patch)
2009-02-12 14:44 EST, Kent Johnson CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2007-08-31 10:13:19 EDT
I20070828-0800

When the 'Unused local or private member' compiler option is enabled, unused public members of private nested classes could also be flagged as unused (need to check the whole enclosing top-level type for references).

Unused public members are already detected inside anonymous classes.

public class Try {
	private static class Nested {
		public void unused1() {} // not found
	}
	
	public static void main(String[] args) {
		new Nested().hashCode();
		new Runnable() {
			public void run() {}
			public void unused2() {} // found to be unused
		};
	}
}
Comment 1 Philipe Mulet CLA 2008-11-10 06:31:03 EST
The trick is that it could be used from outside, even though this would cause a compile error (not accessible member), where in the case of a local type member there would be no possible way to access it (even incorrectly).
Comment 2 Markus Keller CLA 2008-11-10 06:48:41 EST
> The trick is that it could be used from outside, even though this would cause a
> compile error (not accessible member), ...

But that's also the case for unused private members of top-level classes:

public class Try {
    private static void unused3() {}
}

=> Method is also unused, although there could be a reference to Try.unused3() somewhere, with an equivalent "... is not visible" error.
Comment 3 Philipe Mulet CLA 2008-11-10 07:05:24 EST
You're right. I need to look into it.
Comment 4 Philipe Mulet CLA 2009-01-22 03:48:57 EST
Srikanth - pls have a look
Comment 5 Srikanth Sankaran CLA 2009-02-05 03:28:41 EST
(In reply to comment #1)
> The trick is that it could be used from outside, even though this would cause a
> compile error (not accessible member), where in the case of a local type member
> there would be no possible way to access it (even incorrectly).

In any event, the message we issue says "never used locally" or "never read locally" etc, so it should be OK.

I have a fix for this that solves the problem. However, there are about ~320 "failures" in the test suite run - I need to validate that all the changed diagnostics are legit and remaster the expected output. Fun !


Comment 6 Srikanth Sankaran CLA 2009-02-06 05:57:55 EST
Created attachment 124945 [details]
Proposed fix and tests
Comment 7 Srikanth Sankaran CLA 2009-02-09 04:00:04 EST
Created attachment 125106 [details]
Proposed fix and tests


Revised patch to warn on unused interface method specifications too.
Comment 8 Kent Johnson CLA 2009-02-12 14:44:51 EST
Created attachment 125569 [details]
Proposed patch and testcases

Srikanth, please take a look at this patch.

A few of the test changes in MethodVerifyTest and NegativeTest showed problems.

This patch tags all anonymous types as used since we do not seem to detect when an anonymous type really is used & it also tags inherited interface methods as used (not just the concrete implementation).
Comment 9 Srikanth Sankaran CLA 2009-02-13 00:23:13 EST
(In reply to comment #8)
> Created an attachment (id=125569) [details]

[...]

> This patch tags all anonymous types as used since we do not seem to detect when
> an anonymous type really is used & it also tags inherited interface methods as
> used (not just the concrete implementation).

Did you mean to say not just the concrete implementation being enclosed by a private type ? 

The patch looks good.

Comment 10 Kent Johnson CLA 2009-02-13 16:42:11 EST
Yes, not only the concrete method needs to be tagged but the interface methods also need to be tagged.

Released fix and tests for 3.5M6
Comment 11 Frederic Fusier CLA 2009-03-10 07:38:06 EDT
Verified for 3.5M6 using I20090310-0100.