RFR (JDK10/jaxp) 8181154: Fix lint warnings in JAXP repo: deprecation

huizhe wang huizhe.wang at oracle.com
Mon Jul 10 17:40:21 UTC 2017


Hi Lance, Daniel,

I pushed the changeset after adding 'final' to the following, and then 
logged this issue [1] for investigating the deprecated method.

[1] https://bugs.openjdk.java.net/browse/JDK-8184103

Thanks again for the reviews!

Joe

On 7/10/2017 7:56 AM, Lance Andersen wrote:
>
>> On Jul 10, 2017, at 6:10 AM, Daniel Fuchs <daniel.fuchs at oracle.com 
>> <mailto: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/ 
>>> <http://cr.openjdk.java.net/%7Ejoehw/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