Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ subprojects {
errorproneArgs.addAll([
'-Xep:StringCaseLocaleUsage:ERROR',
'-Xep:StringCaseLocaleUsageMethodRef:ERROR',
'-Xep:ComparatorNeverReturnsZero:ERROR',
])
}
}
Expand Down
12 changes: 6 additions & 6 deletions chainbase/src/main/java/org/tron/core/store/AssetIssueStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import static org.tron.common.utils.Commons.ASSET_ISSUE_COUNT_LIMIT_MAX;

import com.google.common.collect.Streams;
import com.google.protobuf.ByteString;
import java.util.Comparator;
import java.util.List;
import java.util.Map.Entry;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -46,12 +48,10 @@ private List<AssetIssueCapsule> getAssetIssuesPaginated(List<AssetIssueCapsule>
if (assetIssueList.size() <= offset) {
return null;
}
assetIssueList.sort((o1, o2) -> {
if (o1.getName() != o2.getName()) {
return o1.getName().toStringUtf8().compareTo(o2.getName().toStringUtf8());
}
return Long.compare(o1.getOrder(), o2.getOrder());
});
assetIssueList.sort(
Comparator.comparing(AssetIssueCapsule::getName,
ByteString.unsignedLexicographicalComparator())
.thenComparingLong(AssetIssueCapsule::getOrder));
limit = limit > ASSET_ISSUE_COUNT_LIMIT_MAX ? ASSET_ISSUE_COUNT_LIMIT_MAX : limit;
long end = offset + limit;
end = end > assetIssueList.size() ? assetIssueList.size() : end;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.tron.core.store;

import com.google.common.collect.Streams;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -31,9 +32,7 @@ public ExchangeCapsule get(byte[] key) throws ItemNotFoundException {
public List<ExchangeCapsule> getAllExchanges() {
return Streams.stream(iterator())
.map(Map.Entry::getValue)
.sorted(
(ExchangeCapsule a, ExchangeCapsule b) -> a.getCreateTime() <= b.getCreateTime() ? 1
: -1)
.sorted(Comparator.comparingLong(ExchangeCapsule::getCreateTime).reversed())
.collect(Collectors.toList());
}
}
}
18 changes: 10 additions & 8 deletions chainbase/src/main/java/org/tron/core/store/ProposalStore.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.tron.core.store;

import com.google.common.collect.Streams;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -32,23 +33,24 @@ public ProposalCapsule get(byte[] key) throws ItemNotFoundException {
public List<ProposalCapsule> getAllProposals() {
return Streams.stream(iterator())
.map(Map.Entry::getValue)
.sorted(
(ProposalCapsule a, ProposalCapsule b) -> a.getCreateTime() <= b.getCreateTime() ? 1
: -1)
.sorted(Comparator.comparingLong(ProposalCapsule::getCreateTime).reversed())
.collect(Collectors.toList());
}

/**
* note: return in asc order by expired time
* Returns proposals in ascending expiration order, ties broken by descending proposal ID.
*
* <p>The descending-id tie-break preserves the execution order for equal-expiration proposals:
* live execution applies the highest id first and the lowest id last (final value), and the
* energy/bandwidth price-history loaders rebuild from the tail, so the lowest id must be last.
*/
public List<ProposalCapsule> getSpecifiedProposals(State state, long code) {
return Streams.stream(iterator())
.map(Map.Entry::getValue)
.filter(proposalCapsule -> proposalCapsule.getState().equals(state))
.filter(proposalCapsule -> proposalCapsule.getParameters().containsKey(code))
.sorted(
(ProposalCapsule a, ProposalCapsule b) -> a.getExpirationTime() > b.getExpirationTime()
? 1 : -1)
.sorted(Comparator.comparingLong(ProposalCapsule::getExpirationTime)
.thenComparing(Comparator.comparingLong(ProposalCapsule::getID).reversed()))
.collect(Collectors.toList());
}
}
}
23 changes: 23 additions & 0 deletions errorprone/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,35 @@ if (!JavaVersion.current().isJava11Compatible()) {
// ErrorProne core requires JDK 11+; skip this module on JDK 8
tasks.withType(JavaCompile).configureEach { enabled = false }
tasks.withType(Jar).configureEach { enabled = false }
tasks.withType(Test).configureEach { enabled = false }
} else {
dependencies {
compileOnly "com.google.errorprone:error_prone_annotations:${errorproneVersion}"
compileOnly "com.google.errorprone:error_prone_check_api:${errorproneVersion}"
compileOnly "com.google.errorprone:error_prone_core:${errorproneVersion}"
compileOnly "com.google.auto.service:auto-service:1.1.1"
annotationProcessor "com.google.auto.service:auto-service:1.1.1"

testImplementation "com.google.errorprone:error_prone_check_api:${errorproneVersion}"
testImplementation "com.google.errorprone:error_prone_core:${errorproneVersion}"
testImplementation "com.google.errorprone:error_prone_test_helpers:${errorproneVersion}"
}

// ErrorProne's CompilationTestHelper drives javac internals; JDK 16+ needs these exports/opens.
tasks.withType(Test).configureEach {
jvmArgs += [
"--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
]
}
}
172 changes: 172 additions & 0 deletions errorprone/src/main/java/errorprone/ComparatorNeverReturnsZero.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
package errorprone;

