Skip to content
This repository has been archived by the owner on Jul 3, 2023. It is now read-only.

ANY23-445 Review spotbugs issues #151

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

ANY23-445 Review spotbugs issues #151

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
61d8320
ANY23-445 Review spotbugs issues
lewismc Oct 16, 2019
03aca0a
Merge branch 'master' into ANY23-445
lewismc Oct 16, 2019
cde3277
ANY23-445 Review spotbugs issues
lewismc Oct 16, 2019
dcef1b4
ANY23-445 Review spotbugs issues
lewismc Oct 17, 2019
842d3f6
ANY23-445 Review spotbugs issues
lewismc Oct 17, 2019
ff3ce37
ANY23-445 Review spotbugs issues
lewismc Oct 17, 2019
ead2757
ANY23-445 Review spotbugs issues
lewismc Oct 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion api/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<artifactId>apache-any23-api</artifactId>

<name>Apache Any23 :: Base API</name>
<description>Any23 library external API.</description>
<description>Any23 library external API</description>

<dependencies>
<dependency>
Expand Down Expand Up @@ -89,6 +89,16 @@
</plugin>
</plugins>
</pluginManagement>
<plugins>
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>${maven-spotbugs-plugin.version}</version>
<configuration>
<omitVisitors>UnreadFields,Naming,FindUnrelatedTypesInGenericContainer</omitVisitors>
</configuration>
</plugin>
</plugins>
</build>

<profiles>
Expand Down
8 changes: 3 additions & 5 deletions api/src/main/java/org/apache/any23/configuration/DefaultConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.io.InputStream;
import java.util.Properties;

/**
Expand Down Expand Up @@ -74,8 +75,8 @@ public static synchronized ModifiableConfiguration copy() {

private static Properties loadDefaultProperties() {
final Properties properties = new Properties();
try {
properties.load( DefaultConfiguration.class.getResourceAsStream(DEFAULT_CONFIG_FILE) );
try(InputStream is = DefaultConfiguration.class.getResourceAsStream(DEFAULT_CONFIG_FILE)) {
properties.load(is);
} catch (IOException ioe) {
throw new IllegalStateException("Error while loading default configuration.", ioe);
}
Expand Down Expand Up @@ -129,9 +130,6 @@ public synchronized int getPropertyIntOrFail(String propertyName) {
@Override
public synchronized boolean getFlagProperty(final String propertyName) {
final String value = getPropertyOrFail(propertyName);
if(value == null) {
return false;
}
if(FLAG_PROPERTY_ON.equals(value)) {
return true;
}
Expand Down
1 change: 1 addition & 0 deletions api/src/main/java/org/apache/any23/extractor/ExtractorGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public boolean allExtractorsSupportAllContentTypes() {
return true;
}

@SuppressWarnings("unlikely-arg-type")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's actually a bug here, so adding @SuppressWarnings kinda defeats the purpose of the spotbugs plugin. getSupportedMIMETypes() returns a list of MIMEType, so checking if it contains a String will always return false!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed both the SupressWarnings javac annotation and thought about this one again.

I've decided to introduce <omitVisitors>FindUnrelatedTypesInGenericContainer</omitVisitors> which will omit spotbugs from identifying this issue as a bug. My justification is that, in this case, because the ExtractorFactory#getSupportedMIMETypes() supports wildcards, and as far as I can see there is currently no way to determine the full Collection (e.g. */*) of supported MimeType's then we cannot enforce another check whether an ExtractorGroup supports all MimeType's.

What do you think about this assessment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I wasn't clear. The code inside this method has a bug, as correctly identified by the spotbugs plugin. The code is testing if a Collection<MIMEType> contains a String. This will always return false, as a String cannot be an instance of MIMEType.

Therefore, the method needs to be fixed, and the spotbugs plugin is doing its job correctly: no suppression of warnings is needed here.

