bugfixes for version 1.13.2

com.sap.adt.abapcleaner.app/abapcleaner.product

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <?pde version="3.5"?>
 
-<product name="ABAP cleaner" uid="com.sap.adt.abapcleaner.app" application="com.sap.adt.abapcleaner.standalone.app" version="1.13.2.qualifier" useFeatures="true" includeLaunchers="true">
+<product name="ABAP cleaner" uid="com.sap.adt.abapcleaner.app" application="com.sap.adt.abapcleaner.standalone.app" version="1.13.3.qualifier" useFeatures="true" includeLaunchers="true">
 
    <configIni use="default">
    </configIni>

com.sap.adt.abapcleaner.app/pom.xml

@@ -3,14 +3,14 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
   <modelVersion>4.0.0</modelVersion>
   <groupId>com.sap.adt.abapcleaner</groupId>
   <artifactId>com.sap.adt.abapcleaner.app</artifactId>
-  <version>1.13.2-SNAPSHOT</version>
+  <version>1.13.3-SNAPSHOT</version>
   <name>com.sap.adt.abapcleaner.app</name>
   <packaging>eclipse-repository</packaging>
 
   <parent>
     <artifactId>parent</artifactId>
     <groupId>com.sap.adt.abapcleaner</groupId>
-    <version>1.13.2-SNAPSHOT</version>
+    <version>1.13.3-SNAPSHOT</version>
     <relativePath>../</relativePath>
   </parent>
 

com.sap.adt.abapcleaner.feature/feature.xml

@@ -2,7 +2,7 @@
 <feature
       id="com.sap.adt.abapcleaner.feature"
       label="ABAP cleaner"
-      version="1.13.2.qualifier">
+      version="1.13.3.qualifier">
 
    <description>
       ABAP cleaner plug-in for ABAP Development Tools

com.sap.adt.abapcleaner.feature/pom.xml

@@ -3,14 +3,14 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
   <modelVersion>4.0.0</modelVersion>
   <artifactId>com.sap.adt.abapcleaner.feature</artifactId>
   <groupId>com.sap.adt.abapcleaner</groupId>
-  <version>1.13.2-SNAPSHOT</version>
+  <version>1.13.3-SNAPSHOT</version>
   <name>com.sap.adt.abapcleaner.feature</name>
   <packaging>eclipse-feature</packaging>
 
   <parent>
     <artifactId>parent</artifactId>
     <groupId>com.sap.adt.abapcleaner</groupId>
-    <version>1.13.2-SNAPSHOT</version>
+    <version>1.13.3-SNAPSHOT</version>
     <relativePath>../</relativePath>
   </parent>
 

com.sap.adt.abapcleaner.gui/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Bundle-Name
 Bundle-SymbolicName: com.sap.adt.abapcleaner.gui;singleton:=true
-Bundle-Version: 1.13.2.qualifier
+Bundle-Version: 1.13.3.qualifier
 Bundle-Vendor: %Provider-Name
 Bundle-Localization: plugin
 Bundle-RequiredExecutionEnvironment: JavaSE-11

com.sap.adt.abapcleaner.gui/pom.xml

@@ -3,14 +3,14 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
   <modelVersion>4.0.0</modelVersion>
   <groupId>com.sap.adt.abapcleaner</groupId>
   <artifactId>com.sap.adt.abapcleaner.gui</artifactId>
-  <version>1.13.2-SNAPSHOT</version>
+  <version>1.13.3-SNAPSHOT</version>
   <name>com.sap.adt.abapcleaner.gui</name>
   <packaging>eclipse-plugin</packaging>
 
   <parent>
     <artifactId>parent</artifactId>
     <groupId>com.sap.adt.abapcleaner</groupId>
-    <version>1.13.2-SNAPSHOT</version>
+    <version>1.13.3-SNAPSHOT</version>
     <relativePath>../</relativePath>
   </parent>
 

com.sap.adt.abapcleaner.gui/src/com/sap/adt/abapcleaner/gui/eclipse/AbapCleanerHandlerBase.java

