Hi, I suggest we use requireNonNull(var, "var") pattern rather than requireNonNull(var).
Then error message would include which variable/field turned out to be null. It would make a big difference in case a constructor verifies multiple parameters like in Correlate: protected Correlate( RelOptCluster cluster, RelTraitSet traitSet, RelNode left, RelNode right, CorrelationId correlationId, ImmutableBitSet requiredColumns, JoinRelType joinType) { super(cluster, traitSet, left, right); assert !joinType.generatesNullsOnLeft() : "Correlate has invalid join type " + joinType; this.joinType = requireNonNull(joinType); this.correlationId = requireNonNull(correlationId); this.requiredColumns = requireNonNull(requiredColumns); If a user makes a mistake and passes null to joinType or correlationId, then Calcite would throw NPE, however, it would be hard to tell which parameter was null. Line numbers are known to drift over time. I suggest we use the following pattern consistently: this.joinType = requireNonNull(joinType, "joinType"); this.correlationId = requireNonNull(correlationId, "correlationId"); this.requiredColumns = requireNonNull(requiredColumns, "requiredColumns"); The PR is https://github.com/apache/calcite/pull/2293 The suggested change automatically aligns single-word message with the first argument to requireNonNull. In other words, "./gradlew style" command would insert/fix the messages automatically. I assume modern IDEs enable developers to configure var => requireNonNull(var, "var") expansion (I use it in IDEA). Currently we have 545 usages of requireNonNull(var) <-- this is to be updated to (var, "var") 359 usages of requireNonNull(var, "var") 37 usages of requireNonNull(var, () -> ..) 33 usages of requireNonNull(var, "custom message") If no-one objects within three days, I'll assume lazy consensus and commit it. Vladimir