From 22f05839f5175832695d8f76baf5bf7319519d66 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Sat, 15 Oct 2022 21:33:30 -0700 Subject: [PATCH] Fix errorprone warnings that should be errors This commit is part of a large scale change to fix errorprone errors that have been downgraded to warnings in the android source tree, so that they can be promoted to errors again. The full list of changes include the following, but not all will be present in any one individual commit: BadAnnotationImplementation BadShiftAmount BanJNDI BoxedPrimitiveEquality ComparableType ComplexBooleanConstant CollectionToArraySafeParameter ConditionalExpressionNumericPromotion DangerousLiteralNull DoubleBraceInitialization DurationFrom DurationTemporalUnit EmptyTopLevelDeclaration EqualsNull EqualsReference FormatString FromTemporalAccessor GetClassOnAnnotation GetClassOnClass HashtableContains IdentityBinaryExpression IdentityHashMapBoxing InstantTemporalUnit InvalidTimeZoneID InvalidZoneId IsInstanceIncompatibleType JUnitParameterMethodNotFound LockOnBoxedPrimitive MathRoundIntLong MislabeledAndroidString MisusedDayOfYear MissingSuperCall MisusedWeekYear ModifyingCollectionWithItself NoCanIgnoreReturnValueOnClasses NonRuntimeAnnotation NullableOnContainingClass NullTernary OverridesJavaxInjectableMethod ParcelableCreator PeriodFrom PreconditionsInvalidPlaceholder ProtoBuilderReturnValueIgnored ProtoFieldNullComparison RandomModInteger RectIntersectReturnValueIgnored ReturnValueIgnored SelfAssignment SelfComparison SelfEquals SizeGreaterThanOrEqualsZero StringBuilderInitWithChar TreeToString TryFailThrowable UnnecessaryCheckNotNull UnusedCollectionModifiedInPlace XorPower See https://errorprone.info/bugpatterns for more information on the checks. Bug: 253827323 Test: m RUN_ERROR_PRONE=true javac-check Change-Id: I7625fa386afe93823b97cb2ecb8fd09a5856c05b --- .../tethering/metrics/TetheringMetrics.java | 3 +- .../android/net/EthernetTetheringTest.java | 2 +- .../tethering/BpfCoordinatorTest.java | 13 +- .../net/module/util/bpf/Tether4Key.java | 2 +- .../net/module/util/bpf/Tether4Value.java | 2 +- .../src/android/net/IpSecAlgorithm.java | 2 +- .../server/ConnectivityServiceTest.java | 161 +++++++++++------- 7 files changed, 110 insertions(+), 75 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java b/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java index 2c6054d61a..ffcea4e360 100644 --- a/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java +++ b/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java @@ -76,8 +76,7 @@ public class TetheringMetrics { .setUpstreamType(UpstreamType.UT_UNKNOWN) .setErrorCode(ErrorCode.EC_NO_ERROR) .setUpstreamEvents(UpstreamEvents.newBuilder()) - .setDurationMillis(0) - .build(); + .setDurationMillis(0); mBuilderMap.put(downstreamType, statsBuilder); } diff --git a/Tethering/tests/integration/src/android/net/EthernetTetheringTest.java b/Tethering/tests/integration/src/android/net/EthernetTetheringTest.java index c61b6eb216..f0f9a31538 100644 --- a/Tethering/tests/integration/src/android/net/EthernetTetheringTest.java +++ b/Tethering/tests/integration/src/android/net/EthernetTetheringTest.java @@ -76,7 +76,7 @@ import android.net.TetheringManager.StartTetheringCallback; import android.net.TetheringManager.TetheringEventCallback; import android.net.TetheringManager.TetheringRequest; import android.net.TetheringTester.TetheredDevice; -import android.net.cts.util.CtsNetUtils;; +import android.net.cts.util.CtsNetUtils; import android.os.Build; import android.os.Handler; import android.os.HandlerThread; diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java index 0bd63806cb..225fed7134 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -138,6 +138,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.Map; @RunWith(AndroidJUnit4.class) @SmallTest @@ -195,13 +196,11 @@ public class BpfCoordinatorTest { UPSTREAM_IFACE2, UPSTREAM_IFINDEX2, MacAddress.fromString("44:55:66:00:00:0c"), NetworkStackConstants.ETHER_MTU); - private static final HashMap UPSTREAM_INFORMATIONS = - new HashMap() {{ - put(UPSTREAM_IFINDEX, new UpstreamInformation(UPSTREAM_IFACE_PARAMS, - PUBLIC_ADDR, NetworkCapabilities.TRANSPORT_CELLULAR, TEST_NET_ID)); - put(UPSTREAM_IFINDEX2, new UpstreamInformation(UPSTREAM_IFACE_PARAMS2, - PUBLIC_ADDR2, NetworkCapabilities.TRANSPORT_WIFI, TEST_NET_ID2)); - }}; + private static final Map UPSTREAM_INFORMATIONS = Map.of( + UPSTREAM_IFINDEX, new UpstreamInformation(UPSTREAM_IFACE_PARAMS, + PUBLIC_ADDR, NetworkCapabilities.TRANSPORT_CELLULAR, TEST_NET_ID), + UPSTREAM_IFINDEX2, new UpstreamInformation(UPSTREAM_IFACE_PARAMS2, + PUBLIC_ADDR2, NetworkCapabilities.TRANSPORT_WIFI, TEST_NET_ID2)); private static final ClientInfo CLIENT_INFO_A = new ClientInfo(DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC, PRIVATE_ADDR, MAC_A); diff --git a/common/src/com/android/net/module/util/bpf/Tether4Key.java b/common/src/com/android/net/module/util/bpf/Tether4Key.java index 8273e6a40b..738256a039 100644 --- a/common/src/com/android/net/module/util/bpf/Tether4Key.java +++ b/common/src/com/android/net/module/util/bpf/Tether4Key.java @@ -75,7 +75,7 @@ public class Tether4Key extends Struct { Inet4Address.getByAddress(src4), Inet4Address.getByAddress(dst4), Short.toUnsignedInt((short) srcPort), Short.toUnsignedInt((short) dstPort)); } catch (UnknownHostException | IllegalArgumentException e) { - return String.format("Invalid IP address", e); + return "Invalid IP address" + e; } } } diff --git a/common/src/com/android/net/module/util/bpf/Tether4Value.java b/common/src/com/android/net/module/util/bpf/Tether4Value.java index 74fdda25c2..449c1daa61 100644 --- a/common/src/com/android/net/module/util/bpf/Tether4Value.java +++ b/common/src/com/android/net/module/util/bpf/Tether4Value.java @@ -91,7 +91,7 @@ public class Tether4Value extends Struct { Short.toUnsignedInt((short) srcPort), Short.toUnsignedInt((short) dstPort), lastUsed); } catch (UnknownHostException | IllegalArgumentException e) { - return String.format("Invalid IP address", e); + return "Invalid IP address" + e; } } } diff --git a/framework-t/src/android/net/IpSecAlgorithm.java b/framework-t/src/android/net/IpSecAlgorithm.java index 10a22ac360..a39a80d338 100644 --- a/framework-t/src/android/net/IpSecAlgorithm.java +++ b/framework-t/src/android/net/IpSecAlgorithm.java @@ -488,4 +488,4 @@ public final class IpSecAlgorithm implements Parcelable { && Arrays.equals(lhs.mKey, rhs.mKey) && lhs.mTruncLenBits == rhs.mTruncLenBits); } -}; +} diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 65e0b10406..d7e1e6a153 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -2689,7 +2689,7 @@ public class ConnectivityServiceTest { final TestNetworkCallback generalCb = new TestNetworkCallback(); final TestNetworkCallback defaultCb = new TestNetworkCallback(); mCm.registerNetworkCallback( - new NetworkRequest.Builder().addTransportType(transport | transport).build(), + new NetworkRequest.Builder().addTransportType(transport).build(), generalCb); mCm.registerDefaultNetworkCallback(defaultCb); @@ -4755,6 +4755,81 @@ public class ConnectivityServiceTest { return new NetworkRequest.Builder().addTransportType(TRANSPORT_WIFI); } + // A NetworkSpecifier subclass that matches all networks but must not be visible to apps. + static class ConfidentialMatchAllNetworkSpecifier extends NetworkSpecifier implements + Parcelable { + public static final Parcelable.Creator CREATOR = + new Parcelable.Creator() { + public ConfidentialMatchAllNetworkSpecifier createFromParcel(Parcel in) { + return new ConfidentialMatchAllNetworkSpecifier(); + } + + public ConfidentialMatchAllNetworkSpecifier[] newArray(int size) { + return new ConfidentialMatchAllNetworkSpecifier[size]; + } + }; + @Override + public boolean canBeSatisfiedBy(NetworkSpecifier other) { + return true; + } + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(Parcel dest, int flags) {} + + @Override + public NetworkSpecifier redact() { + return null; + } + } + + // A network specifier that matches either another LocalNetworkSpecifier with the same + // string or a ConfidentialMatchAllNetworkSpecifier, and can be passed to apps as is. + static class LocalStringNetworkSpecifier extends NetworkSpecifier implements Parcelable { + public static final Parcelable.Creator CREATOR = + new Parcelable.Creator() { + public LocalStringNetworkSpecifier createFromParcel(Parcel in) { + return new LocalStringNetworkSpecifier(in); + } + + public LocalStringNetworkSpecifier[] newArray(int size) { + return new LocalStringNetworkSpecifier[size]; + } + }; + private String mString; + + LocalStringNetworkSpecifier(String string) { + mString = string; + } + + LocalStringNetworkSpecifier(Parcel in) { + mString = in.readString(); + } + + @Override + public boolean canBeSatisfiedBy(NetworkSpecifier other) { + if (other instanceof LocalStringNetworkSpecifier) { + return TextUtils.equals(mString, + ((LocalStringNetworkSpecifier) other).mString); + } + if (other instanceof ConfidentialMatchAllNetworkSpecifier) return true; + return false; + } + + @Override + public int describeContents() { + return 0; + } + @Override + public void writeToParcel(Parcel dest, int flags) { + dest.writeString(mString); + } + } + /** * Verify request matching behavior with network specifiers. * @@ -4764,56 +4839,6 @@ public class ConnectivityServiceTest { */ @Test public void testNetworkSpecifier() throws Exception { - // A NetworkSpecifier subclass that matches all networks but must not be visible to apps. - class ConfidentialMatchAllNetworkSpecifier extends NetworkSpecifier implements - Parcelable { - @Override - public boolean canBeSatisfiedBy(NetworkSpecifier other) { - return true; - } - - @Override - public int describeContents() { - return 0; - } - - @Override - public void writeToParcel(Parcel dest, int flags) {} - - @Override - public NetworkSpecifier redact() { - return null; - } - } - - // A network specifier that matches either another LocalNetworkSpecifier with the same - // string or a ConfidentialMatchAllNetworkSpecifier, and can be passed to apps as is. - class LocalStringNetworkSpecifier extends NetworkSpecifier implements Parcelable { - private String mString; - - LocalStringNetworkSpecifier(String string) { - mString = string; - } - - @Override - public boolean canBeSatisfiedBy(NetworkSpecifier other) { - if (other instanceof LocalStringNetworkSpecifier) { - return TextUtils.equals(mString, - ((LocalStringNetworkSpecifier) other).mString); - } - if (other instanceof ConfidentialMatchAllNetworkSpecifier) return true; - return false; - } - - @Override - public int describeContents() { - return 0; - } - @Override - public void writeToParcel(Parcel dest, int flags) {} - } - - NetworkRequest rEmpty1 = newWifiRequestBuilder().build(); NetworkRequest rEmpty2 = newWifiRequestBuilder().setNetworkSpecifier((String) null).build(); NetworkRequest rEmpty3 = newWifiRequestBuilder().setNetworkSpecifier("").build(); @@ -4897,6 +4922,29 @@ public class ConnectivityServiceTest { return mContext.getAttributionTag(); } + static class NonParcelableSpecifier extends NetworkSpecifier { + @Override + public boolean canBeSatisfiedBy(NetworkSpecifier other) { + return false; + } + } + static class ParcelableSpecifier extends NonParcelableSpecifier implements Parcelable { + public static final Parcelable.Creator CREATOR = + new Parcelable.Creator() { + public NonParcelableSpecifier createFromParcel(Parcel in) { + return new NonParcelableSpecifier(); + } + + public NonParcelableSpecifier[] newArray(int size) { + return new NonParcelableSpecifier[size]; + } + }; + @Override public int describeContents() { + return 0; + } + @Override public void writeToParcel(Parcel p, int flags) {} + } + @Test public void testInvalidNetworkSpecifier() { assertThrows(IllegalArgumentException.class, () -> { @@ -4914,17 +4962,6 @@ public class ConnectivityServiceTest { mContext.getPackageName(), getAttributionTag()); }); - class NonParcelableSpecifier extends NetworkSpecifier { - @Override - public boolean canBeSatisfiedBy(NetworkSpecifier other) { - return false; - } - }; - class ParcelableSpecifier extends NonParcelableSpecifier implements Parcelable { - @Override public int describeContents() { return 0; } - @Override public void writeToParcel(Parcel p, int flags) {} - } - final NetworkRequest.Builder builder = new NetworkRequest.Builder().addTransportType(TRANSPORT_ETHERNET); assertThrows(ClassCastException.class, () -> {