import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.BlockTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ConditionalExpressionTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.ParenthesizedTree;
import com.sun.source.tree.ReturnTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.UnaryTree;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Symbol;
import java.util.ArrayList;
import java.util.List;

/**
* Flags {@link java.util.Comparator}/{@link java.lang.Comparable} implementations that can never
* return {@code 0} (e.g. {@code a <= b ? 1 : -1}).
*
* <p>Such a comparator violates the general contract: when two elements compare "equal" it still
* returns a non-zero value, breaking antisymmetry. At runtime {@code List.sort} (TimSort) throws
* {@code IllegalArgumentException: Comparison method violates its general contract!} once the list
* is large enough and contains equal keys; otherwise it silently produces an undefined order.
*
* <p>The built-in ErrorProne {@code ComparisonContractViolated} checker only inspects
* {@code compare}/{@code compareTo} <em>method declarations</em>, so it cannot see lambda
* comparators such as {@code list.sort((a, b) -> a.t() <= b.t() ? 1 : -1)}. This checker covers
* both the lambda form and the method-declaration form.
*
* <p>This is a deliberately conservative <em>syntactic</em> check: it only flags a body whose every
* return is a non-zero {@code int} constant (a literal, or a ternary of such constants). It does no
* data-flow analysis, so a value laundered through a variable
* ({@code int r = c ? -1 : 1; return r;}) is not flagged.
*
* <p>Fix by returning {@code 0} on equality, e.g. {@code Long.compare(a, b)} or
* {@code Comparator.comparingLong(X::t)} (append {@code .reversed()} for descending order).
*/
@AutoService(BugChecker.class)
@BugPattern(
name = "ComparatorNeverReturnsZero",
summary = "Comparator/compareTo can never return 0 (e.g. `? 1 : -1`), violating the comparison "
+ "contract; List.sort may throw IllegalArgumentException. Return 0 on equality, e.g. "
+ "Long.compare(a, b) or Comparator.comparingLong(...).",
severity = BugPattern.SeverityLevel.ERROR)
public class ComparatorNeverReturnsZero extends BugChecker
implements BugChecker.LambdaExpressionTreeMatcher, BugChecker.MethodTreeMatcher {

private static final Matcher<ExpressionTree> IS_COMPARATOR =
Matchers.isSubtypeOf("java.util.Comparator");

@Override
public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
// Only comparator lambdas: the target functional interface is java.util.Comparator.
// (Comparable is not a functional interface, so it can never be a lambda.)
if (!IS_COMPARATOR.matches(tree, state)) {
return Description.NO_MATCH;
}
return neverReturnsZero(tree.getBody()) ? describeMatch(tree) : Description.NO_MATCH;
}

