RFR (JDK10/jaxp) 8181154: Fix lint warnings in JAXP repo: deprecation
Lance Andersen
lance.andersen at oracle.com
Mon Jul 10 14:56:54 UTC 2017
> On Jul 10, 2017, at 6:10 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
>
> Hi Joe,
>
> Looks good to me. I have two additional comments, one nit - and
> one for some later follow-up - so no need for a new webrev.
>
>
> 1. Nit: I think it should be final as well:
>
> +++ new/src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/OutputPropertiesFactory.java 2017-07-07 21:15:14.418212557 -0700
> [...]
> - private static Integer m_synch_object = new Integer(1);
> + private static Object m_synch_object = new Object();
>
+1
otherwise looks OK (including Daniel’s comment of course below ;-) )
>
> 2. Can you also add the following items to the list of things
> that should be investigated in a followup/cleanup?
> It bothers me that a non-deprecated method still needs to call
> a deprecated API:
>
> +++ new/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/dom/PSVIElementNSImpl.java 2017-07-07 21:15:01.881586697 -0700
> [...]
> + @SuppressWarnings("deprecation")
> public String getSchemaDefault() {
> return fDeclaration == null ? null : fDeclaration.getConstraintValue();
> }
>
> +++ new/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/AttributePSVImpl.java 2017-07-07 21:15:04.749729882 -0700
> [...]
> + @SuppressWarnings("deprecation")
> public String getSchemaDefault() {
> return fDeclaration == null ? null : fDeclaration.getConstraintValue();
> }
>
> +++ new/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/AttributePSVImpl.java 2017-07-07 21:15:04.749729882 -0700
> [...]
> + @SuppressWarnings("deprecation")
> public String getSchemaDefault() {
> return fDeclaration == null ? null : fDeclaration.getConstraintValue();
> }
>
> +++ new/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/ElementPSVImpl.java 2017-07-07 21:15:05.077746257 -0700
> [...]
> + @SuppressWarnings("deprecation")
> public String getSchemaDefault() {
> return fDeclaration == null ? null : fDeclaration.getConstraintValue();
> }
>
>
> cheers,
>
> -- daniel
>
> On 08/07/2017 01:53, huizhe wang wrote:
>> Here's the updated webrev:
>> http://cr.openjdk.java.net/~joehw/jdk10/8181154/webrev01/
>> Thanks,
>> Joe
>> On 7/7/2017 10:59 AM, huizhe wang wrote:
>>>
>>>>> #11
>>>>> com/sun/org/apache/xerces/internal/jaxp/datatype/XMLGregorianCalendarImpl.java
>>>>>
>>>>> 1166 final public void setYear(BigInteger year) {
>>>>>
>>>>> nit: for consistency you should reorder the keywords to:
>>>>>
>>>>> public final void
>>>>>
>>>>> same file:
>>>>>
>>>>> - BigInteger.valueOf((long) 1) => BigInteger.ONE
>>>>> - new BigDecimal(TWELVE) => introduce a constant DECIMAL_TWELVE?
>>>>> - new BigDecimal(TWENTY_FOUR) => introduce a constant DECIMAL_TWENTY_FOUR?
>>>>
>>>
>>> Added the two constants.
>>>
>>>>
>>>>
>>>>>
>>>>> #12
>>>>> com/sun/org/apache/xerces/internal/parsers/AbstractSAXParser.java
>>>>>
>>>>> 81 @SuppressWarnings("deprecation")
>>>>> 82 public abstract class AbstractSAXParser
>>>>> 83 extends AbstractXMLDocumentParser
>>>>> 84 implements PSVIProvider, // PSVI
>>>>> 85 Parser, XMLReader // SAX1, SAX2
>>>>>
>>>>> A better solution would be to deprecate the methods that implement
>>>>> deprecated methods - and avoid using deprecated APIs elsewhere...
>>>>>
>>>>> #13
>>>>> com/sun/org/apache/xerces/internal/util/AttributesProxy.java
>>>>>
>>>>> 36 @SuppressWarnings("deprecation")
>>>>> 37 public final class AttributesProxy
>>>>> 38 implements AttributeList, Attributes2 {
>>>>>
>>>>> I wonder whether deprecating the methods implementing a deprecated
>>>>> API would allow to remove @SuppressWarnings("deprecation") - maybe not
>>>>> in this case since this class needs to implement a deprecated interface
>>>>> and a non deprecated one (and therefore the class itself can't be
>>>>> deprecated). Anyway it might be good to add the @Deprecated
>>>>> annotations to deprecated methods.
>>>
>>> We'll take a closer look at this through [1], for both #12 and #13, and #17 as well. AttributesProxy is used quite widely. We'll need to get into the details to see if we can remove the deprecated AttributeList.
>>>
>>> Since there are import references that are deprecated, the suppress annotation has to be added at the class.
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8183972
>>>
>>>>>
>>>>> #14
>>>>> com/sun/org/apache/xml/internal/dtm/ref/IncrementalSAXSource_Xerces.java
>>>>>
>>>>> if _main use deprecated APIs then maybe _main should be @Deprecated
>>>>> as well.
>>>
>>> Done.
>>>>>
>>>>> #15
>>>>> com/sun/org/apache/xml/internal/serializer/OutputPropertiesFactory.java
>>>>>
>>>>> 207 private static Integer m_synch_object = 1;
>>>>>
>>>>> This is not correct! Since this object is only use as a synchronization
>>>>> lock then please change this to:
>>>>>
>>>>> private static Object m_synch_object = new Object();
>>>
>>> Ok, fixed now. But I don't think the sync is necessary, logged https://bugs.openjdk.java.net/browse/JDK-8184019 to take a look later and can also do some cleanups.
>>>
>>>>>
>>>>> #16
>>>>> com/sun/org/apache/xpath/internal/axes/HasPositionalPredChecker.java
>>>>>
>>>>> Can this class be changed to stop using deprecated APIs instead?
>>>
>>> It can probably be removed, will do so through https://bugs.openjdk.java.net/browse/JDK-8184020.
>>>
>>>>>
>>>>>
>>>>> #17
>>>>> javax/xml/parsers/SAXParser.java
>>>>> org/xml/sax/helpers/ParserAdapter.java
>>>>> org/xml/sax/helpers/XMLReaderAdapter.java
>>>>>
>>>>> Can these classes be changed to stop using deprecated APIs instead?
>>>>> Or at least a cleanup issue logged for that effect?
>>>
>>> Most likely we can't since these are public APIs. DocumentHandler for example, is still usable. We can probably add forRemoval and then remove in the next release.
>>>
>>>>>
>>>>> That's all!
>>>
>>> Thanks for all of that!
>>>
>>> Best,
>>> Joe
>>>
>>>>>
>>>>> best regards,
>>>>>
>>>>> -- daniel
>>>>>
>>>>> On 05/07/2017 19:48, Joe Wang wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This patch fixes deprecation warnings in the jaxp repo. I'm limiting the changes to deprecation or the 102 classes. The cup-generated classes, XPath*.java, could use some cleanup or even rewrite, but that will be left in future works (JDK-8183580).
>>>>>>
>>>>>> Most of the changes are straight-forward, but I did have trouble with the replacing clazz.newInstance() with clazz.getDeclaredConstructor().newInstance() since in some cases I got RuntimePermission problems. So for most of such cases, I've replaced them with getConstructor() instead, thanks Alan for the suggestion. For specific cases such as xsltc/compiler/Parser.java, I'm using getDeclaredConstructor() because some of the classes didn't have a public constructor.
>>>>>>
>>>>>> Again, please bear with me for the cup-generated classes that have super long lines (e.g. XPathParser.java). Please read with Udiffs instead of Sdiffs.
>>>>>>
>>>>>> Some of the boxing (e.g. Integer.valueOf()) in the BCEL classes may not be necessary, but that's how they look in BCEL to which an update is scheduled in JDK 10.
>>>>>>
>>>>>> All tests passed.
>>>>>>
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8181154
>>>>>> webrevs: http://cr.openjdk.java.net/~joehw/jdk10/8181154/webrev/
>>>>>>
>>>>>> Thanks
>>>>>> Joe
>>>>>>
>>>>>
>>>
>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
More information about the core-libs-dev
mailing list