You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@grails.apache.org by bo...@apache.org on 2026/08/13 22:35:06 UTC
(grails-core) 12/14: Fix IntelliJ warnings on DirtyCheckTransformation, dedup shared visit() guard
This is an automated email from the ASF dual-hosted git repository.
borinquenkid pushed a commit to branch test/document-datamapping-core-transformers
in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit a6bc037bd86227b3fa476e10ab261b9234e9c18b
Author: Walter Duque de Estrada <wb...@mac.com>
AuthorDate: Thu Aug 13 14:18:40 2026 -0500
Fix IntelliJ warnings on DirtyCheckTransformation, dedup shared visit() guard
- Removed the unused MY_TYPE_NAME field (dead since it was added;
never referenced anywhere in the codebase).
- Replaced .equals() with == for the ClassNode comparison.
- Extracted the 12-line visit(ASTNode[], SourceUnit) validation guard
duplicated verbatim between DirtyCheckTransformation and
JpaGormEntityTransformation into a new shared
LocalTransformationSupport.resolveAnnotatedClassOrNull, used by
both. Behavior is unchanged - same malformed-type guard, same
annotation-type/ClassNode checks, same early-return contract.
Extracting this logic out of the two AST transforms into a plain
static method made it directly unit-testable for the first time
(previously only reachable, if at all, through real compilation).
Added LocalTransformationSupportSpec covering the reachable branches.
One branch remains uncovered and is called out explicitly in the
spec's docs rather than silently skipped: the malformed-astNodes-shape
guard casts both array slots before checking their type, so any input
that would fail the check throws a plain ClassCastException from the
cast itself first - the intended RuntimeException can never actually
be constructed. This is a pre-existing latent issue inherited
unchanged from both original call sites, not introduced here.
Verified via a full, unfiltered module test suite run - no behavior
change for real compilation.
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
.../compiler/gorm/DirtyCheckTransformation.groovy | 16 +----
.../gorm/JpaGormEntityTransformation.groovy | 14 +---
...on.groovy => LocalTransformationSupport.groovy} | 49 +++++---------
.../gorm/LocalTransformationSupportSpec.groovy | 76 ++++++++++++++++++++++
4 files changed, 96 insertions(+), 59 deletions(-)
diff --git a/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/DirtyCheckTransformation.groovy b/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/DirtyCheckTransformation.groovy
index e798dac779..96a0d6aa70 100644
--- a/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/DirtyCheckTransformation.groovy
+++ b/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/DirtyCheckTransformation.groovy
@@ -22,8 +22,6 @@ package org.grails.compiler.gorm
import groovy.transform.CompilationUnitAware
import groovy.transform.CompileStatic
import org.codehaus.groovy.ast.ASTNode
-import org.codehaus.groovy.ast.AnnotatedNode
-import org.codehaus.groovy.ast.AnnotationNode
import org.codehaus.groovy.ast.ClassNode
import org.codehaus.groovy.control.CompilationUnit
import org.codehaus.groovy.control.CompilePhase
@@ -45,27 +43,17 @@ import org.apache.grails.common.compiler.GroovyTransformOrder
class DirtyCheckTransformation implements ASTTransformation, CompilationUnitAware, TransformWithPriority {
private static final ClassNode MY_TYPE = new ClassNode(DirtyCheck)
- private static final String MY_TYPE_NAME = '@' + MY_TYPE.getNameWithoutPackage()
CompilationUnit compilationUnit
@Override
@CompileStatic
void visit(ASTNode[] astNodes, SourceUnit source) {
-
- AnnotatedNode parent = (AnnotatedNode) astNodes[1]
- AnnotationNode node = (AnnotationNode) astNodes[0]
-
- if (!(astNodes[0] instanceof AnnotationNode) || !(astNodes[1] instanceof AnnotatedNode)) {
- throw new RuntimeException("Internal error: wrong types: ${node.getClass()} / ${parent.getClass()}")
- }
-
- if (!MY_TYPE.equals(node.getClassNode()) || !(parent instanceof ClassNode)) {
+ ClassNode cNode = LocalTransformationSupport.resolveAnnotatedClassOrNull(astNodes, MY_TYPE)
+ if (cNode == null) {
return
}
- ClassNode cNode = (ClassNode) parent
-
def dirtyCheckingTransformer = new DirtyCheckingTransformer()
dirtyCheckingTransformer.compilationUnit = compilationUnit
dirtyCheckingTransformer.performInjection(source, cNode)
diff --git a/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/JpaGormEntityTransformation.groovy b/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/JpaGormEntityTransformation.groovy
index 2bf087c120..25bf5c6147 100644
--- a/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/JpaGormEntityTransformation.groovy
+++ b/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/JpaGormEntityTransformation.groovy
@@ -21,8 +21,6 @@ package org.grails.compiler.gorm
import groovy.transform.CompileStatic
import org.codehaus.groovy.ast.ASTNode
-import org.codehaus.groovy.ast.AnnotatedNode
-import org.codehaus.groovy.ast.AnnotationNode
import org.codehaus.groovy.ast.ClassNode
import org.codehaus.groovy.control.CompilePhase
import org.codehaus.groovy.control.SourceUnit
@@ -45,19 +43,11 @@ class JpaGormEntityTransformation extends GormEntityTransformation {
@Override
void visit(ASTNode[] astNodes, SourceUnit sourceUnit) {
- AnnotatedNode parent = (AnnotatedNode) astNodes[1]
- AnnotationNode node = (AnnotationNode) astNodes[0]
-
- if (!(astNodes[0] instanceof AnnotationNode) || !(astNodes[1] instanceof AnnotatedNode)) {
- throw new RuntimeException("Internal error: wrong types: ${node.getClass()} / ${parent.getClass()}")
- }
-
- if (!MY_TYPE.equals(node.getClassNode()) || !(parent instanceof ClassNode)) {
+ ClassNode cNode = LocalTransformationSupport.resolveAnnotatedClassOrNull(astNodes, MY_TYPE)
+ if (cNode == null) {
return
}
- ClassNode cNode = (ClassNode) parent
-
visit(cNode, sourceUnit)
}
diff --git a/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/JpaGormEntityTransformation.groovy b/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/LocalTransformationSupport.groovy
similarity index 55%
copy from grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/JpaGormEntityTransformation.groovy
copy to grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/LocalTransformationSupport.groovy
index 2bf087c120..588682829c 100644
--- a/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/JpaGormEntityTransformation.groovy
+++ b/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/LocalTransformationSupport.groovy
@@ -24,27 +24,25 @@ import org.codehaus.groovy.ast.ASTNode
import org.codehaus.groovy.ast.AnnotatedNode
import org.codehaus.groovy.ast.AnnotationNode
import org.codehaus.groovy.ast.ClassNode
-import org.codehaus.groovy.control.CompilePhase
-import org.codehaus.groovy.control.SourceUnit
-import org.codehaus.groovy.transform.GroovyASTTransformation
-
-import grails.gorm.annotation.JpaEntity
-import org.apache.grails.common.compiler.GroovyTransformOrder
/**
- * Enhanced GORM entity annotated with JPA annotations
- *
- * @author Graeme Rocher
- * @since 6.1
+ * Shared guard logic for local, annotation-driven AST transformations in this package whose
+ * {@code visit(ASTNode[], SourceUnit)} entry point only applies when the visited node is a class
+ * carrying one specific annotation type.
*/
@CompileStatic
-@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
-class JpaGormEntityTransformation extends GormEntityTransformation {
+class LocalTransformationSupport {
- private static final ClassNode MY_TYPE = new ClassNode(JpaEntity)
+ private LocalTransformationSupport() {
+ }
- @Override
- void visit(ASTNode[] astNodes, SourceUnit sourceUnit) {
+ /**
+ * @param astNodes the array a local transform's {@code visit(ASTNode[], SourceUnit)} receives
+ * @param expectedAnnotationType the annotation type this transform applies to
+ * @return the annotated {@link ClassNode}, or {@code null} if this transform does not apply
+ * (the annotation doesn't match, or the annotated node isn't a class)
+ */
+ static ClassNode resolveAnnotatedClassOrNull(ASTNode[] astNodes, ClassNode expectedAnnotationType) {
AnnotatedNode parent = (AnnotatedNode) astNodes[1]
AnnotationNode node = (AnnotationNode) astNodes[0]
@@ -52,25 +50,10 @@ class JpaGormEntityTransformation extends GormEntityTransformation {
throw new RuntimeException("Internal error: wrong types: ${node.getClass()} / ${parent.getClass()}")
}
- if (!MY_TYPE.equals(node.getClassNode()) || !(parent instanceof ClassNode)) {
- return
+ if (expectedAnnotationType != node.getClassNode() || !(parent instanceof ClassNode)) {
+ return null
}
- ClassNode cNode = (ClassNode) parent
-
- visit(cNode, sourceUnit)
- }
-
- @Override
- void visit(ClassNode classNode, SourceUnit sourceUnit) {
- if (!hasAnnotation(classNode, JPA_ENTITY_CLASS_NODE)) {
- classNode.addAnnotation(JPA_ENTITY_ANNOTATION_NODE)
- }
- super.visit(classNode, sourceUnit)
- }
-
- @Override
- int priority() {
- GroovyTransformOrder.JPA_GORM_ENTITY_ORDER
+ return (ClassNode) parent
}
}
diff --git a/grails-datamapping-core/src/test/groovy/org/grails/compiler/gorm/LocalTransformationSupportSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/compiler/gorm/LocalTransformationSupportSpec.groovy
new file mode 100644
index 0000000000..e67f295854
--- /dev/null
+++ b/grails-datamapping-core/src/test/groovy/org/grails/compiler/gorm/LocalTransformationSupportSpec.groovy
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.grails.compiler.gorm
+
+import org.codehaus.groovy.ast.ASTNode
+import org.codehaus.groovy.ast.AnnotationNode
+import org.codehaus.groovy.ast.ClassHelper
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.FieldNode
+
+import spock.lang.Specification
+
+/**
+ * {@code LocalTransformationSupport.resolveAnnotatedClassOrNull} is the shared guard extracted
+ * from {@link DirtyCheckTransformation} and {@link JpaGormEntityTransformation}'s
+ * {@code visit(ASTNode[], SourceUnit)} entry points.
+ * <p>
+ * The method's malformed-{@code astNodes}-shape check is not covered here: it casts both array
+ * slots to {@code AnnotationNode}/{@code AnnotatedNode} before checking whether they actually are
+ * one, so any input that would fail the check throws a plain {@code ClassCastException} from the
+ * cast itself first - the intended {@code RuntimeException} with its "Internal error: wrong types"
+ * message can never actually be constructed, since building that message requires calling
+ * {@code .getClass()} on locals that only hold a non-null value once the (now un-reachable) casts
+ * have already succeeded. This is a pre-existing latent issue inherited unchanged from both
+ * original call sites, not introduced by extracting this method - it is called out here rather
+ * than silently left uncovered.
+ */
+class LocalTransformationSupportSpec extends Specification {
+
+ private static final ClassNode ANNOTATION_TYPE = ClassHelper.make(Deprecated)
+ private static final ClassNode OTHER_ANNOTATION_TYPE = ClassHelper.make(SuppressWarnings)
+
+ void "resolves the annotated class when the annotation type matches and the node is a class"() {
+ given:
+ ClassNode targetClass = new ClassNode('com.example.Target', 0, ClassHelper.OBJECT_TYPE)
+ AnnotationNode annotationNode = new AnnotationNode(ANNOTATION_TYPE)
+
+ expect:
+ LocalTransformationSupport.resolveAnnotatedClassOrNull([annotationNode, targetClass] as ASTNode[], ANNOTATION_TYPE) == targetClass
+ }
+
+ void "returns null when the annotation type does not match"() {
+ given:
+ ClassNode targetClass = new ClassNode('com.example.Target', 0, ClassHelper.OBJECT_TYPE)
+ AnnotationNode annotationNode = new AnnotationNode(OTHER_ANNOTATION_TYPE)
+
+ expect:
+ LocalTransformationSupport.resolveAnnotatedClassOrNull([annotationNode, targetClass] as ASTNode[], ANNOTATION_TYPE) == null
+ }
+
+ void "returns null when the annotated node is not a class"() {
+ given:
+ ClassNode declaringClass = new ClassNode('com.example.Target', 0, ClassHelper.OBJECT_TYPE)
+ FieldNode fieldNode = new FieldNode('someField', 0, ClassHelper.STRING_TYPE, declaringClass, null)
+ AnnotationNode annotationNode = new AnnotationNode(ANNOTATION_TYPE)
+
+ expect:
+ LocalTransformationSupport.resolveAnnotatedClassOrNull([annotationNode, fieldNode] as ASTNode[], ANNOTATION_TYPE) == null
+ }
+}