Skip to content

Commit

Permalink
add new rule "Break before WHERE clause etc."
Browse files Browse the repository at this point in the history
  • Loading branch information
jmgrassau committed Sep 9, 2024
1 parent cc42753 commit 4428678
Show file tree
Hide file tree
Showing 9 changed files with 433 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,16 @@ private boolean canAddToDdl(Token newToken, Command prevCommand) {
}
}

// start a new Command for a WHERE / GROUP BY / HAVING clause, as well as for EXCEPT / INTERSECT / UNION [ALL]
// (all these keywords never appear in a different context)
if (isAtTopLevel && newToken.isKeyword()) {
if (newToken.isAnyKeyword("WHERE", "GROUP", "HAVING")) {
return false;
} else if (newToken.isAnyKeyword("EXCEPT", "INTERSECT", "UNION")) {
return false;
}
}

// in all other cases, add the new Token to the current Command
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.sap.adt.abapcleaner.rules.ddl.annotations.DdlAnnotationNestingRule;
import com.sap.adt.abapcleaner.rules.ddl.position.DdlPositionAssociationRule;
import com.sap.adt.abapcleaner.rules.ddl.position.DdlPositionBracesRule;
import com.sap.adt.abapcleaner.rules.ddl.position.DdlPositionClausesRule;
import com.sap.adt.abapcleaner.rules.ddl.position.DdlPositionJoinRule;
import com.sap.adt.abapcleaner.rules.ddl.position.DdlPositionSelectRule;
import com.sap.adt.abapcleaner.rules.ddl.annotations.DdlAnnotationLayoutRule;
Expand All @@ -26,7 +27,7 @@
import java.util.*;

