diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/alignment/AlignDeclarationsRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/alignment/AlignDeclarationsRule.java index d29a5879..89342619 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/alignment/AlignDeclarationsRule.java +++ b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/alignment/AlignDeclarationsRule.java @@ -488,9 +488,11 @@ private Token readDeclarationLine(AlignLine line, Token token, int additionalInd Token typeEnd = token; while (typeEnd != null && !typeEnd.isAnyKeyword("LENGTH", "DECIMALS", "VALUE", "READ-ONLY") && !typeEnd.textEqualsAny(".", ",")) { // do not align table declarations with "WITH ... KEY ..." sections, because they usually should not be put on a single line; - // however, do accept the short cases of "WITH EMPTY KEY" and "WITH [UNIQUE | NON-UNIQUE] DEFAULT KEY" - if (typeEnd.isKeyword("WITH") && !typeEnd.matchesOnSiblings(true, "WITH", "EMPTY", "KEY") - && !typeEnd.matchesOnSiblings(true, "WITH", TokenSearch.makeOptional("UNIQUE|NON-UNIQUE"), "DEFAULT", "KEY")) { + // however, do accept the short cases of "WITH EMPTY KEY" and "WITH [UNIQUE | NON-UNIQUE] DEFAULT KEY" and "WITH [UNIQUE | NON-UNIQUE] KEY comp1" + if (typeEnd.isKeyword("WITH") + && !typeEnd.matchesOnSiblings(true, "WITH", "EMPTY", "KEY") + && !typeEnd.matchesOnSiblings(true, "WITH", TokenSearch.makeOptional("UNIQUE|NON-UNIQUE"), "DEFAULT", "KEY") + && !typeEnd.matchesOnSiblings(true, "WITH", TokenSearch.makeOptional("UNIQUE|NON-UNIQUE"), "KEY", TokenSearch.ANY_IDENTIFIER, ",|.")) { return null; } typeEnd = typeEnd.getNextSibling(); diff --git a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/alignment/AlignMethodsDeclarationRule.java b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/alignment/AlignMethodsDeclarationRule.java index abc84e27..1b27397a 100644 --- a/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/alignment/AlignMethodsDeclarationRule.java +++ b/com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/alignment/AlignMethodsDeclarationRule.java @@ -422,7 +422,7 @@ private boolean isDeclarationSuitedForMultiDeclarations(AlignTable table, int me } else if (getAlignConsecutive() == MethodsSequenceAlignment.ALL_TABULAR) { // allow one-liners as well as declarations in 'tabular' layout (or chains of those), i.e. ensure that there are no line breaks... - // - after the keywords METHODS / CLASS-METHODS, + // - after the keywords METHODS / CLASS-METHODS (except when they are followed by a chain colon) // - after the method names, or // - after the access keywords IMPORTING, EXPORTING etc. final Columns[] columnsToCheck = new Columns[] { Columns.KEYWORD, Columns.METHOD_NAME, Columns.ACCESS }; @@ -434,7 +434,7 @@ private boolean isDeclarationSuitedForMultiDeclarations(AlignTable table, int me if (cell == null) continue; Token nextToken = cell.getLastToken().getNextNonCommentSibling(); - if (nextToken != null && nextToken.lineBreaks > 0) { + if (nextToken != null && nextToken.lineBreaks > 0 && !cell.getLastToken().isChainColon()) { // line break found - this is no one-liner or 'tabular' layout // or, as an exception, continue if (lineIndex == methodNameLineIndex && ChangeType.forValue(configContinueAfterKeyword.getValue()) == ChangeType.NEVER)? return false; @@ -458,8 +458,16 @@ private void executeOn(Code code, Command startCommand, AlignTable table, boolea basicIndent += ABAP.INDENT_STEP; int firstLineBreaks = table.getFirstToken().lineBreaks; + // determine whether the table starts or ends in the middle of a declaration chain + boolean tableContainsPartialChains = false; + if (table.getLine(0).getCell(Columns.KEYWORD.getValue()) == null) { + tableContainsPartialChains = true; + } else if (table.getLastLine().getLastToken().matchesOnSiblings(true, TokenSearch.ASTERISK, ABAP.COLON_SIGN_STRING)) { + tableContainsPartialChains = true; + } + // insert line breaks depending on rule configuration - changeLineBreaks(code, table, isTableOfOneLinersOrTabular, basicIndent); + changeLineBreaks(code, table, isTableOfOneLinersOrTabular && !tableContainsPartialChains, basicIndent); // decide whether the DEFAULT or OPTIONAL column should really be aligned (i.e. whether horizontal space should be reserved for them): // if only one single line (or less than 20% of all lines) have content in this column, then join the content into a previous column @@ -495,8 +503,8 @@ private void executeOn(Code code, Command startCommand, AlignTable table, boolea } } - private void changeLineBreaks(Code code, AlignTable table, boolean isTableOfOneLinersOrTabular, int basicIndent) { - boolean isPotentialOneLiner = (table.getLineCount() == 1 && !table.getLine(0).containsComments()); + private void changeLineBreaks(Code code, AlignTable table, boolean alwaysContinueLine, int basicIndent) { + boolean isPotentialOneLiner = (table.getLineCount() == 1 && !table.getLine(0).containsComments()); boolean isCurrentOneLiner = isPotentialOneLiner && !table.getLine(0).containsInnerLineBreaks(); MethodsOneLinerMeasure oneLinerMeasure = MethodsOneLinerMeasure.forValue(configHandleOneLiners.getValue()); @@ -508,8 +516,8 @@ private void changeLineBreaks(Code code, AlignTable table, boolean isTableOfOneL // determine the configuration for this column ChangeType continueLine; - if (isTableOfOneLinersOrTabular) { - continueLine = ChangeType.ALWAYS; + if (alwaysContinueLine) { + continueLine = (colIndex == Columns.KEYWORD.getValue()) ? ChangeType.KEEP_AS_IS : ChangeType.ALWAYS; } else if (isPotentialOneLiner && oneLinerMeasure == MethodsOneLinerMeasure.CREATE) { continueLine = ChangeType.ALWAYS; } else if (isCurrentOneLiner && oneLinerMeasure == MethodsOneLinerMeasure.KEEP_EXISTING) { diff --git a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignDeclarationsTest.java b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignDeclarationsTest.java index 5a7f4213..acab22f3 100644 --- a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignDeclarationsTest.java +++ b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignDeclarationsTest.java @@ -672,12 +672,13 @@ void testDataBeginOfNested() { @Test void testTableDeclarationWithKey() { - // ensure that table declarations with "WITH ... KEY ..." sections are not aligned, because they usually should not be put on a single line + // ensure that table declarations with "WITH ... KEY ..." sections are not aligned (except for very simply cases + // with just one component), because they usually should not be put on a single line buildSrc(" DATA:"); buildSrc(" lt_item TYPE ty_tt_item,"); buildSrc(" lts_buffer TYPE SORTED TABLE OF ty_s_buffer"); - buildSrc(" WITH NON-UNIQUE KEY primary_key,"); + buildSrc(" WITH NON-UNIQUE KEY comp1 comp2 comp3,"); buildSrc(" lv_index TYPE i."); copyExpFromSrc(); diff --git a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignMethodsDeclarationTest.java b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignMethodsDeclarationTest.java index d67f9591..ae5f7b7b 100644 --- a/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignMethodsDeclarationTest.java +++ b/test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignMethodsDeclarationTest.java @@ -1038,9 +1038,9 @@ void testOneLinersAfterChainWithComment() { buildSrc(" method_with_long_name RETURNING VALUE(rv_third) TYPE string."); buildExp(" METHODS: \" comment"); - buildExp(" any_method RETURNING VALUE(rv_any) TYPE abap_bool,"); - buildExp(" other_method RETURNING VALUE(rv_other) TYPE i,"); - buildExp(" method_with_long_name RETURNING VALUE(rv_third) TYPE string."); + buildExp(" any_method RETURNING VALUE(rv_any) TYPE abap_bool,"); + buildExp(" other_method RETURNING VALUE(rv_other) TYPE i,"); + buildExp(" method_with_long_name RETURNING VALUE(rv_third) TYPE string."); putAnyClassDefAroundSrcAndExp(); @@ -1780,10 +1780,30 @@ void testMethodsAbstract() { testRule(); } - /* @Test - void testBreakAfterKeyword() { - rule.configContinueAfterKeyword.setEnumValue(ChangeType.NEVER); + void testBreakAfterKeywordAlignOneLiners() { + rule.configContinueAfterMethodName.setEnumValue(ChangeType.ALWAYS); + rule.configAlignConsecutive.setEnumValue(MethodsSequenceAlignment.ONE_LINERS); + + buildSrc(" METHODS:"); + buildSrc(" any_method IMPORTING iv_any TYPE i,"); + buildSrc(" other_method IMPORTING iv_other TYPE i,"); + buildSrc(" third_method_with_long_name IMPORTING iv_third_param TYPE i."); + buildSrc(""); + buildSrc(" DATA lv_any TYPE i."); + + buildExp(" METHODS:"); + buildExp(" any_method IMPORTING iv_any TYPE i,"); + buildExp(" other_method IMPORTING iv_other TYPE i,"); + buildExp(" third_method_with_long_name IMPORTING iv_third_param TYPE i."); + buildExp(""); + buildExp(" DATA lv_any TYPE i."); + + testRule(); + } + + @Test + void testBreakAfterKeywordAlignAllTabular() { rule.configContinueAfterMethodName.setEnumValue(ChangeType.ALWAYS); rule.configAlignConsecutive.setEnumValue(MethodsSequenceAlignment.ALL_TABULAR); @@ -1791,13 +1811,31 @@ void testBreakAfterKeyword() { buildSrc(" any_method IMPORTING iv_any TYPE i,"); buildSrc(" other_method IMPORTING iv_other TYPE i,"); buildSrc(" third_method_with_long_name IMPORTING iv_third_param TYPE i."); + buildSrc(""); + buildSrc(" DATA lv_any TYPE i."); buildExp(" METHODS:"); buildExp(" any_method IMPORTING iv_any TYPE i,"); buildExp(" other_method IMPORTING iv_other TYPE i,"); buildExp(" third_method_with_long_name IMPORTING iv_third_param TYPE i."); + buildExp(""); + buildExp(" DATA lv_any TYPE i."); + + testRule(); + } + + @Test + void testBreakAfterKeywordButKeepOneLiners() { + rule.configContinueAfterKeyword.setEnumValue(ChangeType.NEVER); + + buildSrc(" METHODS set_value IMPORTING !iv_new_value TYPE i."); + buildSrc(" METHODS get_current_value RETURNING VALUE(rv_result) TYPE i."); + buildSrc(" METHODS get_previous_value RETURNING VALUE(rv_result) TYPE i."); + + buildExp(" METHODS set_value IMPORTING !iv_new_value TYPE i."); + buildExp(" METHODS get_current_value RETURNING VALUE(rv_result) TYPE i."); + buildExp(" METHODS get_previous_value RETURNING VALUE(rv_result) TYPE i."); testRule(); } - */ } \ No newline at end of file