private boolean supportsAllContentTypes(ExtractorFactory<?> factory) {
return factory.getSupportedMIMETypes().contains("*/*");
}
Expand Down
1 change: 0 additions & 1 deletion api/src/main/java/org/apache/any23/vocab/HCard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public static HCard getInstance() {
public IRI Address = createClass(NS, "Address");
public IRI Geo = createClass(NS, "Geo");


public IRI name = createProperty(NS, "name");
public IRI honorific_prefix = createProperty(NS, "honorific-prefix");
public IRI given_name = createProperty(NS, "given-name");
Expand Down
1 change: 0 additions & 1 deletion api/src/main/java/org/apache/any23/vocab/HResume.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public static HResume getInstance() {
public IRI contact = createClass(NS, "contact");
public IRI affiliation = createClass(NS, "affiliation");


public IRI name = createProperty(NS, "name");
public IRI summary = createProperty(NS, "summary");
public IRI skill = createProperty(NS, "skill");
Expand Down
5 changes: 0 additions & 5 deletions api/src/main/java/org/apache/any23/vocab/OGP.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,6 @@ public static OGP getInstance() {
public final IRI audioType = createProperty(AUDIO__TYPE);
public final IRI audioAlt = createProperty(AUDIO__ALT);

@SuppressWarnings("unused")
private IRI createClass(String localName) {
return createClass(NS, localName);
}

private IRI createProperty(String localName) {
return createProperty(NS, localName);
}
Expand Down
5 changes: 0 additions & 5 deletions api/src/main/java/org/apache/any23/vocab/OGPArticle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,6 @@ public static OGPArticle getInstance() {
public final IRI articleSection = createProperty(ARTICLE__SECTION);
public final IRI articleTag = createProperty(ARTICLE__TAG);

@SuppressWarnings("unused")
private IRI createClass(String localName) {
return createClass(NS, localName);
}

private IRI createProperty(String localName) {
return createProperty(NS, localName);
}
Expand Down
5 changes: 0 additions & 5 deletions api/src/main/java/org/apache/any23/vocab/OGPBook.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,6 @@ public static OGPBook getInstance() {
public final IRI bookReleaseDate = createProperty(BOOK__RELEASE_DATE);
public final IRI bookTag = createProperty(BOOK__TAG);

@SuppressWarnings("unused")
private IRI createClass(String localName) {
return createClass(NS, localName);
}

private IRI createProperty(String localName) {
return createProperty(NS, localName);
}
Expand Down
5 changes: 0 additions & 5 deletions api/src/main/java/org/apache/any23/vocab/OGPMusic.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ public static OGPMusic getInstance() {

public final IRI musicCreator = createProperty(MUSIC__CREATOR);

@SuppressWarnings("unused")
private IRI createClass(String localName) {
return createClass(NS, localName);
}

private IRI createProperty(String localName) {
return createProperty(NS, localName);
}
Expand Down
5 changes: 0 additions & 5 deletions api/src/main/java/org/apache/any23/vocab/OGPProfile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,6 @@ public static OGPProfile getInstance() {
public final IRI profileUsername = createProperty(PROFILE__USERNAME);
public final IRI profileGender = createProperty(PROFILE__GENDER);

@SuppressWarnings("unused")
private IRI createClass(String localName) {
return createClass(NS, localName);
}

private IRI createProperty(String localName) {
return createProperty(NS, localName);
}
Expand Down
5 changes: 0 additions & 5 deletions api/src/main/java/org/apache/any23/vocab/OGPVideo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,6 @@ public static OGPVideo getInstance() {
public final IRI videoTag = createProperty(VIDEO__TAG);
public final IRI videoSeries = createProperty(VIDEO__SERIES);

@SuppressWarnings("unused")
private IRI createClass(String localName) {
return createClass(NS, localName);
}

private IRI createProperty(String localName) {
return createProperty(NS, localName);
}
Expand Down
5 changes: 0 additions & 5 deletions api/src/main/java/org/apache/any23/vocab/SINDICE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,6 @@ public static SINDICE getInstance() {
*/
public final IRI date = createProperty(DATE);


private IRI createClass(String localName) {
return createClass(NS, localName);
}

private IRI createProperty(String localName) {
return createProperty(NS, localName);
}
Expand Down
6 changes: 3 additions & 3 deletions api/src/main/java/org/apache/any23/vocab/Vocabulary.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,12 @@ public IRI getProperty(String name, IRI defaultValue) {
*/
public IRI getPropertyCamelCase(String property) {
String[] names = property.split("\\W");
String camelCase = names[0];
StringBuilder camelCase = new StringBuilder(property.length()).append(names[0]);
for (int i = 1; i < names.length; i++) {
String tmp = names[i];
camelCase += tmp.replaceFirst("(.)", tmp.substring(0, 1).toUpperCase(java.util.Locale.ROOT));
camelCase.append(tmp.replaceFirst("(.)", tmp.substring(0, 1).toUpperCase(java.util.Locale.ROOT)));
HansBrende marked this conversation as resolved.
Show resolved Hide resolved
}
return getProperty(camelCase);
return getProperty(camelCase.toString());
}

/**
Expand Down
6 changes: 3 additions & 3 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@
<maven-gpg-plugin.version>1.6</maven-gpg-plugin.version>
<maven-war-plugin.version>3.2.3</maven-war-plugin.version>
<maven-invoker-plugin.version>3.2.1</maven-invoker-plugin.version>
<maven-checkstyle-plugin.version>3.1.12.2</maven-checkstyle-plugin.version>
<maven-spotbugs-plugin.version>3.1.12.2</maven-spotbugs-plugin.version>
<forbiddenapis.version>2.6</forbiddenapis.version>

<!--
Expand Down Expand Up @@ -890,7 +890,7 @@
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>${maven-checkstyle-plugin.version}</version>
<version>${maven-spotbugs-plugin.version}</version>
</plugin>
</plugins>
</build>
Expand Down Expand Up @@ -967,7 +967,7 @@
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>${maven-checkstyle-plugin.version}</version>
<version>${maven-spotbugs-plugin.version}</version>
</plugin>

<!-- Check style report. -->
Expand Down