Skip to content

Add LocalVariableNameFactory. #3271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: 4.0.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-commons</artifactId>
<version>4.0.0-SNAPSHOT</version>
<version>4.0.x-GH-3270-SNAPSHOT</version>

<name>Spring Data Core</name>
<description>Core Spring concepts underpinning every Spring Data module.</description>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import java.util.Map.Entry;

import javax.lang.model.element.Modifier;

import org.jspecify.annotations.Nullable;

import org.springframework.core.ResolvableType;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotationSelectors;
Expand Down Expand Up @@ -54,6 +52,7 @@ public class AotQueryMethodGenerationContext {
private final AotRepositoryFragmentMetadata targetTypeMetadata;
private final MethodMetadata targetMethodMetadata;
private final CodeBlocks codeBlocks;
private final VariableNameFactory variableNameFactory;

AotQueryMethodGenerationContext(RepositoryInformation repositoryInformation, Method method, QueryMethod queryMethod,
AotRepositoryFragmentMetadata targetTypeMetadata) {
Expand All @@ -64,6 +63,7 @@ public class AotQueryMethodGenerationContext {
this.repositoryInformation = repositoryInformation;
this.targetTypeMetadata = targetTypeMetadata;
this.targetMethodMetadata = new MethodMetadata(repositoryInformation, method);
this.variableNameFactory = LocalVariableNameFactory.forMethod(targetMethodMetadata);
this.codeBlocks = new CodeBlocks(targetTypeMetadata);
}

Expand Down Expand Up @@ -127,6 +127,16 @@ public TypeName getReturnTypeName() {
return TypeName.get(getReturnType().getType());
}

/**
* Suggest naming clash free variant for the given intended variable name within the local method context.
*
* @param variableName the intended variable name.
* @return the suggested VariableName
*/
public String suggestLocalVariableName(String variableName) {
return variableNameFactory.generateName(variableName);
}

/**
* Returns the required parameter name for the {@link Parameter#isBindable() bindable parameter} at the given
* {@code parameterIndex} or throws {@link IllegalArgumentException} if the parameter cannot be determined by its
Expand Down Expand Up @@ -227,7 +237,7 @@ public List<String> getBindableParameterNames() {
List<String> result = new ArrayList<>();

for (Parameter parameter : queryMethod.getParameters().getBindableParameters()) {
parameter.getName().map(result::add);
getParameterName(parameter.getIndex());
}

return result;
Expand All @@ -237,14 +247,7 @@ public List<String> getBindableParameterNames() {
* @return list of all parameter names (including non-bindable special parameters).
*/
public List<String> getAllParameterNames() {

List<String> result = new ArrayList<>();

for (Parameter parameter : queryMethod.getParameters()) {
parameter.getName().map(result::add);
}

return result;
return targetMethodMetadata.getMethodArguments().keySet().stream().toList();
}

public boolean hasField(String fieldName) {
Expand All @@ -269,17 +272,7 @@ public String getParameterNameOf(Class<?> type) {
}

public @Nullable String getParameterName(int position) {

if (0 > position) {
return null;
}

List<Entry<String, ParameterSpec>> entries = new ArrayList<>(
targetMethodMetadata.getMethodArguments().entrySet());
if (position < entries.size()) {
return entries.get(position).getKey();
}
return null;
return targetMethodMetadata.getParameterName(position);
}

public void addParameter(ParameterSpec parameter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,15 @@
package org.springframework.data.repository.aot.generate;

import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.lang.reflect.TypeVariable;
import java.util.function.BiConsumer;
import java.util.function.Function;
import java.util.stream.Collectors;

import javax.lang.model.element.Modifier;

import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.MethodParameter;
import org.springframework.core.ResolvableType;
import org.springframework.data.repository.core.RepositoryInformation;
import org.springframework.javapoet.CodeBlock;
import org.springframework.javapoet.MethodSpec;
import org.springframework.javapoet.ParameterSpec;
import org.springframework.javapoet.TypeName;
import org.springframework.javapoet.TypeVariableName;
import org.springframework.util.StringUtils;
Expand All @@ -50,25 +44,7 @@ class AotRepositoryMethodBuilder {
private BiConsumer<AotQueryMethodGenerationContext, MethodSpec.Builder> customizer = (context, body) -> {};

AotRepositoryMethodBuilder(AotQueryMethodGenerationContext context) {

this.context = context;
initParameters(context.getMethod(), context.getRepositoryInformation());
}

private void initParameters(Method method, RepositoryInformation repositoryInformation) {

ResolvableType repositoryInterface = ResolvableType.forClass(repositoryInformation.getRepositoryInterface());

for (Parameter parameter : method.getParameters()) {

MethodParameter methodParameter = MethodParameter.forParameter(parameter);
methodParameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer());
ResolvableType resolvableParameterType = ResolvableType.forMethodParameter(methodParameter, repositoryInterface);

TypeName parameterType = TypeName.get(resolvableParameterType.getType());

this.context.addParameter(ParameterSpec.builder(parameterType, methodParameter.getParameterName()).build());
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright 2025 the original author or 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
*
* https://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 org.springframework.data.repository.aot.generate;

import java.util.Set;

import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;

/**
* 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
*/
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());
}

/**
* 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simplification of this could be:

	LocalVariableNameFactory(Iterable<String> predefinedVariableNames) {
		predefinedVariableNames.forEach((paramName) -> variables.put(paramName, 0L));
	}

Wdyt?


variables = new LinkedMultiValueMap<>();
predefinedVariableNames.forEach(varName -> variables.add(varName, varName));
}

@Override
public String generateName(String intendedVariableName) {

if (!variables.containsKey(intendedVariableName)) {
variables.add(intendedVariableName, intendedVariableName);
return intendedVariableName;
}

String targetName = suggestTargetName(intendedVariableName);
variables.add(intendedVariableName, targetName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to update the original intendedVariableName?

This code is fine but concurrency aside, I still think something like the following that would replace line62-86 would simplify things:

@Override
public String generateName(String suggestedName) {
	var counter = variables.compute(suggestedName,
			(__, currentCount) -> currentCount == null ? 0 : currentCount.longValue() + 1);
	return counter == 0 ? suggestedName : "%s_%s".formatted(suggestedName, counter));
}

variables.add(targetName, targetName);
return targetName;
}

String suggestTargetName(String suggested) {
return suggestTargetName(suggested, 1);
}

String suggestTargetName(String suggested, int counter) {

String targetName = "%s_%s".formatted(suggested, counter);
if (!variables.containsKey(targetName)) {
return targetName;
}
return suggestTargetName(suggested, counter + 1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,16 @@
package org.springframework.data.repository.aot.generate;

import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;

import org.jspecify.annotations.Nullable;

import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.MethodParameter;
import org.springframework.core.ParameterNameDiscoverer;
import org.springframework.core.ResolvableType;
import org.springframework.data.repository.core.RepositoryInformation;
import org.springframework.javapoet.ParameterSpec;
Expand All @@ -31,17 +35,19 @@
* Metadata about an AOT Repository method.
*
* @author Christoph Strobl
* @since 4.0
*/
class MethodMetadata {

private final Map<String, ParameterSpec> methodArguments = new LinkedHashMap<>();
private final ResolvableType actualReturnType;
private final ResolvableType returnType;

public MethodMetadata(RepositoryInformation repositoryInformation, Method method) {
MethodMetadata(RepositoryInformation repositoryInformation, Method method) {

this.returnType = repositoryInformation.getReturnType(method).toResolvableType();
this.actualReturnType = ResolvableType.forType(repositoryInformation.getReturnedDomainClass(method));
this.initParameters(repositoryInformation, method, new DefaultParameterNameDiscoverer());
}

@Nullable
Expand All @@ -54,20 +60,50 @@ public String getParameterNameOf(Class<?> type) {
return null;
}

public ResolvableType getReturnType() {
ResolvableType getReturnType() {
return returnType;
}

public ResolvableType getActualReturnType() {
ResolvableType getActualReturnType() {
return actualReturnType;
}

public void addParameter(ParameterSpec parameterSpec) {
void addParameter(ParameterSpec parameterSpec) {
this.methodArguments.put(parameterSpec.name, parameterSpec);
}

public Map<String, ParameterSpec> getMethodArguments() {
Map<String, ParameterSpec> getMethodArguments() {
return methodArguments;
}

@Nullable
String getParameterName(int position) {

if (0 > position) {
return null;
}

List<Entry<String, ParameterSpec>> entries = new ArrayList<>(methodArguments.entrySet());
if (position < entries.size()) {
return entries.get(position).getKey();
}
return null;
}

private void initParameters(RepositoryInformation repositoryInformation, Method method,
ParameterNameDiscoverer nameDiscoverer) {

ResolvableType repositoryInterface = ResolvableType.forClass(repositoryInformation.getRepositoryInterface());

for (java.lang.reflect.Parameter parameter : method.getParameters()) {

MethodParameter methodParameter = MethodParameter.forParameter(parameter);
methodParameter.initParameterNameDiscovery(nameDiscoverer);
ResolvableType resolvableParameterType = ResolvableType.forMethodParameter(methodParameter, repositoryInterface);

TypeName parameterType = TypeName.get(resolvableParameterType.getType());

addParameter(ParameterSpec.builder(parameterType, methodParameter.getParameterName()).build());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright 2025 the original author or 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
*
* https://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 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
*/
interface VariableNameFactory {

/**
* 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);

}
Loading