@@ -61,13 +61,7 @@ protected String getCommandId() {
 		// This is only used by the super class to check whether the command should be enabled/disabled. 
 		// Since there is no extension framework in ADT yet for 3rd party add-ons, piggyback on an existing ID 
 		// from ADT that does similar things.
-		// - With "com.sap.adt.tools.abapsource.ui.prettyPrintBlock", the command is initially also enabled for
-		//   CDS view definitions, database table definitions etc., but an attempt to run it will simply show
-		//   the message "The chosen operation is not enabled." After this (or after the first regular cleanup
-		//   on an ABAP class etc.), the command is only enabled for editors that indeed allow cleanup.
-		// - With "com.sap.adt.tools.abapsource.ui.cleanup.deleteUnusedVariables", this startup issue could be 
-		//   avoided, however, this would not allow running cleanup on WebDynpro implementations. 
-		return "com.sap.adt.tools.abapsource.ui.prettyPrintBlock"; // ...ui.cleanup.deleteUnusedVariables 
+		return "com.sap.adt.tools.abapsource.ui.prettyPrintBlock";  
 	}
 
 	@Override

com.sap.adt.abapcleaner.updatesite/pom.xml

@@ -2,14 +2,14 @@
 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://maven.apache.org/POM/4.0.0">
   <modelVersion>4.0.0</modelVersion>
   <artifactId>com.sap.adt.abapcleaner.updatesite</artifactId>
-  <version>1.13.2-SNAPSHOT</version>
+  <version>1.13.3-SNAPSHOT</version>
   <name>ABAP Cleaner for ABAP Development Tools (ADT)</name>
   <packaging>eclipse-repository</packaging>
 
   <parent>
     <artifactId>parent</artifactId>
     <groupId>com.sap.adt.abapcleaner</groupId>
-    <version>1.13.2-SNAPSHOT</version>
+    <version>1.13.3-SNAPSHOT</version>
     <relativePath>../</relativePath>
   </parent>
 

com.sap.adt.abapcleaner/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Bundle-Name
 Bundle-SymbolicName: com.sap.adt.abapcleaner;singleton:=true
-Bundle-Version: 1.13.2.qualifier
+Bundle-Version: 1.13.3.qualifier
 Bundle-Vendor: %Provider-Name
 Bundle-Localization: plugin
 Bundle-RequiredExecutionEnvironment: JavaSE-11

com.sap.adt.abapcleaner/pom.xml

@@ -3,14 +3,14 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
   <modelVersion>4.0.0</modelVersion>
   <groupId>com.sap.adt.abapcleaner</groupId>
   <artifactId>com.sap.adt.abapcleaner</artifactId>
-  <version>1.13.2-SNAPSHOT</version>
+  <version>1.13.3-SNAPSHOT</version>
   <name>com.sap.adt.abapcleaner</name>
   <packaging>eclipse-plugin</packaging>
 
   <parent>
     <artifactId>parent</artifactId>
     <groupId>com.sap.adt.abapcleaner</groupId>
-    <version>1.13.2-SNAPSHOT</version>
+    <version>1.13.3-SNAPSHOT</version>
     <relativePath>../</relativePath>
   </parent>
 

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/parser/Command.java

@@ -360,7 +360,7 @@ final boolean isCloserFor(LevelOpener levelOpener) {
 		// 'AT LINE-SELECTION' itself! - can be closed with 'AT LINE-SELECTION' etc. 
 		// obsolete event blocks are included: "START-OF-EDITING", "END-OF-SELECTION", "END-OF-EDITING"
 		final String[] eventClosers = new String[] { "AT LINE-SELECTION", "AT SELECTION-SCREEN", "AT USER-COMMAND", "END-OF-PAGE", 
-																	"INITIALIZATION", "LOAD-OF-PROGRAM", "START-OF-SELECTION", "START-OF-EDITING", "END-OF-SELECTION", "END-OF-EDITING", "TOP-OF-PAGE", "CLASS", "FORM" };
+																	"INITIALIZATION", "LOAD-OF-PROGRAM", "START-OF-SELECTION", "START-OF-EDITING", "END-OF-SELECTION", "END-OF-EDITING", "TOP-OF-PAGE", "CLASS", "FORM", "MODULE" };
 		// the parameter startsLocalVariableContext is usually false, because apart from FORM...ENDFORM and FUNCTION...ENDFUNCTION, 
 		// only the event blocks 'AT SELECTION-SCREEN ...' and 'GET ...' (which are implemented internally as procedures) can contain local data, 
 		// while declarative statements in all other event blocks are part of the global data declarations of the ABAP program, 

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/parser/Token.java