@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (tree.getBody() == null || !isComparatorMethod(tree, state)) {
return Description.NO_MATCH;
}
return neverReturnsZero(tree.getBody()) ? describeMatch(tree) : Description.NO_MATCH;
}

/**
* True if the comparator body ({@code (a,b) -> expr}, {@code (a,b) -> {..}} or a method block)
* provably yields a non-zero {@code int} constant on every path. Conservative: any path whose
* value cannot be proven non-zero (a method call, a subtraction, a plain {@code 0}, ...) makes
* this return {@code false}, so legitimate comparators such as {@code Long.compare(a, b)} are
* never flagged.
*/
private static boolean neverReturnsZero(Tree body) {
if (body instanceof ExpressionTree) {
return alwaysNonZero((ExpressionTree) body);
}
if (body instanceof BlockTree) {
List<ReturnTree> returns = new ArrayList<>();
new ReturnCollector().scan(body, returns);
if (returns.isEmpty()) {
return false;
}
for (ReturnTree r : returns) {
if (r.getExpression() == null || !alwaysNonZero(r.getExpression())) {
return false;
}
}
return true;
}
return false;
}

/** True if {@code e} is provably a non-zero {@code int} constant on every branch. */
private static boolean alwaysNonZero(ExpressionTree e) {
e = stripParens(e);
Integer c = ASTHelpers.constValue(e, Integer.class);
if (c != null) {
return c != 0;
}
// Fall back for `-1` / `+1` in case constant folding did not run.
if (e instanceof UnaryTree
&& (e.getKind() == Tree.Kind.UNARY_MINUS || e.getKind() == Tree.Kind.UNARY_PLUS)) {
Integer operand =
ASTHelpers.constValue(stripParens(((UnaryTree) e).getExpression()), Integer.class);
return operand != null && operand != 0;
}
if (e instanceof ConditionalExpressionTree) {
ConditionalExpressionTree cond = (ConditionalExpressionTree) e;
return alwaysNonZero(cond.getTrueExpression()) && alwaysNonZero(cond.getFalseExpression());
}
return false;
}

private static ExpressionTree stripParens(ExpressionTree e) {
while (e instanceof ParenthesizedTree) {
e = ((ParenthesizedTree) e).getExpression();
}
return e;
}

/**
* True only for methods that genuinely override {@code Comparator.compare} or
* {@code Comparable.compareTo}. Matching by name + arity alone would wrongly flag same-named
* overloads (e.g. {@code compareTo(long)}) and static/private helpers; those override nothing, so
* {@link ASTHelpers#findSuperMethods} returns no comparator super-method for them.
*/
private static boolean isComparatorMethod(MethodTree tree, VisitorState state) {
Symbol.MethodSymbol sym = ASTHelpers.getSymbol(tree);
if (sym == null) {
return false;
}
for (Symbol.MethodSymbol superMethod : ASTHelpers.findSuperMethods(sym, state.getTypes())) {
String ownerName = superMethod.owner.getQualifiedName().toString();
if (ownerName.equals("java.util.Comparator") || ownerName.equals("java.lang.Comparable")) {
return true;
}
}
return false;
}

/** Collects return statements owned by this method/lambda, not by nested functions. */
private static final class ReturnCollector extends TreeScanner<Void, List<ReturnTree>> {
@Override
public Void visitReturn(ReturnTree node, List<ReturnTree> returns) {
returns.add(node);
return super.visitReturn(node, returns);
}

@Override
public Void visitLambdaExpression(LambdaExpressionTree node, List<ReturnTree> returns) {
return null; // a nested lambda's returns are its own
}

@Override
public Void visitClass(ClassTree node, List<ReturnTree> returns) {
return null; // a nested / anonymous class's returns are its own
}
}
}
Loading
Loading