DATAJPA-624 - Fix SonarQube warnings as far as possible/reasonable.

Added private constructor to Jpa21Utils to prevent instantiation. Removed unused parameter query in Querydsl.toOrderSpecifier(…). Inline currently unnecessary computeParameterValue(…) method in ParameterBinder. Return cached crudMethodMetadata instead of always creating and caching a new instance in CrudMethodMetadataPostProcessor. Removed unnecessary final modifier from DefaultCrudMethodMetadata. Introduced constant for error message in SimpleJpaRepository as well as for join alias group index in QueryUtils. Prevent instantiation of BeanDefinitionNames by making it an interface instead of a class.

Reduced the usage of EvaluationContextProvider in JpaQueryLookupStrategy implementation. Removed unnecessary fixed sized set initialization in JpaQueryMethod static initializer block. Made JpaResultConverters final since it is not meant to be sub-classed. Avoid potential unsafe reference based equality check in StringQuery/hasPosition(..). Removed dead store in ClasspathScanningPersistenceUnitPostProessor.scanForMappingFileLocations. Encapsulated access to fields in EntityManagerFactoryBeanDefinition.

Original pull request: #114.

src/main/java/org/springframework/data/jpa/repository/config/BeanDefinitionNames.java

@@ -20,7 +20,7 @@ package org.springframework.data.jpa.repository.config;
  * 
  * @author Oliver Gierke
  */
-class BeanDefinitionNames {
+interface BeanDefinitionNames {
 
 	public static final String JPA_MAPPING_CONTEXT_BEAN_NAME = "jpaMapppingContext";
 }

src/main/java/org/springframework/data/jpa/repository/query/Jpa21Utils.java

@@ -47,6 +47,10 @@ public class Jpa21Utils {
 		}
 	}
 
+	private Jpa21Utils() {
+		// prevent instantiation
+	}
+
 	/**
 	 * Adds a JPA 2.1 fetch-graph or load-graph hint to the given {@link Query} if running under JPA 2.1.
 	 * 

src/main/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategy.java

@@ -50,7 +50,7 @@ public final class JpaQueryLookupStrategy {
 
 		private final EntityManager em;
 		private final QueryExtractor provider;
-		protected final EvaluationContextProvider evaluationContextProvider;
+		private final EvaluationContextProvider evaluationContextProvider;
 
 		/**
 		 * Creates a new {@link AbstractQueryLookupStrategy}.
@@ -81,6 +81,10 @@ public final class JpaQueryLookupStrategy {
 		}
 
 		protected abstract RepositoryQuery resolveQuery(JpaQueryMethod method, EntityManager em, NamedQueries namedQueries);
+
+		protected EvaluationContextProvider getEvaluationContextProvider() {
+			return evaluationContextProvider;
+		}
 	}
 
 	/**
@@ -107,6 +111,7 @@ public final class JpaQueryLookupStrategy {
 						method.toString()), e);
 			}
 		}
+
 	}
 
 	/**
@@ -134,7 +139,7 @@ public final class JpaQueryLookupStrategy {
 		@Override
 		protected RepositoryQuery resolveQuery(JpaQueryMethod method, EntityManager em, NamedQueries namedQueries) {
 
-			RepositoryQuery query = JpaQueryFactory.INSTANCE.fromQueryAnnotation(method, em, this.evaluationContextProvider);
+			RepositoryQuery query = JpaQueryFactory.INSTANCE.fromQueryAnnotation(method, em, getEvaluationContextProvider());
 
 			if (null != query) {
 				return query;
@@ -149,7 +154,7 @@ public final class JpaQueryLookupStrategy {
 			String name = method.getNamedQueryName();
 			if (namedQueries.hasQuery(name)) {
 				return JpaQueryFactory.INSTANCE.fromMethodWithQueryString(method, em, namedQueries.getQuery(name),
-						this.evaluationContextProvider);
+						getEvaluationContextProvider());
 			}
 
 			query = NamedQuery.lookupFrom(method, em);

src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java

@@ -55,7 +55,7 @@ public class JpaQueryMethod extends QueryMethod {
 
 	static {
 
-		Set<Class<?>> types = new HashSet<Class<?>>(4);
+		Set<Class<?>> types = new HashSet<Class<?>>();
 		types.add(byte[].class);
 		types.add(Byte[].class);
 		types.add(char[].class);

src/main/java/org/springframework/data/jpa/repository/query/JpaResultConverters.java

@@ -32,7 +32,7 @@ import org.springframework.util.StreamUtils;
  * @author Thomas Darimont
  * @since 1.6
  */
