Skip to content

Commit fa9cdfb

Browse files
eamonnmcmanusabashev
authored andcommitted
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.
1 parent 2fbf175 commit fa9cdfb

2 files changed

Lines changed: 32 additions & 3 deletions

File tree

‎open-java-format/src/main/java/com/palantir/javaformat/java/JavaCommentsHelper.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,10 +167,12 @@ private static boolean isJBangDirective(String text) {
167167
return matcher.lookingAt() && (matcher.group(1) != null || JBANG_DIRECTIVE_NAMES.contains(matcher.group(2)));
168168
}
169169

170+
private static final Pattern LINE_COMMENT_MISSING_SPACE_PREFIX = Pattern.compile("^(//+)[^\\s/]");
171+
170172
// Preserve special `//noinspection` and `//$NON-NLS-x$` comments used by IDEs, which cannot
171173
// contain leading spaces.
172-
private static final Pattern LINE_COMMENT_MISSING_SPACE_PREFIX =
173-
Pattern.compile("^(//+)(?!noinspection|\\$NON-NLS-\\d+\\$)[^\\s/]");
174+
private static final Pattern LINE_COMMENT_NO_SPACE_PREFIX =
175+
Pattern.compile("^//+(noinspection|\\$NON-NLS-\\d+\\$)");
174176

175177
private static String lineCommentPrefix(String line) {
176178
int prefixLength = 0;
@@ -185,7 +187,7 @@ private List<String> wrapLineComments(List<String> lines, int column0) {
185187
for (String line : lines) {
186188
// Add missing leading spaces to line comments: `//foo` -> `// foo`.
187189
Matcher matcher = LINE_COMMENT_MISSING_SPACE_PREFIX.matcher(line);
188-
if (matcher.find()) {
190+
if (matcher.find() && !LINE_COMMENT_NO_SPACE_PREFIX.matcher(line).find()) {
189191
int length = matcher.group(1).length();
190192
line = "/".repeat(length) + " " + line.substring(length);
191193
}

‎open-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,33 @@ void onlyWrapLineCommentOnWhitespace_noLeadingWhitespace() throws Exception {
367367
+ "}\n");
368368
}
369369

370+
@Test
371+
void lineCommentGetsItsMissingSpaceExceptIdeMarkers() throws Exception {
372+
// `//foo` gets a space after any number of slashes, while the IDE markers `//noinspection` and `//$NON-NLS-n$`
373+
// are recognised by their tools only without one and stay as written.
374+
String input = "class T {\n"
375+
+ " //noinspection unchecked\n"
376+
+ " //$NON-NLS-1$ //$NON-NLS-2$\n"
377+
+ " //$NON-NLS-12$ and more\n"
378+
+ " //foo\n"
379+
+ " ///foo\n"
380+
+ " // bar\n"
381+
+ " // two spaces\n"
382+
+ " void m() {}\n"
383+
+ "}\n";
384+
String expected = "class T {\n"
385+
+ " //noinspection unchecked\n"
386+
+ " //$NON-NLS-1$ //$NON-NLS-2$\n"
387+
+ " //$NON-NLS-12$ and more\n"
388+
+ " // foo\n"
389+
+ " /// foo\n"
390+
+ " // bar\n"
391+
+ " // two spaces\n"
392+
+ " void m() {}\n"
393+
+ "}\n";
394+
assertThat(Formatter.create().formatSource(input)).isEqualTo(expected);
395+
}
396+
370397
@Test
371398
void throwsFormatterException() throws Exception {
372399
assertThatThrownBy(() -> Formatter.create().formatSourceAndFixImports("package foo; public class {"))

0 commit comments

Comments
 (0)