Use Tags.empty() as no-op argument. Reject multiple calls to withLogL…

…evelMap.

Accepting no-op arguments allows ScopedLoggingContext to conditionally add values. A complete no-op, however, allows for syntactic misuse of the builder, calling the same method multiple times as long as all but the last provide null values.

LogLevelMap requires special care, as once a LogLevelMap is set (even if logically empty), every single log statement needs to evaluate the map.

RELNOTES=Enforce calling ScopedLoggingContext.Builder methods at most once, even with no-op params.
PiperOrigin-RevId: 559433662

api/src/main/java/com/google/common/flogger/context/ScopedLoggingContext.java

@@ -137,25 +137,25 @@ public abstract static class Builder {
     private Tags tags = null;
     private ContextMetadata.Builder metadata = null;
     private LogLevelMap logLevelMap = null;
+    private boolean hasLogLevelMap = false;
 
     protected Builder() {}
 
     /**
      * Sets the tags to be used with the context. This method can be called at most once per
-     * builder. Calling with a null value does nothing.
+     * builder.
      */
     @CanIgnoreReturnValue
-    public final Builder withTags(@NullableDecl Tags tags) {
+    public final Builder withTags(Tags tags) {
       checkState(this.tags == null, "tags already set");
-      if (tags != null) {
-        this.tags = tags;
-      }
+      checkNotNull(tags, "tags");
+      this.tags = tags;
       return this;
     }
 
     /**
      * Adds a single metadata key/value pair to the context. This method can be called multiple
-     * times on a builder. Calling with a null value does nothing.
+     * times on a builder. Calling with a null value does not add metadata.
      */
     @CanIgnoreReturnValue
     public final <T> Builder withMetadata(MetadataKey<T> key, @NullableDecl T value) {
@@ -171,14 +171,13 @@ public final <T> Builder withMetadata(MetadataKey<T> key, @NullableDecl T value)
 
     /**
      * Sets the log level map to be used with the context being built. This method can be called at
-     * most once per builder. Calling with a null value does nothing.
+     * most once per builder. Calling with a null value does not set a log level map.
      */
     @CanIgnoreReturnValue
     public final Builder withLogLevelMap(@NullableDecl LogLevelMap logLevelMap) {
-      checkState(this.logLevelMap == null, "log level map already set");
-      if (logLevelMap != null) {
-        this.logLevelMap = logLevelMap;
-      }
+      checkState(!hasLogLevelMap, "log level map already set");
+      hasLogLevelMap = true;
+      this.logLevelMap = logLevelMap;
       return this;
     }
 

api/src/test/java/com/google/common/flogger/testing/AbstractScopedLoggingContextTest.java

@@ -20,6 +20,7 @@
 import static com.google.common.flogger.testing.MetadataSubject.assertThat;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
+import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.flogger.LoggingScope;
@@ -129,11 +130,11 @@ public void testLoggedTags_areMerged() {
   }
 
   @Test
-  public void testNewContext_withNullTags_ignored() {
+  public void testNewContext_withNoOpTags_ignored() {
     assertThat(getTagMap()).isEmpty();
     context
         .newContext()
-        .withTags(null)
+        .withTags(Tags.empty())
         .run(
             () -> {
               assertThat(getTagMap()).isEmpty();
@@ -143,6 +144,17 @@ public void testNewContext_withNullTags_ignored() {
     checkDone();
   }
 
+  @Test
+  public void testNewContext_withTagsTwice_rejected() {
+    try {
+      context.newContext().withTags(Tags.empty()).withTags(Tags.of("foo", "bar")).run(() -> {});
+      fail("Expected IllegalStateException");
+    } catch (IllegalStateException e) {
+      markTestAsDone();
+    }
+    checkDone();
+  }
+
   @Test
   public void testNewContext_withMetadata() {
     assertThat(getMetadata()).hasSize(0);
@@ -205,6 +217,17 @@ public void testNewContext_withNullLogLevelMap_ignored() {
     checkDone();
   }
 
+  @Test
+  public void testNewContext_withLogLevelMapTwice_rejected() {
+    try {
+      context.newContext().withLogLevelMap(null).withLogLevelMap(null).run(() -> {});
+      fail("Expected IllegalStateException");
+    } catch (IllegalStateException e) {
+      markTestAsDone();
+    }
+    checkDone();
+  }
+
   @Test
   public void testNewContext_withMergedTags() {
     assertThat(getTagMap()).isEmpty();