Skip to content

Commit

Permalink
fix Parser for identifiers with namespaces (#359)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmgrassau committed Sep 19, 2024
1 parent 163c524 commit d982d2c
Show file tree
Hide file tree
Showing 12 changed files with 241 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2634,6 +2634,7 @@ public String toHeaderLine() {
sb.append('\t').append("added configuration count");
sb.append('\t').append("total rule count");
sb.append('\t').append("total configuration count");
sb.append('\t').append("options per rule");
sb.append('\t').append("added rule names");
sb.append('\t').append("added technical rule names");
sb.append('\t').append("added configuration names");
Expand All @@ -2646,6 +2647,8 @@ public String toLine() {
sb.append('\t').append(String.valueOf(addedConfigCount));
sb.append('\t').append(String.valueOf(totalRuleCount));
sb.append('\t').append(String.valueOf(totalConfigCount));
double configPerRule = (totalRuleCount == 0) ? 0 : totalConfigCount / (double)totalRuleCount;
sb.append('\t').append(Cult.format(configPerRule, 3));
sb.append('\t').append(String.valueOf(addedRuleNames));
sb.append('\t').append(String.valueOf(addedTechnicalRuleNames));
sb.append('\t').append(String.valueOf(addedConfigNames));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public final class DDL {

public final static String[] listElementSeparators = new String[] { COMMA_SIGN_STRING, SEMICOLON_SIGN_STRING };
public final static String[] arithmeticOperators = new String[] { "+", "-", "*", "/" };
public final static String divisionOperator = "/";
public final static String[] aggregationFunctions = new String[] { "max", "min", "avg", "sum", "count" };

private static class Collocation {
Expand Down Expand Up @@ -505,7 +506,7 @@ public static boolean isCharAllowedForIdentifier(String text, int pos, boolean i

char c = text.charAt(pos);

if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_') {
if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_' || c == '/') { // '_' at start is allowed for aliases, but e.g. not for fields
return true;

} else if (isFirstChar && (c == '$' || c == '#' || c == '@')) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,25 @@ public static int indexOfAny(String string, char[] anyOf, int startIndex) {
return -1;
}

public static int indexOfAny(String string, char[] anyCharOf, String[] anyStringOf, int startIndex) {
if (string == null)
return -1;

int index = startIndex;
while (index < string.length()) {
char c = string.charAt(index);
for (char match : anyCharOf) {
if (c == match) {
return index;
}
}
if (anyStringOf != null && containsAnyAt(string, index, anyStringOf))
return index;
++index;
}
return -1;
}

public static int indexOfAny(String string, char[] anyOf, int startIndex, char escapeChar) {
if (string == null)
return -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2648,7 +2648,7 @@ public final String getDefinedName() {
if (firstCode == null || !firstCode.isKeyword())
return null;
Token token = firstCode.getNextCodeSibling();
if (token.isChainColon())
while (token.isChainColon())
token = token.getNextCodeSibling();
return (token == null || !token.isIdentifier()) ? null : token.getText();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1577,8 +1577,7 @@ final TextBit[] toTextBits(int startIndex, boolean isInOOContext) {
if (i > 0 && isIdentifier != lastWasIdentifier) {
// if the text bit is not part of the keyword or the identifier, it is considered a token operator (e.g. "->", "=>", "(" etc.)
ColorType bitType = lastWasIdentifier ? colType : opColorType;
// in some cases, a Token of type OTHER_OP contains a number, e.g. "ULINE AT /10(20)." and "ULINE AT 10(**)."
if (isDdlOrDcl && AbapCult.stringEquals(text.substring(writtenPos, i), "$projection", true))
if (isDdlOrDcl && AbapCult.stringEqualsAny(true, text.substring(writtenPos, i), DDL.PROJECTION_PREFIX, DDL.PARAMETER_PREFIX))
bitType = ColorType.DDL_KEYWORD;
if (isAbap && ABAP.isInteger(text.substring(writtenPos, i)))
bitType = ColorType.NUMBER;
Expand Down Expand Up @@ -2114,8 +2113,14 @@ private MemoryAccessType determineMemoryAccessType() {

// non-inline declarations
if (firstToken.isAnyKeyword(Command.declarationKeywordsReservingMemory)) {
if (prevToken.isAnyKeyword(Command.declarationKeywords) || prevToken.isComma()) {
if (prevToken.isAnyKeyword(Command.declarationKeywords) || prevToken.isComma()) {
return ABAP.isFieldSymbol(text) ? MemoryAccessType.ASSIGN_TO_FS_OR_DREF : MemoryAccessType.WRITE;
} else if (prevToken.isIdentifier() && prevToken.getOpensLevel() && prevPrevToken != null && (prevPrevToken.isAnyKeyword(Command.declarationKeywords) || prevPrevToken.isComma() || prevPrevToken.isChainColon())) {
// e.g. "lc_length" in "DATA lv_text(lc_length) VALUE 'abcde'."
return MemoryAccessType.READ;
} else if (prevToken.isAnyKeyword("LENGTH", "VALUE")) { // constants can be used for both
// e.g. "lc_length" and "lc_text" in "DATA lv_text TYPE c LENGTH lc_length VALUE lc_text."
return MemoryAccessType.READ;
} else { // e.g. a class name after TYPE REF TO
return MemoryAccessType.NONE;
}
Expand Down Expand Up @@ -3680,4 +3685,16 @@ public boolean isDdlTypeName() {

return false;
}

/** Returns true if this Token can be followed by an arithmetic operator.
* Note that the '-' for negative numbers in NOT considered an arithmetic operator here. */
public boolean mayBeFollowedByArithmeticOp() {
if (textEqualsAny(DDL.PARENS_CLOSE_STRING, DDL.BRACKET_CLOSE_STRING)) {
return true;
} else if (type == TokenType.IDENTIFIER || type == TokenType.LITERAL) {
return true;
} else { // KEYWORDS, ASSIGNMENT_OP, OTHER_OP, COMMA, "("
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@ public class Tokenizer {

private final static char[] abapTokenEndChars = new char[] { ' ', '\u00A0', '\r', '\n', ABAP.COMMA_SIGN, ABAP.DOT_SIGN, ABAP.QUOT_MARK, ABAP.QUOT_MARK2, ABAP.COMMENT_SIGN, ABAP.COLON_SIGN, '(', ')' };
private final static String abapTokenEndCharsToIncludeInToken = "(";
private final static char[] ddlTokenEndChars = new char[] { ' ', '\u00A0', '\r', '\n', DDL.COMMENT_SIGN, DDL.COLON_SIGN, DDL.QUOT_MARK, DDL.COMMA_SIGN, DDL.SEMICOLON_SIGN, DDL.PARENS_OPEN, DDL.PARENS_CLOSE, DDL.BRACE_OPEN, DDL.BRACE_CLOSE, DDL.BRACKET_OPEN, DDL.BRACKET_CLOSE, '<', '>', '=', '+', '-', '*', '/' };
private final static char[] ddlNumericTokenEndChars = new char[] { ' ', '\u00A0', '\r', '\n', DDL.COLON_SIGN, DDL.QUOT_MARK, DDL.COMMA_SIGN, DDL.SEMICOLON_SIGN, DDL.PARENS_OPEN, DDL.PARENS_CLOSE, DDL.BRACE_OPEN, DDL.BRACE_CLOSE, DDL.BRACKET_OPEN, DDL.BRACKET_CLOSE, '<', '>', '=', '+', '-', '*', '/' };
private final static char[] ddlIdentifierEndChars = new char[] { ' ', '\u00A0', '\r', '\n', DDL.COLON_SIGN, DDL.QUOT_MARK, DDL.COMMA_SIGN, DDL.SEMICOLON_SIGN, DDL.PARENS_OPEN, DDL.PARENS_CLOSE, DDL.BRACE_OPEN, DDL.BRACE_CLOSE, DDL.BRACKET_OPEN, DDL.BRACKET_CLOSE, '<', '>', '=', '+', '-', '*' };
private final static String[] ddlIdentifierEndStrings = new String[] { DDL.LINE_END_COMMENT, DDL.ASTERISK_COMMENT_START };
private final static String ddlComparisonOpChars = "<>!=";
private final static String ddlOtherOpChars = "+=*/";
private final static char ddlArithmeticOpDiv = '/';
private final static String ddlArithmeticOpCharsWithoutDiv = "+-*";
private final static char[] nonAbapTokenEndChars = new char[] { '\r', '\n', ABAP.QUOT_MARK, ABAP.COMMENT_SIGN };

private final static char[] stringTemplateEndChars = new char[] { ABAP.BRACE_OPEN, ABAP.PIPE };
Expand Down Expand Up @@ -124,7 +127,7 @@ Token getNext() throws UnexpectedSyntaxException {

} else if (StringUtil.containsAt(text, readPos, DDL.LINE_END_COMMENT) || StringUtil.containsAt(text, readPos, DDL.LINE_END_MINUS_COMMENT)) {
// line end comment: the rest of the line
tokenText = readDdlUntil(lineFeedChars);
tokenText = readDdlUntil(lineFeedChars, null);

} else if (curChar == DDL.QUOT_MARK) {
// read text literal '...', considering escape char ''
Expand All @@ -140,13 +143,22 @@ Token getNext() throws UnexpectedSyntaxException {
// comparison operator or => operator
tokenText = StringUtil.readTillEndOfAllowedChars(text, readPos, ddlComparisonOpChars);

} else if (ddlOtherOpChars.indexOf(curChar) >= 0) {
} else if (Character.isDigit(curChar)) { // || curChar == '-' && readPos + 1 < text.length() && Character.isDigit(text.charAt(readPos + 1))) {
// number, which may start with - and may include . (however, -.5 is not possible)
// unlike identifiers, a number stops at / (and thus automatically at // and /*, therefore no need to supply ddlIdentifierEndStrings)
tokenText = readDdlUntil(ddlNumericTokenEndChars, null);

} else if (curChar == ddlArithmeticOpDiv && !DDL.isCharAllowedForIdentifier(text, readPos + 1, true)) {
// '/' is only a division operator if it is NOT directly followed by a char that could start an identifier
tokenText = Character.toString(curChar);

} else if (ddlArithmeticOpCharsWithoutDiv.indexOf(curChar) >= 0) {
// arithmetic operator (just one char)
tokenText = Character.toString(curChar);

} else {
// normal word, which includes chars from 0-9, a-z, A-Z, as well as ._@# (and < after initial @)
tokenText = readDdlUntil(ddlTokenEndChars);
// normal word, which includes chars from 0-9, a-z, A-Z, / for namespaces, ._@# and < after initial @
tokenText = readDdlUntil(ddlIdentifierEndChars, ddlIdentifierEndStrings);
}

} else if (curChar == ABAP.LINE_COMMENT_SIGN && isAtLineStart) {
Expand Down Expand Up @@ -280,15 +292,15 @@ else if (includeDelimiters != null && includeDelimiters.indexOf(text.charAt(toke
return text.substring(readPos, tokenEnd);
}

private String readDdlUntil(char[] delimiterChars) throws UnexpectedSyntaxException {
private String readDdlUntil(char[] delimiterChars, String[] delimiterStrings) throws UnexpectedSyntaxException {
// read until the first delimiter is found
// in the special case of an initial @, < is allowed afterwards, because annotations for CDS entities
// (except for CDS view entities) may start with @< for the annotation after a list element in a comma-separated or semicolon-separated list.
int start = readPos + 1;
if (text.charAt(readPos) == DDL.ANNOTATION_SIGN && start < text.length() && text.charAt(start) == '<') {
++start;
}
int tokenEnd = StringUtil.indexOfAny(text, delimiterChars, start);
int tokenEnd = StringUtil.indexOfAny(text, delimiterChars, delimiterStrings, start);

if (tokenEnd < 0)
tokenEnd = text.length();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,15 @@ private int executeOnDeclarationCommand(Command command, Command methodStart, Lo
}

private Token executeOnDeclarationLine(Command methodStart, LocalVariables localVariables, Token token, boolean isInOOContext, VariableInfo varInfo) {
// length may be supplied in parentheses: "DATA lv_text(lc_length)."
if (token.getOpensLevel() && token.hasChildren() && token.getFirstChild().isAttached() && token.getFirstChild().isIdentifier()) {
String usedObjectName = LocalVariables.getObjectName(token.getFirstChild().getText(), isInOOContext);
localVariables.addUsageInLikeOrValueClause(token, usedObjectName, methodStart, varInfo);

// continue behind the parenthesis
token = token.getNextCodeSibling();
}

// if the declaration uses "LIKE ...", count that as a usage of that variable or constant
Token next = token.getNextCodeSibling();
if (next != null && next.isKeyword("LIKE") && next.getNextCodeSibling() != null) {
Expand All @@ -406,17 +415,16 @@ private Token executeOnDeclarationLine(Command methodStart, LocalVariables local

}

// if the declaration uses "VALUE <constant>", count that as a usage of <constant>
// if the declaration uses "LENGTH <constant>" and/or "VALUE <constant>", count that as a usage of <constant>
while (token != null && !token.isCommaOrPeriod()) {
if (token.isKeyword("VALUE")) {
if (token.isAnyKeyword("LENGTH", "VALUE")) {
token = token.getNextCodeSibling();
if (token != null && token.isIdentifier()) {
// the Token may contain more than the object name, e.g. 'DATA lv_value TYPE i VALUE if_any_interface=>co_any_value.'
// however, no calculations (lc_any + 1) or substrings (lc_data+4(2)) are possible
String usedObjectName = LocalVariables.getObjectName(token.getText(), isInOOContext);
localVariables.addUsageInLikeOrValueClause(token, usedObjectName, methodStart, varInfo);
}
break;
}
token = token.getNextCodeSibling();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.sap.adt.abapcleaner.base.ABAP;
import com.sap.adt.abapcleaner.base.AbapCult;
import com.sap.adt.abapcleaner.base.StringUtil;

/** encapsulates class or interface definition information: implemented interfaces, defined aliases and method signatures */
public class ClassInfo {
Expand Down Expand Up @@ -39,6 +40,9 @@ public void addMethod(MethodInfo method) {

/** @param methodName - can be a plain method name, an alias, or INTERFACE~METHOD */
public MethodInfo getMethod(String methodName) {
if (StringUtil.isNullOrEmpty(methodName))
return null;

// find the method signature in the class hierarchy (if visible in the code document)
ClassInfo curClass = this;
do {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public String getExample() {
+ LINE_SEP + " -- built-in functions"
+ LINE_SEP + " concat(AnyText,concat('_' ,OtherText)) as AnyTextField,"
+ LINE_SEP + " division(AnyArg *10,OtherArg,2) as ThirdValue,"
+ LINE_SEP + " round(AnyValue/ 100, 5) as RoundedValue,"
+ LINE_SEP + " round(AnyValue* 100, 5) as RoundedValue,"
+ LINE_SEP + ""
+ LINE_SEP + " //you may format commas in ABAP types in a different way,"
+ LINE_SEP + " // because these are not built-in functions"
Expand Down Expand Up @@ -189,14 +189,15 @@ private boolean executeOn(Command command, Token token) {
changed |= token.setSpacesLeftAdjustingIndent(spaces, false);
}

// space before arithmetic operator (+ - * /)
// space before arithmetic operators (+ - * /)
Token prevToken = token.getPrev();
if (prevToken == null && command.getPrev() != null) {
// for list elements, commas belong to the previous Command
prevToken = command.getPrev().getLastToken();
}
Token nextToken = token.getNext();
if (spaceAroundArithmeticOps != ChangeType.KEEP_AS_IS && token.textEqualsAny(DDL.arithmeticOperators)) {
Token prevCode = token.getPrevCodeToken();
if (spaceAroundArithmeticOps != ChangeType.KEEP_AS_IS && token.textEqualsAny(DDL.arithmeticOperators) && prevCode != null && prevCode.mayBeFollowedByArithmeticOp()) {
if (token.textEquals("*") && token.isChildOfAny("ASSOCIATION", "COMPOSITION")) {
// ignore "association [1..*]", "composition [0..*]" etc., because a space between .. and * is a syntax error
} else if (token.textEquals("*") && prevToken != null && prevToken.textEndsWith(".")) { // '!= null' pro forma
Expand All @@ -205,6 +206,9 @@ private boolean executeOn(Command command, Token token) {
// ignore path expression "_Any[*:...]"
} else if (token.textEquals("*") && prevToken != null && prevToken.textEndsWith("(") && nextToken.textStartsWith(")")) { // '!= null' pro forma
// ignore "count(*)"
} else if (token.textEquals(DDL.divisionOperator) && spaceAroundArithmeticOps == ChangeType.NEVER) {
// Spaces around "/" are NOT removed, because attaching "/" to an identifier would be a syntax error, since "/" can be used
// for namespaces: /ANY/Identifier. While "2/" is not a syntax error, it still causes red highlighting in ADT and should therefore be avoided, too.
} else {
int spaces = (spaceAroundArithmeticOps == ChangeType.ALWAYS) ? Math.max(token.spacesLeft, 1) : 0;
changed |= token.setSpacesLeftAdjustingIndent(spaces, true);
Expand All @@ -225,7 +229,10 @@ private boolean executeOn(Command command, Token token) {
changeType = (parentPrev != null && parentPrev.textStartsWith(DDL.TYPED_LITERAL_PREFIX)) ? spaceAfterCommaInAbapType : spaceAfterComma;

} else if (prevToken.textEqualsAny(DDL.arithmeticOperators)) {
changeType = spaceAroundArithmeticOps;
Token prevPrevCode = prevToken.getPrevCodeToken();
if (prevPrevCode != null && prevPrevCode.mayBeFollowedByArithmeticOp()) {
changeType = spaceAroundArithmeticOps;
}
}

if (changeType != ChangeType.KEEP_AS_IS) {
Expand All @@ -237,6 +244,9 @@ private boolean executeOn(Command command, Token token) {
// ignore star in parenthesis as in "(*)"
} else if (prevToken.isDdlParameterColon()) {
// ignore colon that starts a parameter name such as ":P_Any"
} else if (prevToken.textEquals(DDL.divisionOperator) && changeType == ChangeType.NEVER) {
// spaces around "/" are not removed, because attaching "/" to an identifier is a syntax error, since "/" can be used for namespaces: /ANY/Identifier
// while "2/" is not a syntax error, it still causes red highlighting in ADT and should therefore be avoided, too
} else {
int spaces = (changeType == ChangeType.ALWAYS) ? 1 : 0;
changed |= token.setSpacesLeftAdjustingIndent(spaces, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2034,6 +2034,30 @@ void testTokenTypeDdlIdentifierVersusKeyword() {
assertEquals(TokenType.KEYWORD, findToken("right").type);
}

@Test
void testTokenTypeDdlNamespaces() {
buildCommand("define view entity /ANY/I_AnyView as select from /ANY/I_OtherView as /ANY/A/l/i/a/s/ { key /ANY/A/l/i/a/s/./ANY/Field/ as /ANY/Field/ }");

// ensure that identifiers with namespaces are correctly tokenized and parsed
assertEquals(TokenType.IDENTIFIER, findToken("/ANY/I_AnyView").type);
assertEquals(TokenType.IDENTIFIER, findToken("/ANY/I_OtherView").type);
assertEquals(TokenType.IDENTIFIER, findToken("/ANY/A/l/i/a/s/").type);
assertEquals(TokenType.IDENTIFIER, findToken("/ANY/A/l/i/a/s/./ANY/Field/").type);
assertEquals(TokenType.IDENTIFIER, findToken("/ANY/Field/").type);
}

@Test
void testTokenTypeDdlNumber() {
buildCommand("define view entity I_AnyView as select from I_OtherView { -1 as AnyField, 1.5 as OtherField, (-2.5-3.5) as ThirdField }");

// ensure that (positive and negative) numbers with decimals are correctly tokenized and parsed
assertEquals(TokenType.LITERAL, findToken("1").type);
assertEquals(TokenType.LITERAL, findToken("1.5").type);
assertEquals(TokenType.LITERAL, findToken("2.5").type);
assertEquals(TokenType.LITERAL, findToken("3.5").type);
assertEquals(TokenType.OTHER_OP, findToken("-").type);
}

@Test
void testTokenTypeDdlCardinality() {
buildCommand("define view I_Any as select from dtab as d1 association [1..*] to dtab2 as d2 on d1.id = d2.id { Any }");
Expand Down
Loading

0 comments on commit d982d2c

Please sign in to comment.