Namgyu Kim created LUCENE-8460:
----------------------------------
Summary: Javadoc changes in StoredField
Key: LUCENE-8460
URL: https://issues.apache.org/jira/browse/LUCENE-8460
Project: Lucene - Core
Issue Type: Improvement
Components: core/other
Reporter: Namgyu Kim
I have found some invalid Javadocs in StoredField Class.
(and I think Field Class has some problems too :D)
1) Line 45 method
{code:java}
/**
* Expert: allows you to customize the {@link
* ...
* @throws IllegalArgumentException if the field name is null.
*/
protected StoredField(String name, FieldType type) {
super(name, type);
}
{code}
It is misleading because there is no explanation for *type*.
If you follow that super class, you can see the following code(Field class).
{code:java}
/**
* Expert: creates a field with no initial value.
* ...
* @throws IllegalArgumentException if either the name or type
* is null.
*/
protected Field(String name, IndexableFieldType type) {
if (name == null) {
throw new IllegalArgumentException("name must not be null");
}
this.name = name;
if (type == null) {
throw new IllegalArgumentException("type must not be null");
}
this.type = type;
}{code}
Field class has the exception handling(IllegalArgumentException) for *null
IndexableFieldType object*.
For that reason, I changed the Javadoc to:
{code:java}
/**
* Expert: allows you to customize the {@link
* ...
* @throws IllegalArgumentException if the field name or type
* is null.
*/
protected StoredField(String name, FieldType type) {
super(name, type);
}
{code}
2) Line 59 method
{code:java}
/**
* Expert: allows you to customize the {@link
* ...
* @throws IllegalArgumentException if the field name
*/
public StoredField(String name, BytesRef bytes, FieldType type) {
super(name, bytes, type);
}
{code}
It is misleading because there is no explanation for *bytes*.
If you follow that super class, you can see the following code(Field class).
{code:java}
/**
* Create field with binary value.
*
* ...
* @throws IllegalArgumentException if the field name is null,
* or the field's type is indexed()
* @throws NullPointerException if the type is null
*/
public Field(String name, BytesRef bytes, IndexableFieldType type) {
if (name == null) {
throw new IllegalArgumentException("name must not be null");
}
if (bytes == null) {
throw new IllegalArgumentException("bytes must not be null");
}
this.fieldsData = bytes;
this.type = type;
this.name = name;
}
{code}
Field class has the exception handling(IllegalArgumentException) for *null
BytesRef object*.
For that reason, I changed the Javadoc to:
{code:java}
/**
* Expert: allows you to customize the {@link
* ...
* @throws IllegalArgumentException if the field name or value
* is null.
*/
public StoredField(String name, BytesRef bytes, FieldType type) {
super(name, bytes, type);
}
{code}
3) Line 71 method
{code:java}
/**
* Create a stored-only field with the given binary value.
* ...
* @throws IllegalArgumentException if the field name is null.
*/
public StoredField(String name, byte[] value) {
super(name, value, TYPE);
}
{code}
It is misleading because there is no explanation for *byte array*.
If you follow that super class, you can see the following code(Field class).
{code:java}
public Field(String name, byte[] value, IndexableFieldType type) {
this(name, value, 0, value.length, type);
}
// To
public Field(String name, byte[] value, int offset, int length,
IndexableFieldType type) {
this(name, new BytesRef(value, offset, length), type);
}{code}
When declaring a new BytesRef, an Illegal exception will be thrown if value is
null.
{code:java}
public BytesRef(byte[] bytes, int offset, int length) {
this.bytes = bytes;
this.offset = offset;
this.length = length;
assert isValid();
}
public boolean isValid() {
if (bytes == null) {
throw new IllegalStateException("bytes is null");
}
...
}{code}
For that reason, I changed the Javadoc to:
{code:java}
/**
* Create a stored-only field with the given binary value.
* <p>NOTE: the provided byte[] is not copied so be sure
* not to change it until you're done with this field.
* @param name field name
* @param value byte array pointing to binary content (not copied)
* @throws IllegalArgumentException if the field name is null.
* @throws IllegalStateException if the value is null.
*/
public StoredField(String name, byte[] value) {
super(name, value, TYPE);
}
{code}
4) Line 85 method
For the *same reason as "3)"*, I changed the Javadoc to:
{code:java}
/**
* Create a stored-only field with the given binary value.
* <p>NOTE: the provided byte[] is not copied so be sure
* not to change it until you're done with this field.
* @param name field name
* @param value byte array pointing to binary content (not copied)
* @param offset starting position of the byte array
* @param length valid length of the byte array
* @throws IllegalArgumentException if the field name is null.
* @throws IllegalStateException if the value is null.
*/
public StoredField(String name, byte[] value, int offset, int length) {
super(name, value, offset, length, TYPE);
}
{code}
5) Line 97 method
For the *same reason as "2)"*, I changed the Javadoc to:
{code:java}
/**
* Create a stored-only field with the given binary value.
* <p>NOTE: the provided BytesRef is not copied so be sure
* not to change it until you're done with this field.
* @param name field name
* @param value BytesRef pointing to binary content (not copied)
* @throws IllegalArgumentException if the field name or value
* is null.
*/
public StoredField(String name, BytesRef value) {
super(name, value, TYPE);
}
{code}
6) Line 119 method
{code:java}
/**
* Expert: allows you to customize the {@link
* ...
* @throws IllegalArgumentException if the field name or value is null.
*/
public StoredField(String name, String value, FieldType type) {
super(name, value, type);
}
{code}
It is misleading because there is no explanation for *type*.
If you follow that super class, you can see the following code(Field class).
{code:java}
/**
* Create field with String value.
* @param name field name
* @param value string value
* @param type field type
* @throws IllegalArgumentException if either the name or value
* is null, or if the field's type is neither indexed() nor stored(),
* or if indexed() is false but storeTermVectors() is true.
* @throws NullPointerException if the type is null
*/
public Field(String name, String value, IndexableFieldType type) {
if (name == null) {
throw new IllegalArgumentException("name must not be null");
}
if (value == null) {
throw new IllegalArgumentException("value must not be null");
}
if (!type.stored() && type.indexOptions() == IndexOptions.NONE) {
throw new IllegalArgumentException("it doesn't make sense to have a field
that "
+ "is neither indexed nor stored");
}
this.type = type;
this.name = name;
this.fieldsData = value;
}
{code}
Field class has the exception handling(NPE) for *null IndexableFieldType
object*.
(if type is null, NPE can be occured when run type.stored())
For that reason, I changed the Javadoc to:
{code:java}
/**
* Expert: allows you to customize the {@link
* FieldType}.
* @param name field name
* @param value string value
* @param type custom {@link FieldType} for this field
* @throws IllegalArgumentException if the field name or value is null.
* @throws NullPointerException if the type is null.
*/
public StoredField(String name, String value, FieldType type) {
super(name, value, type);
}
{code}
7) Wrong Javadocs in Field Class.
I saw the wrong "NullPointerException" throws in Javadoc.
Line 176, 194 and 210 methods have a NPE throws in Javadoc.
{code:java}
// Line 176 method
/**
* Create field with binary value.
* ...
* @throws NullPointerException if the type is null
*/
public Field(String name, byte[] value, IndexableFieldType type) {
this(name, value, 0, value.length, type);
}
// Line 194 method
/**
* Create field with binary value.
* ...
* @throws NullPointerException if the type is null
*/
public Field(String name, byte[] value, int offset, int length,
IndexableFieldType type) {
this(name, new BytesRef(value, offset, length), type);
}
// Line 210 method
/**
* Create field with binary value.
* ...
* @throws NullPointerException if the type is null
*/
public Field(String name, BytesRef bytes, IndexableFieldType type) {
if (name == null) {
throw new IllegalArgumentException("name must not be null");
}
if (bytes == null) {
throw new IllegalArgumentException("bytes must not be null");
}
this.fieldsData = bytes;
this.type = type;
this.name = name;
}{code}
Line 176 and 194 methods call Line 210 method.
However, this method can not cause the NullPointerException.
If type is null, it is just the following code.
{code:java}
protected final IndexableFieldType type = null;
{code}
In fact, I think the null check is missing.
But it's just my idea, so I can not decide whether to remove Javadoc's throws
or insert a null check code.
If it is decided, I will work on the related issue separately.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
