Index: src/main/org/codehaus/groovy/classgen/ClassCompletionVerifier.java =================================================================== --- src/main/org/codehaus/groovy/classgen/ClassCompletionVerifier.java (revision 19648) +++ src/main/org/codehaus/groovy/classgen/ClassCompletionVerifier.java (working copy) @@ -18,10 +18,12 @@ import org.codehaus.groovy.ast.ClassCodeVisitorSupport; import org.codehaus.groovy.ast.ClassHelper; import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.FieldNode; import org.codehaus.groovy.ast.MethodNode; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; +import org.codehaus.groovy.ast.Variable; import org.codehaus.groovy.ast.expr.BinaryExpression; import org.codehaus.groovy.ast.expr.ConstantExpression; import org.codehaus.groovy.ast.expr.DeclarationExpression; @@ -30,6 +32,7 @@ import org.codehaus.groovy.ast.expr.MapEntryExpression; import org.codehaus.groovy.ast.expr.MethodCallExpression; 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.control.SourceUnit; import org.objectweb.asm.Opcodes; @@ -43,6 +46,8 @@ private ClassNode currentClass; private SourceUnit source; + private boolean inConstructor = false; + private boolean inStaticConstructor = false; public ClassCompletionVerifier(SourceUnit source) { this.source = source; @@ -227,6 +232,8 @@ } public void visitMethod(MethodNode node) { + inConstructor = false; + inStaticConstructor = node.isStaticConstructor(); checkAbstractDeclaration(node); checkRepetitiveMethod(node); checkOverloadingPrivateAndPublic(node); @@ -338,8 +345,54 @@ expression.getRightExpression()); } super.visitBinaryExpression(expression); + + switch (expression.getOperation().getType()){ + case Types.EQUAL: // = assignment + case Types.BITWISE_AND_EQUAL: + case Types.BITWISE_OR_EQUAL: + case Types.BITWISE_XOR_EQUAL: + case Types.PLUS_EQUAL: + case Types.MINUS_EQUAL: + case Types.MULTIPLY_EQUAL: + case Types.DIVIDE_EQUAL: + case Types.INTDIV_EQUAL: + case Types.MOD_EQUAL: + case Types.POWER_EQUAL: + case Types.LEFT_SHIFT_EQUAL: + case Types.RIGHT_SHIFT_EQUAL: + case Types.RIGHT_SHIFT_UNSIGNED_EQUAL: + checkFinalFieldAccess(expression.getLeftExpression()); + break; + default: break; + } } + private void checkFinalFieldAccess(Expression expression) { + if (!(expression instanceof VariableExpression)) return; + VariableExpression ve = (VariableExpression) expression; + Variable v = ve.getAccessedVariable(); + if (v instanceof FieldNode) { + FieldNode fn = (FieldNode) v; + int modifiers = fn.getModifiers(); + + /* + * if it is static final but not accessed inside a static constructor, or, + * if it is an instance final but not accessed inside a instance constructor, it is an error + */ + boolean isFinal = (modifiers & Opcodes.ACC_FINAL) != 0; + boolean isStatic = (modifiers & Opcodes.ACC_STATIC) != 0; + boolean error = isFinal && ((isStatic && !inStaticConstructor) || (!isStatic && !inConstructor)); + + if (error) addError("cannot modify" + (isStatic ? " static" : "") + " final field '" + fn.getName() + + "' outside of " + (isStatic ? "static initialization block." : "constructor."), expression); + } + } + + public void visitConstructor(ConstructorNode node) { + inConstructor = true; + inStaticConstructor = node.isStaticConstructor(); + super.visitConstructor(node); + } public void visitCatchStatement(CatchStatement cs) { if (!(cs.getExceptionType().isDerivedFrom(ClassHelper.make(Throwable.class)))) { addError("Catch statement parameter type is not a subclass of Throwable.", cs); Index: src/main/org/codehaus/groovy/classgen/VariableScopeVisitor.java =================================================================== --- src/main/org/codehaus/groovy/classgen/VariableScopeVisitor.java (revision 19648) +++ src/main/org/codehaus/groovy/classgen/VariableScopeVisitor.java (working copy) @@ -24,8 +24,6 @@ import org.codehaus.groovy.ast.stmt.ForStatement; import org.codehaus.groovy.ast.stmt.Statement; import org.codehaus.groovy.control.SourceUnit; -import org.codehaus.groovy.syntax.Types; -import org.objectweb.asm.Opcodes; import java.util.LinkedList; @@ -46,7 +44,6 @@ private boolean inPropertyExpression = false; private boolean isSpecialConstructorCall = false; private boolean inConstructor = false; - private boolean inStaticConstructor = false; private LinkedList stateStack = new LinkedList(); @@ -424,7 +421,6 @@ protected void visitConstructorOrMethod(MethodNode node, boolean isConstructor) { pushState(node.isStatic()); inConstructor = isConstructor; - inStaticConstructor = node.isStaticConstructor(); node.setVariableScope(currentScope); // GROOVY-2156 @@ -497,51 +493,6 @@ popState(); } - public void visitBinaryExpression(BinaryExpression expression) { - super.visitBinaryExpression(expression); - switch (expression.getOperation().getType()){ - case Types.EQUAL: // = assignment - case Types.BITWISE_AND_EQUAL: - case Types.BITWISE_OR_EQUAL: - case Types.BITWISE_XOR_EQUAL: - case Types.PLUS_EQUAL: - case Types.MINUS_EQUAL: - case Types.MULTIPLY_EQUAL: - case Types.DIVIDE_EQUAL: - case Types.INTDIV_EQUAL: - case Types.MOD_EQUAL: - case Types.POWER_EQUAL: - case Types.LEFT_SHIFT_EQUAL: - case Types.RIGHT_SHIFT_EQUAL: - case Types.RIGHT_SHIFT_UNSIGNED_EQUAL: - checkFinalFieldAccess(expression.getLeftExpression()); - break; - default: break; - } - - } - - private void checkFinalFieldAccess(Expression expression) { - if (!(expression instanceof VariableExpression)) return; - VariableExpression ve = (VariableExpression) expression; - Variable v = ve.getAccessedVariable(); - if (v instanceof FieldNode) { - FieldNode fn = (FieldNode) v; - int modifiers = fn.getModifiers(); - - /* - * if it is static final but not accessed inside a static constructor, or, - * if it is an instance final but not accessed inside a instance constructor, it is an error - */ - boolean isFinal = (modifiers & Opcodes.ACC_FINAL) != 0; - boolean isStatic = (modifiers & Opcodes.ACC_STATIC) != 0; - boolean error = isFinal && ((isStatic && !inStaticConstructor) || (!isStatic && !inConstructor)); - - if (error) addError("cannot modify" + (isStatic ? " static" : "") + " final field '" + fn.getName() + - "' outside of " + (isStatic ? "static initialization block." : "constructor."), expression); - } - } - public void visitProperty(PropertyNode node) { pushState(node.isStatic()); super.visitProperty(node); Index: src/test/groovy/bugs/Groovy4121Bug.groovy =================================================================== --- src/test/groovy/bugs/Groovy4121Bug.groovy (revision 0) +++ src/test/groovy/bugs/Groovy4121Bug.groovy (revision 0) @@ -0,0 +1,45 @@ +/* + * Copyright 2003-2010 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 + * + * http://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 groovy.bugs + +import org.codehaus.groovy.control.MultipleCompilationErrorsException + +class Groovy4121Bug extends GroovyTestCase { + void testAssignmentToAFieldMadeFinalByImmutable() { + try { + new GroovyShell().parse """ + @Immutable + class Account4121 { + BigDecimal balance + String customer + + Account4121 deposit(amount) { + balance = balance + amount + this + } + } + + def acc = new Account4121(0.0, "Test") + acc.deposit(3.1) + assert 3.1 == acc.balance + """ + fail('The compilation should have failed as a final field is being assigned to.') + } catch (MultipleCompilationErrorsException e) { + def syntaxError = e.errorCollector.getSyntaxError(0) + assert syntaxError.message.contains("cannot modify final field 'balance' outside of constructor") + } + } +} \ No newline at end of file