Bug 200724 - [compiler] Assignment with no effect undetected
Summary: [compiler] Assignment with no effect undetected
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4 M3   Edit
Assignee: Maxime Daniel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-08-21 15:55 EDT by Pascal Rapicault CLA
Modified: 2007-10-29 12:49 EDT (History)
1 user (show)

See Also:


Attachments
Proposed fix (1.23 KB, patch)
2007-08-23 11:47 EDT, Olivier Thomann CLA
no flags Details | Diff
Regression tests (1.91 KB, patch)
2007-08-23 11:48 EDT, Olivier Thomann CLA
no flags Details | Diff
Fix + test cases (5.38 KB, patch)
2007-10-17 07:50 EDT, Maxime Daniel CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pascal Rapicault CLA 2007-08-21 15:55:29 EDT
Eclipse 3.3.

In the following code, the detection of "assignment with no effect" does not flag the problem in the start method. Removing the reference to the static class causes the detection to work.


public class DirectorActivator implements BundleActivator {
	public static final String PI_DIRECTOR = "org.eclipse.equinox.prov.director"; //$NON-NLS-1$
	public static BundleContext context;

	public void start(BundleContext ctx) throws Exception {
		DirectorActivator.context = context;
	}

	public void stop(BundleContext ctx) throws Exception {
		DirectorActivator.context = null;
	}

}
Comment 1 Olivier Thomann CLA 2007-08-23 11:47:55 EDT
Created attachment 76783 [details]
Proposed fix

Philippe, please review.
This patch reports an assignment with no effect when the qualified name simply refers to a static field using ClassName.FieldName.
Comment 2 Olivier Thomann CLA 2007-08-23 11:48:49 EDT
Created attachment 76784 [details]
Regression tests
Comment 3 Philipe Mulet CLA 2007-09-13 18:36:28 EDT
What about a situation where the qualified name has the package prefix ?

e.g. some.package.ClassName.field
Comment 4 Olivier Thomann CLA 2007-09-13 19:35:29 EDT
The proposed patch works fine.

package p;
public class X {
        public static Object context;

        public void start(Object ctx) throws Exception {
                p.X.context = context;
        }
}

----------
1. WARNING in D:\tests_sources\p\X.java (at line 6)
	p.X.context = context;
	^^^^^^^^^^^^^^^^^^^^^
The assignment to variable context has no effect
----------
Comment 5 Philipe Mulet CLA 2007-10-12 06:27:12 EDT
Looks good to me. Maxime pls proceed.
Comment 6 Maxime Daniel CLA 2007-10-17 07:47:25 EDT
Fix is OK in that it addresses most common cases involving static fields. Added a few more tests that check variations (package, inner class, swapping the lhs and the rhs) and document a limitation (there are cases for which a static analysis could prove that the runtime type of a variable equates a defined one, in which case the access to the static fields of the said variable would provably point to a well defined field, and we could report additional warnings - see AssignmentTest#48, which is inactive).
Comment 7 Maxime Daniel CLA 2007-10-17 07:50:34 EDT
Created attachment 80549 [details]
Fix + test cases

Fixed a typo in comments and added more tests.
Comment 8 Maxime Daniel CLA 2007-10-18 03:34:16 EDT
Released for 3.4 M3
Comment 9 Frederic Fusier CLA 2007-10-29 12:49:13 EDT
Verified for 3.4M3 using I20071029-0010 build.