Bug 141931 - [1.5][compiler] @Override: upcoming changes of the reference implementation
Summary: [1.5][compiler] @Override: upcoming changes of the reference implementation
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.2   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 3.2.2   Edit
Assignee: Philipe Mulet CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-05-16 01:17 EDT by Maxime Daniel CLA
Modified: 2007-01-15 06:20 EST (History)
3 users (show)

See Also:


Attachments
Test cases: one new, plus some comments changed (4.03 KB, patch)
2006-05-16 02:58 EDT, Maxime Daniel CLA
no flags Details | Diff
Proposed patch (43.28 KB, patch)
2006-10-19 04:15 EDT, Philipe Mulet CLA
no flags Details | Diff
Backport to 3.2.2 (fix plus test cases) (12.83 KB, patch)
2006-10-19 12:04 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 Maxime Daniel CLA 2006-05-16 01:17:15 EDT
The @Override annotation semantics are currently being debated and will likely be refined in upcoming versions of the reference implementation of the compiler. JDT should implement the new semantics as soon as they stabilize.
(Opening this bug to anchor test cases in preparation for the changes.)
Comment 1 Maxime Daniel CLA 2006-05-16 02:58:16 EDT
Created attachment 41553 [details]
Test cases: one new, plus some comments changed

The proposed changes may include tolerating the following case (entered as - inactive - AnnotationTest#200 in the attached patch):
interface I {
  void foo();
}
class X implements I {
  @Override
  public void foo() {}
}

We would probably not mandate such decoration though, since it would make the normal 'class implements interface' coding much heavier. This would hence call for a new definition for the cases in which we complain if the annotation is not present (assuming the current definition is 'could bear @Override but does not').

Marked a few test cases as probably impacted by this bug as well.
Comment 2 Maxime Daniel CLA 2006-05-16 03:07:22 EDT
One more test case:
//https://bugs.eclipse.org/bugs/show_bug.cgi?id=141931
// variant of test200
public void _test201() {
	this.runConformTest(
		new String[] {
			"I.java",
			"public interface I {\n" + 
			"	void foo();\n" + 
			"}\n",
			"X.java",
			"abstract class X implements I {\n" + 
			"}\n",
			"X.java",
			"class Y extends X {\n" + 
			"  @Override\n" +
			"  public void foo() {}\n" +
			"}\n",
		},
		"");
}
Comment 3 Philipe Mulet CLA 2006-10-18 19:09:55 EDT
adopting
Comment 4 Philipe Mulet CLA 2006-10-19 04:15:55 EDT
Created attachment 52296 [details]
Proposed patch

Patch for 3.3
Comment 5 Philipe Mulet CLA 2006-10-19 04:19:08 EDT
Released for 3.3M3 (HEAD)
Should backport fix to 3.2.2, since mandated for compliance 1.6.
In 3.2.x should avoid changing error catalog as done in 3.3 stream (superclass message should thus be reused, no addition on IProblem).

Maxime - pls backport fix. Also should enter a FUP bug for 3.3, so as to add complementary option for missing @Override diagnosis, so as to also suggest missing @Override when implementing interface methods as well (when source 1.6 only), suboption would be off by default.
Comment 6 Philipe Mulet CLA 2006-10-19 04:25:12 EDT
Reopening for addressing in 3.2.2 stream.
Maxime - pls also release your test changes.
I did add AnnotationTest#test214, with some variations of your original testcase in comment 1 only.
Comment 7 Maxime Daniel CLA 2006-10-19 09:01:27 EDT
Released AnnotationTest#215 in HEAD to account for the case reported in comment #2. Other significant test changes have already been released in HEAD.
Now needs to backport.
Comment 8 Maxime Daniel CLA 2006-10-19 12:04:07 EDT
Created attachment 52323 [details]
Backport to 3.2.2 (fix plus test cases)
Comment 9 Maxime Daniel CLA 2006-10-19 12:41:27 EDT
Released for 3.2.2.
Comment 10 Philipe Mulet CLA 2006-10-30 11:07:01 EST
New behavior is only available in 1.6 mode.
Comment 11 David Audel CLA 2006-10-30 11:21:22 EST
Verified for 3.3 M3 using build I20061030-0010
Comment 12 Markus Keller CLA 2006-11-02 06:31:17 EST
This change has caused bug 163192. I'm not sure how stable problemIDs are in general, but this should maybe be added to the build notes or the API changes document.
Comment 13 Philipe Mulet CLA 2006-11-02 09:18:54 EST
In 1.6, a new problem got introduced since the compiler error message must differ (to mention supertypes in general, vs. only superclass in the past).

Maybe the quickfix implementation needs to check for all supertypes as wel... ?

+1 for better documenting this in build notes.
Comment 14 Philipe Mulet CLA 2006-11-02 09:20:30 EST
Maxime - pls make sure our build notes reflect this change in proper What's new section.
Comment 15 Maxime Daniel CLA 2006-11-06 06:23:42 EST
Tuned a bit David's contribution to the N&N of HEAD (thx David) then impacted 3.2 maintenance as well.
Comment 16 Eric Jodet CLA 2007-01-15 05:45:08 EST
verified for 3.2.2 using build M20070112-1200