public abstract class Rule {
public static final int RULE_COUNT = 81;
public static final int RULE_COUNT = 82;
public static final int RULE_GROUP_COUNT = 9;

protected static final String LINE_SEP = ABAP.LINE_SEPARATOR;
Expand Down Expand Up @@ -162,7 +163,8 @@ static Rule[] getAllRules(Profile profile) {
new DdlPositionSelectRule(profile),
new DdlPositionJoinRule(profile),
new DdlPositionAssociationRule(profile),
new DdlPositionBracesRule(profile)
new DdlPositionBracesRule(profile),
new DdlPositionClausesRule(profile)
};

StringBuilder errors = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ public enum RuleID {
DDL_POSITION_SELECT,
DDL_POSITION_JOIN,
DDL_POSITION_ASSOCIATION,
DDL_POSITION_BRACES;
DDL_POSITION_BRACES,
DDL_POSITION_CLAUSES;

public static final int SIZE = java.lang.Integer.SIZE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ protected boolean breakBefore(Token token, DdlLineBreak lineBreak, boolean empty

// adjust the indent of the supplied Token (or the attached non-annotation comments above it)
int lineBreaksMin = emptyLineIfBreaking ? 2 : 1;
setWhitespaceInclAttachedComments(token, lineBreaksMin, 2, indent, false);
int lineBreaksMax = 2;
setWhitespaceInclAttachedComments(token, lineBreaksMin, lineBreaksMax, indent, false);
return false; // .addRuleUse() was already called above for all involved commands
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ public String getExample() {
+ LINE_SEP + " ThirdAlias.AnyNonKeyField }";
}

private final String[] lineBreakSelection = new String[] { "Always", "Keep as is", "Never" };

final ConfigEnumValue<DdlLineBreak> configBreakBeforeOpeningBrace = new ConfigEnumValue<DdlLineBreak>(this, "BreakBeforeOpeningBrace", "Break before opening brace { of select list:", lineBreakSelection, DdlLineBreak.values(), DdlLineBreak.ALWAYS);
final ConfigIntValue configOpeningBraceIndent = new ConfigIntValue(this, "OpeningBraceIndent", "Indent if breaking:", "", 0, 0, MAX_INDENT);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
package com.sap.adt.abapcleaner.rules.ddl.position;

import java.time.LocalDate;

import com.sap.adt.abapcleaner.base.DDL;
import com.sap.adt.abapcleaner.parser.Code;
import com.sap.adt.abapcleaner.parser.Command;
import com.sap.adt.abapcleaner.parser.Token;
import com.sap.adt.abapcleaner.parser.TokenSearch;
import com.sap.adt.abapcleaner.programbase.IntegrityBrokenException;
import com.sap.adt.abapcleaner.programbase.UnexpectedSyntaxAfterChanges;
import com.sap.adt.abapcleaner.programbase.UnexpectedSyntaxBeforeChanges;
import com.sap.adt.abapcleaner.rulebase.ConfigEnumValue;
import com.sap.adt.abapcleaner.rulebase.ConfigInfoValue;
import com.sap.adt.abapcleaner.rulebase.ConfigIntValue;
import com.sap.adt.abapcleaner.rulebase.ConfigValue;
import com.sap.adt.abapcleaner.rulebase.Profile;
import com.sap.adt.abapcleaner.rulebase.RuleID;
import com.sap.adt.abapcleaner.rulebase.RuleReference;
import com.sap.adt.abapcleaner.rulebase.RuleSource;
import com.sap.adt.abapcleaner.rulehelpers.RuleForDdlPosition;

public class DdlPositionClausesRule extends RuleForDdlPosition {
private final static RuleReference[] references = new RuleReference[] { new RuleReference(RuleSource.ABAP_CLEANER) };

@Override
public RuleID getID() { return RuleID.DDL_POSITION_CLAUSES; }

@Override
public String getDisplayName() { return "Break before WHERE clause etc."; }

@Override
public String getDescription() { return "Standardizes line breaks and indentation of keywords that start the clauses WHERE, GROUP BY, HAVING, as well as EXCEPT, INTERSECT and UNION."; }

@Override
public LocalDate getDateCreated() { return LocalDate.of(2024, 9, 9); }

@Override
public RuleReference[] getReferences() { return references; }

@Override
public String getExample() {
return ""
+ LINE_SEP + "define view entity C_AnyEntity"
+ LINE_SEP + " as select from I_AnyEntity as AnyAlias"
+ LINE_SEP + ""
+ LINE_SEP + "{"
+ LINE_SEP + " key AnyAlias.AnyKeyField as AnyKey,"
+ LINE_SEP + " sum(AnyAlias.AnyNumericField) as AnySum,"
+ LINE_SEP + " AnyAlias.UnitField as Unit"
+ LINE_SEP + "}"
+ LINE_SEP + ""
+ LINE_SEP + "where AnyAlias.AnyConditionField = 'X'"
+ LINE_SEP + " and AnyAlias.AnyCategory = 'A' group"
+ LINE_SEP + "by AnyAlias.AnyKeyField,"
+ LINE_SEP + " AnyAlias.AnyNumericField,"
+ LINE_SEP + " AnyAlias.UnitField having AnyAlias.AnyNumericField > 100"
+ LINE_SEP + ""
+ LINE_SEP + "union all"
+ LINE_SEP + " select from I_OtherEntity as OtherAlias"
+ LINE_SEP + ""
+ LINE_SEP + "{"
+ LINE_SEP + " key OtherAlias.OtherKeyField as AnyKey,"
+ LINE_SEP + " sum(OtherAlias.OtherNumericField) as AnySum,"
+ LINE_SEP + " OtherAlias.OtherUnitField as Unit"
+ LINE_SEP + "} where OtherAlias.OtherCategory = 'A'"
+ LINE_SEP + " or OtherAlias.OtherCategory = 'B'"
+ LINE_SEP + ""
+ LINE_SEP + "group by OtherAlias.OtherKeyField,"
+ LINE_SEP + " OtherAlias.OtherNumericField,"
+ LINE_SEP + " OtherAlias.OtherUnitField"
+ LINE_SEP + "";
}

final ConfigEnumValue<DdlLineBreakWithoutNever> configBreakBeforeWhereEtc = new ConfigEnumValue<DdlLineBreakWithoutNever>(this, "BreakBeforeWhereEtc", "Break before WHERE / GROUP BY / HAVING:", lineBreakSelectionWithoutNever, DdlLineBreakWithoutNever.values(), DdlLineBreakWithoutNever.ALWAYS);
final ConfigIntValue configWhereEtcIndent = new ConfigIntValue(this, "WhereEtcIndent", "Indent if breaking:", "", 0, 2, MAX_INDENT);

final ConfigEnumValue<DdlLineBreakWithoutNever> configBreakBeforeUnionEtc = new ConfigEnumValue<DdlLineBreakWithoutNever>(this, "BreakBeforeUnionEtc", "Break before EXCEPT / INTERSECT / UNION:", lineBreakSelectionWithoutNever, DdlLineBreakWithoutNever.values(), DdlLineBreakWithoutNever.ALWAYS);
final ConfigIntValue configUnionEtcIndent = new ConfigIntValue(this, "UnionEtcIndent", "Indent if breaking:", "", 0, 0, MAX_INDENT);

final ConfigInfoValue configInfoBreakBeforeSelect = new ConfigInfoValue(this, "The position of SELECT FROM after a UNION etc. can be changed with the rule '" + DdlPositionSelectRule.defaultDescription + "'");

private final ConfigValue[] configValues = new ConfigValue[] { configBreakBeforeWhereEtc, configWhereEtcIndent, configBreakBeforeUnionEtc, configUnionEtcIndent, configInfoBreakBeforeSelect };

@Override
public ConfigValue[] getConfigValues() { return configValues; }

public DdlPositionClausesRule(Profile profile) {
super(profile);
initializeConfiguration();
}

// -------------------------------------------------------------------------

@Override
protected boolean executeOn(Code code, Command command) throws UnexpectedSyntaxBeforeChanges, UnexpectedSyntaxAfterChanges, IntegrityBrokenException {
if (command.getParent() != null)
return false;

Token token = command.getFirstCodeToken();
if (token == null) // pro forma
return false;

if (token.isAnyKeyword("WHERE", "HAVING")) {
Token lastKeyword = token;
Token clauseEnd = null; // Command.canAddToDdl() starts a new Command after the WHERE / HAVING clause
DdlLineBreak lineBreak = getLineBreak(DdlLineBreakWithoutNever.forValue(configBreakBeforeWhereEtc.getValue()));

return executeOn(command, token, lastKeyword, clauseEnd, lineBreak, configWhereEtcIndent.getValue(), false);

} else if (token.matchesOnSiblings(true, "GROUP", "BY")) {
Token lastKeyword = token.getNextCodeSibling(); // BY
Token clauseEnd = null; // Command.canAddToDdl() starts a new Command after the GROUP BY clause
DdlLineBreak lineBreak = getLineBreak(DdlLineBreakWithoutNever.forValue(configBreakBeforeWhereEtc.getValue()));

return executeOn(command, token, lastKeyword, clauseEnd, lineBreak, configWhereEtcIndent.getValue(), false);

} else if (token.isAnyKeyword("EXCEPT", "INTERSECT", "UNION")) {
Token lastKeyword = token.matchesOnSiblings(true, "UNION", "ALL") ? token.getNextCodeSibling() : token;
Token clauseEnd = token.getLastTokenOnSiblings(true, TokenSearch.ASTERISK, "SELECT");
DdlLineBreak lineBreak = getLineBreak(DdlLineBreakWithoutNever.forValue(configBreakBeforeUnionEtc.getValue()));

Command prevCommand = command.getPrevNonCommentCommand();
boolean isAfterSelectList = (token.getPrevCodeSibling() == null && prevCommand != null && prevCommand.firstCodeTokenTextEqualsAny(DDL.BRACE_CLOSE_STRING));
boolean enforceEmptyLine = !isAfterSelectList;

return executeOn(command, token, lastKeyword, clauseEnd, lineBreak, configUnionEtcIndent.getValue(), enforceEmptyLine);

} else {
return false;
}
}

private boolean executeOn(Command command, Token clauseStart, Token lastKeyword, Token clauseEnd, DdlLineBreak lineBreak, int newIndent, boolean enforceEmptyLine) {
boolean changed = false;
int oldLastKeywordIndex = lastKeyword.getStartIndexInLine();

// break before clause, as configured
changed |= breakBefore(clauseStart, lineBreak, enforceEmptyLine, newIndent);

// condense keyword collocations such as 'GROUP BY' and 'UNION ALL'
if (lastKeyword != clauseStart)
changed |= condense(clauseStart, lastKeyword);


// move the rest of the clause along with how the last keyword was moved horizontally
int addIndent = lastKeyword.getStartIndexInLine() - oldLastKeywordIndex;
if (addIndent != 0)
changed |= command.addIndent(addIndent, 0, lastKeyword.getNext(), clauseEnd);

return changed;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@
import com.sap.adt.abapcleaner.rulehelpers.RuleForDdlPosition;

public class DdlPositionSelectRule extends RuleForDdlPosition {
final static String defaultDescription = "Break before AS SELECT etc.";
private final static RuleReference[] references = new RuleReference[] { new RuleReference(RuleSource.ABAP_CLEANER) };

@Override
public RuleID getID() { return RuleID.DDL_POSITION_SELECT; }

@Override
public String getDisplayName() { return "Break before AS SELECT etc."; }
public String getDisplayName() { return defaultDescription; }

@Override
public String getDescription() { return "Standardizes line breaks and indentation before entity name, WITH PARAMETERS, [AS] SELECT, AS PROJECTION ON and data source."; }
Expand Down Expand Up @@ -74,8 +75,6 @@ public String getExample() {
+ LINE_SEP + "}";
}

private final String[] lineBreakSelection = new String[] { "Always", "Keep as is", "Never" };

final ConfigEnumValue<DdlLineBreak> configBreakBeforeEntityName = new ConfigEnumValue<DdlLineBreak>(this, "BreakBeforeEntityName", "Break before entity name:", lineBreakSelection, DdlLineBreak.values(), DdlLineBreak.NEVER);
final ConfigIntValue configEntityNameIndent = new ConfigIntValue(this, "EntityNameIndent", "Indent if breaking:", "", 0, 2, MAX_INDENT);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1271,8 +1271,8 @@ void testGetLastTokenOfDdlLogicalExpression() {
assertLastTokenOfLogExpr("define view entity I_Any as select from dtab { Any, case when fld = case when other_fld = 'a' then 1 else 2 end then 0 else 1 end as Other }", "when", "then");

assertLastTokenOfLogExpr("define view entity I_Any as select from dtab { Any } where fld = 1 and other_fld is initial", "where", null);
assertLastTokenOfLogExpr("define view entity I_Any as select from dtab { Any } where fld = 1 and other_fld not between 2 and 3 union all select from dtab2 { Any }", "where", "union");
assertLastTokenOfLogExpr("define view entity I_Any as select from dtab { Any } where fld = 1 group by Any", "where", "group");
assertLastTokenOfLogExpr("define view entity I_Any as select from dtab { Any } where fld = 1 and other_fld not between 2 and 3 union all select from dtab2 { Any }", "where", null);
assertLastTokenOfLogExpr("define view entity I_Any as select from dtab { Any } where fld = 1 group by Any", "where", null);

assertLastTokenOfLogExpr("define custom entity I_Any { _assoc : association[1:1] to I_Other on _assoc.fld = I_Any.fld; }", "on", ";");

Expand Down
Loading

0 comments on commit 4428678

Please sign in to comment.