This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-10683
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY-10683 by this push:
new ab38a1c987 GROOVY-11595: error for invalid modifiers on `for` or
`catch` parameters
ab38a1c987 is described below
commit ab38a1c987f358272d164de1a54ccf95cd2a509a
Author: Eric Milles <[email protected]>
AuthorDate: Sun Mar 30 18:10:20 2025 -0500
GROOVY-11595: error for invalid modifiers on `for` or `catch` parameters
---
.../groovy/classgen/ClassCompletionVerifier.java | 90 +++++++++++++++-------
src/test/groovy/ForLoopTest.groovy | 2 +-
2 files changed, 63 insertions(+), 29 deletions(-)
diff --git
a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index 845c0dc5b3..5b0e0d48f4 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -44,6 +44,7 @@ import org.codehaus.groovy.ast.expr.PropertyExpression;
import org.codehaus.groovy.ast.expr.TupleExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.ast.stmt.CatchStatement;
+import org.codehaus.groovy.ast.stmt.ForStatement;
import org.codehaus.groovy.ast.tools.GeneralUtils;
import org.codehaus.groovy.ast.tools.ParameterUtils;
import org.codehaus.groovy.control.SourceUnit;
@@ -58,10 +59,12 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
+import static java.lang.reflect.Modifier.isAbstract;
import static java.lang.reflect.Modifier.isFinal;
import static java.lang.reflect.Modifier.isNative;
import static java.lang.reflect.Modifier.isPrivate;
import static java.lang.reflect.Modifier.isProtected;
+import static java.lang.reflect.Modifier.isPublic;
import static java.lang.reflect.Modifier.isStatic;
import static java.lang.reflect.Modifier.isStrict;
import static java.lang.reflect.Modifier.isSynchronized;
@@ -71,17 +74,10 @@ import static
org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics;
import static org.codehaus.groovy.ast.tools.GenericsUtils.buildWildcardType;
import static
org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpecRecurse;
import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec;
-import static org.objectweb.asm.Opcodes.ACC_ABSTRACT;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
-import static org.objectweb.asm.Opcodes.ACC_NATIVE;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
-import static org.objectweb.asm.Opcodes.ACC_PROTECTED;
-import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ACC_STATIC;
-import static org.objectweb.asm.Opcodes.ACC_STRICT;
-import static org.objectweb.asm.Opcodes.ACC_SYNCHRONIZED;
import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
-import static org.objectweb.asm.Opcodes.ACC_TRANSIENT;
import static org.objectweb.asm.Opcodes.ACC_VOLATILE;
/**
@@ -752,12 +748,46 @@ out: for (ClassNode sc : superTypes) {
@Override
public void visitCatchStatement(final CatchStatement cs) {
+ List<String> modifiers = new ArrayList<>();
+ int mods = cs.getVariable().getModifiers();
+
+ if (isAbstract (mods)) modifiers.add("abstract" );
+ if (isPrivate (mods)) modifiers.add("private" );
+ if (isProtected(mods)) modifiers.add("protected");
+ if (isPublic (mods)) modifiers.add("public" );
+ if (isStatic (mods)) modifiers.add("static" );
+ if (isStrict (mods)) modifiers.add("strictfp" );
+
+ for (String modifier : modifiers) {
+ addError("The catch " + getDescription(cs.getVariable()) + " has
invalid modifier " + modifier + ".", cs);
+ }
+
if
(!(cs.getExceptionType().isDerivedFrom(ClassHelper.THROWABLE_TYPE))) {
addError("Catch statement parameter type is not a subclass of
Throwable.", cs);
}
+
super.visitCatchStatement(cs);
}
+ @Override
+ public void visitForLoop(final ForStatement fs) {
+ List<String> modifiers = new ArrayList<>();
+ int mods = fs.getVariable().getModifiers();
+
+ if (isAbstract (mods)) modifiers.add("abstract" );
+ if (isPrivate (mods)) modifiers.add("private" );
+ if (isProtected(mods)) modifiers.add("protected");
+ if (isPublic (mods)) modifiers.add("public" );
+ if (isStatic (mods)) modifiers.add("static" );
+ if (isStrict (mods)) modifiers.add("strictfp" );
+
+ for (String modifier : modifiers) {
+ addError("The variable '" + fs.getVariable().getName() + "' has
invalid modifier " + modifier + ".", fs);
+ }
+
+ super.visitForLoop(fs);
+ }
+
@Override
public void visitMethodCallExpression(final MethodCallExpression mce) {
super.visitMethodCallExpression(mce);
@@ -772,34 +802,38 @@ out: for (ClassNode sc : superTypes) {
}
}
+ private void checkForInvalidDeclaration(final Expression exp) {
+ if (!(exp instanceof DeclarationExpression)) return;
+ addError("Invalid use of declaration inside method call.", exp);
+ }
+
@Override
public void visitDeclarationExpression(final DeclarationExpression
expression) {
super.visitDeclarationExpression(expression);
+
if (expression.isMultipleAssignmentDeclaration()) return;
- checkInvalidDeclarationModifier(expression, ACC_ABSTRACT, "abstract");
- checkInvalidDeclarationModifier(expression, ACC_NATIVE, "native");
- checkInvalidDeclarationModifier(expression, ACC_PRIVATE, "private");
- checkInvalidDeclarationModifier(expression, ACC_PROTECTED,
"protected");
- checkInvalidDeclarationModifier(expression, ACC_PUBLIC, "public");
- checkInvalidDeclarationModifier(expression, ACC_STATIC, "static");
- checkInvalidDeclarationModifier(expression, ACC_STRICT, "strictfp");
- checkInvalidDeclarationModifier(expression, ACC_SYNCHRONIZED,
"synchronized");
- checkInvalidDeclarationModifier(expression, ACC_TRANSIENT,
"transient");
- checkInvalidDeclarationModifier(expression, ACC_VOLATILE, "volatile");
- if
(ClassHelper.isPrimitiveVoid(expression.getVariableExpression().getOriginType()))
{
- addError("The variable '" +
expression.getVariableExpression().getName() + "' has invalid type void",
expression);
- }
- }
+ var vexp = expression.getVariableExpression();
+ List<String> modifiers = new ArrayList<>();
+ int mods = vexp.getModifiers();
+
+ if (isAbstract (mods)) modifiers.add("abstract" );
+ if (isNative (mods)) modifiers.add("native" );
+ if (isPrivate (mods)) modifiers.add("private" );
+ if (isProtected (mods)) modifiers.add("protected" );
+ if (isPublic (mods)) modifiers.add("public" );
+ if (isStatic (mods)) modifiers.add("static" );
+ if (isStrict (mods)) modifiers.add("strictfp" );
+ if (isSynchronized(mods)) modifiers.add("synchronized" );
+ if (isTransient (mods)) modifiers.add("transient" );
+ if (isVolatile (mods)) modifiers.add("volatile" );
- private void checkInvalidDeclarationModifier(final DeclarationExpression
expression, int modifier, String modName) {
- if ((expression.getVariableExpression().getModifiers() & modifier) !=
0) {
- addError("Modifier '" + modName + "' not allowed here.",
expression);
+ for (String modifier : modifiers) {
+ addError("The variable '" + vexp.getName() + "' has invalid
modifier " + modifier + ".", expression);
}
- }
- private void checkForInvalidDeclaration(final Expression exp) {
- if (!(exp instanceof DeclarationExpression)) return;
- addError("Invalid use of declaration inside method call.", exp);
+ if (ClassHelper.isPrimitiveVoid(vexp.getOriginType())) {
+ addError("The variable '" + vexp.getName() + "' has invalid type
void.", expression);
+ }
}
@Override
diff --git a/src/test/groovy/ForLoopTest.groovy
b/src/test/groovy/ForLoopTest.groovy
index c4603382f9..5608a553fb 100644
--- a/src/test/groovy/ForLoopTest.groovy
+++ b/src/test/groovy/ForLoopTest.groovy
@@ -31,7 +31,7 @@ final class ForLoopTest {
@ParameterizedTest
@ValueSource(strings=[
-
/*'public','private','protected','abstract','static',*/'native',/*'strictfp',*/'synchronized'
+
'public','private','protected','abstract','static','native','strictfp','synchronized'
])
void testFinalParameterInForLoopIsAllowed(String modifier) {
// only 'final' should be allowed; other modifiers should be forbidden
