C/C++ Formatter end suppression (@formatter:on) gets indented

[lrademacher]

Describe the bug
In Eclipse CDT on windows (eclipse-cpp-2023-06-R-win32-x86_64) with a C++ project when using the formatter suppression tags (@formatter:off/@formatter:on) and executing the file formatting, the end-tag of the suppression (@formatter:on) gets indented by one level everytime.

To Reproduce
Steps to reproduce the behavior:

1. Download eclipse-cpp-2023-06-R-win32-x86_64 and start it.
2. Create a new C/C++ Project (C++ Managed Build)
3. Add a new file "test.cpp" with the content:

int main() {
	// @formatter:off
	int x[] = {1,2,34,5,5,5,5,5};
	// @formatter:on
}

4. Go to "Source" -> "Format"
5. See that // @formatter:on is indented by one level e.g.:

int main() {
	// @formatter:off
	int x[] = {1,2,34,5,5,5,5,5};
		// @formatter:on
}

6. Repeat at step 4

Expected behavior
When the formatter is executed two times, the file should not change on the second formatting attempt. => No additional indentation on @formatter:on.

Desktop (please complete the following information):

• OS: Windows 10 Enterprise 64-Bit

[lrademacher]

I just found out, that this does not happen, when the comment directly starts at the beginning of the line, e.g.:

int main() {
// @formatter:off
	int x[] = {1,2,34,5,5,5,5,5};
// @formatter:on
}

[jonahgraham]

Thanks @lrademacher for the bug report. I have created a testcase that shows the error in #431

[jonahgraham]

It seems the issue here is that when @formatter:on/off was implemented it was basically modelled as if @formatter:off was a #if 0 and @formatter:on was an #endif. This leads to slightly different indentation rules around the beginning of line.

I tried marking the end of the comment containing @formatter:on as the end of the inactive region like this:

$ git diff
diff --git a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/formatter/CodeFormatterVisitor.java b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/formatter/CodeFormatterVisitor.java
index 7eee8ee0cf..82e0014043 100644
--- a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/formatter/CodeFormatterVisitor.java
+++ b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/formatter/CodeFormatterVisitor.java
@@ -4973,7 +4973,7 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
                                }
                        } else if (onPos != -1 && onPos > offPos) {
                                if (inInactiveCode) {
-                                       int inactiveCodeEnd = nodeLocation.getNodeOffset();
+                                       int inactiveCodeEnd = nodeLocation.getNodeOffset() + nodeLocation.getNodeLength();
                                        positions.add(new InactivePosition(inactiveCodeStart, inactiveCodeEnd - inactiveCodeStart, false));
                                }
                                inInactiveCode = false;

which resolves this use case, but does cause a regression on this test

cdt/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/CodeFormatterTest.java

Lines 4721 to 4736 in db4bc74

	////@formatter:off
	// int a;
	//
	////@formatter:on
	// int b;
	// int c;
	////@formatter:off
	// int a;
	//
	////@formatter:on
	//int b;
	//int c;
	public void testFormatterOnOff_Bug559669() throws Exception {
	assertFormatterResult();
	}

I just found out, that this does not happen, when the comment directly starts at the beginning of the line, e.g.:

This seems like a reasonable workaround until someone can fix this issue.

[jonahgraham]

I have marked this as a good-first-issue because it is fairly straightforward to look into. The formatter is fairly independent and has a good test suite (see CodeFormatterTest)

[jonahgraham]

Here are some examples tests that can be used to help test this. These tests belong in CodeFormatterTest

	//int main() {
	//	// @formatter:off
	//	int x[] = {1,2,34,5,5,5,5,5};
	//	// @formatter:on
	//}

	//int main() {
	//	// @formatter:off
	//	int x[] = {1,2,34,5,5,5,5,5};
	//	// @formatter:on
	//}
	public void testFormatterOnOff_Issue428() throws Exception {
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_USE_COMMENT_TAG, true);
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_ON_TAG, "@formatter:on");
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_OFF_TAG, "@formatter:off");
		assertFormatterResult();
	}

	//void f() {
	//	//    @formatter:off
	//	int a = 123;
	//	//    @formatter:on
	//}

	//void f() {
	//	//    @formatter:off
	//	int a = 123;
	//	//    @formatter:on
	//}
	public void testFormatterOnOff_Issue428_a() throws Exception {
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_USE_COMMENT_TAG, true);
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_ON_TAG, "@formatter:on");
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_OFF_TAG, "@formatter:off");
		assertFormatterResult();
	}

	//int main() {
	//	int y;
	//// @formatter:off
	//	int x[] = {1,2,34,5,5,5,5,5};
	//// @formatter:on
	//	int z;
	//}

	//int main() {
	//	int y;
	//// @formatter:off
	//	int x[] = {1,2,34,5,5,5,5,5};
	//// @formatter:on
	//	int z;
	//}
	public void testFormatterOnOff_Issue428_b() throws Exception {
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_USE_COMMENT_TAG, true);
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_ON_TAG, "@formatter:on");
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_OFF_TAG, "@formatter:off");
		assertFormatterResult();
	}

	//void f() {
	//    //    @formatter:off
	//    int a = 123;
	//    //    @formatter:on
	//}

	//void f() {
	//    //    @formatter:off
	//    int a = 123;
	//    //    @formatter:on
	//}
	public void testFormatterOnOff_Issue428_c() throws Exception {
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_USE_COMMENT_TAG, true);
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_ON_TAG, "@formatter:on");
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_OFF_TAG, "@formatter:off");
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_TAB_CHAR, CCorePlugin.SPACE);
		assertFormatterResult();
	}

	//void f() {
	////    @formatter:off
	//	int a = 123;
	//	//    @formatter:on
	//}

	//void f() {
	////    @formatter:off
	//	int a = 123;
	//	//    @formatter:on
	//}
	public void testFormatterOnOff_Issue428_d() throws Exception {
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_USE_COMMENT_TAG, true);
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_ON_TAG, "@formatter:on");
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_OFF_TAG, "@formatter:off");
		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_TAB_CHAR, CCorePlugin.SPACE);
		assertFormatterResult();
	}