enhance AlignParametersRule for LOOP AT ... GROUP BY (#330)

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

@@ -2501,10 +2501,13 @@ public Token getEndOfParamsOrComponentsList() {
 			// the built-in function is 'shadowed' (even if the method has a different signature) and the method is called.
 
 		} else if (textEquals("(")) {
-			// stand-alone " ( " only starts a component list if it is a row inside a VALUE or NEW constructor expression, 
-			// not if it is part of arithmetic expressions etc.
-			if (parent == null) {
-				// stand-alone " ( " on top level must be part of an arithmetic expression
+			// stand-alone " ( " only starts a component list in case of LOOP AT ... GROUP BY ( ... ) or if it is a row  
+			// inside a VALUE or NEW constructor expression, not if it is part of arithmetic expressions etc.
+			if (parent == null && this.opensGroupKey()) {
+				// LOOP AT ... GROUP BY ( key1 = dobj1 key2 = dobj2 ... [gs = GROUP SIZE] [gi = GROUP INDEX] ) ...
+				return getNextCodeSibling();
+			} else if (parent == null) {
+				// otherwise, stand-alone " ( " on top level must be part of an arithmetic expression
 				return null;
 			} else if (parent.textEquals("(") || !parent.textEndsWith("(")) {
 				return null;
@@ -2520,8 +2523,9 @@ public Token getEndOfParamsOrComponentsList() {
 			// The above conditions already excluded the case " ( ( ".  
 			// do NOT use getPrevCodeToken() here, as it may return the "#(" of the VALUE or NEW constructor, which has TokenType.OTHER_OP
 			Token prev = getPrevCodeSibling();  
-			if (prev != null && !prev.textEquals(")") && (prev.isAssignmentOperator() || prev.isComparisonOperator() || prev.isOtherOp() || prev.isAnyKeyword("UNTIL", "WHILE", "WHERE")))
+			if (prev != null && !prev.textEquals(")") && (prev.isAssignmentOperator() || prev.isComparisonOperator() || prev.isOtherOp() || prev.isAnyKeyword("UNTIL", "WHILE", "WHERE"))) {
 				return null;
+			}
 
 		} else {
 			// as the token opens a level but is not a stand-alone "(", it must be a method call or a constructor expression,  
@@ -2535,7 +2539,15 @@ public Token getEndOfParamsOrComponentsList() {
 		return getNextSibling();
 	}
 
-
+	public boolean opensGroupKey() {
+		if (!textEquals("(") || !hasChildren() || next.isAttached())
+			return false;
+		// LOOP AT ... GROUP BY ( key1 = dobj1 key2 = dobj2 ... [gs = GROUP SIZE] [gi = GROUP INDEX] ) ...
+		Token prev = getPrevCodeSibling(); 
+		Token prevPrev = (prev == null) ? null : prev.getPrevCodeSibling();
+		return prevPrev != null && prevPrev.isKeyword("GROUP") && prev.isKeyword("BY") && parentCommand.firstCodeTokenIsKeyword("LOOP");
+	}
+
 	/**
 	 * If this Token starts a logical expression, the last code Token of the logical expression is returned; otherwise null.
 	 * @return

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/alignment/AlignParametersRule.java

@@ -36,6 +36,7 @@ public enum ContentType {
 		FUNCTIONAL_CALL_PARAMS, 
 		PROCEDURAL_CALL_PARAMS, 
 		TABLE_KEY, 
+		GROUP_KEY,
 		CONSTRUCTOR_EXPR,
 		ROW_IN_VALUE_OR_NEW_CONSTRUCTOR;
 
@@ -144,12 +145,20 @@ public String getExample() {
 			+ LINE_SEP + "                         ( item_key = '20220040000101'  event_date = '20220401'  total_qty = '30'  qty_unit = 'DAY'  amount = '1500.00'  currency = 'EUR' )" 
 			+ LINE_SEP + "                         ( item_key = '20220050000101'  event_date = '20220501'  total_qty = '30'  qty_unit = 'DAY'  amount = '2000.00'  currency = 'EUR' ) )." 
 			+ LINE_SEP 
-			+ LINE_SEP + "    READ TABLE lt_any_table_name ASSIGNING <ls_table_row> " 
+			+ LINE_SEP + "    READ TABLE lt_any_table_name ASSIGNING <ls_table_row>" 
 			+ LINE_SEP + "         WITH KEY field1 = ls_any_structure-field1" 
 			+ LINE_SEP + "                  fld2 = ls_any_structure-fld2" 
 			+ LINE_SEP + "                  long_field_name3 = ls_any_structure-long_field_name_3." 
 			+ LINE_SEP 
 			+ LINE_SEP + "    result = VALUE #( BASE result ( id = 1 name = 'abc' ) )."
+			+ LINE_SEP 
+			+ LINE_SEP + "    LOOP AT lt_table ASSIGNING <fs>" 
+			+ LINE_SEP + "         GROUP BY ( key1 = <fs>-any_component key2 = get_value( <fs>-other_component )" 
+			+ LINE_SEP + "                     indx = GROUP INDEX count = GROUP SIZE )" 
+			+ LINE_SEP + "         ASSIGNING FIELD-SYMBOL(<group>)." 
+			+ LINE_SEP 
+			+ LINE_SEP + "      cl_demo_output=>write( |{ <group>-indx } { <group>-key1 } { <group>-key2 } { <group>-count }| )." 
+			+ LINE_SEP + "    ENDLOOP." 
 			+ LINE_SEP + "  ENDMETHOD.";
    }
 
@@ -337,7 +346,7 @@ private Token getFirstComponentOfTableKey(Token firstCode) {
 	private ContentType determineContentType(Token token) {
 		if (token.textEquals("(")) { 
 			// stand-alone parenthesis, as opposed to "method_name(" or "type_name(" or "#(" 
-			return ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR;
+			return token.opensGroupKey() ? ContentType.GROUP_KEY : ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR;
 		}
 
 		Token prev = token.getPrevCodeSibling();
@@ -377,7 +386,7 @@ private int determineBaseIndentForRaise(Command command, Token parentToken) {
 	}
 
 	private int determineBaseIndent(Token token, ContentType contentType) {
-		if (contentType == ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR) {
+		if (contentType == ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR || contentType == ContentType.GROUP_KEY) {
 			// rows in a VALUE or NEW constructor expression were already moved when the outer VALUE or NEW constructor itself was processed 
 			return token.getStartIndexInLine();
 		} else { 
@@ -397,7 +406,7 @@ private int determineBaseIndent(Token token, ContentType contentType) {
 	}
 
 	private int determineMinimumIndent(Token token, ContentType contentType) {
-		if (contentType == ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR) {
+		if (contentType == ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR || contentType == ContentType.GROUP_KEY) {
 			// rows in a VALUE or NEW constructor expression were already moved when the outer VALUE or NEW constructor itself was processed 
 			return token.getStartIndexInLine();
 		} else {
@@ -687,7 +696,9 @@ public static void buildAlignTable(AlignTable table, Token parentToken, Token en
 				Token parameter = token;
 				Token keyword = parameter.getPrevCodeToken();
 				if (keyword != null) {
-					if (!keyword.isKeyword() || keyword != parameter.getPrevCodeSibling()) {
+					if (contentType == ContentType.GROUP_KEY) {
+						keyword = null; 
+					} else if (!keyword.isKeyword() || keyword != parameter.getPrevCodeSibling()) {
 						keyword = null; 
 					} else if (keyword.isAnyKeyword("EXPORTING", "IMPORTING", "TABLES", "CHANGING", "RECEIVING", "EXCEPTIONS")) {
 						isInExceptions = keyword.isKeyword("EXCEPTIONS");
@@ -701,7 +712,14 @@ public static void buildAlignTable(AlignTable table, Token parentToken, Token en
 					}
 				}
 				Token assignmentOp = parameter.getNextCodeSibling(); // there may be a comment line between EXPORTING and the first parameter
-				Term expression = Term.createArithmetic(assignmentOp.getNext());
+				Token exprStart = assignmentOp.getNext();
+				Term expression;
+				if (contentType == ContentType.GROUP_KEY && exprStart.matchesOnSiblings(true, "GROUP", "SIZE|INDEX")) {
+					// LOOP AT ... GROUP BY ( key1 = dobj1 key2 = dobj2 ... [gs = GROUP SIZE] [gi = GROUP INDEX] ) ...
+					expression = Term.createForTokenRange(exprStart, exprStart.getNextCodeSibling());
+				} else {
+					expression = Term.createArithmetic(assignmentOp.getNext());
+				}
 
 				// for "RECEIVE RESULTS FROM FUNCTION func" consider the MESSAGE addition after two special exceptions, 
 				// see https://help.sap.com/doc/abapdocu_latest_index_htm/latest/en-US/abapreceive_para.htm:
@@ -770,7 +788,7 @@ public static void buildAlignTable(AlignTable table, Token parentToken, Token en
 				if (token == parentToken.getNext() || token.lineBreaks > 0) {
 					if  (token.isPseudoCommentAfterCode()) {
 						// avoid moving pseudo comments to the next line if they refer to the line of the parentToken 
-					} else if (token.isCommentAfterCode() && contentType != ContentType.CONSTRUCTOR_EXPR && contentType != ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR) {
+					} else if (token.isCommentAfterCode() && contentType != ContentType.CONSTRUCTOR_EXPR && contentType != ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR && contentType != ContentType.GROUP_KEY) {
 						// line-end comments after method calls usually do not refer to the parameters; therefore, keep them in their place, too
 					} else {
 						// store 'other line starts'; in case this is a keyword like "EXPORTING", this may be removed again later
@@ -839,13 +857,14 @@ private boolean forceLineBreakAfterKeywords(ContentType contentType) {
 	private TableStart determineTableStart(Token parentToken, int baseIndent, int minimumIndent, ContentType contentType, AlignTable table, ArrayList<Token> otherLineStarts) {
 		AlignColumn keywordColumn = table.getColumn(Columns.KEYWORD.getValue());
 		boolean hasKeywords = !keywordColumn.isEmpty();
-		int addIndent = (contentType == ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR || hasKeywords) ? ABAP.INDENT_STEP : 2 * ABAP.INDENT_STEP; 
+		int addIndent = (contentType == ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR || contentType == ContentType.GROUP_KEY || hasKeywords) 
+						  ? ABAP.INDENT_STEP : 2 * ABAP.INDENT_STEP; 
 
 		// determine whether to continue on the same line after parentToken (this may be revised later if there is not enough space)
 		// note that continueOnSameLine may refer to the table (if it starts directly after parentToken) or to otherLineStarts
 		boolean continueOnSameLine;
 		Token firstTokenInTable = table.getFirstToken();
-		if ((contentType == ContentType.CONSTRUCTOR_EXPR) || (contentType == ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR)) {
+		if ((contentType == ContentType.CONSTRUCTOR_EXPR) || (contentType == ContentType.ROW_IN_VALUE_OR_NEW_CONSTRUCTOR) || (contentType == ContentType.GROUP_KEY)) {
 			continueOnSameLine = true;
 		} else if (firstTokenInTable != null && parentToken.getNext() == firstTokenInTable) {
 			continueOnSameLine = true;

test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignParametersTest.java

@@ -2981,4 +2981,27 @@ void testClosingParenthesesNotMoved() {
 
 		testRule();
 	}
+
+	@Test
+	void testAlignLoopAtGroupBy() {
+		buildSrc("    LOOP AT lt_table ASSIGNING <fs>");
+		buildSrc("         GROUP BY ( key1 = <fs>-any_component key2 = get_value( <fs>-other_component )");
+		buildSrc("                     indx = GROUP INDEX count = GROUP SIZE )");
+		buildSrc("         ASSIGNING FIELD-SYMBOL(<group>).");
+		buildSrc("      cl_demo_output=>write( |{ <group>-indx } { <group>-key1 } { <group>-key2 } { <group>-count }| ).");
+		buildSrc("    ENDLOOP.");
+
+		buildExp("    LOOP AT lt_table ASSIGNING <fs>");
+		buildExp("         GROUP BY ( key1  = <fs>-any_component");
+		buildExp("                    key2  = get_value( <fs>-other_component )");
+		buildExp("                    indx  = GROUP INDEX");
+		buildExp("                    count = GROUP SIZE )");
+		buildExp("         ASSIGNING FIELD-SYMBOL(<group>).");
+		buildExp("      cl_demo_output=>write( |{ <group>-indx } { <group>-key1 } { <group>-key2 } { <group>-count }| ).");
+		buildExp("    ENDLOOP.");
+
+		putAnyMethodAroundSrcAndExp();
+
+		testRule();
+	}
 }