Skip to content

Commit

Permalink
add new rule "Remove needless CLEAR" (#61, #111)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmgrassau committed Oct 29, 2023
1 parent 36ab1d0 commit 0f027d6
Show file tree
Hide file tree
Showing 11 changed files with 541 additions and 150 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.sap.adt.abapcleaner.parser;

public enum ChainElementAction {
DELETE,
COMMENT_OUT_WITH_ASTERISK,
COMMENT_OUT_WITH_QUOT,
ADD_TODO_COMMENT,
IGNORE;

public int getValue() { return this.ordinal(); }

public static ChainElementAction forValue(int value) {
return values()[value];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2939,6 +2939,126 @@ public final boolean belongsToMacroDefinition() {
}
return false;
}

/**
* Handles the specified chain element (or, if the Command is not a chain, the entire Command) with the specified action.
* @param start - the Token with which the chain element starts; in non-chains, the Token after the ABAP keyword
* @param action - whether to add a comment, comment out the chain element, or remove it
* @param commentText - comment text (without comment sign) in case of ChainElementAction.ADD_TODO_COMMENT
* @return
* @throws UnexpectedSyntaxException
* @throws UnexpectedSyntaxAfterChanges
* @throws IntegrityBrokenException
*/
public final boolean handleChainElement(Token start, ChainElementAction action, String commentText) throws UnexpectedSyntaxException, UnexpectedSyntaxAfterChanges, IntegrityBrokenException {
if (action == ChainElementAction.IGNORE) {
return false;

} else if (action == ChainElementAction.ADD_TODO_COMMENT) {
boolean commentAdded = false;
if (getClosesLevel()) { // in case of 'CATCH ... INTO DATA(...)', the comment cannot be added as a 'previous sibling', because that must be the TRY statement
commentAdded = (appendCommentToLineOf(start, commentText) != null);
} else {
commentAdded = (putCommentAboveLineOf(start, commentText) != null);
}
return commentAdded;
}

// get information on the line to be deleted / commented out and the surrounding lines
Command originalCommand = (this.originalCommand != null) ? this.originalCommand : this;
Token keyword = getFirstToken();
if (keyword.isComment()) // just to be sure; however, with the call to splitOutLeadingCommentLines() below, this should not happen anymore
keyword = keyword.getNextNonCommentSibling();
Token colon = keyword.getNext().isChainColon() ? keyword.getNext() : null;
// boolean isKeywordOnSameLine = (identifier.lineBreaks == 0);
Token commaOrPeriod = start.getLastTokenOnSiblings(true, TokenSearch.ASTERISK, ".|,");
Token lastTokenInLine = (commaOrPeriod.getNext() != null && commaOrPeriod.getNext().isCommentAfterCode()) ? commaOrPeriod.getNext() : commaOrPeriod;
boolean isInChain = isSimpleChain();
boolean isFirstInChain = isInChain && start.getPrevNonCommentToken().isChainColon();
boolean isLastInChain = isInChain && commaOrPeriod.isPeriod();
boolean isOnlyOneInChain = isFirstInChain && isLastInChain;
Token prevComma = (isInChain && !isFirstInChain) ? start.getPrevNonCommentSibling() : null;
Token nextIdentifier = (isInChain && !isLastInChain) ? lastTokenInLine.getNextNonCommentSibling() : null;
Token firstTokenInLine = (isInChain && !isFirstInChain) ? start : keyword;

// make the surrounding chain work without the line to be deleted / commented out
if (isInChain && !isOnlyOneInChain) {
if (isLastInChain) {
prevComma.setText(ABAP.DOT_SIGN_STRING, false);
prevComma.type = TokenType.PERIOD;
} else if (isFirstInChain) {
nextIdentifier.copyWhitespaceFrom(start);
int lineBreaks = (action == ChainElementAction.DELETE) ? keyword.lineBreaks : 1;
nextIdentifier.insertLeftSibling(Token.createForAbap(lineBreaks, keyword.spacesLeft, keyword.getText(), TokenType.KEYWORD, keyword.sourceLineNum));
if (colon != null) {
nextIdentifier.insertLeftSibling(Token.createForAbap(0, colon.spacesLeft, colon.getText(), TokenType.COLON, colon.sourceLineNum));
}
} else if (nextIdentifier != null && nextIdentifier.lineBreaks == 0) {
nextIdentifier.copyWhitespaceFrom(start);
}
}

if (action == ChainElementAction.COMMENT_OUT_WITH_ASTERISK || action == ChainElementAction.COMMENT_OUT_WITH_QUOT) {
// create a comment line
String lineText = Command.sectionToString(firstTokenInLine, lastTokenInLine, true);
int indent = 0;
if (action == ChainElementAction.COMMENT_OUT_WITH_QUOT) {
indent = firstTokenInLine.getStartIndexInLine();
lineText = ABAP.COMMENT_SIGN_STRING + " " + StringUtil.trimStart(lineText);
} else {
lineText = ABAP.LINE_COMMENT_SIGN_STRING + lineText;
}
Token newComment = Token.createForAbap(Math.max(firstTokenInLine.lineBreaks, 1), indent, lineText, TokenType.COMMENT, firstTokenInLine.sourceLineNum);

// insert the comment line, possibly as a new Command before or after the current one
if ((isFirstInChain || isLastInChain) && !isOnlyOneInChain) {
Command newCommand = Command.create(newComment, originalCommand);
try {
newCommand.finishBuild(getSourceTextStart(), getSourceTextEnd());
} catch (ParseException e) {
throw new UnexpectedSyntaxException(this, "parse error in commented-out line");
}

// code.addRuleUse(this, newCommand); is not required, because it will have the same effect as "code.addRuleUse(this, command)" below
if (isFirstInChain)
insertLeftSibling(newCommand);
else
getNext().insertLeftSibling(newCommand);
this.originalCommand = originalCommand;
} else {
firstTokenInLine.insertLeftSibling(newComment, false, true);
}

// remove the old declaration code
// referential integrity cannot be checked at this point, because we may still need to split out leading comment lines
Term.createForTokenRange(firstTokenInLine, lastTokenInLine).removeFromCommand(true);

} else if (action == ChainElementAction.DELETE) {
// remove the declaration code (and possibly the whole Command)
if (isInChain && !isOnlyOneInChain) {
Term termToDelete = Term.createForTokenRange(firstTokenInLine, lastTokenInLine);
termToDelete.removeFromCommand(true);
} else {
// transfer line breaks to next command to keep sections separate
if (next != null && getFirstTokenLineBreaks() > next.getFirstTokenLineBreaks())
next.getFirstToken().lineBreaks = getFirstTokenLineBreaks();
removeFromCode();
}

} else {
throw new IllegalArgumentException("Unknown Action!");
}

// if the (remaining) Command starts or ends with one or several comment lines, create separate Commands from it;
// this may happen if the first/last line of the Command was deleted or commented out, and the adjacent line(s) already were comment lines
if (splitOutLeadingCommentLines(originalCommand))
this.originalCommand = originalCommand;
if (splitOutTrailingCommentLines(originalCommand))
this.originalCommand = originalCommand;

testReferentialIntegrity(true, true);
return true;
}

/** 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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,10 @@ public final Token insertLeftSibling(Token newToken, boolean moveFollowingLinesR
newToken.copyWhitespaceFrom(this);
this.setWhitespace();
}
// if newToken is a comment, ensure a line break before this Token
if (newToken.isComment() && lineBreaks == 0) {
this.setWhitespace(1, parentCommand.getIndent() + ABAP.INDENT_STEP);
}

++parentCommand.tokenCount;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import java.util.*;

public abstract class Rule {
public static final int RULE_COUNT = 60;
public static final int RULE_COUNT = 61;
public static final int RULE_GROUP_COUNT = 7;

protected static final String LINE_SEP = ABAP.LINE_SEPARATOR;
Expand Down Expand Up @@ -71,6 +71,7 @@ static Rule[] getAllRules(Profile profile) {

// declarations
new ChainRule(profile),
new NeedlessClearRule(profile),
new LocalDeclarationOrderRule(profile),
new UnusedVariablesRule(profile),
new ChainOfOneRule(profile),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public enum RuleID {

// Declarations
DECLARATION_CHAIN, // for compatibility, this old RuleID is kept, while the class was renamed from DeclarationChainRule to ChainRule
NEEDLESS_CLEAR,
LOCAL_DECLARATION_ORDER,
UNUSED_VARIABLES,
CHAIN_OF_ONE,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.sap.adt.abapcleaner.rules.declarations;

import com.sap.adt.abapcleaner.parser.ChainElementAction;

public enum NeedlessClearAction {
DELETE(ChainElementAction.DELETE),
ADD_TODO_COMMENT(ChainElementAction.ADD_TODO_COMMENT),
IGNORE(ChainElementAction.IGNORE);

private ChainElementAction chainElementAction;

private NeedlessClearAction(ChainElementAction chainElementAction) {
this.chainElementAction = chainElementAction;
}

public int getValue() { return this.ordinal(); }
public ChainElementAction getCorrespondingChainElementAction() { return chainElementAction; }

public static NeedlessClearAction forValue(int value) {
return values()[value];
}
}
Loading

0 comments on commit 0f027d6

Please sign in to comment.