From fa9cdfb1e5dc77a88de0d867e37d04c17764835c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Sat, 3 Oct 2026 21:43:36 +0300 Subject: [PATCH] Rewrite the missing-space rule without a negative lookahead `//foo` gets a space after its slashes unless the comment is an IDE marker, `//noinspection` or `//$NON-NLS-n$`, which the tools recognise only as written. The exemption was a negative lookahead inside the matching pattern; it is now a second pattern that names the markers, checked after the first one matches. The two forms accept the same lines: the first pattern needs a non-space, non-slash character right after the slashes, so the lookahead was always evaluated after the whole run of slashes, which is where the second pattern looks as well. Taken from google/google-java-format#1460, whose motivation stands here too: regex engines without backtracking do not support lookahead, and the markers are easier to read, and to share with doc/Comment as #23 asks, as a pattern of their own. The rest of that change does not apply: `Strings.repeat` is already `String.repeat` here, and the markdown branch belongs to google-java-format's own `///` handling. FormatterTest pins the behaviour before and after: both markers, a marker followed by text, `//foo`, `///foo`, and comments that already have their space. --- .../javaformat/java/JavaCommentsHelper.java | 8 +++--- .../javaformat/java/FormatterTest.java | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/JavaCommentsHelper.java b/open-java-format/src/main/java/com/palantir/javaformat/java/JavaCommentsHelper.java index 5f4419a76..c3073c242 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/JavaCommentsHelper.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/JavaCommentsHelper.java @@ -167,10 +167,12 @@ private static boolean isJBangDirective(String text) { return matcher.lookingAt() && (matcher.group(1) != null || JBANG_DIRECTIVE_NAMES.contains(matcher.group(2))); } + private static final Pattern LINE_COMMENT_MISSING_SPACE_PREFIX = Pattern.compile("^(//+)[^\\s/]"); + // Preserve special `//noinspection` and `//$NON-NLS-x$` comments used by IDEs, which cannot // contain leading spaces. - private static final Pattern LINE_COMMENT_MISSING_SPACE_PREFIX = - Pattern.compile("^(//+)(?!noinspection|\\$NON-NLS-\\d+\\$)[^\\s/]"); + private static final Pattern LINE_COMMENT_NO_SPACE_PREFIX = + Pattern.compile("^//+(noinspection|\\$NON-NLS-\\d+\\$)"); private static String lineCommentPrefix(String line) { int prefixLength = 0; @@ -185,7 +187,7 @@ private List wrapLineComments(List lines, int column0) { for (String line : lines) { // Add missing leading spaces to line comments: `//foo` -> `// foo`. Matcher matcher = LINE_COMMENT_MISSING_SPACE_PREFIX.matcher(line); - if (matcher.find()) { + if (matcher.find() && !LINE_COMMENT_NO_SPACE_PREFIX.matcher(line).find()) { int length = matcher.group(1).length(); line = "/".repeat(length) + " " + line.substring(length); } diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java index 73d290b1e..cdc8444d0 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java @@ -367,6 +367,33 @@ void onlyWrapLineCommentOnWhitespace_noLeadingWhitespace() throws Exception { + "}\n"); } + @Test + void lineCommentGetsItsMissingSpaceExceptIdeMarkers() throws Exception { + // `//foo` gets a space after any number of slashes, while the IDE markers `//noinspection` and `//$NON-NLS-n$` + // are recognised by their tools only without one and stay as written. + String input = "class T {\n" + + " //noinspection unchecked\n" + + " //$NON-NLS-1$ //$NON-NLS-2$\n" + + " //$NON-NLS-12$ and more\n" + + " //foo\n" + + " ///foo\n" + + " // bar\n" + + " // two spaces\n" + + " void m() {}\n" + + "}\n"; + String expected = "class T {\n" + + " //noinspection unchecked\n" + + " //$NON-NLS-1$ //$NON-NLS-2$\n" + + " //$NON-NLS-12$ and more\n" + + " // foo\n" + + " /// foo\n" + + " // bar\n" + + " // two spaces\n" + + " void m() {}\n" + + "}\n"; + assertThat(Formatter.create().formatSource(input)).isEqualTo(expected); + } + @Test void throwsFormatterException() throws Exception { assertThatThrownBy(() -> Formatter.create().formatSourceAndFixImports("package foo; public class {"))