Skip to content

Commit

Permalink
Cleanup, refactoring (FindBugs vs. SpotBugs annotations, warnings).
Browse files Browse the repository at this point in the history
  • Loading branch information
uhafner committed Mar 1, 2018
1 parent 906c1d3 commit c6f09d7
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 54 deletions.
39 changes: 26 additions & 13 deletions src/main/java/edu/hm/hafner/analysis/IssueBuilder.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package edu.hm.hafner.analysis;

import edu.umd.cs.findbugs.annotations.CheckForNull;

/**
* Creates new {@link Issue issues} using the builder pattern. All properties that have not been set in the builder will
* be set to their default value.
Expand All @@ -24,34 +26,45 @@ public class IssueBuilder {
private int lineEnd = 0;
private int columnStart = 0;
private int columnEnd = 0;
@CheckForNull
private LineRangeList lineRanges;
@CheckForNull
private String category;
@CheckForNull
private String type;
@CheckForNull
private Priority priority;
@CheckForNull
private String message;
@CheckForNull
private String description;
@CheckForNull
private String packageName;
@CheckForNull
private String moduleName;
@CheckForNull
private String origin;
@CheckForNull
private String reference;
@CheckForNull
private String fingerprint;

public IssueBuilder setFingerprint(final String fingerprint) {
public IssueBuilder setFingerprint(@CheckForNull final String fingerprint) {
this.fingerprint = fingerprint;
return this;
}

public IssueBuilder setFileName(final String fileName) {
public IssueBuilder setFileName(@CheckForNull final String fileName) {
this.fileName = fileName;
return this;
}

public IssueBuilder setLineStart(final int lineStart) {
public IssueBuilder setLineStart(@CheckForNull final int lineStart) {
this.lineStart = lineStart;
return this;
}

public IssueBuilder setLineEnd(final int lineEnd) {
public IssueBuilder setLineEnd(@CheckForNull final int lineEnd) {
this.lineEnd = lineEnd;
return this;
}
Expand All @@ -66,47 +79,47 @@ public IssueBuilder setColumnEnd(final int columnEnd) {
return this;
}

public IssueBuilder setCategory(final String category) {
public IssueBuilder setCategory(@CheckForNull final String category) {
this.category = category;
return this;
}

public IssueBuilder setType(final String type) {
public IssueBuilder setType(@CheckForNull final String type) {
this.type = type;
return this;
}

public IssueBuilder setPackageName(final String packageName) {
public IssueBuilder setPackageName(@CheckForNull final String packageName) {
this.packageName = packageName;
return this;
}

public IssueBuilder setModuleName(final String moduleName) {
public IssueBuilder setModuleName(@CheckForNull final String moduleName) {
this.moduleName = moduleName;
return this;
}

public IssueBuilder setOrigin(final String origin) {
public IssueBuilder setOrigin(@CheckForNull final String origin) {
this.origin = origin;
return this;
}

public IssueBuilder setReference(final String reference) {
public IssueBuilder setReference(@CheckForNull final String reference) {
this.reference = reference;
return this;
}

public IssueBuilder setPriority(final Priority priority) {
public IssueBuilder setPriority(@CheckForNull final Priority priority) {
this.priority = priority;
return this;
}

public IssueBuilder setMessage(final String message) {
public IssueBuilder setMessage(@CheckForNull final String message) {
this.message = message;
return this;
}

public IssueBuilder setDescription(final String description) {
public IssueBuilder setDescription(@CheckForNull final String description) {
this.description = description;
return this;
}
Expand Down
22 changes: 9 additions & 13 deletions src/main/java/edu/hm/hafner/analysis/RegexpParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,7 @@ public abstract class RegexpParser extends AbstractParser<Issue> {
protected static final String ANT_TASK = "^(?:.*\\[.*\\])?\\s*";

/** Pattern of compiler warnings. */
private Pattern pattern;

