Skip to content

Commit c1e8447

Browse files
trasktrask
committed
Code review agent: style for null comparisons
1 parent 38ee182 commit c1e8447

24 files changed

Lines changed: 51 additions & 43 deletions

File tree

‎.github/agents/knowledge/general-rules.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th
3131
| Build | Gradle conventions, muzzle, test tasks, plugins | `build.gradle.kts`, `settings.gradle.kts` | `gradle-conventions.md` |
3232
| Build | `testcontainersBuildService` declaration | Testcontainers dependency without `usesService` | `gradle-conventions.md` |
3333
| Style | Prefer instance creation over singletons for stateless interface impls (except on hot paths or Kotlin `object` declarations) | `TextMapGetter`, `TextMapSetter`, `*AttributesGetter`, `AttributesExtractor`, `SpanNameExtractor`, `HttpServerResponseMutator`, enum/static singletons | — |
34+
| Style | Prefer `value == null` / `value != null` over `null == value` / `null != value` | Null comparisons | — |
3435
| Style | No unnecessary explicit type witnesses on generic method calls (`Collections.<String>emptyList()`) | Java generic method calls with explicit type parameters | — |
3536
| Style | Remove redundant null guards on attribute puts | `AttributesBuilder.put`, `onStart`, `onEnd`, attribute extraction methods | — |
3637
| General | No redundant `ByteBuffer.duplicate()` on `Value.getValue()` | `Value.getValue()` with `BYTES` type, `ByteBuffer` handling | — |

‎docs/contributing/style-guide.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ Fields should be declared `final` where possible.
9292

9393
Method parameters and local variables should never be declared `final`.
9494

95+
### Null comparisons
96+
97+
Prefer `value == null` and `value != null` over left-hand null comparisons such as
98+
`null == value` and `null != value`.
99+
100+
This applies throughout the codebase, including Java, Kotlin, and Scala sources.
101+
95102
### Uppercase field names
96103

97104
Use uppercase (`SCREAMING_SNAKE_CASE`) for constant-like fields whose value is treated as a stable

