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

Daniel Fuchs daniel.fuchs at oracle.com
Mon Jul 10 10:10:36 UTC 2017


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();


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
>>>>>
>>>>
>>
> 



More information about the core-libs-dev mailing list