> On Jul 10, 2017, at 6:10 AM, Daniel Fuchs <[email protected]> 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
[email protected] <mailto:[email protected]>
