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.fu...@oracle.com
<mailto:daniel.fu...@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.ander...@oracle.com <mailto:lance.ander...@oracle.com>
