Fix deprecated check routines in ASTNode to also check since value

[jjohnstn]

• fixes #2873

What it does

Fixes deprecated warnings and errors being shown when the release setting is below the since threshold of a deprecated method, type, field.

How to test

See issue.

Author checklist

• [x] I have thoroughly tested my changes
• [x] The change is following the coding conventions
• [x] I have signed the Eclipse Contributor Agreement (ECA)

[jjohnstn]

Hi @stephan-herrmann Can this be reviewed for M2?

[stephan-herrmann]

Hi @stephan-herrmann Can this be reviewed for M2?

yes, M2 sounds realistic.

[jjohnstn]

I dropped some suggestions for technical code simplification.

But I'm not sure we are on 100% firm ground conceptually:

* do we know why editor and problems view show different results? A corresponding test in `ReconcilerTests9` could be helpful to observe the difference.

* I'd expect `since` values to be used mostly by JDK classes (because the version is a JDK version, not the version of 3rd party code) - and in the ticket the affected class was `java.net.URL` indeed.

  * in that case we shouldn't even see the class version with the deprecation annotations, IFF `--release` is used. This, too, is worth being double-checked.

Thanks for the code suggestions. I agree that the since value will likely only be used with JDK classes. I don't know how the release option works under the covers. Should the release option change the URL class in this case since we are talking deprecation and not an API addition, change, or removal? When I use JDK22 and specify compliance 19 with --release option checked, the Javadoc I get from the hover for the URL constructor in question is the JDK22 javadoc specifying the deprecation with since value.

[stephan-herrmann]

I don't know how the release option works under the covers.

--release selects class stubs from withing ct.sym that represent the requested version of all JDK classes. I.e., we basically see the byte code minus the code attribute in exactly the requested version.

Should the release option change the URL class in this case since we are talking deprecation and not an API addition, change, or removal?

The signature entry for URL at versions 20+ contains:

    Deprecated: true
    RuntimeVisibleAnnotations:
      0: #42(#43=s#44)
        java.lang.Deprecated(
          since="20"
        )

When specifying --release 19 we get a version of URL without the deprecation info. Hence the compiler will not issue a warning when one of those contructors is used. This works as specified.

When I use JDK22 and specify compliance 19 with --release option checked, the Javadoc I get from the hover for the URL constructor in question is the JDK22 javadoc specifying the deprecation with since value.

That's a problem: we do have the correct information in byte code format, but we don't have version specific source attachment and javadoc! There is only the JDK22 version in your setup.

[stephan-herrmann]

do we know why editor and problems view show different results? A corresponding test in ReconcilerTests9 could be helpful to observe the difference.

@jjohnstn did you succeed to write a test that demonstrates this difference between compiler and reconciler?

[jjohnstn]

do we know why editor and problems view show different results? A corresponding test in ReconcilerTests9 could be helpful to observe the difference.

@jjohnstn did you succeed to write a test that demonstrates this difference between compiler and reconciler?

No, unfortunately I had to go to a wedding in Texas and have been doing some catch up in getting some UI changes into M2.

[jjohnstn]

Hi @stephan-herrmann I added a test to ReconcilerTests9. Without the patch, there is a warning for both methods (one is deprecated since 9 and the other is deprecated since 10). With the patch, it only shows a warning for the method that is deprecated since 9. Is that sufficient?

[jjohnstn]

@stephan-herrmann The reason for the warning in the editor is that the IMethodBinding returns true for isDeprecated() due to the compiler MethodBinding which sets the flag based on the ASTNode.isMethodUseDeprecated() method which I changed.

[stephan-herrmann]

For the actual difference between editor and builder (which can only be demonstrated with deprecation in an original JDK class) I filed #3168, which comes with its own set of open questions, and hence may not be resolved in the near future. For that reason the current fix is a valid workaround for a situation that ideally shouldn't even occur, but does occur currently.

[stephan-herrmann]

thanks @jjohnstn