-class JpaResultConverters {
+final class JpaResultConverters {
 
 	/**
 	 * {@code private} to prevent instantiation.

src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java

@@ -106,7 +106,7 @@ public class ParameterBinder {
 
 			if (canBindParameter(parameter)) {
 
-				Object value = computeParameterValue(parameter, values[methodParameterPosition], values);
+				Object value = values[methodParameterPosition];
 
 				bind(query, parameter, value, queryParameterPosition++);
 			}

src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java

@@ -92,6 +92,9 @@ public abstract class QueryUtils {
 
 	private static final Map<PersistentAttributeType, Class<? extends Annotation>> ASSOCIATION_TYPES;
 
+	private static final int QUERY_JOIN_ALIAS_GROUP_INDEX = 3;
+	private static final int VARIABLE_NAME_GROUP_INDEX = 4;
+
 	static {
 
 		StringBuilder builder = new StringBuilder();
@@ -254,7 +257,7 @@ public abstract class QueryUtils {
 
 		while (matcher.find()) {
 
-			String alias = matcher.group(3);
+			String alias = matcher.group(QUERY_JOIN_ALIAS_GROUP_INDEX);
 			if (StringUtils.hasText(alias)) {
 				result.add(alias);
 			}
@@ -358,7 +361,7 @@ public abstract class QueryUtils {
 
 		if (countProjection == null) {
 
-			String variable = matcher.matches() ? matcher.group(4) : null;
+			String variable = matcher.matches() ? matcher.group(VARIABLE_NAME_GROUP_INDEX) : null;
 			boolean useVariable = variable != null && StringUtils.hasText(variable) && !variable.startsWith("new")
 					&& !variable.startsWith("count(") && !variable.contains(",");
 

src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java

@@ -423,7 +423,7 @@ class StringQuery {
 		 * @return
 		 */
 		public boolean hasPosition(Integer position) {
-			return position != null && this.name == null && this.position == position;
+			return position != null && this.name == null && position.equals(this.position);
 		}
 
 		/**

src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java

@@ -98,7 +98,7 @@ enum CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor {
 		public Object invoke(MethodInvocation invocation) throws Throwable {
 
 			Method method = invocation.getMethod();
-			Object metadata = TransactionSynchronizationManager.getResource(method);
+			CrudMethodMetadata metadata = (CrudMethodMetadata) TransactionSynchronizationManager.getResource(method);
 
 			if (metadata != null) {
 				return invocation.proceed();
@@ -112,7 +112,7 @@ enum CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor {
 				CrudMethodMetadata tmp = metadataCache.putIfAbsent(method, methodMetadata);
 
 				if (tmp != null) {
-					metadata = tmp;
+					methodMetadata = tmp;
 				}
 			}
 
@@ -152,20 +152,20 @@ enum CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor {
 			this.entityGraph = findEntityGraph(method);
 		}
 
-		private static final JpaEntityGraph findEntityGraph(Method method) {
+		private static JpaEntityGraph findEntityGraph(Method method) {
 
 			EntityGraph entityGraphAnnotation = AnnotationUtils.findAnnotation(method, EntityGraph.class);
 			return entityGraphAnnotation == null ? null : new JpaEntityGraph(entityGraphAnnotation.value(),
 					entityGraphAnnotation.type());
 		}
 
-		private static final LockModeType findLockModeType(Method method) {
+		private static LockModeType findLockModeType(Method method) {
 
 			Lock annotation = AnnotationUtils.findAnnotation(method, Lock.class);
 			return annotation == null ? null : (LockModeType) AnnotationUtils.getValue(annotation);
 		}
 
-		private static final Map<String, Object> findQueryHints(Method method) {
+		private static Map<String, Object> findQueryHints(Method method) {
 
 			Map<String, Object> queryHints = new HashMap<String, Object>();
 			QueryHints queryHintsAnnotation = AnnotationUtils.findAnnotation(method, QueryHints.class);

src/main/java/org/springframework/data/jpa/repository/support/EntityManagerBeanDefinitionRegistrarPostProcessor.java

@@ -51,25 +51,25 @@ public class EntityManagerBeanDefinitionRegistrarPostProcessor implements BeanFa
 	@Override
 	public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException {
 
-		for (EntityManagerFactoryBeanDefinition definitions : getEntityManagerFactoryBeanDefinitions(beanFactory)) {
+		for (EntityManagerFactoryBeanDefinition definition : getEntityManagerFactoryBeanDefinitions(beanFactory)) {
 
-			if (!(definitions.beanFactory instanceof BeanDefinitionRegistry)) {
+			if (!(definition.getBeanFactory() instanceof BeanDefinitionRegistry)) {
 				continue;
 			}
 
 			BeanDefinitionBuilder builder = BeanDefinitionBuilder
 					.rootBeanDefinition("org.springframework.orm.jpa.SharedEntityManagerCreator");
 			builder.setFactoryMethod("createSharedEntityManager");
-			builder.addConstructorArgReference(definitions.beanName);
+			builder.addConstructorArgReference(definition.getBeanName());
 
 			AbstractBeanDefinition emBeanDefinition = builder.getRawBeanDefinition();
 
-			emBeanDefinition.addQualifier(new AutowireCandidateQualifier(Qualifier.class, definitions.beanName));
-			emBeanDefinition.setScope(definitions.beanDefinition.getScope());
-			emBeanDefinition.setSource(definitions.beanDefinition.getSource());
+			emBeanDefinition.addQualifier(new AutowireCandidateQualifier(Qualifier.class, definition.getBeanName()));
+			emBeanDefinition.setScope(definition.getBeanDefinition().getScope());
+			emBeanDefinition.setSource(definition.getBeanDefinition().getSource());
 
 			BeanDefinitionReaderUtils.registerWithGeneratedName(emBeanDefinition,
-					(BeanDefinitionRegistry) definitions.beanFactory);
+					(BeanDefinitionRegistry) definition.getBeanFactory());
 		}
 	}
 }

src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java

@@ -207,6 +207,7 @@ public class JpaMetamodelEntityInformation<T, ID extends Serializable> extends J
 	 * Simple value object to encapsulate id specific metadata.
 	 * 
 	 * @author Oliver Gierke
+	 * @author Thomas Darimont
 	 */
 	private static class IdMetadata<T> implements Iterable<SingularAttribute<? super T, ?>> {
 
@@ -232,18 +233,21 @@ public class JpaMetamodelEntityInformation<T, ID extends Serializable> extends J
 				return idType;
 			}
 
-			Class<?> idType;
+			// lazy initialization of idType field with tolerable benign data-race
+			this.idType = tryExtractIdTypeWithFallbackToIdTypeLookup();
+
+			return this.idType;
+		}
+
+		private Class<?> tryExtractIdTypeWithFallbackToIdTypeLookup() {
 
 			try {
 				Type<?> idType2 = type.getIdType();
-				idType = idType2 == null ? fallbackIdTypeLookup(type) : idType2.getJavaType();
+				return idType2 == null ? fallbackIdTypeLookup(type) : idType2.getJavaType();
 			} catch (IllegalStateException e) {
 				// see https://hibernate.onjira.com/browse/HHH-6951
-				idType = fallbackIdTypeLookup(type);
+				return fallbackIdTypeLookup(type);
 			}
-
-			this.idType = idType;
-			return idType;
 		}
 
 		private static Class<?> fallbackIdTypeLookup(IdentifiableType<?> type) {

src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java

@@ -164,7 +164,7 @@ public class Querydsl {
 		Assert.notNull(query, "Query must not be null!");
 
 		for (Order order : sort) {
-			query.orderBy(toOrderSpecifier(order, query));
+			query.orderBy(toOrderSpecifier(order));
 		}
 
 		return query;
@@ -177,7 +177,7 @@ public class Querydsl {
 	 * @return
 	 */
 	@SuppressWarnings({ "rawtypes", "unchecked" })
-	private OrderSpecifier<?> toOrderSpecifier(Order order, JPQLQuery query) {
+	private OrderSpecifier<?> toOrderSpecifier(Order order) {
 
 		return new OrderSpecifier(order.isAscending() ? com.mysema.query.types.Order.ASC
 				: com.mysema.query.types.Order.DESC, buildOrderPropertyPathFrom(order),

src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java

@@ -65,6 +65,8 @@ import org.springframework.util.Assert;
 public class SimpleJpaRepository<T, ID extends Serializable> implements JpaRepository<T, ID>,
 		JpaSpecificationExecutor<T> {
 
+	private static final String ID_MUST_NOT_BE_NULL = "The given id must not be null!";
+
 	private final JpaEntityInformation<T, ?> entityInformation;
 	private final EntityManager em;
 	private final PersistenceProvider provider;
@@ -132,7 +134,7 @@ public class SimpleJpaRepository<T, ID extends Serializable> implements JpaRepos
 	@Transactional
 	public void delete(ID id) {
 
-		Assert.notNull(id, "The given id must not be null!");
+		Assert.notNull(id, ID_MUST_NOT_BE_NULL);
 
 		T entity = findOne(id);
 
@@ -213,7 +215,7 @@ public class SimpleJpaRepository<T, ID extends Serializable> implements JpaRepos
 	 */
 	public T findOne(ID id) {
 
-		Assert.notNull(id, "The given id must not be null!");
+		Assert.notNull(id, ID_MUST_NOT_BE_NULL);
 
 		Class<T> domainType = getDomainClass();
 
@@ -234,7 +236,7 @@ public class SimpleJpaRepository<T, ID extends Serializable> implements JpaRepos
 	@Override
 	public T getOne(ID id) {
 
-		Assert.notNull(id, "The given id must not be null!");
+		Assert.notNull(id, ID_MUST_NOT_BE_NULL);
 		return em.getReference(getDomainClass(), id);
 	}
 
@@ -244,7 +246,7 @@ public class SimpleJpaRepository<T, ID extends Serializable> implements JpaRepos
 	 */
 	public boolean exists(ID id) {
 
-		Assert.notNull(id, "The given id must not be null!");
+		Assert.notNull(id, ID_MUST_NOT_BE_NULL);
 
 		if (entityInformation.getIdAttribute() == null) {
 			return findOne(id) != null;

src/main/java/org/springframework/data/jpa/support/ClasspathScanningPersistenceUnitPostProcessor.java

@@ -134,7 +134,7 @@ public class ClasspathScanningPersistenceUnitPostProcessor implements Persistenc
 		String path = ResourcePatternResolver.CLASSPATH_ALL_URL_PREFIX + basePackagePathComponent + slash
 				+ mappingFileNamePattern;
 		Set<String> mappingFileUris = new HashSet<String>();
-		Resource[] scannedResources = new Resource[0];
+		Resource[] scannedResources;
 
 		try {
 			scannedResources = resolver.getResources(path);

src/main/java/org/springframework/data/jpa/util/BeanDefinitionUtils.java

@@ -83,10 +83,10 @@ public class BeanDefinitionUtils {
 
 				String transformedName = transformedBeanName(name);
 
-				EntityManagerFactoryBeanDefinition definition = new EntityManagerFactoryBeanDefinition();
-				definition.beanName = transformedName;
-				definition.beanFactory = beanFactory;
-				definition.beanDefinition = beanFactory.getBeanDefinition(transformedName);
+				EntityManagerFactoryBeanDefinition definition = new EntityManagerFactoryBeanDefinition( //
+						transformedName, //
+						beanFactory, //
+						beanFactory.getBeanDefinition(transformedName));
 
 				definitions.add(definition);
 			}
@@ -125,10 +125,39 @@ public class BeanDefinitionUtils {
 		}
 	}
 
+	/**
+	 * @author Oliver Gierke
+	 * @author Thomas Darimont
+	 */
 	public static class EntityManagerFactoryBeanDefinition {
 
-		public String beanName;
-		public BeanDefinition beanDefinition;
-		public BeanFactory beanFactory;
+		private final String beanName;
+		private final BeanFactory beanFactory;
+		private final BeanDefinition beanDefinition;
+
+		/**
+		 * Creates a new {@link EntityManagerFactoryBeanDefinition}.
+		 * 
+		 * @param beanName
+		 * @param beanFactory
+		 * @param beanDefinition
+		 */
+		public EntityManagerFactoryBeanDefinition(String beanName, BeanFactory beanFactory, BeanDefinition beanDefinition) {
+			this.beanName = beanName;
+			this.beanFactory = beanFactory;
+			this.beanDefinition = beanDefinition;
+		}
+
+		public String getBeanName() {
+			return beanName;
+		}
+
+		public BeanFactory getBeanFactory() {
+			return beanFactory;
+		}
+
+		public BeanDefinition getBeanDefinition() {
+			return beanDefinition;
+		}
 	}
 }