private void setPattern(final String warningPattern, final boolean useMultiLine) {
if (useMultiLine) {
pattern = Pattern.compile(warningPattern, Pattern.MULTILINE);
}
else {
pattern = Pattern.compile(warningPattern);
}
}
private final Pattern pattern;

/**
* Creates a new instance of {@link RegexpParser}.
Expand All @@ -42,7 +33,12 @@ private void setPattern(final String warningPattern, final boolean useMultiLine)
protected RegexpParser(final String warningPattern, final boolean useMultiLine) {
super();

setPattern(warningPattern, useMultiLine);
if (useMultiLine) {
pattern = Pattern.compile(warningPattern, Pattern.MULTILINE);
}
else {
pattern = Pattern.compile(warningPattern);
}
}

/**
Expand All @@ -56,14 +52,14 @@ protected RegexpParser(final String warningPattern, final boolean useMultiLine)
* @throws ParsingCanceledException
* indicates that the user canceled the operation
*/
@SuppressWarnings("ReferenceEquality")
@SuppressWarnings({"ReferenceEquality", "PMD.CompareObjectsWithEquals"})
protected void findAnnotations(final String content, final Issues<Issue> issues)
throws ParsingCanceledException {
Matcher matcher = pattern.matcher(content);

while (matcher.find()) {
Issue warning = createIssue(matcher, new IssueBuilder());
if (warning != FALSE_POSITIVE) { // NOPMD
if (warning != FALSE_POSITIVE) {
issues.add(warning);
}
if (Thread.interrupted()) {
Expand Down
14 changes: 6 additions & 8 deletions src/main/java/edu/hm/hafner/analysis/SecureDigester.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package edu.hm.hafner.analysis;

import org.apache.commons.digester3.Digester;
import org.xml.sax.EntityResolver;
import org.xml.sax.InputSource;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

/**
* A secure {@link Digester} implementation that does not resolve external entities.
*
Expand All @@ -16,19 +17,16 @@ public final class SecureDigester extends Digester {
* @param classWithClassLoader the class to get the class loader from
*/
public SecureDigester(final Class<?> classWithClassLoader) {
super();

setClassLoader(classWithClassLoader.getClassLoader());
setValidating(false);
disableFeature("external-general-entities");
disableFeature("external-parameter-entities");
setEntityResolver(new EntityResolver() {
@Override
public InputSource resolveEntity(final String publicId, final String systemId) {
return new InputSource();
}
});
setEntityResolver((publicId, systemId) -> new InputSource());
}

@SuppressWarnings("all")
@SuppressWarnings("all") @SuppressFBWarnings("RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT")
private void disableFeature(final String feature) {
try {
setFeature("http://xml.org/sax/features/" + feature, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import org.dom4j.DocumentException;
import org.xml.sax.SAXException;

import static edu.hm.hafner.analysis.parser.FindBugsParser.PriorityProperty.*;

import edu.hm.hafner.analysis.Issue;
import edu.hm.hafner.analysis.IssueBuilder;
import edu.hm.hafner.analysis.IssueParser;
Expand All @@ -31,7 +33,6 @@
import edu.hm.hafner.analysis.ParsingException;
import edu.hm.hafner.analysis.Priority;
import edu.hm.hafner.analysis.SecureDigester;
import static edu.hm.hafner.analysis.parser.FindBugsParser.PriorityProperty.*;
import edu.hm.hafner.util.VisibleForTesting;
import edu.umd.cs.findbugs.BugAnnotation;
import edu.umd.cs.findbugs.BugInstance;
Expand Down Expand Up @@ -82,6 +83,8 @@ public enum PriorityProperty {
* determines whether to use the rank or confidence when evaluation the {@link Priority}
*/
public FindBugsParser(final PriorityProperty priorityProperty) {
super();

this.priorityProperty = priorityProperty;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ public abstract class AbstractDryParser<T> extends AbstractParser<CodeDuplicatio
* minimum number of duplicate lines for normal priority warnings
*/
protected AbstractDryParser(final int highThreshold, final int normalThreshold) {
super();

this.highThreshold = highThreshold;
this.normalThreshold = normalThreshold;
}
Expand Down
1 change: 1 addition & 0 deletions src/main/java/edu/hm/hafner/util/TreeString.java
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ public boolean isBlank() {
*
* @return the new {@link TreeString} or {@code null} if the parameter is {@code null}
*/
@SuppressWarnings("PMD.ShortMethodName")
@CheckForNull
public static TreeString of(@CheckForNull final String string) {
if (string == null) {
Expand Down
14 changes: 3 additions & 11 deletions src/main/java/edu/hm/hafner/util/TreeStringBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
import java.util.Map;
import java.util.Map.Entry;

import edu.umd.cs.findbugs.annotations.CheckForNull;

/**
* Builds {@link TreeString}s that share common prefixes. Call {@link #intern(String)} and you get the {@link
* TreeString} that represents the same string, but as you interns more strings that share the same prefixes, those
Expand All @@ -30,11 +28,7 @@ public class TreeStringBuilder {
*
* @return the String as {@link TreeString} instance
*/
@CheckForNull
public TreeString intern(@CheckForNull final String string) {
if (string == null) {
return null;
}
public TreeString intern(final String string) {
return getRoot().intern(string).getNode();
}

Expand All @@ -46,10 +40,7 @@ public TreeString intern(@CheckForNull final String string) {
*
* @return the String as {@link TreeString} instance
*/
public TreeString intern(@CheckForNull final TreeString treeString) {
if (treeString == null) {
return null;
}
public TreeString intern(final TreeString treeString) {
return getRoot().intern(treeString.toString()).getNode();
}

Expand Down Expand Up @@ -128,6 +119,7 @@ public Child intern(final String string) {
/**
* Makes sure {@link #children} is writable.
*/
@SuppressWarnings("PMD.CompareObjectsWithEquals")
private void makeWritable() {
if (children == NO_CHILDREN) {
children = new HashMap<>();
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/edu/hm/hafner/analysis/AbstractParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public abstract class AbstractParserTest<T extends Issue> extends ResourceTest {
* the file that should contain some issues
*/
protected AbstractParserTest(final String fileWithIssuesName) {
super();

this.fileWithIssuesName = fileWithIssuesName;
}

Expand Down
2 changes: 2 additions & 0 deletions src/test/java/edu/hm/hafner/analysis/ModuleDetectorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@

import edu.hm.hafner.analysis.ModuleDetector.FileSystem;
import edu.hm.hafner.util.ResourceTest;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

/**
* Tests the class {@link ModuleDetector}.
*/
class ModuleDetectorTest extends ResourceTest {
private static final String MANIFEST = "MANIFEST.MF";
private static final String MANIFEST_NAME = "MANIFEST-NAME.MF";
@SuppressFBWarnings("DMI_HARDCODED_ABSOLUTE_FILENAME")
private static final File ROOT = new File("/tmp");
private static final String PREFIX = normalizeRoot();

Expand Down
1 change: 1 addition & 0 deletions src/test/java/edu/hm/hafner/util/ResourceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
*
* @author Ullrich Hafner
*/
@SuppressWarnings("PMD.AbstractClassWithoutAbstractMethod")
public abstract class ResourceTest {
/**
* Reads the contents of the desired resource. The rules for searching resources associated with this test class are
Expand Down
8 changes: 0 additions & 8 deletions src/test/java/edu/hm/hafner/util/TreeStringBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,6 @@
* @author Kohsuke Kawaguchi
*/
class TreeStringBuilderTest {
@Test
void shouldReturnNullIfNullIfProvided() {
TreeStringBuilder builder = new TreeStringBuilder();

assertThat(builder.intern((String)null)).isNull();
assertThat(builder.intern((TreeString)null)).isNull();
}

@Test
void shouldCreateSimpleTreeStringsWithBuilder() {
TreeStringBuilder builder = new TreeStringBuilder();
Expand Down

0 comments on commit c6f09d7

Please sign in to comment.