Reduce visibility & add documentation (well a little at least)

Author: christophstrobl

src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java

@@ -21,8 +21,8 @@
 import org.springframework.util.MultiValueMap;
 
 /**
- * {@link VariableNameFactory} implementation keeping track of defined names resolving name clashes using internal
- * counter appending {@code _%d} to a suggested name in case of a clash.
+ * Non thread safe {@link VariableNameFactory} implementation keeping track of defined names resolving name clashes
+ * using internal counters appending {@code _%d} to a suggested name in case of a clash.
  *
  * @author Christoph Strobl
  * @since 4.0
@@ -31,36 +31,43 @@ class LocalVariableNameFactory implements VariableNameFactory {
 
 	private final MultiValueMap<String, String> variables;
 
+	/**
+	 * Create a new {@link LocalVariableNameFactory} considering available {@link MethodMetadata#getMethodArguments()
+	 * method arguments}.
+	 * 
+	 * @param methodMetadata source metadata
+	 * @return new instance of {@link LocalVariableNameFactory}.
+	 */
 	static LocalVariableNameFactory forMethod(MethodMetadata methodMetadata) {
 		return of(methodMetadata.getMethodArguments().keySet());
 	}
 
-	static LocalVariableNameFactory empty() {
-		return of(Set.of());
-	}
-
-	static LocalVariableNameFactory of(Set<String> variables) {
-		return new LocalVariableNameFactory(variables);
+	/**
+	 * Create a new {@link LocalVariableNameFactory} with a predefined set of initial variable names.
+	 *
+	 * @param predefinedVariables variables already known to be used in the given context.
+	 * @return new instance of {@link LocalVariableNameFactory}.
+	 */
+	static LocalVariableNameFactory of(Set<String> predefinedVariables) {
+		return new LocalVariableNameFactory(predefinedVariables);
 	}
 
 	LocalVariableNameFactory(Iterable<String> predefinedVariableNames) {
 
 		variables = new LinkedMultiValueMap<>();
-		for (String parameterName : predefinedVariableNames) {
-			variables.add(parameterName, parameterName);
-		}
+		predefinedVariableNames.forEach(varName -> variables.add(varName, varName));
 	}
 
 	@Override
-	public String generateName(String suggestedName) {
+	public String generateName(String intendedVariableName) {
 
-		if (!variables.containsKey(suggestedName)) {
-			variables.add(suggestedName, suggestedName);
-			return suggestedName;
+		if (!variables.containsKey(intendedVariableName)) {
+			variables.add(intendedVariableName, intendedVariableName);
+			return intendedVariableName;
 		}
 
-		String targetName = suggestTargetName(suggestedName);
-		variables.add(suggestedName, targetName);
+		String targetName = suggestTargetName(intendedVariableName);
+		variables.add(intendedVariableName, targetName);
 		variables.add(targetName, targetName);
 		return targetName;
 	}

src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java

@@ -15,12 +15,24 @@
  */
 package org.springframework.data.repository.aot.generate;
 
+import org.springframework.lang.CheckReturnValue;
+
 /**
+ * Name factory for generating clash free variable names checking an intended name against predefined and already used
+ * ones.
+ *
  * @author Christoph Strobl
  * @since 4.0
  */
-public interface VariableNameFactory {
+interface VariableNameFactory {
 
-	String generateName(String suggestedName);
+	/**
+	 * Compare and potentially generate a new name for the given intended variable name.
+	 *
+	 * @param intendedVariableName must not be {@literal null}.
+	 * @return the {@literal intendedVariableName} if no naming clash detected or a clash free generated name.
+	 */
+	@CheckReturnValue
+	String generateName(String intendedVariableName);
 
 }

src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java

@@ -50,6 +50,16 @@ void resolvesNameClashesInNames() {
 		assertThat(variableNameFactory.generateName("name4_1")).isEqualTo("name4_1_3");
 	}
 
+	@Test // GH-3270
+	void worksWithVariablesContainingUnderscores() {
+
+		assertThat(variableNameFactory.generateName("first_name")).isEqualTo("first_name");
+		assertThat(variableNameFactory.generateName("first_name")).isEqualTo("first_name_1");
+		assertThat(variableNameFactory.generateName("first_name")).isEqualTo("first_name_2");
+		assertThat(variableNameFactory.generateName("first_name_3")).isEqualTo("first_name_3");
+		assertThat(variableNameFactory.generateName("first_name")).isEqualTo("first_name_4");
+	}
+
 	@Test // GH-3270
 	void considersPredefinedNames() {
 		assertThat(variableNameFactory.generateName("firstname")).isEqualTo("firstname_1");