diff --git a/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/generator/Consequence.java b/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/generator/Consequence.java index 81a0d3434f2..815a09300b4 100644 --- a/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/generator/Consequence.java +++ b/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/generator/Consequence.java @@ -19,7 +19,6 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -65,7 +64,6 @@ import org.drools.mvelcompiler.MvelCompilerException; import static com.github.javaparser.StaticJavaParser.parseExpression; -import static com.github.javaparser.ast.NodeList.nodeList; import static java.util.stream.Collectors.toSet; import static org.drools.core.util.ClassUtils.getter2property; import static org.drools.core.util.ClassUtils.isGetter; @@ -153,7 +151,7 @@ public MethodCallExpr createCall(RuleDescr ruleDescr, String consequenceString, switch (context.getRuleDialect()) { case JAVA: rewriteReassignedDeclarations(ruleConsequence, usedDeclarationInRHS); - executeCall = executeCall(ruleVariablesBlock, ruleConsequence, usedDeclarationInRHS, onCall, Collections.emptySet()); + executeCall = executeCall(ruleVariablesBlock, ruleConsequence, usedDeclarationInRHS, onCall); break; case MVEL: executeCall = createExecuteCallMvel(consequenceString, ruleVariablesBlock, usedDeclarationInRHS, onCall); @@ -209,9 +207,9 @@ private MethodCallExpr createExecuteCallMvel(String consequenceString, BlockStmt return executeCall(ruleVariablesBlock, compile.statementResults(), usedDeclarationInRHS, - onCall, - compile.getUsedBindings()); + onCall); } + private BlockStmt rewriteConsequence(String consequence) { try { String ruleConsequenceAsBlock = rewriteModifyBlock(consequence.trim()); @@ -268,16 +266,7 @@ public static boolean containsWord(String word, String body) { return m.find(); } - private MethodCallExpr executeCall(BlockStmt ruleVariablesBlock, BlockStmt ruleConsequence, Collection verifiedDeclUsedInRHS, MethodCallExpr onCall, Set modifyProperties) { - - for (String modifiedProperty : modifyProperties) { - NodeList arguments = nodeList(new NameExpr(modifiedProperty)); - MethodCallExpr update = new MethodCallExpr(new NameExpr("drools"), "update", - arguments); - ruleConsequence.getStatements().add(new ExpressionStmt(update)); - } - - + private MethodCallExpr executeCall(BlockStmt ruleVariablesBlock, BlockStmt ruleConsequence, Collection verifiedDeclUsedInRHS, MethodCallExpr onCall) { boolean requireDrools = rewriteRHS(ruleVariablesBlock, ruleConsequence); MethodCallExpr executeCall = new MethodCallExpr(onCall != null ? onCall : new NameExpr("D"), EXECUTE_CALL); LambdaExpr executeLambda = new LambdaExpr(); @@ -361,8 +350,8 @@ private boolean rewriteRHS(BlockStmt ruleBlock, BlockStmt rhs) { if (context.isPropertyReactive(updatedClass)) { if ( !initializedBitmaskFields.contains( updatedVar ) ) { - Set modifiedProps = findModifiedProperties( methodCallExprs, updateExpr, updatedVar, updatedClass ); - modifiedProps.addAll(findModifiedPropertiesFromAssignment( assignExprs, updateExpr, updatedVar, updatedClass )); + Set modifiedProps = findModifiedProperties(methodCallExprs, updatedVar, updatedClass ); + modifiedProps.addAll(findModifiedPropertiesFromAssignment(assignExprs, updatedVar)); MethodCallExpr bitMaskCreation = createBitMaskInitialization( updatedClass, modifiedProps ); AssignExpr bitMaskAssign = createBitMaskField(updatedVar, bitMaskCreation); if (!DrlxParseUtil.hasDuplicateExpr(ruleBlock, bitMaskAssign)) { @@ -412,9 +401,9 @@ private AssignExpr createBitMaskField(String updatedVar, MethodCallExpr bitMaskC return new AssignExpr(bitMaskVar, bitMaskCreation, AssignExpr.Operator.ASSIGN); } - private Set findModifiedProperties( List methodCallExprs, MethodCallExpr updateExpr, String updatedVar, Class updatedClass ) { + private Set findModifiedProperties( List methodCallExprs, String updatedVar, Class updatedClass ) { Set modifiedProps = new HashSet<>(); - for (MethodCallExpr methodCall : methodCallExprs.subList(0, methodCallExprs.indexOf(updateExpr))) { + for (MethodCallExpr methodCall : methodCallExprs) { if (!isDirectExpression(methodCall)) { continue; // don't evaluate a method which is a part of other expression } @@ -464,7 +453,7 @@ private Set findModifiedProperties( List methodCallExprs return modifiedProps; } - private Set findModifiedPropertiesFromAssignment(List assignExprs, MethodCallExpr updateExpr, String updatedVar, Class updatedClass) { + private Set findModifiedPropertiesFromAssignment(List assignExprs, String updatedVar) { Set modifiedProps = new HashSet<>(); for (AssignExpr assignExpr : assignExprs) { Expression target = assignExpr.getTarget(); diff --git a/drools-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/PropertyReactivityMatrixTest.java b/drools-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/PropertyReactivityMatrixTest.java index c18e87024e8..719e599e1f7 100644 --- a/drools-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/PropertyReactivityMatrixTest.java +++ b/drools-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/PropertyReactivityMatrixTest.java @@ -82,7 +82,6 @@ public int getValue2() { public void setValue2(int value2) { this.value2 = value2; } - } private String setValueStatement(String property, int value) { @@ -163,6 +162,52 @@ public void updateInsideIfBlockInsideAndOutsideAssignment_shouldTriggerReactivit " }"); } + @Test + public void assignmentAfterModify_shouldTriggerReactivity() { + // DROOLS-7497 + statementInsideBlock_shouldTriggerReactivity(" modify($fact) {\n" + + " " + setValueStatement("value2", 2) + "\n" + + " }\n" + + " $fact." + setValueStatement("value1", 2) + "\n"); + } + + @Test + public void assignmentBeforeAndAfterModify_shouldTriggerReactivity() { + // DROOLS-7497 + statementInsideBlock_shouldTriggerReactivity(" $fact." + setValueStatement("value2", 2) + "\n" + + " modify($fact) {\n" + + " }\n" + + " $fact." + setValueStatement("value1", 2) + "\n"); + } + + @Test + public void assignmentAfterIfBlockModify_shouldTriggerReactivity() { + // DROOLS-7497 + statementInsideBlock_shouldTriggerReactivity(" if (true) {\n" + + " modify($fact) {\n" + + " " + setValueStatement("value2", 2) + "\n" + + " }\n" + + " }\n" + + " $fact." + setValueStatement("value1", 2) + "\n"); + } + + @Test + public void assignmentBeforeAndAfterUpdate_shouldTriggerReactivity() { + // DROOLS-7497 + statementInsideBlock_shouldTriggerReactivity(" $fact." + setValueStatement("value2", 2) + "\n" + + " update($fact);\n" + + " $fact." + setValueStatement("value1", 2) + "\n"); + } + + @Test + public void assignmentAfterIfBlockUpdate_shouldTriggerReactivity() { + // DROOLS-7497 + statementInsideBlock_shouldTriggerReactivity(" if (true) {\n" + + " update($fact);\n" + + " }\n" + + " $fact." + setValueStatement("value1", 2) + "\n"); + } + private void statementInsideBlock_shouldTriggerReactivity(String rhs) { final String str = "import " + Fact.class.getCanonicalName() + ";\n" + @@ -192,4 +237,100 @@ private void statementInsideBlock_shouldTriggerReactivity(String rhs) { assertThat(results).as("Should trigger property reactivity on value1") .containsExactly("R2 fired"); } + + @Test + public void modifyInsideIfFalseAndTrueBlock_shouldTriggerReactivity() { + // DROOLS-7493 + statementInsideBlock_shouldTriggerReactivityWithLoop(" if (false) {\n" + + " $fact." + setValueStatement("value1", 2) + "\n" + // this line is not executed, but analyzed as a property reactivity + " }\n" + + " if (true) {\n" + + " modify($fact) {\n" + + " }\n" + + " }\n"); + } + + @Test + public void updateInsideIfFalseAndTrueBlock_shouldTriggerReactivity() { + statementInsideBlock_shouldTriggerReactivityWithLoop(" if (false) {\n" + + " $fact." + setValueStatement("value1", 2) + "\n" + // this line is not executed, but analyzed as a property reactivity + " }\n" + + " if (true) {\n" + + " update($fact);\n" + + " }\n"); + } + + private void statementInsideBlock_shouldTriggerReactivityWithLoop(String rhs) { + final String str = + "import " + Fact.class.getCanonicalName() + ";\n" + + "dialect \"" + dialect.name().toLowerCase() + "\"\n" + + "global java.util.List results;\n" + + "rule R1\n" + + " when\n" + + " $fact : Fact( value1 == 1 )\n" + + " then\n" + + rhs + + "end\n"; + + KieSession ksession = getKieSession(str); + List results = new ArrayList<>(); + ksession.setGlobal("results", results); + + Fact fact = new Fact(1); + ksession.insert(fact); + int fired = ksession.fireAllRules(10); + + assertThat(fired).as("Should trigger property reactivity on value1 and result in a loop") + .isEqualTo(10); + } + + @Test + public void modifyInsideIfFalseBlock_shouldNotTriggerReactivity() { + // DROOLS-7493 + statementInsideIfFalseBlock_shouldNotTriggerReactivityNorSelfLoop(" if (false) {\n" + + " modify($fact) {\n" + + " " + setValueStatement("value1", 2) + "\n" + + " }\n" + + " }\n"); + } + + @Test + public void updateInsideIfFalseBlock_shouldNotTriggerReactivity() { + statementInsideIfFalseBlock_shouldNotTriggerReactivityNorSelfLoop(" if (false) {\n" + + " $fact." + setValueStatement("value1", 2) + "\n" + + " update($fact);\n" + + " }\n"); + } + + private void statementInsideIfFalseBlock_shouldNotTriggerReactivityNorSelfLoop(String rhs) { + final String str = + "import " + Fact.class.getCanonicalName() + ";\n" + + "dialect \"" + dialect.name().toLowerCase() + "\"\n" + + "global java.util.List results;\n" + + "rule R1\n" + + " when\n" + + " $fact : Fact( value1 == 1 )\n" + + " then\n" + + rhs + + "end\n" + + "rule R2\n" + + " when\n" + + " $fact : Fact( value1 == 2 )\n" + + " then\n" + + " results.add(\"R2 fired\");\n" + + "end\n"; + + KieSession ksession = getKieSession(str); + List results = new ArrayList<>(); + ksession.setGlobal("results", results); + + Fact fact = new Fact(1); + ksession.insert(fact); + int fired = ksession.fireAllRules(10); + assertThat(fired).as("Don't cause a loop") + .isEqualTo(1); + + assertThat(results).as("Shouldn't trigger R2, because modify is not executed") + .isEmpty(); + } } diff --git a/drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/MvelCompiler.java b/drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/MvelCompiler.java index 55c72bdb687..9891bca634a 100644 --- a/drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/MvelCompiler.java +++ b/drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/MvelCompiler.java @@ -18,17 +18,20 @@ import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.stream.Stream; import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.expr.MethodCallExpr; +import com.github.javaparser.ast.expr.NameExpr; import com.github.javaparser.ast.stmt.BlockStmt; -import com.github.javaparser.ast.stmt.Statement; import org.drools.mvel.parser.MvelParser; import org.drools.mvel.parser.ast.expr.ModifyStatement; import org.drools.mvelcompiler.ast.TypedExpression; import org.drools.mvelcompiler.context.MvelCompilerContext; +import static com.github.javaparser.ast.NodeList.nodeList; import static java.util.stream.Collectors.toList; public class MvelCompiler { @@ -72,8 +75,15 @@ public CompiledBlockResult compileStatement(String mvelBlock) { .setUsedBindings(allUsedBindings); } - private Stream transformStatementWithPreprocessing(Statement s) { + private Stream transformStatementWithPreprocessing(ModifyStatement s) { + Optional parentNode = s.getParentNode(); PreprocessPhase.PreprocessPhaseResult invoke = preprocessPhase.invoke(s); + parentNode.ifPresent(p -> { + BlockStmt parentBlock = (BlockStmt) p; + for (String modifiedFact : invoke.getUsedBindings()) { + parentBlock.addStatement(new MethodCallExpr(new NameExpr("drools"), "update", nodeList(new NameExpr(modifiedFact)))); + } + }); s.remove(); return invoke.getUsedBindings().stream(); } diff --git a/drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/PreprocessPhase.java b/drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/PreprocessPhase.java index 8cd34e3b340..45ec20bcde3 100644 --- a/drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/PreprocessPhase.java +++ b/drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/PreprocessPhase.java @@ -104,17 +104,22 @@ private PreprocessPhaseResult modifyPreprocessor(ModifyStatement modifyStatement final Expression scope = modifyStatement.getModifyObject(); modifyStatement .findAll(AssignExpr.class) - .replaceAll(assignExpr -> assignToFieldAccess(result, scope, assignExpr)); + .replaceAll(assignExpr -> assignToFieldAccess(scope, assignExpr)); // Do not use findAll as we should only process top level expressions modifyStatement .getExpressions() - .replaceAll(e -> addScopeToMethodCallExpr(result, scope, e)); + .replaceAll(e -> addScopeToMethodCallExpr(scope, e)); NodeList statements = wrapToExpressionStmt(modifyStatement.getExpressions()); // delete modify statement and replace its own block of statements modifyStatement.replace(new BlockStmt(statements)); + // even if no property is modified inside modify block, need to call update because its properties may be modified in RHS + if (scope.isNameExpr() || scope instanceof DrlNameExpr) { + result.addUsedBinding(printNode(scope)); + } + return result; } @@ -128,7 +133,7 @@ private NodeList wrapToExpressionStmt(NodeList expressions .collect(Collectors.toList())); } - private Statement addScopeToMethodCallExpr(PreprocessPhaseResult result, Expression scope, Statement e) { + private Statement addScopeToMethodCallExpr(Expression scope, Statement e) { if (e != null && e.isExpressionStmt()) { Expression expression = e.asExpressionStmt().getExpression(); if (expression.isMethodCallExpr()) { @@ -141,16 +146,10 @@ private Statement addScopeToMethodCallExpr(PreprocessPhaseResult result, Express MethodCallExpr rootMcExpr = rootExpr.asMethodCallExpr(); Expression enclosed = new EnclosedExpr(scope); rootMcExpr.setScope(enclosed); - - if (scope.isNameExpr() || scope instanceof DrlNameExpr) { // some classes such "AtomicInteger" have a setter called "set" - result.addUsedBinding(printNode(scope)); - } - return new ExpressionStmt(mcExpr); } else if (rootExpr instanceof DrlNameExpr) { throwExceptionIfSameDrlName(rootExpr, scope); // Unknown name. Assume a property of the fact - result.addUsedBinding(printNode(scope)); replaceRootExprWithFieldAccess(scope, (DrlNameExpr) rootExpr); } } @@ -191,8 +190,7 @@ private Expression findRootScope(Expression expr) { return scope; } - private AssignExpr assignToFieldAccess(PreprocessPhaseResult result, Expression scope, AssignExpr assignExpr) { - result.addUsedBinding(printNode(scope)); + private AssignExpr assignToFieldAccess(Expression scope, AssignExpr assignExpr) { Expression target = assignExpr.getTarget(); if (target instanceof DrlNameExpr) { // e.g. age = 10 diff --git a/drools-model/drools-mvel-compiler/src/test/java/org/drools/mvelcompiler/MvelCompilerTest.java b/drools-model/drools-mvel-compiler/src/test/java/org/drools/mvelcompiler/MvelCompilerTest.java index 37e2a121458..fa781eaea39 100644 --- a/drools-model/drools-mvel-compiler/src/test/java/org/drools/mvelcompiler/MvelCompilerTest.java +++ b/drools-model/drools-mvel-compiler/src/test/java/org/drools/mvelcompiler/MvelCompilerTest.java @@ -341,7 +341,7 @@ public void testSetterStringWithNull() { public void testSetterBigDecimalConstantModify() { test(ctx -> ctx.addDeclaration("$p", Person.class), "{ modify ( $p ) { salary = 50000 }; }", - "{ { $p.setSalary(new java.math.BigDecimal(50000)); } }", + "{ { $p.setSalary(new java.math.BigDecimal(50000)); } drools.update($p);}", result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); } @@ -349,7 +349,7 @@ public void testSetterBigDecimalConstantModify() { public void testSetterBigDecimalLiteralModify() { test(ctx -> ctx.addDeclaration("$p", Person.class), "{ modify ( $p ) { salary = 50000B }; }", - "{ { $p.setSalary(new java.math.BigDecimal(\"50000\")); } }", + "{ { $p.setSalary(new java.math.BigDecimal(\"50000\")); } drools.update($p);}", result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); } @@ -357,7 +357,7 @@ public void testSetterBigDecimalLiteralModify() { public void testSetterBigDecimalLiteralModifyNegative() { test(ctx -> ctx.addDeclaration("$p", Person.class), "{ modify ( $p ) { salary = -50000B }; }", - "{ { $p.setSalary(new java.math.BigDecimal(\"-50000\")); } }", + "{ { $p.setSalary(new java.math.BigDecimal(\"-50000\")); } drools.update($p);}", result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); } @@ -407,10 +407,64 @@ public void testDoNotConvertAdditionInStringConcatenation() { " list.add(\"before \" + $p + \", money = \" + $p.getSalary()); " + " { $p.setSalary(new java.math.BigDecimal(50000)); }" + " list.add(\"after \" + $p + \", money = \" + $p.getSalary()); " + + " drools.update($p);" + "}\n", result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); } + @Test + public void modifyInsideIfTrueBlock() { + test(ctx -> ctx.addDeclaration("$p", Person.class), + "{" + + " if (true) { " + + " modify ( $p ) { salary = 50000 }; " + + " }" + + "}", + "{\n" + + " if (true) {" + + " { $p.setSalary(new java.math.BigDecimal(50000)); }" + + " drools.update($p);" + + " }" + + "}", + result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); + } + + @Test + public void modifyInsideIfFalseBlock() { + test(ctx -> ctx.addDeclaration("$p", Person.class), + "{" + + " if (false) { " + + " modify ( $p ) { salary = 50000 }; " + + " }" + + "}", + "{\n" + + " if (false) {" + + " { $p.setSalary(new java.math.BigDecimal(50000)); }" + + " drools.update($p);" + + " }" + + "}", + result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); + } + + @Test + public void emptyModifyInsideIfBlockAndSetterOutside() { + test(ctx -> ctx.addDeclaration("$p", Person.class), + "{" + + " $p.salary = 50000;" + + " if (false) { " + + " modify ( $p ) { }; " + + " }" + + "}", + "{\n" + + " $p.setSalary(new java.math.BigDecimal(50000));" + + " if (false) {" + + " { }" + + " drools.update($p);" + + " }" + + "}", + result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); + } + @Test public void testSetterBigIntegerLiteral() { test(ctx -> { @@ -734,7 +788,7 @@ public void testPromotionOfIntToBigDecimalOnField() { public void testModify() { test(ctx -> ctx.addDeclaration("$p", Person.class), "{ modify ( $p ) { name = \"Luca\", age = 35 }; }", - "{\n {\n $p.setName(\"Luca\");\n $p.setAge(35);\n }\n }", + "{\n {\n $p.setName(\"Luca\");\n $p.setAge(35);\n } drools.update($p);\n }", result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); } @@ -745,7 +799,7 @@ public void testModifyMap() { ctx.addDeclaration("$p2", Person.class); }, "{ modify ( $p ) { items = $p2.items }; }", - "{\n {\n $p.setItems($p2.getItems());\n }\n }", + "{\n {\n $p.setItems($p2.getItems());\n } drools.update($p);\n }", result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); } @@ -753,7 +807,7 @@ public void testModifyMap() { public void testModifySemiColon() { test(ctx -> ctx.addDeclaration("$p", Person.class), "{ modify($p) { setAge(1); }; }", - "{ { $p.setAge(1); } }", + "{ { $p.setAge(1); } drools.update($p);}", result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); } @@ -761,7 +815,7 @@ public void testModifySemiColon() { public void testModifyWithAssignment() { test(ctx -> ctx.addDeclaration("$p", Person.class), "{ modify($p) { age = $p.age+1 }; }", - "{ { $p.setAge($p.getAge() + 1); } }", + "{ { $p.setAge($p.getAge() + 1); } drools.update($p); }", result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); } @@ -769,7 +823,7 @@ public void testModifyWithAssignment() { public void testModifyWithMethodCall() { test(ctx -> ctx.addDeclaration("$p", Person.class), "{ modify($p) { addresses.clear() }; }", - "{ { $p.getAddresses().clear(); } }", + "{ { $p.getAddresses().clear(); } drools.update($p);}", result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); } @@ -815,6 +869,7 @@ public void testModifyInsideIfBlock() { " {\n" + " $p.setName(\"without_parent\");\n" + " }\n" + + " drools.update($p);\n" + " } " + "}", result -> assertThat(allUsedBindings(result)).containsExactlyInAnyOrder("$p")); @@ -839,6 +894,7 @@ public void testModifyOrdering() { "{ " + " $person.setAddress($newAddress);\n" + "}\n" + + "drools.update($person);\n" + "}"); } diff --git a/drools-mvel/src/main/java/org/drools/mvel/asm/AsmUtil.java b/drools-mvel/src/main/java/org/drools/mvel/asm/AsmUtil.java index da91702f5ed..db7b4e72f16 100644 --- a/drools-mvel/src/main/java/org/drools/mvel/asm/AsmUtil.java +++ b/drools-mvel/src/main/java/org/drools/mvel/asm/AsmUtil.java @@ -268,7 +268,7 @@ private static void rewriteDescr(final RuleBuildContext context, switch (d.getType()) { case MODIFY: - rewriteModifyDescr(context, d, originalBlock, consequence, declr, obj); + rewriteModifyDescr(context, d, analysis, originalBlock, consequence, declr, obj); break; case UPDATE: rewriteUpdateDescr(context, d, analysis, consequence, declr, obj); @@ -281,6 +281,7 @@ private static void rewriteDescr(final RuleBuildContext context, private static void rewriteModifyDescr( RuleBuildContext context, JavaBlockDescr d, + JavaAnalysisResult analysis, String originalBlock, StringBuilder consequence, Declaration declr, @@ -301,7 +302,8 @@ private static void rewriteModifyDescr( RuleBuildContext context, } BitMask modificationMask = isPropertyReactive ? getEmptyPropertyReactiveMask(settableProperties.size()) : allSetButTraitBitMask(); if (isPropertyReactive) { - modificationMask = getModificationMask( consequence, obj, modificationMask, typeDeclaration, settableProperties, statement, false ); + // collect modification outside modify block + modificationMask = getModificationMask( analysis.getAnalyzedExpr(), obj, modificationMask, typeDeclaration, settableProperties, statement, false ); } int end = originalBlock.indexOf("{"); @@ -327,6 +329,7 @@ private static void rewriteModifyDescr( RuleBuildContext context, start = end + exprStr.length(); if (typeDeclaration != null) { + // collect modification inside modify block modificationMask = parseModifiedProperties(statement, settableProperties, typeDeclaration, isPropertyReactive, modificationMask, exprStr); } } @@ -365,17 +368,17 @@ private static void rewriteUpdateDescr( RuleBuildContext context, context.getRule().getConsequenceMetaData().addStatement(statement); if (isPropertyReactive) { - modificationMask = getModificationMask( consequence, obj, modificationMask, typeDeclaration, settableProperties, statement, true ); + modificationMask = getModificationMask( analysis.getAnalyzedExpr(), obj, modificationMask, typeDeclaration, settableProperties, statement, true ); } } appendUpdateStatement(consequence, declr, obj, modificationMask, typeClass); } - private static BitMask getModificationMask( StringBuilder consequence, String obj, BitMask modificationMask, TypeDeclaration typeDeclaration, List settableProperties, ConsequenceMetaData.Statement statement, boolean isUpdate ) { + private static BitMask getModificationMask( String originalConsequence, String obj, BitMask modificationMask, TypeDeclaration typeDeclaration, List settableProperties, ConsequenceMetaData.Statement statement, boolean isUpdate ) { boolean parsedExprOnce = false; // a late optimization to include this for-loop within this if - for (String expr : splitStatementsAcrossBlocks( consequence )) { + for (String expr : splitStatementsAcrossBlocks( originalConsequence )) { String updateExpr = expr.replaceFirst("^\\Q" + obj + "\\E\\s*\\.", ""); if (!updateExpr.equals(expr)) { parsedExprOnce = true;