Bug 561484 - Clean up to remove unused parameter from private methods
Summary: Clean up to remove unused parameter from private methods
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.14   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 4.24 M1   Edit
Assignee: Thomas M??der CLA
QA Contact: Jeff Johnston CLA
URL:
Whiteboard:
Keywords: usability
Depends on:
Blocks:
 
Reported: 2020-03-26 07:13 EDT by Lars Vogel CLA
Modified: 2023-06-20 14:46 EDT (History)
7 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Vogel CLA 2020-03-26 07:13:48 EDT 
Would be nice to have a clean-up / save action which removes unused parameters from private methods.
Comment 1 Thomas M??der CLA 2022-02-08 10:05:45 EST 
There already is a "unused code" cleanup (UnusedCodeCleanUpCore). IMO, it would make more sense to add this cleanup as part of that class.
Comment 2 Thomas M??der CLA 2022-02-11 05:23:02 EST 
The "unused code" cleanups/fixes rely on problems reported by the compiler, I guess because it's easier than to do the usage analysis on the AST ourselves. There is a problem type "org.eclipse.jdt.core.compiler.IProblem.ArgumentIsNeverUsed" which can be used to find unused method parameters and there is even a fix being computed for the problem (see org.eclipse.jdt.internal.corext.fix.UnusedCodeFixCore.isUnusedMember(IProblemLocationCore)). However, there are two problemsi with this

1. The fix does not clean up any usages, so we're possibly introducing compile problems
2. The fix is also available for non-private methods where the parameter might be used in another implementation of the method

As I understand it, cleanups should be localized to the open file, not reach out to other source code.
Comment 3 Thomas M??der CLA 2022-02-11 07:16:29 EST 
My plan is this: for efficiency's sake, I would make this cleanup part of "UnusedCoreCleanupCore", but only apply the cleanup for private methods and create our own rewrite operation that also removes any usages in the CU.
Comment 4 Eclipse Genie CLA 2022-02-21 10:53:26 EST 
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/191020
Comment 6 Jeff Johnston CLA 2022-03-10 17:45:56 EST 
Released for 4.24 M1
Comment 7 Eclipse Genie CLA 2022-03-16 05:49:47 EDT 
New Gerrit change created: https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/191946
Comment 9 Lars Vogel CLA 2022-03-22 15:44:14 EDT 
Thanks for this great feature, Thomas and welcome back. AFAICS you had a small break of Eclipse development starting 2004 until recently. :-)
Comment 10 Lars Vogel CLA 2022-03-30 06:29:37 EDT 
If a parameter is annotated I think it should not be removed. Example:

@Inject
@Optional
private void subscribeTopicAppStartup(@UIEventTopic(UIEvents.UILifeCycle.APP_STARTUP_COMPLETE) Event event) {
	// stuff
}

If you agree, I can open a new bug / GH issue for this additional change.
Comment 11 Jeff Johnston CLA 2022-04-06 15:10:38 EDT 
Verified for 4.24 M1 using I20220406 build
Comment 12 Jeff Johnston CLA 2022-04-06 15:12:10 EDT 
(In reply to Lars Vogel from comment #10)
> If a parameter is annotated I think it should not be removed. Example:
> 
> @Inject
> @Optional
> private void
> subscribeTopicAppStartup(@UIEventTopic(UIEvents.UILifeCycle.
> APP_STARTUP_COMPLETE) Event event) {
> 	// stuff
> }
> 
> If you agree, I can open a new bug / GH issue for this additional change.

I suggest you open a new bug and cc/assign Thomas to comment.
Comment 13 Holger Voormann CLA 2022-06-06 11:04:03 EDT 
The New and Noteworthy of Eclipse 4.24 tells about "cleanup and quick-assist" (<https://www.eclipse.org/eclipse/news/4.24/jdt.php#strconcat-to-textblock>), but I cannot see any quick assist here.

I guess "and quick-assist" is just a copy and paste error from the following Eclipse 4.22 New and Noteworthy item (see also the not adapted anchor name "strconcat-to-textblock"):
https://www.eclipse.org/eclipse/news/4.22/jdt.php#strconcat-to-textblock

So "and quick-assist" has to be removed from Eclipse 4.24 New and Noteworthy (and the anchor name should be adapted), right?
Comment 14 Peter Bull CLA 2022-08-13 14:50:21 EDT 
Disabling this option does not seem to actually disable this cleanup behavior.
Comment 15 Holger Voormann CLA 2022-08-15 01:55:55 EDT 
(In reply to Peter Bull from comment #14)
> Disabling this option does not seem to actually disable this cleanup
> behavior.

I cannot reproduce what you are saying.

If the error is reproducible with the latest Eclipse SDK integration build (https://download.eclipse.org/eclipse/downloads/), please report it with a code example here:
https://github.com/eclipse-jdt/eclipse.jdt.ui/issues
Comment 16 Peter Bull CLA 2023-06-20 14:46:47 EDT 
(In reply to Holger Voormann from comment #15)
> (In reply to Peter Bull from comment #14)
> > Disabling this option does not seem to actually disable this cleanup
> > behavior.
> 
> I cannot reproduce what you are saying.
> 
> If the error is reproducible with the latest Eclipse SDK integration build
> (https://download.eclipse.org/eclipse/downloads/), please report it with a
> code example here:
> https://github.com/eclipse-jdt/eclipse.jdt.ui/issues

OK, please see https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/313