Skip to content

Commit

Permalink
fix AlignDeclarationsRule for mixed TYPE clauses with tables (SAP#186)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jmgrassau
jmgrassau committed Nov 13, 2023
1 parent 9914319 commit 2b2a5bb
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 18 deletions.
3 changes: 3 additions & 0 deletions com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/base/ABAP.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ public static enum SyField {
"tstmp_add_seconds(", "tstmp_current_utctimestamp(", "tstmp_is_valid(", "tstmp_seconds_between(", "tstmp_to_dats(", "tstmp_to_dst(", "tstmp_to_tims(", "tstmpl_from_utcl(", "tstmpl_to_utcl(", "unit_conversion(", "upper(", "utcl_add_seconds(",
"utcl_current(", "utcl_seconds_between(", "uuid(", "weekday(" };

public final static String[] builtInAbapTypes = new String[] { "b", "c", "d", "decfloat16", "decfloat34", "f", "i", "int8", "n", "p", "s", "string", "t", "utclong", "x", "xstring" };
public final static String[] builtInDdicTypes = new String[] { "abap_bool", "abap_boolean", "d16n", "d34n", "datn", "geom_ewkb", "ggm1", "int1", "int2", "int4", "timn", "utcl" };


/** all built-in functions, cp. https://help.sap.com/doc/abapdocu_latest_index_htm/latest/en-US/abenbuilt_in_functions_overview.htm */
public final static String[] builtInFunctions = new String[] {"boolc(", "boolx(", "xsdbool(", "contains(", "contains_any_of(", "contains_any_not_of(", "matches(", "line_exists(", "abs(", "ceil(", "floor(", "frac(", "sign(", "trunc(", "ipow(", "nmax(", "nmin(", "acos(", "asin(", "atan(", "cos(", "sin(", "tan(", "cosh(", "sinh(", "tanh(", "exp(", "log(", "log10(", "sqrt(", "round(", "rescale(", "charlen(", "dbmaxlen(", "numofchar(", "strlen(", "char_off(", "cmax(", "cmin(", "count(", "count_any_of(", "count_any_not_of(", "distance(", "condense(", "concat_lines_of(", "escape(", "find(", "find_end(", "find_any_of(", "find_any_not_of(", "insert(", "match(", "repeat(", "replace(", "reverse(", "segment(", "shift_left(", "shift_right(", "substring(", "substring_after(", "substring_from(", "substring_before(", "substring_to(", "to_upper(", "to_lower(", "to_mixed(", "from_mixed(", "translate(", "xstrlen(", "bit-set(", "utclong_current(", "utclong_add(", "utclong_diff(", "lines(", "line_index("};
Expand Down
49 changes: 33 additions & 16 deletions com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/parser/Obfuscator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ private IdentifierType(String infix, String shortInfix) {
}
}

private final static String getKey(String identifier) {
return identifier.toUpperCase();
}

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

