Skip to content

Commit

Permalink
avoid multiple charAt calls for injectedLogSite equals()
Browse files Browse the repository at this point in the history
RELNOTES=n/a
PiperOrigin-RevId: 700422386
  • Loading branch information
nick-someone authored and Flogger Team committed Nov 27, 2024
1 parent fc6e93b commit cb3989a
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 9 deletions.
1 change: 1 addition & 0 deletions api/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ gen_java_tests(
":testing",
"@google_bazel_common//third_party/java/auto:service",
"@google_bazel_common//third_party/java/guava",
"@google_bazel_common//third_party/java/guava:testlib",
"@google_bazel_common//third_party/java/jspecify_annotations",
"@google_bazel_common//third_party/java/junit",
"@google_bazel_common//third_party/java/mockito",
Expand Down
17 changes: 8 additions & 9 deletions api/src/main/java/com/google/common/flogger/LogSite.java
Original file line number Diff line number Diff line change
Expand Up @@ -223,16 +223,15 @@ private static boolean internalNamesCompatible(String s1, String s2) {
}

for (int i = 0; i < s1.length(); i++) {
if (s1.charAt(i) == s2.charAt(i)) {
continue;
char c1 = s1.charAt(i);
char c2 = s2.charAt(i);
if (c1 != c2) {
// If the characters are not equal, but the slash/dot difference is accounted for, then
// consider them equivalent.
if (!(c1 == '/' && c2 == '.') && !(c1 == '.' && c2 == '/')) {
return false;
}
}
if (s1.charAt(i) == '/' && s2.charAt(i) == '.') {
continue;
}
if (s1.charAt(i) == '.' && s2.charAt(i) == '/') {
continue;
}
return false;
}
return true;
}
Expand Down
64 changes: 64 additions & 0 deletions api/src/test/java/com/google/common/flogger/LogSiteTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright (C) 2024 The Flogger Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.common.flogger;

import static com.google.common.truth.Truth.assertThat;

import com.google.common.testing.EqualsTester;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public final class LogSiteTest {

@Test
public void injectedLogSite_internalVsExternalName() {
String className = "com.google.common.flogger.LogSiteTest";
String internalClassName = "com/google/common/flogger/LogSiteTest";

// Using the same class, but represented by the internal vs external name, should
// be considered equal.
new EqualsTester()
.addEqualityGroup(logSite(className), logSite(internalClassName))
.addEqualityGroup(logSite(className + "2"), logSite(internalClassName + "2"))
.addEqualityGroup(logSite(className + "3"), logSite(internalClassName + "3"))
.testEquals();

// The public "className()" method will be the same
assertThat(logSite(className).getClassName()).isEqualTo(className);
assertThat(logSite(internalClassName).getClassName()).isEqualTo(className);
}

// This technically passes, but is not what we want, since the botched name isn't a real class
// identifier. The complexity of tracking the separator is not worth it, however.
@Test
public void injectedLogSite_invalidClassName_stillValid() {
String className = "com.google.common.flogger.LogSiteTest";
String internalClassName = "com/google/common/flogger/LogSiteTest";
String botchedClassName = "com.google/common.flogger/LogSiteTest";

new EqualsTester()
.addEqualityGroup(logSite(className), logSite(internalClassName), logSite(botchedClassName))
.testEquals();
}

@SuppressWarnings("deprecation") // Intentionally calling injectedLogSite to test it.
private static LogSite logSite(String className) {
return LogSite.injectedLogSite(className, "someMethod", 42, "MyFile.java");
}
}

0 comments on commit cb3989a

Please sign in to comment.