opendev/gerrit
Code Issues Proposed changes

Error Prone: Enable and fix NarrowingCompoundAssignment

Browse Source 
I'm not going to pretend I understand the code in IdGenerator; the
replacement was exactly what was suggested by Error Prone, and based on
the documentation[1], this construction is just making explicit the way
that Java was already interpreting it.

[1] https://github.com/google/error-prone/blob/master/docs/bugpattern/NarrowingCompoundAssignment.md

Change-Id: I2437b71e774b607871916c4386ee9e42e75554cd
This commit is contained in:
Dave Borowitz
2019-05-24 10:52:43 -07:00
committed by David Pursehouse
parent dc00c1d8f5
commit cd7efe1d11
3 changed files with 15 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
java/com/google/gerrit/server/index/change/AllChangesIndexer.java
View File

@@ -23,6 +23,7 @@ import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
import com.google.common.base.Stopwatch; import com.google.common.base.Stopwatch;
import com.google.common.collect.ComparisonChain; import com.google.common.collect.ComparisonChain;
import com.google.common.flogger.FluentLogger; import com.google.common.flogger.FluentLogger;
import com.google.common.primitives.Ints;
import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.index.SiteIndexer; import com.google.gerrit.index.SiteIndexer;
@@ -108,7 +109,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
int projectsFailed = 0; int projectsFailed = 0;
for (Project.NameKey name : projectCache.all()) { for (Project.NameKey name : projectCache.all()) {
try (Repository repo = repoManager.openRepository(name)) { try (Repository repo = repoManager.openRepository(name)) {
long size = estimateSize(repo); int size = estimateSize(repo);
changeCount += size; changeCount += size;
projects.add(new ProjectHolder(name, size)); projects.add(new ProjectHolder(name, size));
} catch (IOException e) { } catch (IOException e) {
@@ -126,15 +127,17 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
return indexAll(index, projects); return indexAll(index, projects);
} }
private long estimateSize(Repository repo) throws IOException { private int estimateSize(Repository repo) throws IOException {
// Estimate size based on IDs that show up in ref names. This is not perfect, since patch set // Estimate size based on IDs that show up in ref names. This is not perfect, since patch set
// refs may exist for changes whose metadata was never successfully stored. But that's ok, as // refs may exist for changes whose metadata was never successfully stored. But that's ok, as
// the estimate is just used as a heuristic for sorting projects. // the estimate is just used as a heuristic for sorting projects.
return repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES).stream() long size =
.map(r -> Change.Id.fromRef(r.getName())) repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES).stream()
.filter(Objects::nonNull) .map(r -> Change.Id.fromRef(r.getName()))
.distinct() .filter(Objects::nonNull)
.count(); .distinct()
.count();
return Ints.saturatedCast(size);
} }
private SiteIndexer.Result indexAll(ChangeIndex index, SortedSet<ProjectHolder> projects) { private SiteIndexer.Result indexAll(ChangeIndex index, SortedSet<ProjectHolder> projects) {

8
java/com/google/gerrit/server/util/IdGenerator.java
View File

@@ -45,8 +45,8 @@ public class IdGenerator {
public static int mix(int salt, int in) { public static int mix(int salt, int in) {
short v0 = hi16(in); short v0 = hi16(in);
short v1 = lo16(in); short v1 = lo16(in);
v0 += ((v1 << 2) + 0 ^ v1) + (salt ^ (v1 >>> 3)) + 1; v0 += (short) (((v1 << 2) + 0 ^ v1) + (salt ^ (v1 >>> 3)) + 1);
v1 += ((v0 << 2) + 2 ^ v0) + (salt ^ (v0 >>> 3)) + 3; v1 += (short) (((v0 << 2) + 2 ^ v0) + (salt ^ (v0 >>> 3)) + 3);
return result(v0, v1); return result(v0, v1);
} }
@@ -54,8 +54,8 @@ public class IdGenerator {
static int unmix(int in) { static int unmix(int in) {
short v0 = hi16(in); short v0 = hi16(in);
short v1 = lo16(in); short v1 = lo16(in);
v1 -= ((v0 << 2) + 2 ^ v0) + (salt ^ (v0 >>> 3)) + 3; v1 -= (short) (((v0 << 2) + 2 ^ v0) + (salt ^ (v0 >>> 3)) + 3);
v0 -= ((v1 << 2) + 0 ^ v1) + (salt ^ (v1 >>> 3)) + 1; v0 -= (short) (((v1 << 2) + 0 ^ v1) + (salt ^ (v1 >>> 3)) + 1);
return result(v0, v1); return result(v0, v1);
} }

2
tools/BUILD
View File

@@ -74,7 +74,7 @@ java_package_configuration(
"-Xep:MissingFail:ERROR", "-Xep:MissingFail:ERROR",
"-Xep:MissingOverride:ERROR", "-Xep:MissingOverride:ERROR",
"-Xep:MutableConstantField:ERROR", "-Xep:MutableConstantField:ERROR",
#"-Xep:NarrowingCompoundAssignment:ERROR", "-Xep:NarrowingCompoundAssignment:ERROR",
"-Xep:StringEquality:ERROR", "-Xep:StringEquality:ERROR",
"-Xep:MissingCasesInEnumSwitch:ERROR", "-Xep:MissingCasesInEnumSwitch:ERROR",
"-Xep:WildcardImport:ERROR", "-Xep:WildcardImport:ERROR",