Hi Joe,
Waouh, lost of good cleanup! A few comments:
#1:
com/sun/org/apache/xalan/internal/xsltc/dom/NodeSortRecordFactory.java:
154 sortRecord =
(NodeSortRecord)_class.getConstructor().newInstance();
Maybe it would be good to add a comment to say that NodeSortRecord
subclasses are generated with a public empty constructor (which is
if I'm not mistaken, what
com.sun.org.apache.xalan.internal.xsltc.compiler.Sort::compileInit
does).
#2:
com/sun/org/apache/xalan/internal/xsltc/runtime/Attributes.java:
30 @SuppressWarnings("deprecation")
31 public final class Attributes implements AttributeList {
I wonder if the correct thing to do here would be to deprecate this
class, since it doesn't seem to be anything but the implementation
of a deprecated interface. Or alternatively, change it to implement
the org.xml.sax.Attributes interface?
Maybe the @SuppresWarnings can be kept as a stop gap solution for
now, and a new issue logged to investigate what's the proper thing
to do - as I suspect the answer might not be straightforward...
#3:
com/sun/org/apache/xalan/internal/xsltc/trax/TrAXFilter.java
com/sun/org/apache/xalan/internal/xsltc/trax/Util.java
com/sun/org/apache/xerces/internal/impl/xs/traversers/XSDHandler.java
com/sun/org/apache/xpath/internal/SourceTreeManager.java
51 @SuppressWarnings("deprecation")
//org.xml.sax.helpers.XMLReaderFactory
Same here: maybe we should log a further issue to investigate
whether javax.xml.parsers.SAXParserFactory could be used instead
of the deprecated factory in these classes?
#4:
com/sun/org/apache/xerces/internal/dom/CoreDOMImplementationImpl.java
Is this fixing a different issue?
#5:
com/sun/org/apache/xerces/internal/dom/PSVIAttrNSImpl.java
com/sun/org/apache/xerces/internal/dom/PSVIElementNSImpl.java
com/sun/org/apache/xerces/internal/impl/xs/AttributePSVImpl.java
com/sun/org/apache/xerces/internal/impl/xs/ElementPSVImpl.java
114 @SuppressWarnings("deprecation")
115 public String getSchemaDefault() {
Use fDeclaration.getValueConstraintValue().getNormalizedValue() instead.
126 @SuppressWarnings("deprecation")
127 public String getSchemaNormalizedValue() {
245 @SuppressWarnings("deprecation")
246 public Object getActualNormalizedValue() {
253 @SuppressWarnings("deprecation")
254 public short getActualNormalizedValueType() {
261 @SuppressWarnings("deprecation")
262 public ShortList getItemValueTypes() {
I believe the correct thing to do here instead would be to deprecate
these methods - that should get rid of the warnings.
#6
com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java
com/sun/org/apache/xerces/internal/impl/XMLEntityScanner.java
An alternative solution would be to use Boolean.TRUE and Boolean.FALSE.
Not sure if it matters though. That was just a thought.
#7
com/sun/org/apache/xerces/internal/impl/dtd/models/CMLeaf.java
Not sure why you converted StringBuffer to StringBuilder in the
previous file but not there.
#8
com/sun/org/apache/xerces/internal/impl/xs/XSAttributeDecl.java
com/sun/org/apache/xerces/internal/impl/xs/XSAttributeUseImpl.java
com/sun/org/apache/xerces/internal/impl/xs/XSElementDecl.java
161 @SuppressWarnings("deprecation")
162 public String getConstraintValue() {
198 @SuppressWarnings("deprecation")
199 public Object getActualVC() {
205 @SuppressWarnings("deprecation")
206 public short getActualVCType() {
212 @SuppressWarnings("deprecation")
213 public ShortList getItemValueTypes() {
These methods should be marked deprecated since they implement
deprecated methods. If I'm not mistaken then this should allow
you to remove the @SuppressWarnings("deprecation") from their
declaration.
#9
com/sun/org/apache/xerces/internal/impl/xs/traversers/XSDComplexTypeTraverser.java
- fGlobalStore[fGlobalStorePos++] = new Integer((fDerivedBy <<
16) + fFinal);
- fGlobalStore[fGlobalStorePos++] = new Integer((fBlock << 16) +
fContentType);
+ fGlobalStore[fGlobalStorePos++] = (fDerivedBy << 16) + fFinal;
+ fGlobalStore[fGlobalStorePos++] = (fBlock << 16) + fContentType;
I wonder if there can be some subtle type conversion side effects here,
given that all the operands are shorts, and that we expect an int in
the end?
#10
com/sun/org/apache/xerces/internal/jaxp/SAXParserImpl.java
66 @SuppressWarnings("deprecation")
Why is this needed?
#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?
#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.
#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.
#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();
#16
com/sun/org/apache/xpath/internal/axes/HasPositionalPredChecker.java
Can this class be changed to stop using deprecated APIs instead?
#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?
That's all!
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