‎instrumentation/akka/akka-actor-fork-join-2.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkaforkjoin/AkkaForkJoinTaskInstrumentation.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ public static Scope enter(@Advice.This ForkJoinTask<?> thiz) {
6969
Scope newScope =
7070
TaskAdviceHelper.makePropagatedContextCurrent(
7171
VirtualFields.RUNNABLE_PROPAGATED_CONTEXT, (Runnable) thiz);
72-
if (null != newScope) {
73-
if (null != scope) {
72+
if (newScope != null) {
73+
if (scope != null) {
7474
newScope.close();
7575
} else {
7676
scope = newScope;
@@ -81,8 +81,8 @@ public static Scope enter(@Advice.This ForkJoinTask<?> thiz) {
8181
Scope newScope =
8282
TaskAdviceHelper.makePropagatedContextCurrent(
8383
VirtualFields.CALLABLE_PROPAGATED_CONTEXT, (Callable<?>) thiz);
84-
if (null != newScope) {
85-
if (null != scope) {
84+
if (newScope != null) {
85+
if (scope != null) {
8686
newScope.close();
8787
} else {
8888
scope = newScope;

‎instrumentation/akka/akka-http-10.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/akkahttp/AkkaHttpTestAsyncWebServer.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ object AkkaHttpTestAsyncWebServer {
6363
private var binding: ServerBinding = _
6464

6565
def start(port: Int): Unit = synchronized {
66-
if (null == binding) {
66+
if (binding == null) {
6767
import scala.concurrent.duration._
6868
binding = Await.result(
6969
Http().bindAndHandleAsync(asyncHandler, "localhost", port),
@@ -73,7 +73,7 @@ object AkkaHttpTestAsyncWebServer {
7373
}
7474

7575
def stop(): Unit = synchronized {
76-
if (null != binding) {
76+
if (binding != null) {
7777
binding.unbind()
7878
system.terminate()
7979
binding = null

‎instrumentation/akka/akka-http-10.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/akkahttp/AkkaHttpTestServerSourceWebServer.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ object AkkaHttpTestServerSourceWebServer {
9393
private var binding: ServerBinding = null
9494

9595
def start(port: Int): Unit = synchronized {
96-
if (null == binding) {
96+
if (binding == null) {
9797
import scala.concurrent.duration._
9898
binding = Await.result(
9999
Http()
@@ -107,7 +107,7 @@ object AkkaHttpTestServerSourceWebServer {
107107
}
108108

109109
def stop(): Unit = synchronized {
110-
if (null != binding) {
110+
if (binding != null) {
111111
binding.unbind()
112112
system.terminate()
113113
binding = null

‎instrumentation/akka/akka-http-10.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/akkahttp/AkkaHttpTestSyncWebServer.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ object AkkaHttpTestSyncWebServer {
6060
private var binding: ServerBinding = null
6161

6262
def start(port: Int): Unit = synchronized {
63-
if (null == binding) {
63+
if (binding == null) {
6464
import scala.concurrent.duration._
6565
binding = Await.result(
6666
Http().bindAndHandleSync(syncHandler, "localhost", port),
@@ -70,7 +70,7 @@ object AkkaHttpTestSyncWebServer {
7070
}
7171

7272
def stop(): Unit = synchronized {
73-
if (null != binding) {
73+
if (binding != null) {
7474
binding.unbind()
7575
system.terminate()
7676
binding = null

‎instrumentation/akka/akka-http-10.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/akkahttp/AkkaHttpTestWebServer.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,15 +92,15 @@ object AkkaHttpTestWebServer {
9292
private var binding: ServerBinding = null
9393

9494
def start(port: Int): Unit = synchronized {
95-
if (null == binding) {
95+
if (binding == null) {
9696
import scala.concurrent.duration._
9797
binding =
9898
Await.result(Http().bindAndHandle(route, "localhost", port), 10.seconds)
9999
}
100100
}
101101

102102
def stop(): Unit = synchronized {
103-
if (null != binding) {
103+
if (binding != null) {
104104
binding.unbind()
105105
system.terminate()
106106
binding = null

‎instrumentation/async-http-client/async-http-client-1-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/asynchttpclient/common/ResponseInstrumentation.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public static Scope onEnter(
6565

6666
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
6767
public static void onExit(@Advice.Enter @Nullable Scope scope) {
68-
if (null != scope) {
68+
if (scope != null) {
6969
scope.close();
7070
}
7171
}
@@ -90,7 +90,7 @@ public static Scope onEnter(
9090

9191
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
9292
public static void onExit(@Advice.Enter @Nullable Scope scope) {
93-
if (null != scope) {
93+
if (scope != null) {
9494
scope.close();
9595
}
9696
}

‎instrumentation/async-http-client/async-http-client-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/asynchttpclient/v2_0/AsyncCompletionHandlerInstrumentation.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public static Scope onEnter(
6666

6767
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
6868
public static void onExit(@Advice.Enter @Nullable Scope scope) {
69-
if (null != scope) {
69+
if (scope != null) {
7070
scope.close();
7171
}
7272
}
@@ -91,7 +91,7 @@ public static Scope onEnter(
9191

9292
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
9393
public static void onExit(@Advice.Enter @Nullable Scope scope) {
94-
if (null != scope) {
94+
if (scope != null) {
9595
scope.close();
9696
}
9797
}

‎instrumentation/executors/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/executors/JavaForkJoinTaskInstrumentation.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ public static Scope enter(@Advice.This ForkJoinTask<?> task) {
7070
Scope newScope =
7171
TaskAdviceHelper.makePropagatedContextCurrent(
7272
RUNNABLE_PROPAGATED_CONTEXT, (Runnable) task);
73-
if (null != newScope) {
74-
if (null != scope) {
73+
if (newScope != null) {
74+
if (scope != null) {
7575
newScope.close();
7676
} else {
7777
scope = newScope;
@@ -82,8 +82,8 @@ public static Scope enter(@Advice.This ForkJoinTask<?> task) {
8282
Scope newScope =
8383
TaskAdviceHelper.makePropagatedContextCurrent(
8484
CALLABLE_PROPAGATED_CONTEXT, (Callable<?>) task);
85-
if (null != newScope) {
86-
if (null != scope) {
85+
if (newScope != null) {
86+
if (scope != null) {
8787
newScope.close();
8888
} else {
8989
scope = newScope;

0 commit comments

Comments
 (0)