Community
Participate
Working Groups
For this code: public class CompilerTest { private int hideIt; private static class InnerTest { private int hideIt; } public CompilerTest() { hideIt = 1; } } Under M6 get the compiler warning: "The field CompilerTest.InnerTest.hideIt is hiding a field from type CompilerTest" Lighten up - it's declared static. My code uses static inner classes in several places that are now being flagged this way.
The language spec tolerates field hiding. As a courtesy we will warn the user if the optional diagnosis for field hiding is enabled. You can turn this preference off in: Preferences>Java>Compiler>Advanced>Field declaration hides another field or variable. Note that it is off by default. When enabled, it doesn't distinguish static vs. non static, as all name collisions are just considered as poor style. As said before, even instance field collisions are allowed by language. Generally, naming conventions should enforce distinguishing in between static vs. non-static (statics would be capitalized at least first letter).
*** Bug 45779 has been marked as a duplicate of this bug. ***
*** Bug 50253 has been marked as a duplicate of this bug. ***
*** Bug 53827 has been marked as a duplicate of this bug. ***
*** Bug 53048 has been marked as a duplicate of this bug. ***
I thought this warning was intended for contexts in which an expression could refer to either one of the variables. In the given example there is no such context. > Generally, naming conventions should enforce distinguishing in between > static vs. non-static (statics would be capitalized at least first letter). You seem to be talking about static variables but the example is about static inner classes. The example respects the conventions, as far as I can tell.
Has been reported 5 times.
If you read the spec carefully, you'll notice that field hiding is legal. The example is legal code, and though it is legal code, we added a warning to help people resolve situations where variable names are confusing and may introduce nasty bugs down the road. The fact variables are field and/or static and/or in static context doesn't change this issue, the end program will be confusing, and could mislead a developper to use a slot in the wrong way. This is only a warning, it is off by default. If you don't want field hiding diagnosis, then simply disable the warning, adding a special case when going through the static context would simply defeat the purpose of this enhanced diagnosis. I am still missing something obvious ?
Yes, the obvious thing is that in this case variable names cannot introduce any bugs. All of us who reported this bug enabled the field hiding warning. Because we find it useful, but only in situations where it makes sense, not in this one. Maybe an option "Include static inner class fields and variables hiding outer class non-static fields" can be added at the same level with "Include constructor or setter method parameters".
These variables names are in 2 different name domains: the static members and the instance members. Philippe tries to enforce detection of when an access to an enclosing instance field is made from a static innerclass (which would be a compiler error because there is no enclosing instance to a static inner class) BUT THAT static innerclass just happen to have the same field name, and the coder would use it by mistake. I (we) say that coders often create utility static innerclasses that carry similar values from enclosing instance (very frequent for non-anonymous runnables, listeners, etc...) and the warning is not founded because we know that we access a static-innerclass-local field. For example, you want to define some treenode static innerclass to be used in a tree class. Tree and treenode both have a "children" and "size" variable. Because of that warning, coders must figure out alternate names for these variable, like "_children" and "_size". It is just plain annoying. Beside, I would like to have field hiding as an error, not a warning, because it is a major concern for avoiding bugs, just like I put "if(boo=true)" as an error too. I just don't want sane code to show up as a problem. Please do not leave on WONTFIX, it is sending a very negative message. It is a good option, and we understand it could be done LATER, but not never. Thanks.
Maybe an option "Include static fields" could be added as well, since some of us already have an error set on "Non-static access to static member" and "Indirect access to static member". This way fields used by reflection, like serialVersionUID during serialization, could be named the same without any confusion about which field is being referred to.
*** Bug 55023 has been marked as a duplicate of this bug. ***
*** Bug 49030 has been marked as a duplicate of this bug. ***
I agree that a warning is a good idea and that such code is often bad. However, what confuses is the term "hides" which in programming has some meaning. I would suggest some other terminology like "duplicate name" or "name already in used". I am reopening this one since the number of dups really indicates that this is a problem for users.
We could indeed work the problem differently to rather mention a name collision as it is really what we are diagnosing. Would it be an acceptable tradeoff to all ? "The field CompilerTest.InnerTest.hideIt name is colliding with a field from type CompilerTest"
as long as comment #9 is implemented.
Comment #9 would not be implemented. The argument is that this warning would only blame the name, not mention hiding any longer.
Slight improvement: The name of field {0}.{1} is already used by a field from type {2}. Though it is legal, it should be named differently to avoid confusion
If we do so we must not forget to update the JDT UI preference page.
a field in a static inner class cannot hide an outer class field with the same name so there should be no warning. To me a hide field warning is usefull for subclasses, having a subclass with a field that is a duplicate of one in the base class is almost always a mistake. While an inner class having a field with the same name may not be. I have done this many times without inadvertantly hiding an outer class field How about having a checkbox (or another option) for giving warnings on non- static inner classes would be OK with me
Ok, you convinced me. We will look into addressing this one for M9, with/without an extra option (if no extra option, then the offending scenario will no longer be reported). Note that in case a static field is existing in the enclosing type, then this would still be reported as a potentially hiding issue.
Created attachment 9771 [details] patch fixing the bug
The patch fixes the only case where the bug happened. This case is the one described in the first message : when a field is in a static inner class with the same name of a field in an outer class. The compiler shouldn't warn us in this case.
Thanks David. Patch integrated. Fixed
Verified in 200405180816
This bug (or feature) still turns up; it seems the actual scope is not checked with the submitted patch. It's a very nice feature to have (thanks), so please look into it. Example code (anonymous inner class): public class EclipseHidingBug { public void method() { int i = 0; Object b = new Object() { void m() { i = 1; // error, i not visible (correct) int i = 2; // warning - if enabled, hiding (incorrect) } } } }
Are you saying the original issue is still there, or did you find another bug ? Closing. Pls do not resurrect old fixed bugs, and rather log new ones for new scenarii.
Retagging as verified
Entered bug 79168 for subsequent issue in comment 26.
