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