diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/parser/Command.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/parser/Command.java index 62f52dab..e602d919 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/parser/Command.java +++ b/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() { diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rulebase/Rule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rulebase/Rule.java index e71e1b49..9fc924f7 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rulebase/Rule.java +++ b/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 unchain(Code code, Command command, boolean allowUnchain) throws UnexpectedSyntaxAfterChanges { + ArrayList 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; + } } \ No newline at end of file diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/AddToEtcRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/AddToEtcRule.java index d2efcca7..1751f2dd 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/AddToEtcRule.java +++ b/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 configReplacementStyleForOldRelease = new ConfigEnumValue(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 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 { diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/AssertClassRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/AssertClassRule.java index 24467378..69421afa 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/AssertClassRule.java +++ b/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 -item_key <= ms_parameters-last_item_key." + LINE_SEP + " ASSERT lv_quantity <= 100." + LINE_SEP + " ASSERT abs( -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 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; diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CallMethodRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CallMethodRule.java index cdff136a..0de9268a 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CallMethodRule.java +++ b/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,9 +77,23 @@ 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(); @@ -86,9 +101,23 @@ public CallMethodRule(Profile profile) { @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 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; diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckInLoopRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckInLoopRule.java index 6acb249b..307a3c26 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckInLoopRule.java +++ b/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 configKeepCondition = new ConfigEnumValue(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(); diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckOutsideLoopRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckOutsideLoopRule.java index f21668a0..d9bda877 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckOutsideLoopRule.java +++ b/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 configKeepCondition = new ConfigEnumValue(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(); diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckStatementRuleBase.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckStatementRuleBase.java index af19c3b5..f6523552 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckStatementRuleBase.java +++ b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CheckStatementRuleBase.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.*; @@ -21,7 +22,20 @@ public CheckStatementRuleBase(Profile profile) { * transforms "CHECK {logical expression}." into "IF {negated logical expression}. CONTINUE/RETURN. ENDIF." * @throws IntegrityBrokenException */ - protected final boolean executeOn(Code code, Command command, boolean isInLoop, NegationStyle negationStyle, boolean convertAbapFalseAndAbapTrue, int releaseRestriction) throws UnexpectedSyntaxBeforeChanges, UnexpectedSyntaxAfterChanges { + protected final boolean executeOn(Code code, Command command, boolean isInLoop, boolean processChains, NegationStyle negationStyle, boolean convertAbapFalseAndAbapTrue, int releaseRestriction) throws UnexpectedSyntaxBeforeChanges, UnexpectedSyntaxAfterChanges { + ArrayList unchainedCommands = unchain(code, command, processChains); + if (unchainedCommands == null || unchainedCommands.isEmpty()) + return false; + + for (Command unchainedCommand : unchainedCommands) { + if (executeOnUnchained(code, unchainedCommand, isInLoop, negationStyle, convertAbapFalseAndAbapTrue, releaseRestriction)) { + code.addRuleUse(this, unchainedCommand); + } + } + return false; // addRuleUse() was already called above + } + + private final boolean executeOnUnchained(Code code, Command command, boolean isInLoop, NegationStyle negationStyle, boolean convertAbapFalseAndAbapTrue, int releaseRestriction) throws UnexpectedSyntaxBeforeChanges, UnexpectedSyntaxAfterChanges { Token firstToken = command.getFirstToken(); Token period = command.getLastNonCommentToken(); int indent = firstToken.getStartIndexInLine(); @@ -68,7 +82,6 @@ protected final boolean executeOn(Code code, Command command, boolean isInLoop, (new UnexpectedSyntaxAfterChanges(this, null)).addToLog(); return false; } - code.addRuleUse(this, command); return true; } } diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CreateObjectRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CreateObjectRule.java index 97cd52f2..477ebe93 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CreateObjectRule.java +++ b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/CreateObjectRule.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.*; @@ -70,9 +71,24 @@ public String getExample() { + LINE_SEP + " EXCEPTIONS" + LINE_SEP + " cx_message = 1" + LINE_SEP + " others = 2." + + LINE_SEP + + LINE_SEP + " \" chains can only be processed if they are first unchained" + + LINE_SEP + " CREATE OBJECT: lx_message, lx_other_message." + + LINE_SEP + + LINE_SEP + " CREATE OBJECT: lo_any TYPE cl_any_class," + + LINE_SEP + " lo_other TYPE (lv_class_name)," + + LINE_SEP + " lo_third TYPE cl_othird_class" + + LINE_SEP + " EXPORTING io_contract = me." + LINE_SEP + " ENDMETHOD."; } + final ConfigBoolValue configProcessChains = new ConfigBoolValue(this, "ProcessChains", "Unchain CREATE OBJECT: 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 CreateObjectRule(Profile profile) { super(profile); initializeConfiguration(); @@ -81,11 +97,26 @@ public CreateObjectRule(Profile profile) { @Override protected boolean executeOn(Code code, Command command, int releaseRestriction) throws UnexpectedSyntaxAfterChanges { Token firstToken = command.getFirstToken(); - if (!firstToken.matchesOnSiblings(false, "CREATE", "OBJECT", TokenSearch.ANY_IDENTIFIER)) + if (!firstToken.matchesOnSiblings(false, "CREATE", "OBJECT", TokenSearch.makeOptional(":"), TokenSearch.ANY_IDENTIFIER)) + return false; + + ArrayList unchainedCommands = unchain(code, command, configProcessChains.getValue()); + if (unchainedCommands == null || unchainedCommands.isEmpty()) return false; - else if (command.isLateChain()) + + 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, "CREATE", "OBJECT", TokenSearch.ANY_IDENTIFIER)) return false; - + Token identifier = firstToken.getNext().getNext(); Token next = identifier.getNextCodeSibling(); @@ -130,7 +161,6 @@ else if (command.isLateChain()) ((ExportingKeywordRule) parentProfile.getRule(RuleID.EXPORTING_KEYWORD)).executeOn(code, command, typeInfo, false, releaseRestriction); command.invalidateMemoryAccessType(); - return true; } } diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/MoveToRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/MoveToRule.java index f091bd5c..a9a7578d 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/MoveToRule.java +++ b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/MoveToRule.java @@ -1,12 +1,11 @@ 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.*; import com.sap.adt.abapcleaner.rulebase.*; -import com.sap.adt.abapcleaner.rules.declarations.ChainOfOneRule; -import com.sap.adt.abapcleaner.rules.declarations.ChainRule; public class MoveToRule extends RuleForCommands { private final static RuleReference[] references = new RuleReference[] { @@ -62,10 +61,14 @@ public String getExample() { + LINE_SEP + " \" another comment" + LINE_SEP + " EXACT iv_data TO ev_data," + LINE_SEP + " io_instance ?TO eo_instance." + + LINE_SEP + "" + + LINE_SEP + " \" with the MOVE chain, get_next_value( ) is also called 3 times," + + LINE_SEP + " \" but without the chain, that's much clearer:" + + LINE_SEP + " MOVE get_next_value( ) TO: ev_any, ev_other, ev_third." + LINE_SEP + " ENDMETHOD."; } - final ConfigBoolValue configProcessChains = new ConfigBoolValue(this, "ProcessChains", "Process MOVE: chains", true, false, LocalDate.of(2023, 6, 5)); + final ConfigBoolValue configProcessChains = new ConfigBoolValue(this, "ProcessChains", "Unchain MOVE: chains (required for processing them with this rule)", true, false, LocalDate.of(2023, 10, 27)); private final ConfigValue[] configValues = new ConfigBoolValue[] { configProcessChains }; @@ -82,68 +85,16 @@ protected boolean executeOn(Code code, Command command, int releaseRestriction) if (!command.firstCodeTokenIsKeyword("MOVE")) return false; - if (!command.containsChainColon()) { - return executeOnNonChain(code, command); - - } else if (configProcessChains.getValue() && command.containsChainColon()) { - // check whether all parts of the chain match the expected pattern - boolean isChainOfOne = true; - if (command.isSimpleChain()) { - // expect "MOVE: [EXACT] TO , [EXACT] TO , ..." - Token checkToken = command.getFirstToken().getNextCodeSibling().getNextCodeSibling(); - while (checkToken != null) { - Token lastToken = checkToken.getLastTokenOnSiblings(true, TokenSearch.makeOptional("EXACT"), TokenSearch.ANY_TERM, "TO|?TO", TokenSearch.ANY_IDENTIFIER, ".|,"); - if (lastToken == null || !lastToken.isCommaOrPeriod()) - return false; - if (lastToken.isComma()) - isChainOfOne = false; - checkToken = lastToken.getNextCodeSibling(); - } - } else { - // expect "MOVE [EXACT] TO: , , ...." - Token chainColon = command.getFirstToken().getLastTokenOnSiblings(true, "MOVE", TokenSearch.makeOptional("EXACT"), TokenSearch.ANY_TERM, "TO|?TO", ":"); - if (chainColon == null) - return false; - Token checkToken = chainColon; - while (checkToken != null) { - checkToken = checkToken.getNextCodeSibling(); - if (checkToken == null) - break; - if (!checkToken.isIdentifier()) - return false; - checkToken = checkToken.getNextCodeSibling(); - if (checkToken == null || !checkToken.isCommaOrPeriod()) - return false; - if (checkToken.isComma()) - isChainOfOne = false; - } - } - - // unchain MOVE: into multiple commands - Command prevCommand = command.getPrev(); - Command endCommand = command.getNext(); - boolean unchained; - if (isChainOfOne) { - unchained = ((ChainOfOneRule)parentProfile.getRule(RuleID.CHAIN_OF_ONE)).executeOn(code, command, false); - } else { - unchained = ((ChainRule)parentProfile.getRule(RuleID.DECLARATION_CHAIN)).executeOn(code, command, false); - } - if (!unchained) - return false; - - // process unchained MOVE commands - Command changeCommand = (prevCommand == null) ? code.firstCommand : prevCommand.getNext(); - while (changeCommand != endCommand) { - if (changeCommand.isCommentLine() || executeOnNonChain(code, changeCommand)) { - code.addRuleUse(this, changeCommand); - } - changeCommand = changeCommand.getNext(); - } - return true; - - } else { - return false; - } + ArrayList unchainedCommands = unchain(code, command, configProcessChains.getValue()); + if (unchainedCommands == null || unchainedCommands.isEmpty()) + return false; + + for (Command unchainedCommand : unchainedCommands) { + if (executeOnNonChain(code, unchainedCommand)) { + code.addRuleUse(this, unchainedCommand); + } + } + return false; // addRuleUse() was already called above } private boolean executeOnNonChain(Code code, Command command) throws UnexpectedSyntaxBeforeChanges, UnexpectedSyntaxAfterChanges { @@ -194,7 +145,6 @@ private boolean executeOnNonChain(Code code, Command command) throws UnexpectedS term.addIndent(term.firstToken.getStartIndexInLine() - termPos); command.invalidateMemoryAccessType(); - return true; } } diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/TranslateRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/TranslateRule.java index c2a006a9..1e49aaf6 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/TranslateRule.java +++ b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/commands/TranslateRule.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.Code; @@ -84,14 +85,20 @@ public String getExample() { + LINE_SEP + " \" translate( ) must have FROM = `c`, since FROM = `c+` TO = `C` would remove the `+`" + LINE_SEP + " DATA lv_c_plus_plus TYPE string VALUE `c++`." + LINE_SEP + " TRANSLATE lv_c_plus_plus USING 'cC+'." + + LINE_SEP + + LINE_SEP + " \" chains can only be processed if they are first unchained" + + LINE_SEP + " TRANSLATE: lv_text TO LOWER CASE, lv_other_text TO UPPER CASE." + + LINE_SEP + + LINE_SEP + " TRANSLATE lv_abc USING: '1 2 3 ', 'a-b-c-'." + LINE_SEP + " ENDMETHOD."; } final ConfigBoolValue configReplaceTranslateToUpperLower = new ConfigBoolValue(this, "ReplaceTranslateToUpperLower", "Replace TRANSLATE ... TO UPPER|LOWER", true); final ConfigBoolValue configReplaceTranslateUsing = new ConfigBoolValue(this, "ReplaceTranslateUsing", "Replace TRANSLATE ... USING", true); final ConfigBoolValue configReplaceUnevenMasks = new ConfigBoolValue(this, "ReplaceUnevenMasks", "Replace TRANSLATE ... USING if mask has an uneven number of chars", true); + final ConfigBoolValue configProcessChains = new ConfigBoolValue(this, "ProcessChains", "Unchain TRANSLATE: chains (required for processing them with this rule)", true, false, LocalDate.of(2023, 10, 27)); - private final ConfigValue[] configValues = new ConfigValue[] { configReplaceTranslateToUpperLower, configReplaceTranslateUsing, configReplaceUnevenMasks }; + private final ConfigValue[] configValues = new ConfigValue[] { configReplaceTranslateToUpperLower, configReplaceTranslateUsing, configReplaceUnevenMasks, configProcessChains }; @Override public ConfigValue[] getConfigValues() { return configValues; } @@ -103,30 +110,38 @@ public TranslateRule(Profile profile) { @Override protected boolean executeOn(Code code, Command command, int releaseRestriction) throws UnexpectedSyntaxBeforeChanges, UnexpectedSyntaxAfterChanges { - if (command.containsChainColon()) - return false; - // for the syntax of the deprecated TRANSLATE statement, see https://help.sap.com/doc/abapdocu_latest_index_htm/latest/en-US/index.htm?file=abaptranslate.htm Token firstToken = command.getFirstToken(); - if (firstToken == null || !firstToken.isKeyword("TRANSLATE")) + if (firstToken == null) return false; - if (firstToken.matchesOnSiblings(false, "TRANSLATE", TokenSearch.ANY_IDENTIFIER, "TO", "UPPER|LOWER", "CASE")) { - if (configReplaceTranslateToUpperLower.getValue() && replaceTranslateToUpperOrLower(firstToken)) { - command.invalidateMemoryAccessType(); - return true; - } - } else if (firstToken.matchesOnSiblings(false, "TRANSLATE", TokenSearch.ANY_IDENTIFIER, "USING", TokenSearch.ANY_LITERAL)) { - if (configReplaceTranslateUsing.getValue() && replaceTranslateUsing(firstToken)){ - command.invalidateMemoryAccessType(); - return true; + if ( !firstToken.matchesOnSiblings(true, "TRANSLATE", TokenSearch.ASTERISK, "TO", "UPPER|LOWER", "CASE") + && !firstToken.matchesOnSiblings(true, "TRANSLATE", TokenSearch.ASTERISK, "USING")) { + return false; + } + + ArrayList unchainedCommands = unchain(code, command, configProcessChains.getValue()); + if (unchainedCommands == null || unchainedCommands.isEmpty()) + return false; + + for (Command unchainedCommand : unchainedCommands) { + firstToken = unchainedCommand.getFirstCodeToken(); + if (firstToken == null) { + // skip Command + } else if (firstToken.matchesOnSiblings(false, "TRANSLATE", TokenSearch.ANY_IDENTIFIER, "TO", "UPPER|LOWER", "CASE")) { + if (configReplaceTranslateToUpperLower.getValue() && replaceTranslateToUpperOrLower(unchainedCommand, firstToken)) { + code.addRuleUse(this, unchainedCommand); + } + } else if (firstToken.matchesOnSiblings(false, "TRANSLATE", TokenSearch.ANY_IDENTIFIER, "USING", TokenSearch.ANY_LITERAL)) { + if (configReplaceTranslateUsing.getValue() && replaceTranslateUsing(unchainedCommand, firstToken)){ + code.addRuleUse(this, unchainedCommand); + } } } - - return false; + return false; // addRuleUse() was already called above } - private boolean replaceTranslateToUpperOrLower(Token firstToken) throws UnexpectedSyntaxAfterChanges, IntegrityBrokenException { + private boolean replaceTranslateToUpperOrLower(Command command, Token firstToken) throws UnexpectedSyntaxAfterChanges, IntegrityBrokenException { Token identifier1 = firstToken.getNextCodeSibling(); Token toToken = identifier1.getNextCodeSibling(); Token upperLowerToken = toToken.getNextCodeSibling(); @@ -153,10 +168,12 @@ private boolean replaceTranslateToUpperOrLower(Token firstToken) throws Unexpect toToken.removeFromCommand(); upperLowerToken.removeFromCommand(); caseToken.removeFromCommand(); + + command.invalidateMemoryAccessType(); return true; } - private boolean replaceTranslateUsing(Token firstToken) throws UnexpectedSyntaxAfterChanges, IntegrityBrokenException { + private boolean replaceTranslateUsing(Command command, Token firstToken) throws UnexpectedSyntaxAfterChanges, IntegrityBrokenException { Token identifier1 = firstToken.getNextCodeSibling(); Token usingToken = identifier1.getNextCodeSibling(); Token maskToken = usingToken.getNextCodeSibling(); @@ -217,6 +234,8 @@ else if (maskToken.getNextCodeSibling() == null || !maskToken.getNextCodeSibling usingToken.removeFromCommand(); maskToken.removeFromCommand(); + + command.invalidateMemoryAccessType(); return true; } } diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/declarations/ChainRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/declarations/ChainRule.java index cfb880ab..322cf07e 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/declarations/ChainRule.java +++ b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/declarations/ChainRule.java @@ -291,6 +291,9 @@ public boolean executeOn(Code code, Command command, boolean addRuleUse) throws if (addRuleUse) { code.addRuleUse(this, newCommand); } + // now reduce number of line breaks to 1 (otherwise, comment lines that will now have to be split out + // would have too many lineBreaks) + firstTokenOfPartA.lineBreaks = 1; // continue with loop } else { diff --git a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rulebase/RuleTest.java b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rulebase/RuleTest.java index f3766f21..bc97caff 100644 --- a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rulebase/RuleTest.java +++ b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rulebase/RuleTest.java @@ -253,7 +253,7 @@ void testWasAddedSince() { @Test void testWasEnhancedSince() { - Rule translateRule = profile.getRule(RuleID.TRANSLATE); + Rule translateRule = profile.getRule(RuleID.EXPORTING_KEYWORD); assertFalse(translateRule.wasEnhancedSince(null)); Release[] releases = Program.getReleases(); diff --git a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/AddToEtcTest.java b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/AddToEtcTest.java index 445433dd..29c2f0ca 100644 --- a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/AddToEtcTest.java +++ b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/AddToEtcTest.java @@ -18,6 +18,7 @@ class AddToEtcTest extends RuleTestBase { @BeforeEach void setUp() { // setup default test configuration (may be modified in the individual test methods) + rule.configProcessChains.setValue(true); rule.configReplacementStyleForOldRelease.setEnumValue(AddToReplacementStyleForOldRelease.KEEP); } @@ -194,10 +195,12 @@ void testAssignmentTargetWithOffsetAndLength() { @Test void testChainUnchanged() { - buildSrc(" ADD 1 TO : lv_value_a, lv_value_b."); + rule.configProcessChains.setValue(false); + + buildSrc(" ADD 1 TO: lv_value_a, lv_value_b."); buildSrc(" SUBTRACT: 1 FROM lv_value_a, 2 FROM lv_value_b."); - buildSrc(" MULTIPLY iv_value BY : 2, 3, 4."); - buildSrc(" DIVIDE iv_value : BY 2, BY 3, BY 4."); + buildSrc(" MULTIPLY iv_value BY: 2, 3, 4."); + buildSrc(" DIVIDE iv_value: BY 2, BY 3, BY 4."); copyExpFromSrc(); @@ -206,6 +209,27 @@ void testChainUnchanged() { testRule(); } + @Test + void testChainChanged() { + buildSrc(" ADD 1 TO: lv_value_a, lv_value_b."); + buildSrc(" SUBTRACT: 1 FROM lv_value_a, 2 FROM lv_value_b."); + buildSrc(" MULTIPLY iv_value BY: 2, 3, 4."); + buildSrc(" DIVIDE iv_value: BY 2, BY 3, BY 4."); + + buildExp(" lv_value_a += 1."); + buildExp(" lv_value_b += 1."); + buildExp(" lv_value_a -= 1."); + buildExp(" lv_value_b -= 2."); + buildExp(" iv_value *= 2."); + buildExp(" iv_value *= 3."); + buildExp(" iv_value *= 4."); + buildExp(" iv_value /= 2."); + buildExp(" iv_value /= 3."); + buildExp(" iv_value /= 4."); + + testRule(); + } + @Test void testOperandLiteralToOldSyntax() { setAbapReleaseOfCode("753"); diff --git a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/AssertClassTest.java b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/AssertClassTest.java index 0e897474..b3b47085 100644 --- a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/AssertClassTest.java +++ b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/AssertClassTest.java @@ -18,6 +18,7 @@ class AssertClassTest extends RuleTestBase { void setUp() { // setup default test configuration (may be modified in the individual test methods) rule.configAssertClassName.setValue("cx_any_assert"); + rule.configProcessChains.setValue(true); } @Test @@ -170,6 +171,8 @@ void testOtherAssertClassName() { @Test void testChainUnchanged() { + rule.configProcessChains.setValue(false); + buildSrc(" ASSERT: a = 1, b = 2."); buildSrc(" ASSERT NOT: lo_item IS BOUND,"); buildSrc(" IS ASSIGNED."); @@ -242,4 +245,37 @@ void testAssertCondition() { testRule(); } + @Test + void testChainProcessed() { + rule.configProcessChains.setValue(true); + + buildSrc(" ASSERT: sy-subrc = 0,"); + buildSrc(" io_instance IS BOUND,"); + buildSrc(" \" comment"); + buildSrc(" iv_is_valid = abap_false."); + + buildExp(" cx_any_assert=>assert_subrc( )."); + buildExp(" cx_any_assert=>assert_bound( io_instance )."); + buildExp(" \" comment"); + buildExp(" cx_any_assert=>assert_false( iv_is_valid )."); + + putAnyMethodAroundSrcAndExp(); + + testRule(); + } + + @Test + void testChainKept() { + rule.configProcessChains.setValue(false); + + buildSrc(" ASSERT: sy-subrc = 0,"); + buildSrc(" io_instance IS BOUND,"); + buildSrc(" iv_is_valid = abap_false."); + + copyExpFromSrc(); + + putAnyMethodAroundSrcAndExp(); + + testRule(); + } } diff --git a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CallMethodTest.java b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CallMethodTest.java index 2d4157a9..882652b2 100644 --- a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CallMethodTest.java +++ b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CallMethodTest.java @@ -1,15 +1,25 @@ package com.sap.adt.abapcleaner.rules.commands; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import com.sap.adt.abapcleaner.rulebase.RuleID; import com.sap.adt.abapcleaner.rulebase.RuleTestBase; class CallMethodTest extends RuleTestBase { + private CallMethodRule rule; + CallMethodTest() { super(RuleID.CALL_METHOD); + rule = (CallMethodRule)getRule(); } + @BeforeEach + void setUp() { + // setup default test configuration (may be modified in the individual test methods) + rule.configProcessChains.setValue(true); + } + @Test void testCallWithoutParentheses() { buildSrc(" CALL METHOD any_method."); @@ -87,6 +97,8 @@ void testDynamicMethodCallUnchanged() { @Test void testChainUnchanged() { + rule.configProcessChains.setValue(false); + buildSrc(" CALL METHOD:any_method,"); buildSrc(" other_method."); buildSrc(" CALL METHOD : any_method( ),"); @@ -239,4 +251,43 @@ void testPragmaAfterCallMethod() { testRule(); } + @Test + void testChainProcessed() { + rule.configProcessChains.setValue(true); + + buildSrc(" CALL METHOD: any_method,"); + buildSrc(" other_method EXPORTING iv_value = iv_value,"); + buildSrc(" mo_any_instance->(iv_dynamic_method_name)."); + buildSrc(""); + buildSrc(" CALL METHOD third_method EXPORTING iv_name =: 'abc', 'def', 'ghi'."); + + buildExp(" any_method( )."); + buildExp(" other_method( iv_value = iv_value )."); + buildExp(" CALL METHOD mo_any_instance->(iv_dynamic_method_name)."); + buildExp(""); + buildExp(" third_method( iv_name = 'abc' )."); + buildExp(" third_method( iv_name = 'def' )."); + buildExp(" third_method( iv_name = 'ghi' )."); + + putAnyMethodAroundSrcAndExp(); + + testRule(); + } + + @Test + void testChainKept() { + rule.configProcessChains.setValue(false); + + buildSrc(" CALL METHOD: any_method,"); + buildSrc(" other_method EXPORTING iv_value = iv_value,"); + buildSrc(" mo_any_instance->(iv_dynamic_method_name)."); + buildSrc(""); + buildSrc(" CALL METHOD third_method EXPORTING iv_name =: 'abc', 'def', 'ghi'."); + + copyExpFromSrc(); + + putAnyMethodAroundSrcAndExp(); + + testRule(); + } } diff --git a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CheckInLoopTest.java b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CheckInLoopTest.java index e86ec910..e6f806ad 100644 --- a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CheckInLoopTest.java +++ b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CheckInLoopTest.java @@ -21,6 +21,7 @@ void setUp() { rule.configKeepCondition.setEnumValue(KeepCheckInLoopCondition.NEVER); rule.configNegationStyle.setEnumValue(NegationStyle.AVOID_INNER_NEGATIONS); rule.configConvertAbapFalseAndAbapTrue.setValue(true); + rule.configProcessChains.setValue(true); } @Test @@ -318,7 +319,8 @@ void testDontConvertAbapBool() { @Test void testChainsUnchanged() { - buildSrc(" \" chains are currently ignored:"); + rule.configProcessChains.setValue(false); + buildSrc(" DO 10 TIMES."); buildSrc(" CHECK: its_table_1 IS INITIAL,"); buildSrc(" its_table_2 IS INITIAL."); @@ -432,4 +434,46 @@ void testCheckInSelectEndSelect() { testRule(); } + + @Test + void testChainProcessed() { + rule.configProcessChains.setValue(true); + + buildSrc(" DO 5 TIMES."); + buildSrc(" CHECK: its_table IS NOT INITIAL,"); + buildSrc(" sy-index > 1."); + buildSrc(" lv_counter += 1."); + buildSrc(" ENDDO."); + + buildExp(" DO 5 TIMES."); + buildExp(" IF its_table IS INITIAL."); + buildExp(" CONTINUE."); + buildExp(" ENDIF."); + buildExp(" IF sy-index <= 1."); + buildExp(" CONTINUE."); + buildExp(" ENDIF."); + buildExp(" lv_counter += 1."); + buildExp(" ENDDO."); + + putAnyMethodAroundSrcAndExp(); + + testRule(); + } + + @Test + void testChainKept() { + rule.configProcessChains.setValue(false); + + buildSrc(" DO 5 TIMES."); + buildSrc(" CHECK: its_table IS NOT INITIAL,"); + buildSrc(" sy-index > 1."); + buildSrc(" lv_counter += 1."); + buildSrc(" ENDDO."); + + copyExpFromSrc(); + + putAnyMethodAroundSrcAndExp(); + + testRule(); + } } diff --git a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CheckOutsideLoopTest.java b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CheckOutsideLoopTest.java index ed3b23eb..cb20c6f3 100644 --- a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CheckOutsideLoopTest.java +++ b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CheckOutsideLoopTest.java @@ -22,6 +22,7 @@ void setUp() { rule.configNegationStyle.setEnumValue(NegationStyle.AVOID_INNER_NEGATIONS); rule.configConvertAbapFalseAndAbapTrue.setValue(true); rule.configAllowCheckAfterCheckpoints.setValue(true); + rule.configProcessChains.setValue(true); } @Test @@ -426,9 +427,32 @@ void testDontConvertAbapBool() { } @Test - void testChainsUnchanged() { - // as of now, expect chains to be ignored + void testChainsProcessed() { + rule.configProcessChains.setValue(true); + + buildSrc(" lv_value = 1."); + buildSrc(""); + buildSrc(" CHECK: its_table_1 IS INITIAL,"); + buildSrc(" its_table_2 IS INITIAL."); + + buildExp(" lv_value = 1."); + buildExp(""); + buildExp(" IF its_table_1 IS NOT INITIAL."); + buildExp(" RETURN."); + buildExp(" ENDIF."); + buildExp(" IF its_table_2 IS NOT INITIAL."); + buildExp(" RETURN."); + buildExp(" ENDIF."); + + putAnyMethodAroundSrcAndExp(); + testRule(); + } + + @Test + void testChainsUnchanged() { + rule.configProcessChains.setValue(false); + buildSrc(" lv_value = 1."); buildSrc(""); buildSrc(" CHECK: its_table_1 IS INITIAL,"); diff --git a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CreateObjectTest.java b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CreateObjectTest.java index 1e09cde5..c1e4a2c3 100644 --- a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CreateObjectTest.java +++ b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/CreateObjectTest.java @@ -1,5 +1,6 @@ package com.sap.adt.abapcleaner.rules.commands; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import com.sap.adt.abapcleaner.base.ABAP; @@ -7,10 +8,19 @@ import com.sap.adt.abapcleaner.rulebase.RuleTestBase; class CreateObjectTest extends RuleTestBase { + private CreateObjectRule rule; + CreateObjectTest() { super(RuleID.CREATE_OBJECT); + rule = (CreateObjectRule)getRule(); } + @BeforeEach + void setUp() { + // setup default test configuration (may be modified in the individual test methods) + rule.configProcessChains.setValue(true); + } + @Test void testOldAbapRelease() { // ensure that NEW is NOT introduced if the code must compile against an ABAP Release prior to 7.40, @@ -163,6 +173,8 @@ void testCallWithExceptionsUnchanged() { @Test void testChainUnchanged() { + rule.configProcessChains.setValue(false); + buildSrc(" CREATE OBJECT:lo_any_object,"); buildSrc(" lo_other_object."); buildSrc(" CREATE OBJECT lo_any_object"); @@ -256,4 +268,67 @@ void testPragmaBeforeExporting() { testRule(); } + @Test + void testChainProcessed() { + rule.configProcessChains.setValue(true); + + buildSrc(" CREATE OBJECT: lx_message, lx_other_message."); + buildSrc(""); + buildSrc(" CREATE OBJECT: lo_any TYPE cl_any_class,"); + buildSrc("* comment1"); + buildSrc("* comment2"); + buildSrc(" lo_other TYPE (lv_class_name),"); + buildSrc(" lo_third TYPE cl_othird_class"); + buildSrc(" EXPORTING io_contract = me."); + + buildExp(" lx_message = NEW #( )."); + buildExp(" lx_other_message = NEW #( )."); + buildExp(""); + buildExp(" lo_any = NEW cl_any_class( )."); + buildExp("* comment1"); + buildExp("* comment2"); + buildExp(" CREATE OBJECT lo_other TYPE (lv_class_name)."); + buildExp(" lo_third = NEW cl_othird_class("); + buildExp(" io_contract = me )."); + + putAnyMethodAroundSrcAndExp(); + + testRule(); + } + + @Test + void testNoLineBreakAboveComment() { + // ensure that the 2 line breaks above CREATE OBJECT do NOT result in two line breaks above '* comment' + buildSrc(" any_method( )."); + buildSrc(""); + buildSrc(" CREATE OBJECT: lo_any,"); + buildSrc("* comment"); + buildSrc(" lo_other."); + + buildExp(" any_method( )."); + buildExp(""); + buildExp(" lo_any = NEW #( )."); + buildExp("* comment"); + buildExp(" lo_other = NEW #( )."); + + testRule(); + } + + @Test + void testChainKept() { + rule.configProcessChains.setValue(false); + + buildSrc(" CREATE OBJECT: lx_message, lx_other_message."); + buildSrc(""); + buildSrc(" CREATE OBJECT: lo_any TYPE cl_any_class,"); + buildSrc(" lo_other TYPE (lv_class_name),"); + buildSrc(" lo_third TYPE cl_othird_class"); + buildSrc(" EXPORTING io_contract = me."); + + copyExpFromSrc(); + + putAnyMethodAroundSrcAndExp(); + + testRule(); + } } diff --git a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/TranslateTest.java b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/TranslateTest.java index c1e0bd5f..2c0b570a 100644 --- a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/TranslateTest.java +++ b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/commands/TranslateTest.java @@ -20,6 +20,7 @@ void setUp() { rule.configReplaceTranslateToUpperLower.setValue(true); rule.configReplaceTranslateUsing.setValue(true); rule.configReplaceUnevenMasks.setValue(true); + rule.configProcessChains.setValue(true); } @Test @@ -230,4 +231,42 @@ void testUnescapingAndEscaping() { testRule(); } + + @Test + void testChainKept() { + rule.configProcessChains.setValue(false); + + buildSrc(" TRANSLATE: lv_text TO LOWER CASE, lv_other_text TO UPPER CASE."); + buildSrc(""); + buildSrc(" TRANSLATE lv_abc USING: '1 2 3 ', 'a-b-c-'."); + + copyExpFromSrc(); + + putAnyMethodAroundSrcAndExp(); + + testRule(); + } + + @Test + void testChainProcessed() { + rule.configProcessChains.setValue(true); + + buildSrc(" TRANSLATE: lv_text TO LOWER CASE, lv_other_text TO UPPER CASE."); + buildSrc(""); + buildSrc(" TRANSLATE lv_abc USING: '1 2 3 ', 'a-b-c-'."); + + buildExp(" lv_text = to_lower( lv_text )."); + buildExp(" lv_other_text = to_upper( lv_other_text )."); + buildExp(""); + buildExp(" lv_abc = translate( val = lv_abc"); + buildExp(" from = `123`"); + buildExp(" to = ` ` )."); + buildExp(" lv_abc = translate( val = lv_abc"); + buildExp(" from = `abc`"); + buildExp(" to = `---` )."); + + putAnyMethodAroundSrcAndExp(); + + testRule(); + } } \ No newline at end of file