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