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

Daniel Fuchs daniel.fuchs at oracle.com
Thu Jul 6 11:03:34 UTC 2017


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
> 



More information about the core-libs-dev mailing list