Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-16427: Enable error-prone PatternMatchingInstanceof rule #2867

Merged
Merged
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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,7 @@ private boolean hasInheritedJavadocs(Element element) {
}

// Check for methods up the types tree.
if (element instanceof ExecutableElement) {
ExecutableElement thisMethod = (ExecutableElement) element;
if (element instanceof ExecutableElement thisMethod) {
Iterable<Element> superTypes =
() -> superTypeForInheritDoc(thisMethod.getEnclosingElement()).iterator();

Expand Down
2 changes: 1 addition & 1 deletion gradle/validation/error-prone.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ allprojects { prj ->
'-Xep:Overrides:WARN',
// '-Xep:OverridesGuiceInjectableMethod:OFF', // we don't use guice
'-Xep:ParameterName:WARN',
// '-Xep:PatternMatchingInstanceof:WARN', // todo check if useful or comment why not
'-Xep:PatternMatchingInstanceof:WARN',
'-Xep:PreconditionsCheckNotNullRepeated:WARN',
'-Xep:PrimitiveAtomicReference:WARN',
'-Xep:ProtectedMembersInFinalClass:WARN',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,7 @@ public String toString() {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof Surrogate)) return false;
Surrogate surrogate = (Surrogate) o;
if (!(o instanceof Surrogate surrogate)) return false;
return hashCode.equals(surrogate.hashCode)
&& identityHashcode.equals(surrogate.identityHashcode);
}
Expand Down
12 changes: 4 additions & 8 deletions solr/core/src/java/org/apache/solr/api/AnnotatedApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,7 @@ static class Cmd {
}

private void readPayloadType(Type t) {
if (t instanceof ParameterizedType) {
ParameterizedType typ = (ParameterizedType) t;
if (t instanceof ParameterizedType typ) {
if (typ.getRawType() == PayloadObj.class) {
isWrappedInPayloadObj = true;
if (typ.getActualTypeArguments().length == 0) {
Expand All @@ -271,8 +270,7 @@ private void readPayloadType(Type t) {
return;
}
Type t1 = typ.getActualTypeArguments()[0];
if (t1 instanceof ParameterizedType) {
ParameterizedType parameterizedType = (ParameterizedType) t1;
if (t1 instanceof ParameterizedType parameterizedType) {
parameterClass = (Class<?>) parameterizedType.getRawType();
} else {
parameterClass = (Class<?>) typ.getActualTypeArguments()[0];
Expand Down Expand Up @@ -345,9 +343,8 @@ public int hashCode() {
public boolean equals(Object rhs) {
if (null == rhs) return false;
if (this == rhs) return true;
if (!(rhs instanceof Cmd)) return false;
if (!(rhs instanceof Cmd rhsCast)) return false;

final Cmd rhsCast = (Cmd) rhs;
return Objects.equals(command, rhsCast.command)
&& Objects.equals(method, rhsCast.method)
&& Objects.equals(obj, rhsCast.obj)
Expand All @@ -373,8 +370,7 @@ public static Map<String, Object> createSchema(Method m) {
t = types[2]; // (SolrQueryRequest req, SolrQueryResponse rsp, PayloadObj<PluginMeta>)
if (types.length == 1) t = types[0]; // (PayloadObj<PluginMeta>)
if (t != null) {
if (t instanceof ParameterizedType) {
ParameterizedType typ = (ParameterizedType) t;
if (t instanceof ParameterizedType typ) {
if (typ.getRawType() == PayloadObj.class) {
t = typ.getActualTypeArguments()[0];
}
Expand Down
13 changes: 5 additions & 8 deletions solr/core/src/java/org/apache/solr/api/ApiBag.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,14 @@ protected void attachValueToNode(PathTrie<Api>.Node node, Api o) {

// If 'o' and 'node.obj' aren't both AnnotatedApi's then we can't aggregate the commands, so
// fallback to the default behavior
if ((!(o instanceof AnnotatedApi)) || (!(node.getObject() instanceof AnnotatedApi))) {
if ((!(o instanceof AnnotatedApi beingRegistered))
|| (!(node.getObject() instanceof AnnotatedApi alreadyRegistered))) {
super.attachValueToNode(node, o);
return;
}

final AnnotatedApi beingRegistered = (AnnotatedApi) o;
final AnnotatedApi alreadyRegistered = (AnnotatedApi) node.getObject();
if (alreadyRegistered instanceof CommandAggregatingAnnotatedApi) {
final CommandAggregatingAnnotatedApi alreadyRegisteredAsCollapsing =
(CommandAggregatingAnnotatedApi) alreadyRegistered;
if (alreadyRegistered
instanceof CommandAggregatingAnnotatedApi alreadyRegisteredAsCollapsing) {
alreadyRegisteredAsCollapsing.combineWith(beingRegistered);
} else {
final CommandAggregatingAnnotatedApi wrapperApi =
Expand Down Expand Up @@ -404,11 +402,10 @@ public void registerLazy(PluginBag.PluginHolder<SolrRequestHandler> holder, Plug

public static SpecProvider constructSpec(PluginInfo info) {
Object specObj = info == null ? null : info.attributes.get("spec");
if (specObj != null && specObj instanceof Map) {
if (specObj != null && specObj instanceof Map<?, ?> map) {
// Value from Map<String,String> can be a Map because in PluginInfo(String, Map) we assign a
// Map<String, Object>
// assert false : "got a map when this should only be Strings";
Map<?, ?> map = (Map<?, ?>) specObj;
return () -> ValidatingJsonMap.getDeepCopy(map, 4, false);
} else {
return HANDLER_NAME_SPEC_PROVIDER;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ static class PluginMetaHolder {

@Override
public boolean equals(Object obj) {
if (obj instanceof PluginMetaHolder) {
PluginMetaHolder that = (PluginMetaHolder) obj;
if (obj instanceof PluginMetaHolder that) {
return Objects.equals(this.original, that.original);
}
return false;
Expand Down Expand Up @@ -466,8 +465,7 @@ public static <T extends MapWriter> Class<T> getConfigClass(ConfigurablePlugin<T
do {
Type[] interfaces = klas.getGenericInterfaces();
for (Type type : interfaces) {
if (type instanceof ParameterizedType) {
ParameterizedType parameterizedType = (ParameterizedType) type;
if (type instanceof ParameterizedType parameterizedType) {
Type rawType = parameterizedType.getRawType();
if (rawType == ConfigurablePlugin.class
||
Expand Down
6 changes: 2 additions & 4 deletions solr/core/src/java/org/apache/solr/cli/ExportTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ public synchronized void accept(SolrDocument doc) throws IOException {
}
}
field = constructDateStr(field);
if (field instanceof List) {
List<?> list = (List<?>) field;
if (field instanceof List<?> list) {
if (hasdate(list)) {
ArrayList<Object> listCopy = new ArrayList<>(list.size());
for (Object o : list) listCopy.add(constructDateStr(o));
Expand Down Expand Up @@ -443,8 +442,7 @@ public synchronized void accept(SolrDocument doc) throws IOException {
}
}
field = constructDateStr(field);
if (field instanceof List) {
List<?> list = (List<?>) field;
if (field instanceof List<?> list) {
if (hasdate(list)) {
ArrayList<Object> listCopy = new ArrayList<>(list.size());
for (Object o : list) listCopy.add(constructDateStr(o));
Expand Down
3 changes: 1 addition & 2 deletions solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,7 @@ public int hashCode() {
public boolean equals(Object obj) {
if (this == obj) return true;
if (obj == null) return false;
if (!(obj instanceof ReplicaHealth)) return true;
ReplicaHealth that = (ReplicaHealth) obj;
if (!(obj instanceof ReplicaHealth that)) return true;
return this.shard.equals(that.shard) && this.isLeader == that.isLeader;
}

Expand Down
3 changes: 1 addition & 2 deletions solr/core/src/java/org/apache/solr/cli/StreamTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,7 @@ public void runImpl(CommandLine cli) throws Exception {

Object o = tuple.get(outputHeaders[i]);
if (o != null) {
if (o instanceof List) {
List outfields = (List) o;
if (o instanceof List outfields) {
outLine.append(listToString(outfields, arrayDelimiter));
} else {
outLine.append(o);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,7 @@ public void writeResults(ResultContext ctx, JavaBinCodec codec) throws IOExcepti
/** A list of streams, non-null. */
private List<ContentStream> getContentStreams(SolrRequest<?> request) throws IOException {
if (request.getMethod() == SolrRequest.METHOD.GET) return List.of();
if (request instanceof ContentStreamUpdateRequest) {
final ContentStreamUpdateRequest csur = (ContentStreamUpdateRequest) request;
if (request instanceof ContentStreamUpdateRequest csur) {
final Collection<ContentStream> cs = csur.getContentStreams();
if (cs != null) return new ArrayList<>(cs);
}
Expand Down
3 changes: 1 addition & 2 deletions solr/core/src/java/org/apache/solr/cloud/Overseer.java
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,7 @@ public void run() {
// Return true whenever the exception thrown by ZkStateWriter is correspond
// to a invalid state or 'bad' message (in this case, we should remove that message from queue)
private boolean isBadMessage(Exception e) {
if (e instanceof KeeperException) {
KeeperException ke = (KeeperException) e;
if (e instanceof KeeperException ke) {
return ke.code() == KeeperException.Code.NONODE
|| ke.code() == KeeperException.Code.NODEEXISTS;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,7 @@ public int hashCode() {
@Override
public boolean equals(Object obj) {
if (this == obj) return true;
if (!(obj instanceof QueueEvent)) return false;
QueueEvent other = (QueueEvent) obj;
if (!(obj instanceof QueueEvent other)) return false;
return Objects.equals(id, other.id);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,8 @@ public boolean onTermChanged(ShardTerms terms) {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof RecoveringCoreTermWatcher)) return false;
if (!(o instanceof RecoveringCoreTermWatcher that)) return false;

RecoveringCoreTermWatcher that = (RecoveringCoreTermWatcher) o;
return coreDescriptor.getName().equals(that.coreDescriptor.getName());
}

Expand Down
6 changes: 2 additions & 4 deletions solr/core/src/java/org/apache/solr/cloud/ZkController.java
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ public int hashCode() {
@Override
public boolean equals(Object obj) {
if (this == obj) return true;
if (!(obj instanceof ContextKey)) return false;
ContextKey other = (ContextKey) obj;
if (!(obj instanceof ContextKey other)) return false;
return Objects.equals(collection, other.collection)
&& Objects.equals(coreNodeName, other.coreNodeName);
}
Expand Down Expand Up @@ -2833,8 +2832,7 @@ public synchronized boolean onStateChanged(DocCollection collectionState) {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof UnloadCoreOnDeletedWatcher)) return false;
UnloadCoreOnDeletedWatcher that = (UnloadCoreOnDeletedWatcher) o;
if (!(o instanceof UnloadCoreOnDeletedWatcher that)) return false;
return Objects.equals(coreNodeName, that.coreNodeName)
&& Objects.equals(shard, that.shard)
&& Objects.equals(coreName, that.coreName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ public void call(ClusterState clusterState, ZkNodeProps message, NamedList<Objec
SolrException.ErrorCode.BAD_REQUEST,
"Unknown target collection: " + sourceCollectionName);
}
if (!(sourceCollection.getRouter() instanceof CompositeIdRouter)) {
if (!(sourceCollection.getRouter() instanceof CompositeIdRouter sourceRouter)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Source collection must use a compositeId router");
}
if (!(targetCollection.getRouter() instanceof CompositeIdRouter)) {
if (!(targetCollection.getRouter() instanceof CompositeIdRouter targetRouter)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Target collection must use a compositeId router");
}
Expand All @@ -115,8 +115,6 @@ public void call(ClusterState clusterState, ZkNodeProps message, NamedList<Objec
SolrException.ErrorCode.BAD_REQUEST, "The split.key cannot be null or empty");
}

CompositeIdRouter sourceRouter = (CompositeIdRouter) sourceCollection.getRouter();
CompositeIdRouter targetRouter = (CompositeIdRouter) targetCollection.getRouter();
Collection<Slice> sourceSlices =
sourceRouter.getSearchSlicesSingle(splitKey, null, sourceCollection);
if (sourceSlices.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,8 +490,7 @@ public Action(RoutedAlias sourceAlias, ActionType actionType, String targetColle
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof Action)) return false;
Action action = (Action) o;
if (!(o instanceof Action action)) return false;
return Objects.equals(sourceAlias, action.sourceAlias)
&& actionType == action.actionType
&& Objects.equals(targetCollection, action.targetCollection);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1126,8 +1126,7 @@ public static String fillRanges(
}
}
} else if (splitKey != null) {
if (router instanceof CompositeIdRouter) {
CompositeIdRouter compositeIdRouter = (CompositeIdRouter) router;
if (router instanceof CompositeIdRouter compositeIdRouter) {
List<DocRouter.Range> tmpSubRanges = compositeIdRouter.partitionRangeByKey(splitKey, range);
if (tmpSubRanges.size() == 1) {
throw new SolrException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ public SliceMutator(SolrCloudManager cloudManager) {
}

static SolrZkClient getZkClient(SolrCloudManager cloudManager) {
if (cloudManager instanceof SolrClientCloudManager) {
SolrClientCloudManager manager = (SolrClientCloudManager) cloudManager;
if (cloudManager instanceof SolrClientCloudManager manager) {
return manager.getZkClient();
} else {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ public void added(ContainerPluginsRegistry.ApiInfo plugin) {
return;
}
Object instance = plugin.getInstance();
if (instance instanceof ClusterEventListener) {
ClusterEventListener listener = (ClusterEventListener) instance;
if (instance instanceof ClusterEventListener listener) {
clusterEventProducer.registerListener(listener);
} else if (instance instanceof ClusterEventProducer) {
if (ClusterEventProducer.PLUGIN_NAME.equals(plugin.getInfo().name)) {
Expand Down Expand Up @@ -162,8 +161,7 @@ public void deleted(ContainerPluginsRegistry.ApiInfo plugin) {
return;
}
Object instance = plugin.getInstance();
if (instance instanceof ClusterEventListener) {
ClusterEventListener listener = (ClusterEventListener) instance;
if (instance instanceof ClusterEventListener listener) {
clusterEventProducer.unregisterListener(listener);
} else if (instance instanceof ClusterEventProducer) {
if (ClusterEventProducer.PLUGIN_NAME.equals(plugin.getInfo().name)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,9 @@ public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof MetricImpl)) {
if (!(o instanceof MetricImpl<?> that)) {
return false;
}
MetricImpl<?> that = (MetricImpl<?>) o;
return name.equals(that.getName())
&& internalName.equals(that.getInternalName())
&& converter.equals(that.converter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,12 @@ public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof NodeMetricImpl)) {
if (!(o instanceof NodeMetricImpl<?> that)) {
return false;
}
if (!super.equals(o)) {
return false;
}
NodeMetricImpl<?> that = (NodeMetricImpl<?>) o;
return registry == that.registry;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,9 @@ public boolean equals(Object obj) {
if (obj == this) {
return true;
}
if (!(obj instanceof NodeImpl)) {
if (!(obj instanceof NodeImpl other)) {
return false;
}
NodeImpl other = (NodeImpl) obj;
return Objects.equals(this.nodeName, other.nodeName);
}

Expand Down Expand Up @@ -307,10 +306,9 @@ public boolean equals(Object obj) {
if (obj == this) {
return true;
}
if (!(obj instanceof ShardImpl)) {
if (!(obj instanceof ShardImpl other)) {
return false;
}
ShardImpl other = (ShardImpl) obj;
return Objects.equals(this.shardName, other.shardName)
&& Objects.equals(this.collection, other.collection)
&& Objects.equals(this.shardState, other.shardState)
Expand Down Expand Up @@ -467,10 +465,9 @@ public boolean equals(Object obj) {
if (obj == this) {
return true;
}
if (!(obj instanceof ReplicaImpl)) {
if (!(obj instanceof ReplicaImpl other)) {
return false;
}
ReplicaImpl other = (ReplicaImpl) obj;
return Objects.equals(this.replicaName, other.replicaName)
&& Objects.equals(this.coreName, other.coreName)
&& Objects.equals(this.shard, other.shard)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,10 +555,9 @@ public int hashCode() {

@Override
public boolean equals(Object o) {
if (!(o instanceof WeightedNode)) {
if (!(o instanceof WeightedNode on)) {
return false;
} else {
WeightedNode on = (WeightedNode) o;
if (this.node == null) {
return on.node == null;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ public void added(ContainerPluginsRegistry.ApiInfo plugin) {
}
// register new api
Object instance = plugin.getInstance();
if (instance instanceof ClusterSingleton) {
ClusterSingleton singleton = (ClusterSingleton) instance;
if (instance instanceof ClusterSingleton singleton) {
singletonMap.put(singleton.getName(), singleton);
// check to see if we should immediately start this singleton
if (isReady() && runSingletons.get()) {
Expand All @@ -89,8 +88,7 @@ public void deleted(ContainerPluginsRegistry.ApiInfo plugin) {
return;
}
Object instance = plugin.getInstance();
if (instance instanceof ClusterSingleton) {
ClusterSingleton singleton = (ClusterSingleton) instance;
if (instance instanceof ClusterSingleton singleton) {
singleton.stop();
singletonMap.remove(singleton.getName());
}
Expand Down
Loading
Loading