@@ -2858,16 +2858,36 @@ public boolean condenseUpTo(Token last, int maxLineLength, int indent, boolean k
 		boolean changed = false;
 		int indexInLine = token.getEndIndexInLine();
 		token = token.getNext();
+		Token lastMovableToken = null;
 		while (token != null) {
-			// if needed, move Token to the next line, otherwise directly behind the previous Token
+			// determine whether the Token is (or should be) attached; otherwise, it can be moved to the next line
 			int spacesLeft = (token.isAttached() || token.isCommaOrPeriod() || token.isChainColon()) ? 0 : 1;
+			if (spacesLeft > 0)
+				lastMovableToken = token;
+
+			// if needed, move Token(s) to the next line, otherwise move it directly behind the previous Token
 			if (token.isAsteriskCommentLine() || keepQuotMarkCommentLines && token.isQuotMarkCommentLine()) {
 				// do nothing
-			} else if (token.getPrev().isComment() || indexInLine + spacesLeft + token.getTextLength() > maxLineLength) {
+
+			} else if (token.getPrev().isComment()) {
+				// after a comment, even a comma or period must be kept on the next line
 				if (token.setWhitespace(1, indent)) {
 					changed = true;
 				}
-				indexInLine = indent + token.getTextLength();
+				lastMovableToken = null;
+				indexInLine = token.getEndIndexInLine();
+
+			} else if (lastMovableToken != null && indexInLine + spacesLeft + token.getTextLength() > maxLineLength) {
+				// break the line before the last movable (= the last non-attached) Token
+				if (lastMovableToken.setWhitespace(1, indent)) {
+					changed = true;
+					if (token != lastMovableToken) {
+						token.setWhitespace(0, spacesLeft);
+					}
+				}
+				lastMovableToken = null;
+				indexInLine = token.getEndIndexInLine();
+
 			} else {
 				if (token.setWhitespace(0, spacesLeft)) {
 					changed = true;

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/declarations/UnusedVariablesRule.java

@@ -156,6 +156,14 @@ protected void executeOn(Code code, ClassInfo classInfo, int releaseRestriction)
 
 	@Override
 	protected void executeOn(Code code, Command methodStart, LocalVariables localVariables, int releaseRestriction) throws UnexpectedSyntaxAfterChanges, IntegrityBrokenException {
+		// even if there are no local variables, remove obsolete comments that were generated in previous runs of this cleanup rule 
+		// but are no more attached to a declaration
+		try {
+			removeObsoleteComments(code, methodStart);
+		} catch (UnexpectedSyntaxException e) {
+			throw new UnexpectedSyntaxAfterChanges(this, e);
+		}
+
 		// skip this method if macros or dynamic ASSIGN are used inside the method to avoid deletion of variables 
 		// that are used inside the macros or dynamically (note that macro code may be local or 'out of sight')
 		if (localVariables.isEmpty() || localVariables.getMethodUsesMacrosOrTestInjection() || localVariables.getMethodUsesDynamicAssign())
@@ -206,6 +214,89 @@ protected void executeOn(Code code, Command methodStart, LocalVariables localVar
 		}
 	}
 
+	private void removeObsoleteComments(Code code, Command methodStart) throws UnexpectedSyntaxException, UnexpectedSyntaxAfterChanges {
+		// remove obsolete comments (both comment lines and comment Tokens at line end) that are no more attached to a declaration
+
+		Command command = methodStart;
+		while (command != null && command != methodStart.getNextSibling()) {
+			Command nextCommand = command.getNext();
+			if (isCommandBlocked(command))  {
+				// skip this Command
+
+			} else if (command.isCommentLine() ) {
+				// does the comment line match a message generated by this rule?
+				Token firstToken = command.getFirstToken();
+				if (firstToken.textEqualsAny(constMessages) || firstToken.textEqualsAny(varMessages)) {
+					// remove the comment from the Code, unless it is attached to a declaration
+					if (!isAttachedToDeclaration(command)) {
+						if (nextCommand.getFirstToken().setLineBreaks(firstToken.lineBreaks)) {
+							code.addRuleUse(this, nextCommand);
+						}
+						command.removeFromCode();
+					}
+				}
+
+			} else if (!command.isDeclaration()) {
+				// does a (full line or line-end) comment Token match a message generated by this rule?
+				Token token = command.getFirstToken();
+				while (token != null) {
+					Token nextToken = token.getNext();
+					if (token.isComment() && (token.textEqualsAny(constMessages) || token.textEqualsAny(varMessages))) {
+						// remove the comment from the Command, unless it is attached to a declaration
+						if (!isAttachedToDeclaration(token)) {
+							if (token.lineBreaks > 0 && token.getNext() != null) {
+								token.getNext().setLineBreaks(Math.max(token.lineBreaks, token.getNext().lineBreaks));
+							}
+							token.removeFromCommand();
+							code.addRuleUse(this, command);
+						}
+					}
+					token = nextToken;
+				}
+			}
+			command = nextCommand;
+		}
+	}
+
+	private boolean isAttachedToDeclaration(Command command) {
+		// determine whether the comment is attached to a declaration
+		Command test = command.getNext();
+		while (test != null && test.getFirstTokenLineBreaks() == 1 && test.isCommentLine()) {
+			test = test.getNext();
+		}
+		return (test != null && test.getFirstTokenLineBreaks() == 1 && test.isDeclaration());
+	}
+
+	private boolean isAttachedToDeclaration(Token token) {
+		if (token.lineBreaks == 0) {
+			// for line-end comments, search for inline declarations before the token
+			Token test = token.getPrev();
+			while (test != null) {
+				if (test.opensInlineDeclaration()) 
+					return true;
+				if (test.lineBreaks > 0) 
+					break;
+				test = test.getPrev();
+			}
+			return false;
+
+		} else {
+			// for comment lines within the Command, search for inline declarations in the line that directly follows the token
+			Token test = token.getNext();
+			if (test != null && test.lineBreaks > 1)
+				return false;
+			while (test != null) {
+				if (test.opensInlineDeclaration())
+					return true;
+				test = test.getNext();
+				if (test != null && test.lineBreaks > 0) {
+					return false;
+				}
+			}
+			return false;
+		}
+	}
+
 	private UnusedVariableAction getAction(VariableInfo varInfo) {
 		UnusedVariableAction action;
 		if (varInfo.isAssigned()) {

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/prettyprinter/IndentRule.java

@@ -162,6 +162,10 @@ else if (alignCommand.firstCodeTokenIsAnyKeyword("CATCH", "CLEANUP"))
 
 	public final void executeOn(Code code, Command command, int newIndent) {
 		Token firstToken = command.getFirstToken();
+		// skip commands that follow on the same line as the previous command
+		if (firstToken.lineBreaks == 0 && !command.isFirstCommandInCode()) {
+			return;
+		}
 		if (firstToken.spacesLeft != newIndent) {
 			command.addIndent(newIndent - firstToken.spacesLeft, 0);
 			code.addRuleUse(this, command);

docs/release-notes.md

@@ -6,10 +6,20 @@ as well as bugfixes of reported issues, i.e. anything that enhances or changes t
 For a complete list of changes (including documentation, tests, refactoring etc.), please refer to 
 the list of [commits](../../../commits/main).
 
+## 2024-02-27 (version 1.13.2)
+
+**Thanks** a lot to [**JoachimEck**](https://github.com/JoachimEck), [**Falcon7EH**](https://github.com/Falcon7EH) 
+and [**VladGhitulescu**](https://github.com/VladGhitulescu) for the bug reports behind these fixes!
+
+* Enhanced rule '**Delete unused variables**' to **clean up obsolete comments** ([#257](../../../issues/257))
+* Fixed rule '**Rearrange local declarations**' for **MODULE** ... ENDMODULE ([#258](../../../issues/258))
+* Fixed rule '**Align SELECT lists**' for line breaks with **inline declarations** ([#251](../../../issues/251))
+* Fixed rule '**Indent lines**' for multiple **commands on same line** ([#248](../../../issues/248))
+
 ## 2024-02-12 (version 1.13.1)
 
 Many **thanks** to [**flying-crane**](https://github.com/flying-crane), [**owelzel**](https://github.com/owelzel), 
-[**ConjuringCoffee**](https://github.com/ConjuringCoffee), [**I034943**](https://github.com/I034943),, [**xczar0**](https://github.com/xczar0),
+[**ConjuringCoffee**](https://github.com/ConjuringCoffee), [**I034943**](https://github.com/I034943), [**xczar0**](https://github.com/xczar0),
 [**fmabap**](https://github.com/fmabap), [**this-yash**](https://github.com/this-yash) and [**mraht**](https://github.com/mraht) 
 for your contributions, ideas and bug reports that led to these improvements!
 

pom.xml

@@ -3,7 +3,7 @@
   <modelVersion>4.0.0</modelVersion>
   <groupId>com.sap.adt.abapcleaner</groupId>
   <artifactId>parent</artifactId>
-  <version>1.13.2-SNAPSHOT</version>
+  <version>1.13.3-SNAPSHOT</version>
   <name>ABAP cleaner</name>
   <packaging>pom</packaging>
   <modules>

test/com.sap.adt.abapcleaner.test/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: Unit Tests for com.sap.adt.abapcleaner
 Bundle-SymbolicName: com.sap.adt.abapcleaner.test;singleton:=true
-Bundle-Version: 1.13.2.qualifier
+Bundle-Version: 1.13.3.qualifier
 Bundle-Vendor: abap-dev
 Fragment-Host: com.sap.adt.abapcleaner
 Bundle-RequiredExecutionEnvironment: JavaSE-11

test/com.sap.adt.abapcleaner.test/pom.xml

@@ -3,14 +3,14 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
   <modelVersion>4.0.0</modelVersion>
   <groupId>com.sap.adt.abapcleaner</groupId>
   <artifactId>com.sap.adt.abapcleaner.test</artifactId>
-  <version>1.13.2-SNAPSHOT</version>
+  <version>1.13.3-SNAPSHOT</version>
   <name>com.sap.adt.abapcleaner.test</name>
   <packaging>eclipse-test-plugin</packaging>
 
   <parent>
     <artifactId>tests</artifactId>
     <groupId>com.sap.adt.abapcleaner</groupId>
-    <version>1.13.2-SNAPSHOT</version>
+    <version>1.13.3-SNAPSHOT</version>
     <relativePath>../</relativePath>
   </parent>
 

test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/alignment/AlignSelectListsTest.java

@@ -827,4 +827,25 @@ void testSelectSingleForUpdate() {
 
 		testRule();
 	}
+
+	@Test
+	void testIntoListWithInlineDeclarations() {
+		// ensure that the line break is introduced before @DATA(, NOT after it, because the inline declaration must be kept together
+		rule.configMaxLineLength.setValue(100);
+
+		buildSrc("    SELECT f1, f2, f3");
+		buildSrc("      FROM any_dtab");
+		buildSrc("      INTO ( @DATA(lv_var_1_with_long_name), @DATA(lv_var_2_with_very_long_name), @DATA(lv_var_3_with_long_name) ).");
+		buildSrc("    ENDSELECT.");
+
+		buildExp("    SELECT f1, f2, f3");
+		buildExp("      FROM any_dtab");
+		buildExp("      INTO ( @DATA(lv_var_1_with_long_name), @DATA(lv_var_2_with_very_long_name),");
+		buildExp("             @DATA(lv_var_3_with_long_name) ).");
+		buildExp("    ENDSELECT.");
+
+		putAnyMethodAroundSrcAndExp();
+
+		testRule();
+	}
 }