Issue 574 #1861
Issue 574 #1861
Conversation
-Implements test case for @SerializedName -Adjusts UnreadFields file to skip fields with @SerializedName annotation -Confirms URF_UNREAD_FIELD bugs in Issue574Test.java
-Refactors code initially placed in "UnreadFields.java" to new util class ("UnreadFieldsAnnotationChecker.java")
-Adds support for checking for XmlAttribute and XmlValue tags
-Separates test classes for different annotation groups
…xtension -Added support for suppressing warnings related to @RegisterExtension annotation
| * a given field, then the "URF_UNREAD_FIELD" detector should be skipped for the field. | ||
| * @see <a href="https://github.com/spotbugs/spotbugs/issues/574">GitHub issue</a> | ||
| */ | ||
| public final class UnreadFieldsAnnotationChecker { |
There was a problem hiding this comment.
Only one class is depending on this util class, then we do not need to make this util class.
Instead, implement it as a method in the UnreadFields class.
| /** | ||
| * Array of annotations which, if present, signal that "URF_UNREAD_FIELD" detector should be skipped. | ||
| */ | ||
| private static final Class[] IGNORE_RULE_FOR_THESE_ANNOTATIONS = { |
There was a problem hiding this comment.
Use String[] instead, then we do not need to depend on libraries in runtime.
| @@ -94,6 +93,7 @@ dependencies { | |||
| api project(':spotbugs-annotations') | |||
|
|
|||
| api "com.google.code.gson:gson:2.8.9" | |||
| api 'org.junit.jupiter:junit-jupiter:5.8.2' | |||
There was a problem hiding this comment.
Do not depend on thirdparty libraries only for annotations.
| @@ -76,7 +76,6 @@ dependencies { | |||
| exclude group: 'jaxen', module: 'jaxen' | |||
| exclude group: 'javax.xml.stream', module: 'stax-api' | |||
| exclude group: 'net.java.dev.msv', module: 'xsdlib' | |||
| exclude group: 'javax.xml.bind', module: 'jaxb-api' | |||
There was a problem hiding this comment.
Do not depend on thirdparty libraries only for annotations.
| */ | ||
| public final class Issue574SerializedName { | ||
|
|
||
| @SerializedName("name") |
There was a problem hiding this comment.
Keep each class owns only one responsibility. Currently this class has two responsibilities, so better to split into two classes:
- Class to run JUnit test cases.
- Class to serialize by Gson.
| public static void testSerializedName() { | ||
| Issue574SerializedName target = new Issue574SerializedName("v1", "v2"); | ||
| Gson gson = new Gson(); | ||
| String json = gson.toJson(target); | ||
| System.out.println(json); | ||
| } |
There was a problem hiding this comment.
Test method should include one assert method invocation, or ./gradlew test cannot verify implementations.
There was a problem hiding this comment.
And we do not have to test Gson's feature in our project. I guess that we can remove this file completely from this PR.
| if (throwable instanceof FileNotFoundException) { | ||
| return; | ||
| } | ||
| throw throwable; |
There was a problem hiding this comment.
Please do not use TestExecutionExceptionHandler for this purpose, because we cannot judge which FileNotFoundException is acceptable.
To keep our code readable and intuitive, use assertThrows() method instead.
There was a problem hiding this comment.
Or I guess that we can remove this code from this PR. Current code does not apply Issue574RegisterExtension during JUnit test executions.
| import javax.xml.bind.annotation.XmlValue; | ||
|
|
||
| /** | ||
| * Tests that "URF_UNREAD_FIELD" warning is suppressed for fields with @XmlElement, @XmlAttribute, and @XmlValue |
There was a problem hiding this comment.
@ in javadoc is treated as a javadoc tag. It is better to wrap by the code tag like {@code @FooBar}, or the link tag like {@link FooBar}.
Attempt at addressing #574 to ignore URF_UNREAD_FIELD warning for fields with certain annotations (e.g. @SerializedName, @xmlelement, and @RegisterExtension). New contributor / completed as part of university project.
Make sure these boxes are checked before submitting your PR -- thank you!
CHANGELOG.mdif you have changed SpotBugs code