[jira] [Commented] (LUCENE-8460) Javadoc changes in StoredField
[
https://issues.apache.org/jira/browse/LUCENE-8460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16593741#comment-16593741
]
Adrien Grand commented on LUCENE-8460:
--
Looking further at the patch, the "throws IllegalStateException" javadocs are
not really correct, since this exception is only thrown by BytesRef#isValid
which is called via an assert? Let's check for null values before the BytesRef
constructor and throw an IllegalArgumentException?
bq. Are the above throws NullPointerException codes okay?
An IllegalArgumentException would be more consistent with other constructors,
but a NullPointerException isn't bad either. I'm fine either way.
bq. The current issue name is "Javadoc changes in StoredField". However, I
think that the content has changed so much that it needs to be modified. What
do you think a good name is?
Not sure, maybe something like "Better argument validation in StoredField"?
> 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
>Priority: Major
> Labels: javadocs, newbie
> Attachments: LUCENE-8460.patch, LUCENE-8460.patch
>
>
> 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 Bytes
[jira] [Commented] (LUCENE-8460) Javadoc changes in StoredField
[
https://issues.apache.org/jira/browse/LUCENE-8460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16592435#comment-16592435
]
Lucene/Solr QA commented on LUCENE-8460:
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
|| || || || {color:brown} Prechecks {color} ||
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
20s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Release audit (RAT) {color} |
{color:green} 0m 18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Check forbidden APIs {color} |
{color:green} 0m 18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Validate source patterns {color} |
{color:green} 0m 18s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 11m
13s{color} | {color:green} core in the patch passed. {color} |
| {color:black}{color} | {color:black} {color} | {color:black} 13m 26s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Issue | LUCENE-8460 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12937064/LUCENE-8460.patch |
| Optional Tests | compile javac unit ratsources checkforbiddenapis
validatesourcepatterns |
| uname | Linux lucene1-us-west 4.4.0-130-generic #156~14.04.1-Ubuntu SMP Thu
Jun 14 13:51:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | ant |
| Personality |
/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
|
| git revision | master / f26dd13 |
| ant | version: Apache Ant(TM) version 1.9.3 compiled on July 24 2018 |
| Default Java | 1.8.0_172 |
| Test Results |
https://builds.apache.org/job/PreCommit-LUCENE-Build/79/testReport/ |
| modules | C: lucene/core U: lucene/core |
| Console output |
https://builds.apache.org/job/PreCommit-LUCENE-Build/79/console |
| Powered by | Apache Yetus 0.7.0 http://yetus.apache.org |
This message was automatically generated.
> 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
>Priority: Major
> Labels: javadocs, newbie
> Attachments: LUCENE-8460.patch, LUCENE-8460.patch
>
>
> 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
> */
[jira] [Commented] (LUCENE-8460) Javadoc changes in StoredField
[
https://issues.apache.org/jira/browse/LUCENE-8460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16592158#comment-16592158
]
Namgyu Kim commented on LUCENE-8460:
Hi, [~jpountz].
Thank you for your reply.
I uploaded a new patch.
■ Added content
*1) Add Null Check (Field)*
{code:java}
if (type == null) {
throw new IllegalArgumentException("type must not be null");
}
{code}
I added a null check for "IndexableFieldType".
*2) Edit Javadoc (Field, StoredField)*
I changed Javadoc to fit the new "Field" class.
(like delete throws NullPointerException)
*3) Very Small Refactoring (Field)*
before
{code:java}
// Line 210 method
public Field(String name, BytesRef bytes, IndexableFieldType type) {
...
this.fieldsData = bytes;
this.type = type;
this.name = name;
}
// Line 234 method
public Field(String name, String value, IndexableFieldType type) {
...
this.type = type;
this.name = name;
this.fieldsData = value;
}
{code}
after
{code:java}
// Line 210 method
public Field(String name, BytesRef bytes, IndexableFieldType type) {
...
this.name = name;
this.fieldsData = bytes;
this.type = type;
}
// Line 234 method
public Field(String name, String value, IndexableFieldType type) {
...
this.name = name;
this.fieldsData = value;
this.type = type;
}
{code}
■ Discussion
1) Remaining NullPointerException throws
{code:java}
// Line 112 method
/**
* Create field with Reader value.
* ...
* @throws NullPointerException if the reader is null
*/
public Field(String name, Reader reader, IndexableFieldType type) {
...
if (reader == null) {
throw new NullPointerException("reader must not be null");
}
...
}
// Line 144 method
/**
* Create field with TokenStream value.
* ...
* @throws NullPointerException if the tokenStream is null
*/
public Field(String name, TokenStream tokenStream, IndexableFieldType type) {
...
if (tokenStream == null) {
throw new NullPointerException("tokenStream must not be null");
}
...
}{code}
Are the above throws NullPointerException codes okay?
2) Issue Name
The current issue name is "Javadoc changes in StoredField".
However, I think that the content has changed so much that it needs to be
modified.
What do you think a good name is?
> 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
>Priority: Major
> Labels: javadocs, newbie
> Attachments: LUCENE-8460.patch, LUCENE-8460.patch
>
>
> 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 Ille
[jira] [Commented] (LUCENE-8460) Javadoc changes in StoredField
[
https://issues.apache.org/jira/browse/LUCENE-8460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16591299#comment-16591299
]
Adrien Grand commented on LUCENE-8460:
--
bq. In fact, I think the null check is missing.
+1 Let's update the patch to add the missing null checks and only throw
instances of IllegalArgumentException?
> 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
>Priority: Major
> Labels: javadocs, newbie
> Attachments: LUCENE-8460.patch
>
>
> 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.
> * NOTE: the provided byte[] is not copie
[jira] [Commented] (LUCENE-8460) Javadoc changes in StoredField
[
https://issues.apache.org/jira/browse/LUCENE-8460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16586661#comment-16586661
]
Lucene/Solr QA commented on LUCENE-8460:
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
|| || || || {color:brown} Prechecks {color} ||
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
20s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
20s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
20s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Release audit (RAT) {color} |
{color:green} 0m 20s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Check forbidden APIs {color} |
{color:green} 0m 20s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Validate source patterns {color} |
{color:green} 0m 20s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 11m
38s{color} | {color:green} core in the patch passed. {color} |
| {color:black}{color} | {color:black} {color} | {color:black} 13m 47s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Issue | LUCENE-8460 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12936302/LUCENE-8460.patch |
| Optional Tests | compile javac unit ratsources checkforbiddenapis
validatesourcepatterns |
| uname | Linux lucene1-us-west 4.4.0-130-generic #156~14.04.1-Ubuntu SMP Thu
Jun 14 13:51:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | ant |
| Personality |
/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
|
| git revision | master / 5eab1c3 |
| ant | version: Apache Ant(TM) version 1.9.3 compiled on July 24 2018 |
| Default Java | 1.8.0_172 |
| Test Results |
https://builds.apache.org/job/PreCommit-LUCENE-Build/77/testReport/ |
| modules | C: lucene/core U: lucene/core |
| Console output |
https://builds.apache.org/job/PreCommit-LUCENE-Build/77/console |
| Powered by | Apache Yetus 0.7.0 http://yetus.apache.org |
This message was automatically generated.
> 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
>Priority: Major
> Labels: newbie
> Attachments: LUCENE-8460.patch
>
>
> 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
[jira] [Commented] (LUCENE-8460) Javadoc changes in StoredField
[
https://issues.apache.org/jira/browse/LUCENE-8460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16586289#comment-16586289
]
Namgyu Kim commented on LUCENE-8460:
Because of the "Field Class", I set it to major issue.
But if someone doesn't think it's valid, I'll change it to minor issue.
> 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
>Priority: Major
> Labels: newbie
> Attachments: LUCENE-8460.patch
>
>
> 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.
> * NOTE: the provided byte[] is not copied so be sure
> * not to change it until you're
[jira] [Commented] (LUCENE-8460) Javadoc changes in StoredField
[
https://issues.apache.org/jira/browse/LUCENE-8460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16586281#comment-16586281
]
Namgyu Kim commented on LUCENE-8460:
Because of the "Field Class", I first set it to major issue.
But if you do not think it's valid, I'll change it to minor issue.
> 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
>Priority: Major
> Labels: newbie
>
> 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.
> * NOTE: the provided byte[] is not copied so be sure
> * not to change it until you're done with this field.
> * @param name fie
