enhance 8 rules in RuleGroup.COMMANDS with 'unchain' option (SAP#120)

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/parser/Command.java

@@ -230,6 +230,8 @@ private boolean isLevelCloser(String closingKeyword) {
 
 	public final boolean containsChainColon() { return (chainColonCount > 0); }
 
+	public final boolean containsComma() { return firstToken.matchesOnSiblings(true, TokenSearch.ASTERISK, ","); }
+
 	public final boolean isLateChain() { return containsChainColon() && !isSimpleChain(); }
 
 	public final boolean startsLoop() { return getOpensLevel() && firstCodeTokenIsAnyKeyword(ABAP.loopKeywords); }
@@ -3080,7 +3082,6 @@ public void setErrorStateBeforeCleanup(int errorCount) {
 		this.errorCountBeforeCleanup = errorCount;
 	}
 
-
 	/** Returns true if the Command matches a hard-coded pattern or condition.
 	 * This method can be used during development to search for examples in all sample code files. */
 	public final boolean matchesPattern() {

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rulebase/Rule.java

@@ -521,4 +521,37 @@ else if (configValues.length != otherConfigValues.length)
 	public boolean hasSameConfigurationAs(Rule otherRule, String configName) {
 		return getString(configName).equals(otherRule.getString(configName));
 	}
+
+	/** returns an ArrayList of non-chained Commands; the ArrayList is empty if the supplied Command was chained,  
+	 * but unchaining is not allowed or not possible */
+	protected ArrayList<Command> unchain(Code code, Command command, boolean allowUnchain) throws UnexpectedSyntaxAfterChanges {
+   	ArrayList<Command> unchainedCommands = new ArrayList<>();
+	   if (!command.containsChainColon()) {
+	   	unchainedCommands.add(command);
+	   	return unchainedCommands;
+	   } else if (!allowUnchain) {
+   		return unchainedCommands;
+	   }
+
+	   Command prevCommand = command.getPrev();
+   	Command endCommand = command.getNext();
+
+   	boolean unchained;
+   	if (command.containsComma()) {
+	   	unchained = ((ChainRule)parentProfile.getRule(RuleID.DECLARATION_CHAIN)).executeOn(code, command, false);
+   	} else {
+   		unchained = ((ChainOfOneRule)parentProfile.getRule(RuleID.CHAIN_OF_ONE)).executeOn(code, command, false);
+   	}
+   	if (!unchained) 
+   		return unchainedCommands;
+
+   	// process unchained MOVE commands
+   	Command unchainedCommand = (prevCommand == null) ? code.firstCommand : prevCommand.getNext();
+   	while (unchainedCommand != endCommand) {
+   		if (!unchainedCommand.isCommentLine()) 
+   			unchainedCommands.add(unchainedCommand);
+   		unchainedCommand = unchainedCommand.getNext();
+   	} 
+   	return unchainedCommands;
+	}
 }

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/AddToEtcRule.java

@@ -1,6 +1,7 @@
 package com.sap.adt.abapcleaner.rules.commands;
 
 import java.time.LocalDate;
+import java.util.ArrayList;
 
 import com.sap.adt.abapcleaner.base.ABAP;
 import com.sap.adt.abapcleaner.parser.*;
@@ -47,7 +48,7 @@ public String getExample() {
       return "" 
 			+ LINE_SEP + "  METHOD replace_obsolete_add_to_etc." 
 			+ LINE_SEP + "    ADD 1 TO ls_struc-component." 
-			+ LINE_SEP + "    ADD iv_value  TO lv_length." 
+			+ LINE_SEP + "    ADD iv_value TO lv_length." 
 			+ LINE_SEP 
 			+ LINE_SEP + "    SUBTRACT lo_typedesc->length FROM lv_length." 
 			+ LINE_SEP + "    SUBTRACT is_struc-component FROM lv_length." 
@@ -62,13 +63,23 @@ public String getExample() {
 			+ LINE_SEP + "    DIVIDE lv_value BY lo_struc-component." 
 			+ LINE_SEP + "    DIVIDE lv_value BY class_name( )=>get_tool( )->get_value( iv_param  = 5" 
 			+ LINE_SEP + "                                                              iv_param2 = 'abc' )." 
+			+ LINE_SEP 
+			+ LINE_SEP + "    \" chains can only be processed if they are first unchained" 
+			+ LINE_SEP + "    ADD 10 TO: lv_value, lv_other." 
+			+ LINE_SEP 
+			+ LINE_SEP + "    SUBTRACT: 1 FROM lv_value, 2 FROM lv_other." 
+			+ LINE_SEP 
+			+ LINE_SEP + "    MULTIPLY iv_value BY: 2, 3, 5." 
+			+ LINE_SEP 
+			+ LINE_SEP + "    DIVIDE iv_value: BY lv_any, BY lv_other." 
 			+ LINE_SEP + "  ENDMETHOD.";
    }
 
 	final ConfigEnumValue<AddToReplacementStyleForOldRelease> configReplacementStyleForOldRelease = new ConfigEnumValue<AddToReplacementStyleForOldRelease>(this, "AddToReplacementStyle", "If cleanup is restricted to NetWeaver < 7.54 syntax,",
 			new String[] { "keep obsolete statements", "replace with 'a = a + ...' etc." }, AddToReplacementStyleForOldRelease.REPLACE_WITHOUT_ASSIGNMENT_OP, AddToReplacementStyleForOldRelease.KEEP, LocalDate.of(2022, 8, 26));
+	final ConfigBoolValue configProcessChains = new ConfigBoolValue(this, "ProcessChains", "Unchain ADD:, SUBTRACT: etc. chains (required for processing them with this rule)", true, false, LocalDate.of(2023, 10, 27));
 
-	private final ConfigValue[] configValues = new ConfigValue[] { configReplacementStyleForOldRelease };
+	private final ConfigValue[] configValues = new ConfigValue[] { configReplacementStyleForOldRelease, configProcessChains };
 
 	@Override
 	public ConfigValue[] getConfigValues() { return configValues; }
@@ -84,19 +95,37 @@ public AddToEtcRule(Profile profile) {
 
 	@Override
 	protected boolean executeOn(Code code, Command command, int releaseRestriction) throws UnexpectedSyntaxBeforeChanges, UnexpectedSyntaxAfterChanges {
-		if (command.containsChainColon())
+		Token firstToken = command.getFirstCodeToken();
+		if (firstToken == null)
 			return false;
 
+		if (	!firstToken.matchesOnSiblings(true, "ADD", TokenSearch.ASTERISK, "TO")
+			&& !firstToken.matchesOnSiblings(true, "SUBTRACT", TokenSearch.ASTERISK, "FROM")
+			&& !firstToken.matchesOnSiblings(true, "MULTIPLY", TokenSearch.ASTERISK, "BY")
+			&& !firstToken.matchesOnSiblings(true, "DIVIDE", TokenSearch.ASTERISK, "BY")) {
+			return false;
+		}
+
 		// determine whether introducing calculation assignment operators is allowed by the release restrictions;
 		// if not, determine whether the obsolete statement should be kept or replaced with 'a = a + ...' etc.
 		boolean areCalcAssignOpsAllowed = isCleanupAllowedFor(ABAP.REQUIRED_RELEASE_754, code, releaseRestriction);
 		if (!areCalcAssignOpsAllowed && getReplacementStyleForOldRelease() == AddToReplacementStyleForOldRelease.KEEP)
 			return false;
 
-		return replaceAddOrSubtract(code, command, "ADD", "TO", "+", areCalcAssignOpsAllowed) 
-		 	 || replaceAddOrSubtract(code, command, "SUBTRACT", "FROM", "-", areCalcAssignOpsAllowed)
-			 || replaceMultiplyOrDivide(code, command, "MULTIPLY", "BY", "*", areCalcAssignOpsAllowed) 
-			 || replaceMultiplyOrDivide(code, command, "DIVIDE", "BY", "/", areCalcAssignOpsAllowed);
+		ArrayList<Command> unchainedCommands = unchain(code, command, configProcessChains.getValue());
+		if (unchainedCommands == null || unchainedCommands.isEmpty())
+			return false;
+
+		for (Command unchainedCommand : unchainedCommands) {
+			if (	replaceAddOrSubtract(code, unchainedCommand, "ADD", "TO", "+", areCalcAssignOpsAllowed) 
+				|| replaceAddOrSubtract(code, unchainedCommand, "SUBTRACT", "FROM", "-", areCalcAssignOpsAllowed)
+				|| replaceMultiplyOrDivide(code, unchainedCommand, "MULTIPLY", "BY", "*", areCalcAssignOpsAllowed) 
+				|| replaceMultiplyOrDivide(code, unchainedCommand, "DIVIDE", "BY", "/", areCalcAssignOpsAllowed)) {
+
+				code.addRuleUse(this, unchainedCommand);
+			}
+   	}
+		return false; // addRuleUse() was already called above
 	}
 
 	private boolean replaceAddOrSubtract(Code code, Command command, String keyword1Text, String keyword2Text, String operator, boolean areCalcAssignOpsAllowed) throws UnexpectedSyntaxBeforeChanges, UnexpectedSyntaxAfterChanges {

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/AssertClassRule.java

@@ -1,6 +1,7 @@
 package com.sap.adt.abapcleaner.rules.commands;
 
 import java.time.LocalDate;
+import java.util.ArrayList;
 
 import com.sap.adt.abapcleaner.base.*;
 import com.sap.adt.abapcleaner.parser.*;
@@ -59,7 +60,7 @@ public String getExample() {
 			+ LINE_SEP + "    ASSERT is_parameters-component_name IS INITIAL." 
 			+ LINE_SEP + "    ASSERT io_any_instance IS NOT INITIAL." 
 			+ LINE_SEP 
-			+ LINE_SEP + "    ASSERT sy-subrc = 0. \" defitem must exist" 
+			+ LINE_SEP + "    ASSERT sy-subrc = 0. \" item must exist" 
 			+ LINE_SEP + "    ASSERT sy-subrc = 4." 
 			+ LINE_SEP + "    ASSERT sy-subrc = get_expected_subrc_value( param_a = 'abc' " 
 			+ LINE_SEP + "                                                param_b = 'def' )." 
@@ -83,6 +84,11 @@ public String getExample() {
 			+ LINE_SEP + "    ASSERT <ls_row>-item_key <= ms_parameters-last_item_key." 
 			+ LINE_SEP + "    ASSERT lv_quantity <= 100." 
 			+ LINE_SEP + "    ASSERT abs( <ls_any_field_symbol>-sum_quantity ) > 0." 
+			+ LINE_SEP 
+			+ LINE_SEP + "    \" chains can only be processed if they are first unchained" 
+			+ LINE_SEP + "    ASSERT: sy-subrc = 0," 
+			+ LINE_SEP + "            io_instance IS BOUND," 
+			+ LINE_SEP + "            iv_is_valid = abap_false." 
 			+ LINE_SEP + "  ENDMETHOD."
 			+ LINE_SEP + "ENDCLASS."
 			+ LINE_SEP 
@@ -127,8 +133,9 @@ public String getExample() {
 	}
 
 	final ConfigTextValue configAssertClassName = new ConfigTextValue(this, "AssertClassName", "Assert class name:", "cx_assert", ConfigTextType.ABAP_CLASS);
+	final ConfigBoolValue configProcessChains = new ConfigBoolValue(this, "ProcessChains", "Unchain ASSERT: chains (required for processing them with this rule)", true, false, LocalDate.of(2023, 10, 27));
 
-	private final ConfigValue[] configValues = new ConfigValue[] { configAssertClassName };
+	private final ConfigValue[] configValues = new ConfigValue[] { configAssertClassName, configProcessChains };
 
 	@Override
 	public ConfigValue[] getConfigValues() { return configValues; }
@@ -144,14 +151,28 @@ private String getAssertClassCall(String methodName) {
 
 	@Override
 	protected boolean executeOn(Code code, Command command, int releaseRestriction) throws UnexpectedSyntaxBeforeChanges, UnexpectedSyntaxAfterChanges {
-		if (command.containsChainColon())
-			return false;
 		if (!command.firstCodeTokenIsKeyword("ASSERT"))
 			return false;
 
+		ArrayList<Command> unchainedCommands = unchain(code, command, configProcessChains.getValue());
+		if (unchainedCommands == null || unchainedCommands.isEmpty())
+			return false;
+
+		for (Command unchainedCommand : unchainedCommands) {
+			if (executeOnUnchained(code, unchainedCommand)) {
+				code.addRuleUse(this, unchainedCommand);
+			}
+		}
+		return false; // addRuleUse() was already called above
+	}
+
+
+	private boolean executeOnUnchained(Code code, Command command) throws UnexpectedSyntaxBeforeChanges, UnexpectedSyntaxAfterChanges {
 		// exclude cases with optional additions ID or FIELDS:
 		// ASSERT [ [ID group [SUBKEY sub]] [FIELDS val1 val2 ...] CONDITION ] log_exp. 
 		Token assertToken = command.getFirstCodeToken();
+		if (assertToken == null || !assertToken.isKeyword("ASSERT"))
+			return false;
 		Token next = assertToken.getNextCodeSibling(); 
 		if (next.isAnyKeyword("ID", "FIELDS"))
 			return false;

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CallMethodRule.java

@@ -1,6 +1,7 @@
 package com.sap.adt.abapcleaner.rules.commands;
 
 import java.time.LocalDate;
+import java.util.ArrayList;
 
 import com.sap.adt.abapcleaner.parser.*;
 import com.sap.adt.abapcleaner.programbase.UnexpectedSyntaxAfterChanges;
@@ -45,7 +46,7 @@ public class CallMethodRule extends RuleForCommands {
 	public RuleReference[] getReferences() { return references; }
 
 	@Override
-	public RuleID[] getDependentRules() { return new RuleID[] { RuleID.ALIGN_PARAMETERS } ; }
+	public RuleID[] getDependentRules() { return new RuleID[] { RuleID.UPPER_AND_LOWER_CASE, RuleID.ALIGN_PARAMETERS } ; }
 
 	@Override
 	public boolean isEssential() { return true; }
@@ -76,19 +77,47 @@ public String getExample() {
 			+ LINE_SEP 
 			+ LINE_SEP + "    CALL METHOD (iv_dynamic_class_name)=>(iv_dynamic_method_name)" 
 			+ LINE_SEP + "      EXPORTING iv_par = iv_par. " 
+			+ LINE_SEP 
+			+ LINE_SEP + "    \" chains can only be processed if they are first unchained" 
+			+ LINE_SEP + "    CALL METHOD: any_method," 
+			+ LINE_SEP + "                 other_method EXPORTING iv_value = iv_value," 
+			+ LINE_SEP + "                 mo_any_instance->(iv_dynamic_method_name)."
+			+ LINE_SEP 
+			+ LINE_SEP + "    CALL METHOD third_method EXPORTING iv_name =: 'abc', 'def', 'ghi'." 
 			+ LINE_SEP + "  ENDMETHOD.";
    }
 
+	final ConfigBoolValue configProcessChains = new ConfigBoolValue(this, "ProcessChains", "Unchain CALL METHOD: chains (required for processing them with this rule)", true, false, LocalDate.of(2023, 10, 27));
+
+   private final ConfigValue[] configValues = new ConfigValue[] { configProcessChains };
+
+	@Override
+	public ConfigValue[] getConfigValues() { return configValues; }
+
 	public CallMethodRule(Profile profile) {
 		super(profile);
 		initializeConfiguration();
 	}
 
 	@Override
 	protected boolean executeOn(Code code, Command command, int releaseRestriction) throws UnexpectedSyntaxAfterChanges {
-		if (command.containsChainColon())
+		Token firstToken = command.getFirstToken();
+		if (!firstToken.matchesOnSiblings(false, "CALL", "METHOD", TokenSearch.makeOptional(":"), TokenSearch.ANY_IDENTIFIER))
+			return false;
+
+		ArrayList<Command> unchainedCommands = unchain(code, command, configProcessChains.getValue());
+		if (unchainedCommands == null || unchainedCommands.isEmpty())
 			return false;
 
+		for (Command unchainedCommand : unchainedCommands) {
+			if (executeOnUnchained(code, unchainedCommand, releaseRestriction)) {
+				code.addRuleUse(this, unchainedCommand);
+			}
+		}
+		return false; // addRuleUse() was already called above
+	}
+
+	private boolean executeOnUnchained(Code code, Command command, int releaseRestriction) throws UnexpectedSyntaxAfterChanges {
 		Token firstToken = command.getFirstToken();
 		if (!firstToken.matchesOnSiblings(false, "CALL", "METHOD", TokenSearch.ANY_IDENTIFIER))
 			return false;

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckInLoopRule.java

@@ -54,6 +54,10 @@ public String getExample() {
 			+ LINE_SEP + "        CHECK ls_row-min_id <> 0." 
 			+ LINE_SEP + "        CHECK ls_row-processed = abap_false." 
 			+ LINE_SEP 
+			+ LINE_SEP + "        \" chains can only be processed if they are first unchained" 
+			+ LINE_SEP + "        CHECK: ls_row-max_id > 0," 
+			+ LINE_SEP + "               ls_row-flag IS NOT INITIAL." 
+			+ LINE_SEP 
 			+ LINE_SEP + "        WHILE lv_id < ls_row-max_id." 
 			+ LINE_SEP + "          lv_id += 1." 
 			+ LINE_SEP 
@@ -73,8 +77,9 @@ public String getExample() {
    }
 
    final ConfigEnumValue<KeepCheckInLoopCondition> configKeepCondition = new ConfigEnumValue<KeepCheckInLoopCondition>(this, "KeepCondition", "Keep CHECK statement in LOOP:", new String[] { "never", "at loop start" }, KeepCheckInLoopCondition.NEVER);
+	final ConfigBoolValue configProcessChains = new ConfigBoolValue(this, "ProcessChains", "Unchain CHECK: chains in loops (required for processing them with this rule)", true, false, LocalDate.of(2023, 10, 27));
 
-	private final ConfigValue[] configValues = new ConfigValue[] { configKeepCondition, configNegationStyle, configConvertAbapFalseAndAbapTrue };
+	private final ConfigValue[] configValues = new ConfigValue[] { configKeepCondition, configNegationStyle, configConvertAbapFalseAndAbapTrue, configProcessChains };
 
 	@Override
 	public ConfigValue[] getConfigValues() { return configValues; }
@@ -93,6 +98,7 @@ public void executeOn(Code code, int releaseRestriction) throws UnexpectedSyntax
 		KeepCheckInLoopCondition convertUpTo = KeepCheckInLoopCondition.NEVER; // tells which condition(s) would still be satisfied at the position of the current Command
 		NegationStyle negationStyle = NegationStyle.forValue(configNegationStyle.getValue());
 		boolean convertAbapFalseAndAbapTrue = configConvertAbapFalseAndAbapTrue.getValue();
+		boolean processChains = configProcessChains.getValue();
 
 		Command command = code.firstCommand;
 		int loopLevel = 0;
@@ -112,9 +118,9 @@ public void executeOn(Code code, int releaseRestriction) throws UnexpectedSyntax
 				convertUpTo = KeepCheckInLoopCondition.KEEP_AT_LOOP_START;
 			}
 
-			if (loopLevel > 0 && !isCommandBlocked(command) && firstToken.isKeyword("CHECK") && !command.containsChainColon()
+			if (loopLevel > 0 && !isCommandBlocked(command) && firstToken.isKeyword("CHECK")
 					&& keepCondition.getValue() <= convertUpTo.getValue()) {
-				executeOn(code, command, true, negationStyle, convertAbapFalseAndAbapTrue, releaseRestriction);
+				executeOn(code, command, true, processChains, negationStyle, convertAbapFalseAndAbapTrue, releaseRestriction);
 			}
 
 			command = command.getNext();

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckOutsideLoopRule.java

@@ -65,6 +65,10 @@ public String getExample() {
 			+ LINE_SEP + "    \" CHECKs only preceded by declarations and CLEAR" 
 			+ LINE_SEP + "    CHECK a = abap_false AND b > 3 OR a = abap_true AND b <= 10." 
 			+ LINE_SEP 
+			+ LINE_SEP + "    \" chains can only be processed if they are first unchained" 
+			+ LINE_SEP + "    CHECK: a IS NOT INITIAL," 
+			+ LINE_SEP + "           b < 5." 
+			+ LINE_SEP 
 			+ LINE_SEP + "    lv_value = 1." 
 			+ LINE_SEP 
 			+ LINE_SEP + "    \" CHECKs inside the method" 
@@ -90,8 +94,9 @@ public String getExample() {
 
    final ConfigEnumValue<KeepCheckOutsideLoopCondition> configKeepCondition = new ConfigEnumValue<KeepCheckOutsideLoopCondition>(this, "KeepCondition", "Keep CHECK statement:", new String[] { "never", "at method start", "after declarations", "after declarations and CLEAR statements" }, KeepCheckOutsideLoopCondition.KEEP_AFTER_DECLARATIONS);
 	final ConfigBoolValue configAllowCheckAfterCheckpoints = new ConfigBoolValue(this, "AllowCheckAfterCheckpoints", "Allow CHECK after ASSERT, BREAK-POINT and LOG-POINT", true, false, LocalDate.of(2023, 10, 10));
-
-	private final ConfigValue[] configValues = new ConfigValue[] { configKeepCondition, configNegationStyle, configConvertAbapFalseAndAbapTrue, configAllowCheckAfterCheckpoints };
+	final ConfigBoolValue configProcessChains = new ConfigBoolValue(this, "ProcessChains", "Unchain CHECK: chains outside loops (required for processing them with this rule)", true, false, LocalDate.of(2023, 10, 27));
+
+	private final ConfigValue[] configValues = new ConfigValue[] { configKeepCondition, configNegationStyle, configConvertAbapFalseAndAbapTrue, configAllowCheckAfterCheckpoints, configProcessChains };
 
 	@Override
 	public ConfigValue[] getConfigValues() { return configValues; }
@@ -111,6 +116,7 @@ public void executeOn(Code code, int releaseRestriction) throws UnexpectedSyntax
 		NegationStyle negationStyle = NegationStyle.forValue(configNegationStyle.getValue());
 		boolean convertAbapFalseAndAbapTrue = configConvertAbapFalseAndAbapTrue.getValue();
 		boolean allowCheckAfterCheckpoints = configAllowCheckAfterCheckpoints.getValue();
+		boolean processChains = configProcessChains.getValue();
 		boolean isInsideMethod = false; 
 
 		Command command = code.firstCommand;
@@ -135,8 +141,8 @@ public void executeOn(Code code, int releaseRestriction) throws UnexpectedSyntax
 			}
 
 			// only change the statement if we are SURE to be inside a method, because if only a code snippet from a LOOP body is processed, CHECK may be erroneously converted to IF ... RETURN! 
-			if (isInsideMethod && !isCommandBlocked(command) && command.firstCodeTokenIsKeyword("CHECK") && !command.containsChainColon() && keepCondition.getValue() <= convertUpTo.getValue())
-				executeOn(code, command, false, negationStyle, convertAbapFalseAndAbapTrue, releaseRestriction);
+			if (isInsideMethod && !isCommandBlocked(command) && command.firstCodeTokenIsKeyword("CHECK") && keepCondition.getValue() <= convertUpTo.getValue())
+				executeOn(code, command, false, processChains, negationStyle, convertAbapFalseAndAbapTrue, releaseRestriction);
 
 			if (command.getOpensLevel() && command.firstCodeTokenIsAnyKeyword(ABAP.loopKeywords))
 				command = command.getNextSibling();