Skip to content

Commit

Permalink
fix IndentRule for WITH ... ENDWITH (#98)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jmgrassau
jmgrassau committed Sep 4, 2023
1 parent ae3c2ff commit 6825739
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 19 deletions.
49 changes: 30 additions & 19 deletions com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/parser/Command.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,9 @@ final boolean isCloserFor(LevelOpener levelOpener) {
// Processing External Data

// ABAP SQL
addInnerLevelOpener("SELECT", "ENDSELECT"); // under certain conditions, SELECT has no ENDSELECT, see .finishBuild()
// under certain conditions, SELECT / WITH have no ENDSELECT / ENDWITH, see .finishBuild() and .opensSelectLoop()
addInnerLevelOpener("SELECT", "ENDSELECT");
addInnerLevelOpener("WITH", "ENDWITH");

// Native SQL
addInnerLevelOpener("EXEC", "ENDEXEC");
Expand Down Expand Up @@ -911,16 +913,16 @@ public final void finishBuild(int sourceTextStart, int sourceTextEnd) throws Par
isClassImplementationStart = firstCode != null && firstCode.matchesOnSiblings(true, "CLASS", TokenSearch.ASTERISK, "IMPLEMENTATION");
}

// in a SELECT statement, keep the LevelOpener only if an ENDSELECT is required
if (firstCode != null && firstCode.isKeyword("SELECT")) {
boolean requiresEndSelect;
// in a SELECT or WITH statement, keep the LevelOpener only if an ENDSELECT or ENDWITH is required
if (firstCode != null && firstCode.isAnyKeyword("SELECT", "WITH")) {
boolean opensSelectLoop;
try {
requiresEndSelect = requiresEndSelect();
opensSelectLoop = opensSelectLoop();
} catch (NullPointerException ex) {
// unexpected syntax
requiresEndSelect = false;
opensSelectLoop = false;
}
if (!requiresEndSelect)
if (!opensSelectLoop)
usedLevelOpener = null;
}

Expand Down Expand Up @@ -1028,9 +1030,10 @@ private void updateChainColonCount() {
}
}

private boolean requiresEndSelect() throws NullPointerException {
private boolean opensSelectLoop() throws NullPointerException {
// determine whether a SELECT command requires an ENDSELECT and therefore opens a level
// see https://help.sap.com/doc/abapdocu_latest_index_htm/latest/en-US/index.htm?file=abapselect.htm
// similarly, determine whether WITH requires ENDWITH by analyzing its main SELECT query

// "In the following cases, the statement SELECT opens a loop that must be closed using ENDSELECT.
// 1. If an assignment is made to a non-table-like target range, that is, a SELECT statement without the
Expand All @@ -1043,31 +1046,39 @@ private boolean requiresEndSelect() throws NullPointerException {
// 2. If an assignment is made to a table-like target range, that is, a SELECT statement with the addition
// INTO|APPENDING ... TABLE, a loop closed by ENDSELECT occurs whenever the addition PACKAGE SIZE is used."

// is the result put into a table? - then ENDSELECT is only required if PACKAGE SIZE is used (case 2)
Token firstCode = getFirstCodeToken();
if (firstCode == null)
Token selectToken = getFirstCodeToken();
if (selectToken == null)
return false;
if (firstCode.matchesOnSiblings(true, TokenSearch.ASTERISK, "INTO|APPENDING", TokenSearch.makeOptional("CORRESPONDING FIELDS OF"), "TABLE"))
return firstCode.matchesOnSiblings(true, TokenSearch.ASTERISK, "PACKAGE SIZE");

// for WITH commands, move to the main SELECT query (skipping parenthesized subquery SELECT clauses from table expression definitions)
if (selectToken.isKeyword("WITH")) {
selectToken = selectToken.getLastTokenOnSiblings(true, TokenSearch.ASTERISK, "SELECT");
if (selectToken == null)
return false;
}

// is the result put into a table? - then ENDSELECT is only required if PACKAGE SIZE is used (case 2)
if (selectToken.matchesOnSiblings(true, TokenSearch.ASTERISK, "INTO|APPENDING", TokenSearch.makeOptional("CORRESPONDING FIELDS OF"), "TABLE"))
return selectToken.matchesOnSiblings(true, TokenSearch.ASTERISK, "PACKAGE SIZE");

// is SELECT SINGLE used? - then ENDSELECT is not required (case 1a)
if (firstCode.matchesOnSiblings(true, "SELECT", "SINGLE"))
if (selectToken.matchesOnSiblings(true, "SELECT", "SINGLE"))
return false;

// process case 1b

// is GROUP BY or UNION specified? - then ENDSELECT is required (case 1b.iii)
if (firstCode.matchesOnSiblings(true, TokenSearch.ASTERISK, "GROUP BY"))
if (selectToken.matchesOnSiblings(true, TokenSearch.ASTERISK, "GROUP BY"))
return true;
if (firstCode.matchesOnSiblings(true, TokenSearch.ASTERISK, "UNION"))
if (selectToken.matchesOnSiblings(true, TokenSearch.ASTERISK, "UNION"))
return true;

// determine start and end of the select clause
// (see https://help.sap.com/doc/abapdocu_latest_index_htm/latest/en-US/index.htm?file=abapselect_mainquery.htm)
Token selectClauseStart = firstCode.getNextCodeSibling();
Token selectClauseStart = selectToken.getNextCodeSibling();
Token selectClauseEnd;
if (selectClauseStart.isKeyword("FROM")) {
selectClauseStart = firstCode.getLastTokenOnSiblings(true, TokenSearch.ASTERISK, "FIELDS");
selectClauseStart = selectToken.getLastTokenOnSiblings(true, TokenSearch.ASTERISK, "FIELDS");
selectClauseStart = selectClauseStart.getNextCodeSibling();
// the INTO clause may come earlier than specified in the ABAP reference
selectClauseEnd = selectClauseStart.getLastTokenOnSiblings(true, TokenSearch.ASTERISK, "FOR ALL ENTRIES IN|WHERE|GROUP BY|HAVING|ORDER BY|%_HINTS|INTO");
Expand Down Expand Up @@ -2743,7 +2754,7 @@ public final boolean matchesPattern() {
// return changesSyField(ABAP.SyField.SUBRC) && SyFieldAnalyzer.getSyFieldReadersFor(ABAP.SyField.SUBRC, this).size() >= 2;
// - getCommandsRelatedToPatternMatch() can then return SyFieldAnalyzer.getSyFieldReadersFor(ABAP.SyField.SUBRC, this);

return changeControl.wasRuleUsed(RuleID.ALIGN_LOGICAL_EXPRESSIONS) && this.containsCommentBetween(firstToken, null);
return this.getFirstToken().isAnyKeyword("WITH", "ENDWITH");
//return false;
}

Expand Down
66 changes: 66 additions & 0 deletions ....sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/prettyprinter/IndentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -823,4 +823,70 @@ void testAlignCommentWithCleanup() {

testRule();
}

@Test
void testWithNotStartingSelectLoop() {
// expect the any_method call to be moved to the same indentation as WITH, because no SELECT loop is started

buildSrc(" WITH");
buildSrc(" +cte1 AS ( SELECT fld1, fld2 FROM dtab1");
buildSrc(" JOIN dtab2 ON dtab1~id = dtab2~id");
buildSrc(" WHERE dtab1~id = @id ),");
buildSrc(" +cte2 AS ( SELECT COUNT(*) AS cnt FROM +cte1 )");
buildSrc(" SELECT * FROM +cte2");
buildSrc(" CROSS JOIN +cte1");
buildSrc(" ORDER BY fld1, fld2");
buildSrc(" INTO CORRESPONDING FIELDS OF TABLE @itab.");
buildSrc("");
buildSrc(" any_method( itab ).");

buildExp(" WITH");
buildExp(" +cte1 AS ( SELECT fld1, fld2 FROM dtab1");
buildExp(" JOIN dtab2 ON dtab1~id = dtab2~id");
buildExp(" WHERE dtab1~id = @id ),");
buildExp(" +cte2 AS ( SELECT COUNT(*) AS cnt FROM +cte1 )");
buildExp(" SELECT * FROM +cte2");
buildExp(" CROSS JOIN +cte1");
buildExp(" ORDER BY fld1, fld2");
buildExp(" INTO CORRESPONDING FIELDS OF TABLE @itab.");
buildExp("");
buildExp(" any_method( itab ).");

putAnyMethodAroundSrcAndExp();

testRule();
}

@Test
void testWithStartingSelectLoop() {
// expect the any_method call to get additional indentation, because WITH starts a SELECT loop

buildSrc(" WITH");
buildSrc(" +cte AS ( SELECT FROM dtab1 FIELDS f1, f2 )");
buildSrc(" SELECT FROM dtab2 AS b");
buildSrc(" INNER JOIN +cte AS a ON b~g1 = a~f1");
buildSrc(" FIELDS a~f2, s~g2");
buildSrc(" WHERE s~g1 = 'NN'");
buildSrc(" INTO @FINAL(wa)");
buildSrc(" UP TO 10 ROWS.");
buildSrc("");
buildSrc(" any_method( wa ).");
buildSrc(" ENDWITH.");

buildExp(" WITH");
buildExp(" +cte AS ( SELECT FROM dtab1 FIELDS f1, f2 )");
buildExp(" SELECT FROM dtab2 AS b");
buildExp(" INNER JOIN +cte AS a ON b~g1 = a~f1");
buildExp(" FIELDS a~f2, s~g2");
buildExp(" WHERE s~g1 = 'NN'");
buildExp(" INTO @FINAL(wa)");
buildExp(" UP TO 10 ROWS.");
buildExp("");
buildExp(" any_method( wa ).");
buildExp(" ENDWITH.");

putAnyMethodAroundSrcAndExp();

testRule();
}
}

0 comments on commit 6825739

Please sign in to comment.