jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/376559 )
Change subject: Adding spotbugs to the build. ...................................................................... Adding spotbugs to the build. Spotbugs catches at least one real bug (this patch contains the correction). It looks like a good idea to enable it. Note that it probably increases the build time slightly (no metric at this point). Another patch will add some more configuration to check non-nullity. Change-Id: Ib6141bde067792ee4c7702cdf36ac07502007c32 --- M README.md M pom.xml M src/main/java/org/wikimedia/search/extra/analysis/filters/PreserveOriginalFilter.java M src/main/java/org/wikimedia/search/extra/fuzzylike/FuzzyLikeThisQueryBuilder.java M src/main/java/org/wikimedia/search/extra/latency/SearchLatencyListener.java M src/main/java/org/wikimedia/search/extra/regex/SourceRegexQueryBuilder.java 6 files changed, 75 insertions(+), 6 deletions(-) Approvals: jenkins-bot: Verified DCausse: Looks good to me, approved diff --git a/README.md b/README.md index c2fe12f..9f8a318 100644 --- a/README.md +++ b/README.md @@ -56,3 +56,19 @@ ```bash ./bin/elasticsearch-plugin install org.wikimedia.search:extra:x.y.z ``` + +Build +----- +[Spotbugs](https://spotbugs.github.io/) is run during the `verify` phase of the +build to find common issues. The build will break if any issue is found. The +issues will be reported on the console. + +To run just the check, use `mvn spotbugs:check` on a project that was already +compiled (`mvn compile`). `mvn spotbugs:gui` will provide a graphical UI that +might be easier to read. + +Like all tools, spotbugs is much dumber than you. If you find a false positive, +you can ignore it with the `@SuppressFBWarnings` annotation. You can provide a +justification to make document why this rule should be ignored in this specific +case. Some rules don't make sense for this project and they can be ignored via +[`src/dev-tools/spotbugs-excludes.xml`](https://spotbugs.readthedocs.io/en/latest/filter.html). diff --git a/pom.xml b/pom.xml index fa0f932..b1e55d3 100644 --- a/pom.xml +++ b/pom.xml @@ -50,6 +50,7 @@ <maven.compiler.target>1.8</maven.compiler.target> <maven.compiler.source>1.8</maven.compiler.source> <maven.compiler.showWarnings>true</maven.compiler.showWarnings> + <spotbugs.version>3.1.0-RC5</spotbugs.version> </properties> <build> @@ -63,6 +64,31 @@ </resource> </resources> <plugins> + <plugin> + <groupId>com.github.hazendaz.spotbugs</groupId> + <artifactId>spotbugs-maven-plugin</artifactId> + <version>3.0.6</version> + <dependencies> + <dependency> + <groupId>com.github.spotbugs</groupId> + <artifactId>spotbugs</artifactId> + <version>${spotbugs.version}</version> + </dependency> + </dependencies> + <configuration> + <effort>high</effort> + <threshold>low</threshold> + </configuration> + <executions> + <execution> + <id>findbugs-check</id> + <goals> + <goal>check</goal> + </goals> + <phase>verify</phase> + </execution> + </executions> + </plugin> <plugin> <groupId>com.carrotsearch.randomizedtesting</groupId> <artifactId>junit4-maven-plugin</artifactId> @@ -354,6 +380,24 @@ <dependencies> <dependency> + <groupId>com.github.spotbugs</groupId> + <artifactId>spotbugs-annotations</artifactId> + <version>${spotbugs.version}</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>net.jcip</groupId> + <artifactId>jcip-annotations</artifactId> + <version>1.0</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>com.google.code.findbugs</groupId> + <artifactId>jsr305</artifactId> + <version>3.0.2</version> + <scope>provided</scope> + </dependency> + <dependency> <groupId>org.projectlombok</groupId> <artifactId>lombok</artifactId> <version>1.16.16</version> diff --git a/src/main/java/org/wikimedia/search/extra/analysis/filters/PreserveOriginalFilter.java b/src/main/java/org/wikimedia/search/extra/analysis/filters/PreserveOriginalFilter.java index d59b8cd..e121db4 100644 --- a/src/main/java/org/wikimedia/search/extra/analysis/filters/PreserveOriginalFilter.java +++ b/src/main/java/org/wikimedia/search/extra/analysis/filters/PreserveOriginalFilter.java @@ -1,5 +1,6 @@ package org.wikimedia.search.extra.analysis.filters; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.apache.lucene.analysis.TokenFilter; import org.apache.lucene.analysis.TokenStream; import org.apache.lucene.analysis.miscellaneous.KeywordRepeatFilter; @@ -16,6 +17,12 @@ * position. The purpose is very similar to {@link KeywordRepeatFilter}+{@link RemoveDuplicatesTokenFilter} * but this approach does not require that the filters support the keyword attribute. */ +// TODO: check if the behaviour of equals() is actually what is expected. Read +// https://sourceforge.net/p/findbugs/bugs/1379/ before blindly adding an +// equals() method to PreserveOriginalFilter. +@SuppressFBWarnings( + value = "EQ_DOESNT_OVERRIDE_EQUALS", + justification = "equals() as defined in org.apache.lucene.util.AttributeSource seems strong enough.") public class PreserveOriginalFilter extends TokenFilter { private final CharTermAttribute cattr; private final PositionIncrementAttribute posIncr; diff --git a/src/main/java/org/wikimedia/search/extra/fuzzylike/FuzzyLikeThisQueryBuilder.java b/src/main/java/org/wikimedia/search/extra/fuzzylike/FuzzyLikeThisQueryBuilder.java index e08b2c9..98dab2d 100644 --- a/src/main/java/org/wikimedia/search/extra/fuzzylike/FuzzyLikeThisQueryBuilder.java +++ b/src/main/java/org/wikimedia/search/extra/fuzzylike/FuzzyLikeThisQueryBuilder.java @@ -19,6 +19,7 @@ package org.wikimedia.search.extra.fuzzylike; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import lombok.Getter; import lombok.Setter; import lombok.experimental.Accessors; @@ -57,6 +58,7 @@ @Deprecated @Accessors(fluent = true, chain = true) @Getter @Setter +@SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "We don't care about exposing representation in a builder") public class FuzzyLikeThisQueryBuilder extends AbstractQueryBuilder<FuzzyLikeThisQueryBuilder> { public static final ParseField NAME = new ParseField("fuzzy_like_this", "flt", "fuzzyLikeThis"); diff --git a/src/main/java/org/wikimedia/search/extra/latency/SearchLatencyListener.java b/src/main/java/org/wikimedia/search/extra/latency/SearchLatencyListener.java index 3d078e5..0e9792e 100644 --- a/src/main/java/org/wikimedia/search/extra/latency/SearchLatencyListener.java +++ b/src/main/java/org/wikimedia/search/extra/latency/SearchLatencyListener.java @@ -140,7 +140,7 @@ } synchronized double getMillisAtPercentile(double percentile) { - return current.getValueAtPercentile(percentile) / TimeValue.NSEC_PER_MSEC; + return current.getValueAtPercentile(percentile) / ((double)TimeValue.NSEC_PER_MSEC); } synchronized TimeValue getTimeValueAtPercentile(double percentile) { diff --git a/src/main/java/org/wikimedia/search/extra/regex/SourceRegexQueryBuilder.java b/src/main/java/org/wikimedia/search/extra/regex/SourceRegexQueryBuilder.java index 7bf63a7..50023d5 100644 --- a/src/main/java/org/wikimedia/search/extra/regex/SourceRegexQueryBuilder.java +++ b/src/main/java/org/wikimedia/search/extra/regex/SourceRegexQueryBuilder.java @@ -39,11 +39,11 @@ public class SourceRegexQueryBuilder extends AbstractQueryBuilder<SourceRegexQueryBuilder> { public static final ParseField NAME = new ParseField("source_regex", "sourceRegex", "source-regex"); - public static ParseField FIELD = new ParseField("field"); - public static ParseField REGEX = new ParseField("regex"); - public static ParseField LOAD_FROM_SOURCE = new ParseField("load_from_source"); - public static ParseField NGRAM_FIELD = new ParseField("ngram_field"); - public static ParseField GRAM_SIZE = new ParseField("gram_size"); + public static final ParseField FIELD = new ParseField("field"); + public static final ParseField REGEX = new ParseField("regex"); + public static final ParseField LOAD_FROM_SOURCE = new ParseField("load_from_source"); + public static final ParseField NGRAM_FIELD = new ParseField("ngram_field"); + public static final ParseField GRAM_SIZE = new ParseField("gram_size"); public static final boolean DEFAULT_LOAD_FROM_SOURCE = true; public static final int DEFAULT_GRAM_SIZE = 3; -- To view, visit https://gerrit.wikimedia.org/r/376559 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib6141bde067792ee4c7702cdf36ac07502007c32 Gerrit-PatchSet: 5 Gerrit-Project: search/extra Gerrit-Branch: master Gerrit-Owner: Gehel <guillaume.leder...@wikimedia.org> Gerrit-Reviewer: DCausse <dcau...@wikimedia.org> Gerrit-Reviewer: EBernhardson <ebernhard...@wikimedia.org> Gerrit-Reviewer: Gehel <guillaume.leder...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
