Bug 298563 - Overwriting a method in a deprecated class should not create a "deprecation" warning
Summary: Overwriting a method in a deprecated class should not create a "deprecation" ...
Status: VERIFIED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 3.6 M6   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-28 04:04 EST by Ulli Hafner CLA
Modified: 2010-03-09 05:54 EST (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ulli Hafner CLA 2009-12-28 04:04:34 EST
When I'm overwriting a method of a deprecated class, then Eclipse creates a deprecation warning on the method level. I think Eclipse should only create such a warning if the method is marked as deprecated, and not if only the class is marked as deprecated.

Our usecase: we have an internal class that is marked as deprecated because it should not be used anymore internally (lets call this class 'Definition', the user does not see this class.) If you derive from this class a new class 'Middle', then you correctly get a deprecation warning on the class extends keyword. This derived class is in our API and visible to the users. This derived API class 'Middle' also provides some default implementations for methods in the deprecated class 'Definition' (Note that the methods are not marked as deprecated). When an API user now creates a subclass of 'Middle' and overrides one of the default implementation he will get a deprecation warning on the overridden method, which is not correct. The user should never get a deprecation warning on a method level because he only works with the class 'Middle' which has no deprecation annotations at all.

Here a test case, expected behavior is a deprecation warning on the expression 'Middle extends Definition' but nowhere else.

@Deprecated
public class Definition {
    void method1() {

    }

    private static class Middle extends Definition {
        /** {@inheritDoc} */
        @Override
        void method1() {
        }
    }

    private static class Usage extends Middle {
        /** {@inheritDoc} */
        @Override
        void method1() {
        }
    }
}

Using this snippet, you will see that Eclipse creates a warning in the class Usage for the overwritten method which is not correct.
Comment 1 Frederic Fusier CLA 2009-12-30 06:18:56 EST
What is your build ID?

Using your snippet, I can only reproduce iff both preferences:
 - 'Signal use of deprecated API inside deprecated code'
 - 'Signal overriding or implementing deprecated method'
are checked
(you can find these preferences on the Java -> Compiler -> Error/Warnings page, 
inside the 'Deprecated and restricted API' group)

Note that doing that I also get the same warning on the Middle.method1() method declaration...

Did you also turn these two compiler option ON? If so, it's the expected behavior and to get rid of the warnings, you just need to turn one of this preference OFF (typically the second one)...

Or maybe did I miss something?
Comment 2 Frederic Fusier CLA 2009-12-30 06:20:32 EST
(In reply to comment #1)
I forgot to precise that the behavior is the same in this area for 3.4.2, 3.5.1 and 3.6M4 builds...
Comment 3 Olivier Thomann CLA 2010-01-06 12:54:29 EST
Ulli, please answer questions from Frédéric in comment 1.
Closing for now. Please reopen if you get this behavior without having both preferences enabled.
Comment 4 Srikanth Sankaran CLA 2010-01-25 06:39:01 EST
Verified for 3.6M5
Comment 5 Ulli Hafner CLA 2010-02-04 03:21:36 EST
Sorry about the late response, seems that my spam filter ate the mails from bugzilla.

My build ID now is M20090917-0800.
In the example above, I had both check boxes enabled. When I remove the first check box, then the warning of the snippet correctly goes away as you noticed. 

However, the warning in my actual code still is shown so I needed to modify my snippet somewhat:

@Deprecated
public class Definition {
    void method1(Object object) {
    }
}

class Middle extends Definition {
}

class Usage extends Middle {
    @Override
    void method1(Object object) {}
}

When I set
FALSE - 'Signal use of deprecated API inside deprecated code'
TRUE - 'Signal overriding or implementing deprecated method'
then I still get the warning even thought the overwritten method is not deprecated (only the class).
Comment 6 Srikanth Sankaran CLA 2010-02-04 06:08:13 EST
(In reply to comment #5)

> However, the warning in my actual code still is shown so I needed to modify my
> snippet somewhat:
> 
> @Deprecated
> public class Definition {
>     void method1(Object object) {
>     }
> }
> 
> class Middle extends Definition {
> }
> 
> class Usage extends Middle {
>     @Override
>     void method1(Object object) {}
> }
> 
> When I set
> FALSE - 'Signal use of deprecated API inside deprecated code'
> TRUE - 'Signal overriding or implementing deprecated method'
> then I still get the warning even thought the overwritten method is not
> deprecated (only the class).

When a class is deprecated all its methods are "implicitly deprecated".
Hence overriding such a method produces the warning when the concerned
setting is on. At least this is the way things are now.

Relevant code sits in org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding.resolveTypesFor(MethodBinding)  and

org.eclipse.jdt.internal.compiler.lookup.MethodVerifier.checkAgainstInheritedMethods(MethodBinding, MethodBinding[], int, MethodBinding[])
Comment 7 Ulli Hafner CLA 2010-02-05 03:07:46 EST
I see. But isn't the assumption "class is deprecated => all methods are deprecated" wrong? It would be nice if this assumption could be toggled at least. 

Our use case: we have a top level class that should not be used any more internally, hence it is marked with @deprecated. Instead of the top level class a deriving class (Middle) should be used by internal code. However, the users of the Middle API class should still be allowed to override methods from the classes Definition and Middle. So we need to mark the class as deprecated but not all of the methods.
Comment 8 Srikanth Sankaran CLA 2010-02-05 04:57:55 EST
See also: https://bugs.eclipse.org/bugs/show_bug.cgi?id=86330
Comment 9 Olivier Thomann CLA 2010-02-09 13:31:44 EST
(In reply to comment #7)
> I see. But isn't the assumption "class is deprecated => all methods are
> deprecated" wrong? It would be nice if this assumption could be toggled at
> least.
yes, this is a legitimate assumption as far as I can tell.

>TRUE - 'Signal overriding or implementing deprecated method'
If you set that to >true<, then you request this deprecation warning to be reported. If you don't care, why do you set it up ?
Comment 10 Ulli Hafner CLA 2010-02-11 04:38:43 EST
(In reply to comment #9)
>	> I see. But isn't the assumption "class is deprecated => all methods are
>	> deprecated" wrong? It would be nice if this assumption could be toggled at
>	> least.

> yes, this is a legitimate assumption as far as I can tell.

Then, to be consequent, a warning should be issued if you use @deprecated on a method of a @deprecated class. (Since this is not done neither in Eclipse nor javac it is a small indication that your assumption is wrong:-) 

>	>TRUE - 'Signal overriding or implementing deprecated method'

> If you set that to >true<, then you request this deprecation warning to be reported. If you don't care, why do you set it up ?

I actually care! That is exactly the point. I want to be notified about all deprecations, but in my case the deprecation warning is wrong, at least in my opinion. (Or is there a better way to fulfill the use case in comment #7) 

But anyway, good to know that there is a @SupressWarnings annotation for false positives...
Comment 11 Olivier Thomann CLA 2010-02-12 13:52:21 EST
(In reply to comment #10)
> Then, to be consequent, a warning should be issued if you use @deprecated on a
> method of a @deprecated class. (Since this is not done neither in Eclipse nor
> javac it is a small indication that your assumption is wrong:-) 
You mean we should report that @deprecated is unnecessary ?
We don't report a warning for unnecessary modifiers when you have "public final static" on an interface field. It doesn't add anything to specify these modifiers, but it is not an error to add them.

If you want to match javac behavior for deprecation detection, you should uncheck this option.
Comment 12 Olivier Thomann CLA 2010-02-19 14:42:17 EST
Resolved as WONTFIX. javac deprecation warning doesn't match all Eclipse compiler options for deprecation.
There is no equivalent to:
'Signal overriding or implementing deprecated method'
in javac.
No plan to change the existing behavior.
Comment 13 Jay Arthanareeswaran CLA 2010-03-09 05:54:03 EST
Verified for 3.6M6.