private final boolean methodScope;
Expand All @@ -68,6 +72,7 @@ private IdentifierType(String infix, String shortInfix) {
private final boolean obfuscateLiterals;

private HashMap<String, String> types = new HashMap<>();
private HashMap<String, String> builtInTypes = new HashMap<>();
private HashMap<String, String> variables = new HashMap<>();
private HashMap<String, String> components = new HashMap<>();
private HashMap<String, String> classes = new HashMap<>();
Expand All @@ -89,6 +94,16 @@ private void clearMaps(boolean clearMethods, boolean clearClasses) {
variables.clear();
components.clear();

// only initialize builtInTypes the first time
if (builtInTypes.isEmpty()) {
for (String builtInAbapType : ABAP.builtInAbapTypes) {
builtInTypes.put(getKey(builtInAbapType), builtInAbapType);
}
for (String builtInDdicType : ABAP.builtInDdicTypes) {
builtInTypes.put(getKey(builtInDdicType), builtInDdicType);
}
}

stringLiterals.clear();
stringLiterals.put(" ", " ");
stringLiterals.put("X", "X");
Expand Down Expand Up @@ -209,13 +224,13 @@ else if (condenseTillLineEnd && !token.isAttached())
boolean oldTextMayBeVarName = ABAP.mayBeVariableName(oldText, false);
boolean prevIfFirstCode = (prevCode == command.getFirstCodeToken());
if (prevCode != null && prevCode.isAnyKeyword("METHOD", "METHODS") && oldTextMayBeVarName) {
newText = getNewNameFor(oldText, methods, "", IdentifierType.METHOD);
newText = getNewNameFor(getKey(oldText), methods, "", IdentifierType.METHOD);

} else if (prevCode != null && prevCode.isKeyword("CLASS") && prevIfFirstCode && oldTextMayBeVarName) {
newText = getTypeName(oldText, classes, "cl_");
newText = getTypeName(oldText, classes, "cl_", false);

} else if (prevCode != null && prevCode.isKeyword("INTERFACE") && prevIfFirstCode && oldTextMayBeVarName) {
newText = getTypeName(oldText, classes, "if_");
newText = getTypeName(oldText, classes, "if_", false);

} else {
newText = obfuscateIdentifier(token);
Expand Down Expand Up @@ -246,7 +261,7 @@ private String obfuscateIdentifier(Token token) {
boolean isLastBit = (i + 1 == bits.size());
String nextBit = isLastBit ? "" : bits.get(i + 1);
String newBit = null;

sbKey.append(bit);

// if chains like 'lo_object->if_any~method(' shall be removed, simply skip if~, cl_any=>, cl_any->, struc- and proceed with the rest of the identifier
Expand All @@ -255,7 +270,7 @@ private String obfuscateIdentifier(Token token) {
continue;
}
// if chains are removed, the key must consist of the whole chain, so 'cl_any=>method(' gets a different identifier than 'cl_other=>method('
String key = simplify ? sbKey.toString() : bit;
String key = getKey(simplify ? sbKey.toString() : bit);

if (ABAP.isFieldSymbol(bit)) {
newBit = ABAP.FIELD_SYMBOL_START_STRING + getVariableName(bit.substring(1, bit.length() - 1), variables, "ls_") + ABAP.FIELD_SYMBOL_END_STRING;
Expand All @@ -273,7 +288,7 @@ private String obfuscateIdentifier(Token token) {
newBit = getNewNameFor(key, classes, "if_", IdentifierType.INTERFACE);

} else if (nextBit.equals("=>")) {
newBit = getTypeName(bit, classes, "cl_");
newBit = getTypeName(bit, classes, "cl_", false);

} else if (nextBit.equals("->")) {
if (AbapCult.stringEquals(bit, "me", true)) {
Expand All @@ -284,14 +299,14 @@ private String obfuscateIdentifier(Token token) {

} else if (nextBit.equals("(") && nextCode != null && !nextCode.isAttached()) {
if (token.isTypeIdentifier()) {
newBit = getTypeName(bit, types, "ty_");
newBit = getTypeName(bit, types, "ty_", false);
} else {
newBit = getNewNameFor(key, methods, "", IdentifierType.METHOD);
}

} else if (prevBit.equals("~")) {
if (token.isTypeIdentifier()) {
newBit = getTypeName(bit, types, "ty_");
newBit = getTypeName(bit, types, "ty_", false);
} else {
newBit = getNewNameFor(key, methods, "", IdentifierType.METHOD);
}
Expand Down Expand Up @@ -326,7 +341,7 @@ private String obfuscateIdentifier(Token token) {

} else {
if (token.isTypeIdentifier()) {
newBit = getTypeName(bit, types, "ty_");
newBit = getTypeName(bit, types, "ty_", (bits.size() == 1));
} else if (simplify && sb.length() == 0) {
newBit = getVariableName(bit, variables, "lv_", key);
} else {
Expand All @@ -339,7 +354,7 @@ private String obfuscateIdentifier(Token token) {
}

private String getVariableName(String oldText, HashMap<String, String> variables, String defaultPrefix) {
return getVariableName(oldText, variables, defaultPrefix, oldText);
return getVariableName(oldText, variables, defaultPrefix, getKey(oldText));
}

private String getVariableName(String oldText, HashMap<String, String> variables, String defaultPrefix, String key) {
Expand Down Expand Up @@ -395,14 +410,16 @@ private String getVariableName(String oldText, HashMap<String, String> variables
return getNewNameFor(key, variables, prefix, identifierType);
}

private String getTypeName(String oldText, HashMap<String, String> types, String defaultPrefix) {
return getTypeName(oldText, types, defaultPrefix, oldText);
private String getTypeName(String oldText, HashMap<String, String> types, String defaultPrefix, boolean mayBeBuiltInType) {
return getTypeName(oldText, types, defaultPrefix, mayBeBuiltInType, getKey(oldText));
}

private String getTypeName(String oldText, HashMap<String, String> types, String defaultPrefix, String key) {
if (types.containsKey(oldText)) {
return types.get(oldText);
}
private String getTypeName(String oldText, HashMap<String, String> types, String defaultPrefix, boolean mayBeBuiltInType, String key) {
if (mayBeBuiltInType && builtInTypes.containsKey(key))
return builtInTypes.get(key);
if (types.containsKey(key))
return types.get(key);


String prefix = determinePrefix(oldText, defaultPrefix, typePrefixes);

Expand Down
10 changes: 8 additions & 2 deletions ...ap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/alignment/AlignDeclarationsRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,11 @@ private Token readDeclarationLine(AlignLine line, Token token, int additionalInd
// (not specifically considered here, but probably irrelevant, as they are skipped): [tabkeys], [INITIAL SIZE n] [WITH HEADER LINE]
Token typeStart = token;
Token typeEnd = token;
boolean isTable = false;
while (typeEnd != null && !typeEnd.isAnyKeyword("LENGTH", "DECIMALS", "VALUE", "READ-ONLY") && !typeEnd.textEqualsAny(".", ",")) {
if (typeEnd.isKeyword("TABLE"))
isTable = true;

// 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" and "WITH [UNIQUE | NON-UNIQUE] KEY comp1 [comp2 [comp3]]"
if (typeEnd.isKeyword("ASSOCIATION")
Expand All @@ -574,13 +578,15 @@ private Token readDeclarationLine(AlignLine line, Token token, int additionalInd
// only align up to (but excluding) "WITH" or "ASSOCIATION", thus leaving the WITH or ASSOCIATION section(s) unchanged
// and keeping possible line breaks as well as manual alignment
Term typeInfo = Term.createForTokenRange(typeStart, typeEnd.getPrev());
line.setCell(Columns.TYPE.getValue(), new AlignCellTerm(typeInfo));
line.setCell(Columns.TYPE.getValue(), AlignCellTerm.createSpecial(typeInfo, 0, true));
return typeEnd.getLastTokenOnSiblings(true, TokenSearch.ASTERISK, ".|,");
}
typeEnd = typeEnd.getNextSibling();
}
// for TYPE ... TABLE OF ..., override text width with 1 to avoid expanding the TYPE column with this cell
Term typeInfo = Term.createForTokenRange(typeStart, typeEnd.getPrev());
line.setCell(Columns.TYPE.getValue(), new AlignCellTerm(typeInfo));
newCell = AlignCellTerm.createSpecial(typeInfo, 0, isTable);
line.setCell(Columns.TYPE.getValue(), newCell );
token = typeEnd;

// the remaining components only appear in DATA and TYPES, not with FIELD-SYMBOLS
Expand Down
19 changes: 19 additions & 0 deletions ...t.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignDeclarationsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1587,4 +1587,23 @@ void testMoveOverlengthValuesBelowNameOrTypeAlignNameOnly() {
testRule();
}


@Test
void testTypeTableAndLengthColumn() {
// ensure that the declaration TYPE STANDARD TABLE OF ... does NOT expand the TYPE column for the other declarations

buildSrc("DATA: gt_any_table TYPE STANDARD TABLE OF ty_any_type,");
buildSrc(" gv_any_value TYPE decfloat16 LENGTH 2 DECIMALS 3,");
buildSrc(" gv_other_value TYPE p LENGTH 2 DECIMALS 0,");
buildSrc(" gv_third_value TYPE abap_bool.");

buildExp("DATA: gt_any_table TYPE STANDARD TABLE OF ty_any_type,");
buildExp(" gv_any_value TYPE decfloat16 LENGTH 2 DECIMALS 3,");
buildExp(" gv_other_value TYPE p LENGTH 2 DECIMALS 0,");
buildExp(" gv_third_value TYPE abap_bool.");

putAnyMethodAroundSrcAndExp();

testRule();
}
}

0 comments on commit 2b2a5bb

Please sign in to comment.