Skip to content

Commit

Permalink
fix UnusedVariablesRule for non-OO variables with special chars (#202)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmgrassau committed Nov 16, 2023
1 parent 3d415e4 commit 842dbaf
Show file tree
Hide file tree
Showing 19 changed files with 373 additions and 143 deletions.
111 changes: 87 additions & 24 deletions com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/base/ABAP.java
Original file line number Diff line number Diff line change
Expand Up @@ -426,11 +426,10 @@ public static boolean isAbapKeywordCollocationStart(String text) {
return !StringUtil.isNullOrEmpty(text) && abapKeywordCollocationStarts.contains(getAbapKeywordKey(text));
}

public static boolean isCharAllowedForVariableNames(char c) {
return isCharAllowedForVariableNames(c, false, false);
}

public static boolean isCharAllowedForVariableNames(char c, boolean isFirstChar, boolean allowFieldSymbols) {
public static boolean isCharAllowedForVariableNames(String text, int pos, boolean isFirstChar, boolean allowFieldSymbols, boolean isInOOContext) {
char c = text.charAt(pos);

// criteria valid both in the object-oriented context and outside of it:
if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_' || c == NAMESPACE_SIGN) {
return true;

Expand All @@ -439,21 +438,84 @@ public static boolean isCharAllowedForVariableNames(char c, boolean isFirstChar,

} else if (allowFieldSymbols && ((isFirstChar && c == '<') || c == '>')) {
return true;

} else if (c == '%' && pos + 1 < text.length() && text.charAt(pos + 1) == '_') {
// the special case of %_... is even allowed in an object-oriented context
return true;
}

} else {
// the remaining criteria are only valid in a non-object-oriented context:
if (isInOOContext) {
return false;

} else if (c == '%' || c == '$' || c == '*' || c == '?') {
return true;

} else if (isFirstChar && (c == '&' || c == '<')) {
return true;

} else if (!isFirstChar && c == '#') {
return true;

} else if (c == '>' && (pos + 1 == text.length() || !isCharAllowedForVariableNames(text, pos + 1, false, allowFieldSymbols, isInOOContext))) {
// '>' is only allowed as the very last char in a non-OO context
return true;

} else {
// * errors include the following ASCII chars: ! " ' ( ) + , . / : ; = @ [ \ ] ^ ` { | } ~ DEL
// (ASCII codes: 21, 22, 27, 28, 29, 2B, 2C, 2E, 2F, 3A, 3B, 3D, 40, 5B, 5C, 5D, 5E, 60, 7B, 7C, 7D, 7E, 7F)
// * '-' would produce a warning (but no error) for data objects or procedure parameters; however, this is
// currently NOT supported due to possible confusions with the structure component selector, as in 'sy-subrc' or 'struc-comp1'.
// For procedure names in a non-OO context, '-' would be fine, but those are NOT covered by this method anyway.
return false;
}
}

public static boolean isCharAllowedForTypeNames(char c, boolean isFirstChar) {
public static boolean isCharAllowedForTypeNames(String text, int pos, boolean isFirstChar, boolean isInOOContext) {
char c = text.charAt(pos);

// criteria valid both in the object-oriented context and outside of it:
if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_' || c == NAMESPACE_SIGN) {
return true;

} else if (!isFirstChar) {
} else if (!isFirstChar && (c >= '0' && c <= '9')) {
return true;

} else if (!isFirstChar && c == '=' && pos + 1 < text.length() && text.charAt(pos + 1) == '>') {
// type names may contain =>, e.g. 'IF_ANY_INTERFACE=>TY_S_ANY_STRUCTURE'
return (c >= '0' && c <= '9') || c == '=' || c == '>';
return true;

} else if (!isFirstChar && c == '>' && pos > 0 && text.charAt(pos - 1) == '=') {
// type names may contain =>, e.g. 'IF_ANY_INTERFACE=>TY_S_ANY_STRUCTURE'
return true;

} else if (c == '%' && pos + 1 < text.length() && text.charAt(pos + 1) == '_') {
// the special case of %_... is even allowed in an object-oriented context
return true;
}

} else {
// the remaining criteria are only valid in a non-object-oriented context:
if (isInOOContext) {
return false;

} else if (c == '%' || c == '$' || c == '*' || c == '?') {
return true;

} else if (isFirstChar && (c == '&' || c == '<')) {
return true;

} else if (!isFirstChar && c == '#') {
return true;

} else if (c == '>' && (pos + 1 == text.length() || !isCharAllowedForTypeNames(text, pos + 1, false, isInOOContext))) {
// '>' is only allowed as the very last char in a non-OO context
return true;

} else {
// * errors include the following ASCII chars: ! " ' ( ) + , . / : ; = @ [ \ ] ^ ` { | } ~ DEL
// (ASCII codes: 21, 22, 27, 28, 29, 2B, 2C, 2E, 2F, 3A, 3B, 3D, 40, 5B, 5C, 5D, 5E, 60, 7B, 7C, 7D, 7E, 7F)
// * also, for data types(!), '-' is never allowed
// * however, in ABAP cleaner, type names may contain / (NAMESPACE SIGN) as well as '=' and '>'
return false;
}
}
Expand Down Expand Up @@ -765,21 +827,22 @@ public static boolean isPositionAndLength(String text) {
}

public static String toVariableName(String name) {
// always assume an object-oriented context
final boolean isInOOContext = true;
if (name == null)
return "";
StringBuilder varName = new StringBuilder(name.length());
boolean isFirstChar = true;
char[] nameChars = name.toCharArray();
for (char c : nameChars) {
varName.append(isCharAllowedForVariableNames(c, isFirstChar, false) ? c : '_');
for (int pos = 0; pos < name.length(); ++pos) {
varName.append(isCharAllowedForVariableNames(name, pos, isFirstChar, false, isInOOContext) ? name.charAt(pos) : '_');
isFirstChar = false;
if (varName.length() >= MAX_VARIABLE_NAME_LENGTH)
break;
}
return AbapCult.toLower(varName.toString());
}

public static boolean mayBeVariableName(String name, boolean allowFieldSymbols) {
public static boolean mayBeVariableName(String name, boolean allowFieldSymbols, boolean isInOOContext) {
if (name == null || name.length() == 0)
return false;
if (name.length() > MAX_VARIABLE_NAME_LENGTH)
Expand All @@ -796,13 +859,13 @@ public static boolean mayBeVariableName(String name, boolean allowFieldSymbols)
boolean isFirstChar = true;
int namespaceLength = 0; // is set to > 0 while reading a namespace '/.../' within the identifier
while (pos < name.length()) {
char c = name.charAt(pos);
if (!isCharAllowedForVariableNames(c, isFirstChar, allowFieldSymbols))
if (!isCharAllowedForVariableNames(name, pos, isFirstChar, allowFieldSymbols, isInOOContext))
return false;

isFirstChar = false;

// ensure that in FIELD-SYMBOLS, < is only found at the beginning and > only at the end
char c = name.charAt(pos);
if ((c == FIELD_SYMBOL_START_SIGN && pos > 0) || (c == FIELD_SYMBOL_END_SIGN && pos != name.length() - 1))
return false;

Expand Down Expand Up @@ -848,28 +911,28 @@ public static boolean mayBeVariableName(String name, boolean allowFieldSymbols)
* @param allowFieldSymbols - true if field symbols are allowed as variables
* @return - the variable name that was identified, or null if start position exceeded line length
*/
public static String readTillEndOfVariableName(String line, int start, boolean allowFieldSymbols) {
public static String readTillEndOfVariableName(String line, int start, boolean allowFieldSymbols, boolean isInOOContext) {
if (line == null)
return null;
if (start >= line.length())
return null;
boolean isFirstChar = true;
int end = start;
while (end < line.length() && isCharAllowedForVariableNames(line.charAt(end), isFirstChar, allowFieldSymbols)) {
while (end < line.length() && isCharAllowedForVariableNames(line, end, isFirstChar, allowFieldSymbols, isInOOContext)) {
isFirstChar = false;
++end;
}
return line.substring(start, end);
}

public static String readTillEndOfTypeName(String line, int start) {
public static String readTillEndOfTypeName(String line, int start, boolean isInOOContext) {
int end = start;
if (line == null)
return null;
if (start >= line.length())
return null;
boolean isFirstChar = true;
while (end < line.length() && isCharAllowedForTypeNames(line.charAt(end), isFirstChar)) {
while (end < line.length() && isCharAllowedForTypeNames(line, end, isFirstChar, isInOOContext)) {
isFirstChar = false;
++end;
}
Expand Down Expand Up @@ -948,21 +1011,21 @@ public static String getReleaseRestrictionName(int releaseRestriction) {
/** splits composed identifiers such as "any_class=>any_structure-any_component", or "any_class=>any_method(",
* or "any_interface~any_method" etc. into an array of identifiers, e.g. ["any_class", "=>", "any_structure", "-", "any_component"]
* or ["any_class", "=>", "any_method", "("] etc. */
public static ArrayList<String> splitIdentifier(String identifier, boolean allowFieldSymbols) {
public static ArrayList<String> splitIdentifier(String identifier, boolean allowFieldSymbols, boolean isInOOContext) {
ArrayList<String> results = new ArrayList<String>();
if (StringUtil.isNullOrEmpty(identifier))
return results;
int start = 0;
// a field symbol could only be at the beginning, e.g. '<ls_any>-obj_ref->attribute'
if (allowFieldSymbols && identifier.charAt(0) == ABAP.FIELD_SYMBOL_START_SIGN) {
String fieldSymbol = readTillEndOfVariableName(identifier, start, true);
String fieldSymbol = readTillEndOfVariableName(identifier, start, true, isInOOContext);
results.add(fieldSymbol);
start += fieldSymbol.length();
}
while (start < identifier.length()) {
boolean isVarName = isCharAllowedForVariableNames(identifier.charAt(start), true, false);
boolean isVarName = isCharAllowedForVariableNames(identifier, start, true, false, isInOOContext);
int end = start + 1;
while (end < identifier.length() && (isVarName == isCharAllowedForVariableNames(identifier.charAt(end), false, false))) {
while (end < identifier.length() && (isVarName == isCharAllowedForVariableNames(identifier, end, false, false, isInOOContext))) {
++end;
}
results.add(identifier.substring(start, end));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ public final ArrayList<DisplayLine> toDisplayLines(int indexOffset) {
Command command = firstCommand;
while (command != null) {

boolean isInOOContext = command.isInOOContext();
Token token = command.firstToken;
while (token != null) {
if (token.lineBreaks > 0) {
Expand All @@ -174,7 +175,7 @@ public final ArrayList<DisplayLine> toDisplayLines(int indexOffset) {
// add the Token's text, and the TextBits for coloring this text
int startIndex = line.length();
line.append(token.text);
textBits.addAll(Arrays.asList(token.toTextBits(startIndex)));
textBits.addAll(Arrays.asList(token.toTextBits(startIndex, isInOOContext)));

token = token.getNext();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

/**
* Enumeration of the color types available to display ABAP code.
* {@link Token#toTextBits(int)} splits a {@link Token} into (one or several) {@link TextBit}s
* {@link Token#toTextBits(int, boolean)} splits a {@link Token} into (one or several) {@link TextBit}s
* of one color type each.
*/
public enum ColorType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3103,7 +3103,18 @@ public boolean containsKeywordDeep(String keyword) {
return (firstToken.getNextTokenOfTypeAndText(TokenType.KEYWORD, keyword) != null);
}
}


public boolean isInOOContext() {
Command command = this;
while (command != null) {
if (command.firstCodeTokenIsAnyKeyword("CLASS", "INTERFACE", "METHOD")) {
return true;
}
command = command.getParent();
}
return false;
}

/** Returns true if the Command matches a hard-coded pattern or condition.
* This method can be used during development to search for examples in all sample code files. */
public final boolean matchesPattern() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ private void obfuscate(Command command) throws UnexpectedSyntaxAfterChanges {
clearMaps(true, true);
}

boolean isInOOContext = command.isInOOContext();
Token token = command.getFirstToken();
boolean condenseTillLineEnd = false;
while (token != null) {
Expand Down Expand Up @@ -221,7 +222,7 @@ else if (condenseTillLineEnd && !token.isAttached())
Token prevCode = token.getPrevCodeToken();

String newText;
boolean oldTextMayBeVarName = ABAP.mayBeVariableName(oldText, false);
boolean oldTextMayBeVarName = ABAP.mayBeVariableName(oldText, false, isInOOContext);
boolean prevIfFirstCode = (prevCode == command.getFirstCodeToken());
if (prevCode != null && prevCode.isAnyKeyword("METHOD", "METHODS") && oldTextMayBeVarName) {
newText = getNewNameFor(getKey(oldText), methods, "", IdentifierType.METHOD);
Expand All @@ -233,7 +234,7 @@ else if (condenseTillLineEnd && !token.isAttached())
newText = getTypeName(oldText, classes, "if_", false);

} else {
newText = obfuscateIdentifier(token);
newText = obfuscateIdentifier(token, isInOOContext);
}
token.setText(newText, true);
}
Expand All @@ -243,12 +244,12 @@ else if (condenseTillLineEnd && !token.isAttached())
}
}

private String obfuscateIdentifier(Token token) {
private String obfuscateIdentifier(Token token, boolean isInOOContext) {
String oldText = token.getText();
Token nextCode = token.getNextCodeToken();
Token parent = token.getParent();

ArrayList<String> bits = ABAP.splitIdentifier(oldText, true);
ArrayList<String> bits = ABAP.splitIdentifier(oldText, true, isInOOContext);
if (bits.size() == 3 && AbapCult.stringEqualsAny(true, bits.get(0) + bits.get(1), ABAP.SY_PREFIX, ABAP.SYST_PREFIX, ABAP.TEXT_SYMBOL_PREFIX)) {
return oldText;
}
Expand All @@ -275,7 +276,7 @@ private String obfuscateIdentifier(Token token) {
if (ABAP.isFieldSymbol(bit)) {
newBit = ABAP.FIELD_SYMBOL_START_STRING + getVariableName(bit.substring(1, bit.length() - 1), variables, "ls_") + ABAP.FIELD_SYMBOL_END_STRING;

} else if (!ABAP.isCharAllowedForVariableNames(bit.charAt(0), true, false)) {
} else if (!ABAP.isCharAllowedForVariableNames(bit, 0, true, false, isInOOContext)) {
sb.append(bit);
continue;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,7 @@ else if (this != parentCommand.getFirstCodeToken())
return false;
}

final TextBit[] toTextBits(int startIndex) {
final TextBit[] toTextBits(int startIndex, boolean isInOOContext) {
boolean isSimpleCase = false;

// in most cases, the whole Token is of one ColorType
Expand Down Expand Up @@ -1380,7 +1380,8 @@ final TextBit[] toTextBits(int startIndex) {
boolean lastWasIdentifier = false;
for (int i = 0; i < text.length(); ++i) {
char c = text.charAt(i);
boolean isIdentifier = (type == TokenType.KEYWORD) ? ABAP.isCharAllowedForAnyKeyword(c) : ABAP.isCharAllowedForVariableNames(c);
boolean isIdentifier = (type == TokenType.KEYWORD) ? ABAP.isCharAllowedForAnyKeyword(c)
: ABAP.isCharAllowedForVariableNames(text, i, (i == 0), false, isInOOContext);
// in some cases, initial '/' does NOT start an identifier with a namespace, e.g. "ULINE AT /.", "ULINE AT /10(20)." or "ULINE at /pos(20)."
if (i == 0 && c == '/' && (text.length() <= 1 || text.indexOf('/', 1) < 0)) {
isIdentifier = false;
Expand Down
Loading

0 comments on commit 842dbaf

Please sign in to comment.