Bug 312989 - Accepts illegal method-local classes if hidden by generics parameters
Summary: Accepts illegal method-local classes if hidden by generics parameters
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: PC Windows Vista
: P3 normal (vote)
Target Milestone: 3.7 M1   Edit
Assignee: Srikanth Sankaran CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-14 18:30 EDT by Chris West (Faux) CLA
Modified: 2010-08-03 07:23 EDT (History)
3 users (show)

See Also:
satyam.kandula: review+


Attachments
Add regression tests (3.32 KB, patch)
2010-05-14 19:52 EDT, Olivier Thomann CLA
no flags Details | Diff
Patch under consideration (6.33 KB, patch)
2010-05-17 07:36 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Revised patch under test (7.48 KB, patch)
2010-05-17 09:44 EDT, Srikanth Sankaran CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris West (Faux) CLA 2010-05-14 18:30:15 EDT
Build Identifier: I20100506-0800

I believe both of the following classes are illegal for the same reason that class C is:

class C {
 void foo() {
     class C {}}}
Eclipse:   ^ The nested type C cannot hide an enclosing type.
sun6javac: ^ C is already defined in unnamed package

Eclipse, however, unlike Sun javac (1.6.0_19-b04 win64), does not issue that error for the following classes:

class A {
 <A> void foo() {
  class A {}}}

class B<B> {
 void foo() {
  class B {}}}


This code is not sensible as (as far as I'm aware) it's not possible to refer to the method-local classes in any way.

Reproducible: Always

Steps to Reproduce:
1. Paste class A or B into any Java editor.
2. Observe error.
Comment 1 Olivier Thomann CLA 2010-05-14 19:46:17 EDT
Srikanth, please review.
Comment 2 Olivier Thomann CLA 2010-05-14 19:52:42 EDT
Created attachment 168624 [details]
Add regression tests
Comment 3 Srikanth Sankaran CLA 2010-05-17 07:36:26 EDT
Created attachment 168713 [details]
Patch under consideration
Comment 4 Srikanth Sankaran CLA 2010-05-17 08:17:01 EDT
Here is an odd behavior from javac:

case (1)
class Y {
    class X {}
    void foo() {
        class X {}
    }
}

doesn't elicit any messages while

case (2)
class Y {
    class X {}
    <X> void foo() {
        class X {}
    }
}

elicits:

Y.java:4: X is already defined in <X>foo()
        class X {}
        ^
1 error

With respect to case (1), eclipse behavior is similar to javac
with or without this patch.

With respect to case(2) the proposed patch implements the same
behavior, but that is not appropriate and the compiler's behavior
should be similar to case (1)
Comment 5 Srikanth Sankaran CLA 2010-05-17 09:44:29 EDT
Created attachment 168727 [details]
Revised patch under test
Comment 6 Srikanth Sankaran CLA 2010-05-17 09:47:06 EDT
Targetting 3.7M1
Comment 7 Srikanth Sankaran CLA 2010-06-02 06:26:26 EDT
JLS3 sections 8.1 and 9.1 disallow class A and B of comment# 0
("A compile-time error occurs if a class has the same simple
name as any of its enclosing classes or interfaces.") Note that
this says nothing about the enclosing type being visible there
(i.e, not already being shadowed)

This is the subject matter of the current defect and is
addressed by the proposed patch.

The shadowing rules for type variables are ill
specified in the JLS (as acknowledged in http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5060485).

14.3 Local Class Declarations is silent about type variables
in scope having the same simple name as the local type.

6.3.1 Shadowing Declarations is silent about type variables.

4.4 Type Variables defers to 6.3 for scoping rules for type
variables, but that section does not give the shadowing rules.

8.4.4 Generic Methods: gives the scoping rules for type parameters
but is silent about legalities of shadowing.

Eclipse compiler starting with https://bugs.eclipse.org/bugs/show_bug.cgi?id=165662, has always allowed a local type to shadow a type variable
in scope. The current patch leaves this behavior unchanged.

Thus the following code:
class Y {
    class X {}
    <X> void foo() {
        class X {}
    }
}
which refuses to compile with javac continues to compile without
a problem (as before) with this patch.
Comment 8 Srikanth Sankaran CLA 2010-06-17 09:49:00 EDT
Satyam, please review -- Thanks.
Comment 9 Satyam Kandula CLA 2010-06-21 01:14:22 EDT
It will help if the bug numbers are associated with the new tests added. 
All the other changes look good. 
+1
Comment 10 Srikanth Sankaran CLA 2010-06-21 05:48:58 EDT
Thanks Satyam. I have added defect URLs to unit tests.
Released in HEAD for 3.7 M1
Comment 11 Ayushman Jain CLA 2010-08-03 07:23:21 EDT
Verified for 3.7M1 using build I20100802-1800.