diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..695521c1e --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,35 @@ +# Working in this repository + +Building and testing: the README's "Building and testing locally" section. Gradle runs on the Temurin 21 that +`mise install` provides, `./gradlew test` is what CI runs, and `./gradlew formatDiff` formats the files you changed. + +## Conventions for new code + +- **Collections.** Plain `java.util` types. `List.of`, `List.copyOf`, `Map.of` and `Stream.toList()` give + unmodifiable collections, so new code does not need Guava's `ImmutableList` or `ImmutableMap`; use them only + where an existing API already hands one over. +- **Nullability.** One annotation, `javax.annotation.Nullable` (jsr305), the one the code uses; do not add another + `Nullable`. +- **Local variables.** `var` wherever the initializer gives the type unchanged, in `for` loops too. Not where it + would box a primitive (`for (var n : List)`), and not without an initializer that has a type of its own + (a lambda, `null`). +- **Immutability annotations.** Do not add Error Prone's `@Immutable` or `@SuppressWarnings("Immutable")`. +- **License header.** A new file starts with the Apache 2.0 header alone, without a copyright line: + + ```java + /* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + ``` + + Files inherited from google-java-format and palantir-java-format keep their original headers. diff --git a/open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java b/open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java index d8d04b160..ae3e0852d 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java @@ -30,14 +30,18 @@ import com.palantir.javaformat.doc.DocBuilder; import com.palantir.javaformat.doc.FillMode; import com.palantir.javaformat.doc.NonBreakingSpace; +import com.palantir.javaformat.doc.ReservedWidth; import com.palantir.javaformat.doc.State; import com.palantir.javaformat.doc.Token; import com.palantir.javaformat.doc.Token.RealOrImaginary; import com.palantir.javaformat.java.FormatterDiagnostic; import com.palantir.javaformat.java.InputMetadata; import com.palantir.javaformat.java.InputMetadataBuilder; +import com.palantir.javaformat.java.NonNlsMarkers; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; /** An {@code OpsBuilder} creates a list of {@link Op}s, which is turned into a {@link Doc} by {@link DocBuilder}. */ @@ -505,6 +509,10 @@ public OpsOutput build() { markForPartialFormat(); // Rewrite the ops to insert comments. Multimap tokOps = ArrayListMultimap.create(); + // The room after a string literal for the NON-NLS markers written on its line, see Comment.makeNonNlsMarkers. + var reservations = ArrayListMultimap.create(); + var tokenOps = new HashMap(); + Map tokenByIndex = null; int opsN = ops.size(); for (int i = 0; i < opsN; i++) { Op op = ops.get(i); @@ -528,6 +536,7 @@ public OpsOutput build() { * Regular input token. Copy out toksBefore before token, and toksAfter after it. Insert * this token's toksBefore at position j. */ + tokenOps.put(token, i); int newlines = 0; // Count of newlines in a row. boolean space = false; // Do we need an extra space after a previous "/*" comment? boolean lastWasComment = false; // Was the last thing we output a comment? @@ -591,7 +600,29 @@ public OpsOutput build() { } else { tokOps.put(tokAfterPos, SPACE); } - tokOps.putAll(tokAfterPos, makeComment(tokAfter)); + if (tokAfter.isSlashSlashComment() && NonNlsMarkers.isMarkersOnly(tokAfter.getText())) { + if (tokenByIndex == null) { + tokenByIndex = new HashMap<>(); + for (Input.Token t : input.getTokens()) { + tokenByIndex.put(t.getTok().getIndex(), t); + } + } + var literals = nonNlsLiterals(token, tokAfter, tokenByIndex, tokenOps); + var numbers = NonNlsMarkers.numbers(tokAfter.getText()); + for (var m = 0; m < literals.size(); m++) { + var literalOp = literals.get(m) < 0 + ? null + : tokenOps.get(tokenByIndex.get(literals.get(m))); + if (literalOp != null) { + var marker = NonNlsMarkers.marker(numbers.get(m)); + reservations.put(literalOp + 1, ReservedWidth.make(marker.length() + 1)); + } + } + tokOps.put(tokAfterPos, Comment.makeNonNlsMarkers(tokAfter, literals)); + tokOps.put(tokAfterPos, Break.makeForced()); + } else { + tokOps.putAll(tokAfterPos, makeComment(tokAfter)); + } if (breakAfter) { tokOps.put(tokAfterPos, Break.make(FillMode.FORCED, "", ZERO)); } @@ -630,6 +661,7 @@ public OpsOutput build() { ImmutableList.Builder newOps = ImmutableList.builder(); boolean afterForcedBreak = false; // Was the last Op a forced break? If so, suppress spaces. for (int i = 0; i < opsN; i++) { + newOps.addAll(reservations.get(i)); for (Op op : tokOps.get(i)) { if (!(afterForcedBreak && op instanceof NonBreakingSpace)) { newOps.add(op); @@ -662,6 +694,39 @@ private static boolean isNonNlsComment(Input.Tok tokAfter) { return tokAfter.isSlashSlashComment() && tokAfter.getText().contains("$NON-NLS"); } + /** + * For each marker of a {@code //$NON-NLS-1$ //$NON-NLS-2$} comment that trails {@code token}, the index of the + * string literal it marks, the n-th one on the comment's input line, or -1 when there is none. A literal in + * another partial-format region counts as none too: a partial format of its region would otherwise lose the + * marker, which belongs to the comment's region. So does a literal without an op of its own. + */ + private List nonNlsLiterals( + Input.Token token, + Input.Tok comment, + Map tokenByIndex, + Map tokenOps) { + var line = input.getLineNumber(token.getTok().getPosition()) - 1; + var toks = input.getRanges(line); + var literals = new ArrayList(); + for (int k = toks.lowerEndpoint(); k < Math.min(toks.upperEndpoint(), comment.getIndex()); k++) { + var candidate = tokenByIndex.get(k); + if (candidate != null && candidate.getTok().getText().startsWith("\"")) { + literals.add(k); + } + } + var region = inputMetadataBuilder.regionContaining(comment.getIndex()); + var result = new ArrayList(); + for (int n : NonNlsMarkers.numbers(comment.getText())) { + var literal = 1 <= n && n <= literals.size() ? literals.get(n - 1) : -1; + var usable = literal >= 0 + && region != null + && region.contains(literal) + && tokenOps.containsKey(tokenByIndex.get(literal)); + result.add(usable ? literal : -1); + } + return List.copyOf(result); + } + private static boolean isForcedBreak(Op op) { return op instanceof Break b && b.isForced(); } diff --git a/open-java-format/src/main/java/com/palantir/javaformat/Output.java b/open-java-format/src/main/java/com/palantir/javaformat/Output.java index f4a7a12d1..f6689a4b2 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/Output.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/Output.java @@ -17,6 +17,7 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.Range; import com.palantir.javaformat.doc.State; +import java.util.List; /** An output from the formatter. */ public abstract class Output extends InputOutput { @@ -36,6 +37,13 @@ public abstract class Output extends InputOutput { */ public abstract void append(State state, String text, Range range); + /** + * Output a comment of {@code //$NON-NLS-n$} markers, see {@link com.palantir.javaformat.java.NonNlsMarkers}. + * + * @param literals for each marker, the index of the string literal it marks, or -1 + */ + public abstract void appendNonNlsMarkers(State state, String comment, Range range, List literals); + @Override public String toString() { return MoreObjects.toStringHelper(this).add("super", super.toString()).toString(); diff --git a/open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java b/open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java index 3750f74cf..49297093e 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java @@ -19,20 +19,25 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.Iterators; import com.google.common.collect.Range; -import com.google.errorprone.annotations.Immutable; import com.palantir.javaformat.CommentsHelper; import com.palantir.javaformat.Input; import com.palantir.javaformat.Newlines; import com.palantir.javaformat.Op; import com.palantir.javaformat.Output; +import com.palantir.javaformat.java.JavaCommentsHelper; +import com.palantir.javaformat.java.NonNlsMarkers; +import java.util.List; /** A leaf node in a {@link Doc} for a non-token. */ -@Immutable public final class Comment extends Doc implements Op { private final Input.Tok tok; - private Comment(Input.Tok tok) { + /** For a comment of NON-NLS markers, the index of the string literal each marker marks, or -1; else empty. */ + private final List nonNlsLiterals; + + private Comment(Input.Tok tok, List nonNlsLiterals) { this.tok = tok; + this.nonNlsLiterals = nonNlsLiterals; } /** @@ -42,7 +47,18 @@ private Comment(Input.Tok tok) { * @return the new {@code Tok} */ public static Comment make(Input.Tok tok) { - return new Comment(tok); + return new Comment(tok, List.of()); + } + + /** + * A comment of {@code //$NON-NLS-n$} markers, see {@link NonNlsMarkers}. A marker with a literal is written at + * the end of the literal's output line and takes its room there, as a {@link ReservedWidth} after the literal; + * only the markers without one take room here. + * + * @param literals for each marker, the index of the string literal it marks, or -1 + */ + public static Comment makeNonNlsMarkers(Input.Tok tok, List literals) { + return new Comment(tok, List.copyOf(literals)); } @Override @@ -52,14 +68,25 @@ public void add(DocBuilder builder) { @Override protected float computeWidth() { + if (!nonNlsLiterals.isEmpty()) { + var width = 0; + var numbers = NonNlsMarkers.numbers(tok.getOriginalText()); + for (var i = 0; i < numbers.size(); i++) { + if (nonNlsLiterals.get(i) < 0) { + width += (width > 0 ? 1 : 0) + + NonNlsMarkers.marker(numbers.get(i)).length(); + } + } + return width; + } int idx = Newlines.firstBreak(tok.getOriginalText()); // only count the first line of multi-line block comments if (tok.isComment()) { if (idx > 0) { return idx; - } else if (tok.isSlashSlashComment() && !tok.getOriginalText().startsWith("// ")) { - // Account for line comments with missing spaces, see computeFlat. - return tok.length() + 1; + } else if (tok.isSlashSlashComment()) { + // The flat text may have gained the space after the slashes, see computeFlat. + return getFlat().length(); } else { return tok.length(); } @@ -69,11 +96,11 @@ protected float computeWidth() { @Override protected String computeFlat() { - // TODO(cushon): commentsHelper.rewrite doesn't get called for spans that fit in a single - // line. That's fine for multi-line comment reflowing, but problematic for adding missing - // spaces in line comments. - if (tok.isSlashSlashComment() && !tok.getOriginalText().startsWith("// ")) { - return "// " + tok.getOriginalText().substring("//".length()); + // A level that fits on one line is written from its flat text, and commentsHelper.rewrite is not called for + // the comments in it. So the space a line comment gets after its slashes is added here as well, by the rule + // that rewrite applies: a trailing `//foo` and a trailing `//$NON-NLS-1$` then come out the same either way. + if (tok.isSlashSlashComment()) { + return JavaCommentsHelper.addMissingSpace(tok.getOriginalText()); } return tok.getOriginalText(); } @@ -101,7 +128,11 @@ public State computeBreaks( @Override public void write(State state, Output output) { - output.append(state, state.getTokText(this), range()); + if (nonNlsLiterals.isEmpty()) { + output.append(state, state.getTokText(this), range()); + } else { + output.appendNonNlsMarkers(state, state.getTokText(this), range(), nonNlsLiterals); + } } @Override diff --git a/open-java-format/src/main/java/com/palantir/javaformat/doc/ReservedWidth.java b/open-java-format/src/main/java/com/palantir/javaformat/doc/ReservedWidth.java new file mode 100644 index 000000000..bb2c80927 --- /dev/null +++ b/open-java-format/src/main/java/com/palantir/javaformat/doc/ReservedWidth.java @@ -0,0 +1,72 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.javaformat.doc; + +import com.google.common.base.MoreObjects; +import com.google.common.collect.Range; +import com.palantir.javaformat.CommentsHelper; +import com.palantir.javaformat.Op; +import com.palantir.javaformat.Output; + +/** + * A leaf node in a {@link Doc} that takes room on its line without writing anything: the room for a + * {@code //$NON-NLS-n$} marker after the string literal it marks, which {@link Comment} writes at the end of the + * literal's line once the line breaks are known. So the layout already counts the marker on that line. + */ +public final class ReservedWidth extends Doc implements Op { + private final int width; + + private ReservedWidth(int width) { + this.width = width; + } + + public static ReservedWidth make(int width) { + return new ReservedWidth(width); + } + + @Override + public void add(DocBuilder builder) { + builder.add(this); + } + + @Override + protected float computeWidth() { + return width; + } + + @Override + protected String computeFlat() { + return ""; + } + + @Override + protected Range computeRange() { + return Doc.EMPTY_RANGE; + } + + @Override + public State computeBreaks( + CommentsHelper commentsHelper, int maxWidth, State state, Obs.ExplorationNode observationNode) { + return state.withColumn(state.column() + width); + } + + @Override + public void write(State state, Output output) {} + + @Override + public String toString() { + return MoreObjects.toStringHelper(this).add("width", width).toString(); + } +} diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadataBuilder.java b/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadataBuilder.java index ef0669f3c..7bc9846d4 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadataBuilder.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadataBuilder.java @@ -24,6 +24,7 @@ import com.palantir.javaformat.OpsBuilder.BlankLineWanted; import java.util.HashMap; import java.util.Map; +import javax.annotation.Nullable; public final class InputMetadataBuilder { private final Map blankLines = new HashMap<>(); // Info on blank lines. @@ -50,6 +51,12 @@ public void markForPartialFormat(Input.Token start, Input.Token end) { partialFormatRanges.add(Range.closed(lo, hi)); } + /** The region marked for partial formatting that holds tok {@code k}, or null. */ + @Nullable + public Range regionContaining(int k) { + return partialFormatRanges.rangeContaining(k); + } + public InputMetadata build() { return new InputMetadata(ImmutableMap.copyOf(blankLines), ImmutableRangeSet.copyOf(partialFormatRanges)); } 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 83996c93d..6c0f89f43 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 @@ -174,6 +174,21 @@ private static boolean isJBangDirective(String text) { private static final Pattern LINE_COMMENT_NO_SPACE_PREFIX = Pattern.compile("^//+(noinspection|\\$NON-NLS-\\d+\\$)"); + /** + * Adds the missing space after the slashes of a line comment: {@code //foo} becomes {@code // foo}. The IDE markers + * {@code //noinspection} and {@code //$NON-NLS-n$} stay as written, since their tools recognise them only without + * the space. {@link com.palantir.javaformat.doc.Comment#computeFlat()} applies the same rule to a comment that is + * written flat, for which {@link #rewrite} is not called. + */ + public static String addMissingSpace(String line) { + var matcher = LINE_COMMENT_MISSING_SPACE_PREFIX.matcher(line); + if (matcher.find() && !LINE_COMMENT_NO_SPACE_PREFIX.matcher(line).find()) { + var length = matcher.group(1).length(); + return "/".repeat(length) + " " + line.substring(length); + } + return line; + } + private static String lineCommentPrefix(String line) { int prefixLength = 0; while (prefixLength < line.length() && line.charAt(prefixLength) == '/') { @@ -191,14 +206,9 @@ private List wrapLineComments(List lines, int column0) { // be read from the trimmed text: otherwise the prefix comes back empty and the wrapped // remainder is emitted as bare code. line = CharMatcher.whitespace().trimLeadingFrom(line); - // Add missing leading spaces to line comments: `//foo` -> `// foo`. - Matcher matcher = LINE_COMMENT_MISSING_SPACE_PREFIX.matcher(line); - if (matcher.find() && !LINE_COMMENT_NO_SPACE_PREFIX.matcher(line).find()) { - int length = matcher.group(1).length(); - line = "/".repeat(length) + " " + line.substring(length); - } - if (line.startsWith("// MOE:")) { - // don't wrap comments for https://github.com/google/MOE + line = addMissingSpace(line); + if (NonNlsMarkers.isMarkersOnly(line)) { + // Eclipse reads the markers only on the line of their literal, so a run of them is never wrapped. result.add(line); continue; } diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/JavaOutput.java b/open-java-format/src/main/java/com/palantir/javaformat/java/JavaOutput.java index 36b015bc2..1b254f680 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/JavaOutput.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/JavaOutput.java @@ -155,6 +155,66 @@ public void append(State state, String text, Range range) { } } + /** + * Writes each marker of a {@code //$NON-NLS-1$ //$NON-NLS-2$} comment, see {@link NonNlsMarkers}, to the output + * line of the literal it marks, numbered by the position of the literal among the string literals written on + * that line, so that every marker stays with its literal when the statement is wrapped (#23). A marker without a + * literal stays here. + */ + @Override + public void appendNonNlsMarkers(State state, String comment, Range range, List literals) { + var numbers = NonNlsMarkers.numbers(comment); + var current = mutableLines.size(); + if (numbers.size() != literals.size() || range.isEmpty() || newlinesPending > 0 || ranges.size() <= current) { + // Not a comment as paired with its literals, or not trailing the code of the current line. + append(state, comment, range); + return; + } + var remaining = new ArrayList(); + for (var i = 0; i < numbers.size(); i++) { + int literal = literals.get(i); + var line = literal < 0 ? -1 : outputLineOf(literal, current); + if (line < 0) { + remaining.add(NonNlsMarkers.marker(numbers.get(i))); + } else if (line == current) { + remaining.add(NonNlsMarkers.marker(stringLiteralsOnOutputLine(line, literal))); + } else { + var marker = NonNlsMarkers.marker(stringLiteralsOnOutputLine(line, literal)); + mutableLines.set(line, mutableLines.get(line) + " " + marker); + } + } + if (remaining.isEmpty()) { + // Every marker went to an earlier line. The comment still counts to this line, where it was written, so + // that a partial format of the statement covers it. + ranges.set(current, union(ranges.get(current), range)); + lastK = range.upperEndpoint(); + } else { + append(state, String.join(" ", remaining), range); + } + } + + /** The output line, searched back from {@code current}, that holds tok {@code k}, or -1 if it is not written. */ + private int outputLineOf(int k, int current) { + for (var line = current; line >= 0; line--) { + if (ranges.get(line).contains(k)) { + return line; + } + } + return -1; + } + + /** The position of the string literal {@code k} among the string literals written on an output line. */ + private int stringLiteralsOnOutputLine(int line, int k) { + var count = 0; + for (int i = ranges.get(line).lowerEndpoint(); i <= k; i++) { + var tok = javaInput.getToken(i).getTok(); + if (tok.getIndex() == i && tok.getText().startsWith("\"")) { + count++; + } + } + return count; + } + @Override public void indent(int indent) { spacesPending.append(" ".repeat(indent)); diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/NonNlsMarkers.java b/open-java-format/src/main/java/com/palantir/javaformat/java/NonNlsMarkers.java new file mode 100644 index 000000000..312760db3 --- /dev/null +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/NonNlsMarkers.java @@ -0,0 +1,54 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.javaformat.java; + +import java.util.List; +import java.util.regex.Pattern; + +/** + * The markers with which Eclipse exempts a string literal from its warning about non-externalized strings: a trailing + * line comment {@code //$NON-NLS-n$}, where n is the position of the literal among the string literals of its line, + * one marker per exempted literal, as in {@code //$NON-NLS-1$ //$NON-NLS-2$}. Eclipse reads a marker only with this + * exact spelling and only on the line of its literal, so the formatter leaves the spelling alone, never wraps a comment + * of markers, and when it moves a literal to another line it moves the marker along and renumbers it (#23): see + * {@link com.palantir.javaformat.OpsBuilder#build()}, which pairs each marker with its literal, and + * {@link JavaOutput#appendNonNlsMarkers}, which writes it on the literal's line. + */ +public final class NonNlsMarkers { + private static final Pattern MARKER = Pattern.compile("//\\$NON-NLS-(\\d{1,9})\\$"); + + /** A line comment that holds markers and nothing else. */ + private static final Pattern MARKERS_ONLY = + Pattern.compile("//\\$NON-NLS-\\d{1,9}\\$([ \\t]+//\\$NON-NLS-\\d{1,9}\\$)*"); + + public static boolean isMarkersOnly(String comment) { + return comment.startsWith("//$NON-NLS-") + && MARKERS_ONLY.matcher(comment).matches(); + } + + /** The numbers of the markers in a comment, in order; an unmodifiable list. */ + public static List numbers(String comment) { + return MARKER.matcher(comment) + .results() + .map(result -> Integer.parseInt(result.group(1))) + .toList(); + } + + public static String marker(int n) { + return "//$NON-NLS-" + n + "$"; + } + + private NonNlsMarkers() {} +} diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/StringWrapper.java b/open-java-format/src/main/java/com/palantir/javaformat/java/StringWrapper.java index 28c954f19..d62ea7e1d 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/StringWrapper.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/StringWrapper.java @@ -43,6 +43,7 @@ import com.sun.tools.javac.util.Position.LineMap; import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Comparator; import java.util.Deque; import java.util.HashMap; import java.util.Iterator; @@ -50,6 +51,7 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; +import javax.annotation.Nullable; /** Wraps string literals that exceed the column limit. */ public final class StringWrapper { @@ -165,10 +167,13 @@ protected TreeRangeMap getReflowReplacements() { List longStringLiterals = new ArrayList<>(); // Paths to text blocks to be re-indented. List textBlocks = new ArrayList<>(); - new LongStringsAndTextBlockScanner(longStringLiterals, textBlocks).scan(new TreePath(unit), null); + // Every string literal, for the numbering of the NON-NLS markers of a line. + List stringLiterals = new ArrayList<>(); + new LongStringsAndTextBlockScanner(longStringLiterals, textBlocks, stringLiterals) + .scan(new TreePath(unit), null); TreeRangeMap replacements = TreeRangeMap.create(); indentTextBlocks(replacements, textBlocks); - wrapLongStrings(replacements, longStringLiterals); + wrapLongStrings(replacements, longStringLiterals, stringLiterals); return replacements; } @@ -176,10 +181,13 @@ private class LongStringsAndTextBlockScanner extends TreePathScanner private final List longStringLiterals; private final List textBlocks; + private final List stringLiterals; - LongStringsAndTextBlockScanner(List longStringLiterals, List textBlocks) { + LongStringsAndTextBlockScanner( + List longStringLiterals, List textBlocks, List stringLiterals) { this.longStringLiterals = longStringLiterals; this.textBlocks = textBlocks; + this.stringLiterals = stringLiterals; } @Override @@ -187,6 +195,7 @@ public Void visitLiteral(LiteralTree literalTree, Void aVoid) { if (literalTree.getKind() != Kind.STRING_LITERAL) { return null; } + stringLiterals.add(literalTree); int pos = getStartPosition(literalTree); if (input.substring(pos, Math.min(input.length(), pos + TEXT_BLOCK_DELIMITER.length())) .equals(TEXT_BLOCK_DELIMITER)) { @@ -351,8 +360,26 @@ private String computePrefixIndentation( return " ".repeat(extraIndent + startParentColumn); } + /** + * A long literal with its neighbours in a chain of {@code +}, reflowed as one text; see {@link #flatten}. + * {@code plain} is its text reflowed without regard to markers, in pieces, one per line. + */ + private record Chain( + List flat, + boolean firstInChain, + int start, + int end, + int lineEnd, + int startColumn, + int firstLineColumn, + List plain) {} + @SuppressWarnings("for-rollout:NullAway") - private void wrapLongStrings(TreeRangeMap replacements, List longStringLiterals) { + private void wrapLongStrings( + TreeRangeMap replacements, + List longStringLiterals, + List stringLiterals) { + var chains = new ArrayList(); for (TreePath path : longStringLiterals) { // Find the outermost contiguous enclosing concatenation expression TreePath enclosing = path; @@ -391,10 +418,180 @@ private void wrapLongStrings(TreeRangeMap replacements, List components = stringComponents(input, unit, flat); + var plain = reflow(columnLimit, trailing, components, first.get(), startColumn, fistLineCol, "", ""); + chains.add(new Chain( + flat, + first.get(), + getStartPosition(flat.get(0)), + end, + lineEnd, + startColumn, + fistLineCol, + plain)); + } + for (var chain : chains) { + var joined = join(chain, chain.plain(), "", ""); + if (joined.equals(input.substring(chain.start(), chain.end()))) { + // Every literal on a line past the limit is a candidate; this one does not change. + continue; + } + var markers = lineMarkers(chains, chain, stringLiterals); + if (markers == null) { + replacements.put(Range.closedOpen(chain.start(), chain.end()), joined); + continue; + } + var components = stringComponents(input, unit, chain.flat()); + var pieces = reflow( + columnLimit, + markers.tail(2).length(), + components, + chain.firstInChain(), + chain.startColumn(), + chain.firstLineColumn(), + markers.firstLine(), + markers.otherLines()); replacements.put( - Range.closedOpen(getStartPosition(flat.get(0)), getEndPosition(unit, getLast(flat))), - reflow(separator, columnLimit, trailing, components, first.get(), startColumn, fistLineCol)); + Range.closedOpen(chain.start(), chain.lineEnd()), + join(chain, pieces, markers.firstLine(), markers.otherLines()) + markers.tail(pieces.size())); + } + } + + private String join(Chain chain, List pieces, String firstLineMarkers, String otherLineMarkers) { + return render( + separator, + pieces, + chain.firstInChain() ? chain.firstLineColumn() + 4 : chain.startColumn() - 2, + firstLineMarkers, + otherLineMarkers); + } + + /** + * The NON-NLS markers of the line a chain ends on, see {@link NonNlsMarkers}, sorted by where they go once the + * chain is reflowed: a marker of a literal before the chain keeps its number on the first line; the chain, if + * marked, gets a marker after every piece, numbered by the piece's line; a marker of a literal after the chain + * is counted from the last piece on the last line; and a marker without a literal stays at the end. Null when + * the markers are left alone: when the comment holds anything else, or when another chain of the line + * changes too, since the chain rewrites the rest of its line. + */ + @Nullable + private LineMarkers lineMarkers(List chains, Chain chain, List stringLiterals) { + var tail = input.substring(chain.end(), chain.lineEnd()); + var comment = lineCommentStart(tail); + if (comment < 0 + || !NonNlsMarkers.isMarkersOnly(tail.substring(comment).stripTrailing())) { + return null; + } + var lastLine = lineMap.getLineNumber(chain.end() - 1); + for (var other : chains) { + // The same chain is found once per long literal in it. + if (other.start() != chain.start() + && lineMap.getLineNumber(other.end() - 1) == lastLine + && !join(other, other.plain(), "", "").equals(input.substring(other.start(), other.end()))) { + return null; + } + } + var literals = literalsOnLine(stringLiterals, lastLine); + var lastOfChain = literals.indexOf(getLast(chain.flat())); + if (lastOfChain < 0) { + // A text block, which starts on another line. + return null; + } + var firstOfChain = lastOfChain; + while (firstOfChain > 0 && chain.flat().contains(literals.get(firstOfChain - 1))) { + firstOfChain--; + } + var before = new ArrayList(); + var after = new ArrayList(); + var unmatched = new ArrayList(); + var chainMarked = false; + for (int n : NonNlsMarkers.numbers(tail.substring(comment))) { + var i = n - 1; + if (i < 0 || i >= literals.size()) { + unmatched.add(NonNlsMarkers.marker(n)); + } else if (i < firstOfChain) { + before.add(NonNlsMarkers.marker(n)); + } else if (i <= lastOfChain) { + chainMarked = true; + } else { + after.add(i - lastOfChain); + } } + var beforeOnFirstLine = literalsOnLine(stringLiterals, lineMap.getLineNumber(chain.start())).stream() + .filter(literal -> getStartPosition(literal) < chain.start()) + .count(); + return new LineMarkers( + List.copyOf(before), + (int) beforeOnFirstLine, + chainMarked, + List.copyOf(after), + List.copyOf(unmatched), + tail.substring(0, comment)); + } + + /** See {@link #lineMarkers}; {@code code} is the text between the chain and its line's comment. */ + private record LineMarkers( + List before, + int beforeOnFirstLine, + boolean chainMarked, + List after, + List unmatched, + String code) { + + /** The markers after the first piece, when it ends its line. */ + String firstLine() { + var markers = new ArrayList<>(before); + if (chainMarked) { + markers.add(NonNlsMarkers.marker(beforeOnFirstLine + 1)); + } + return markers.isEmpty() ? "" : " " + String.join(" ", markers); + } + + /** The markers after a piece in the middle. */ + String otherLines() { + return chainMarked ? " " + NonNlsMarkers.marker(1) : ""; + } + + /** The text after the last of {@code pieces}: the code, then the markers of that line. */ + String tail(int pieces) { + var markers = new ArrayList(); + var chainPosition = pieces > 1 ? 1 : beforeOnFirstLine + 1; + if (pieces == 1) { + markers.addAll(before); + } + if (chainMarked) { + markers.add(NonNlsMarkers.marker(chainPosition)); + } + for (int position : after) { + markers.add(NonNlsMarkers.marker(chainPosition + position)); + } + markers.addAll(unmatched); + return markers.isEmpty() ? code.stripTrailing() : code + String.join(" ", markers); + } + } + + /** The string literals that start on a line, in order. */ + private List literalsOnLine(List stringLiterals, long line) { + return stringLiterals.stream() + .filter(literal -> lineMap.getLineNumber(getStartPosition(literal)) == line) + .sorted(Comparator.comparingInt(StringWrapper::getStartPosition)) + .toList(); + } + + /** The index of the line comment in a line of code, the first {@code //} outside a literal, or -1. */ + private static int lineCommentStart(String line) { + for (var i = 0; i < line.length(); i++) { + var c = line.charAt(i); + if (c == '"' || c == '\'') { + for (i++; i < line.length() && line.charAt(i) != c; i++) { + if (line.charAt(i) == '\\') { + i++; + } + } + } else if (c == '/' && line.startsWith("//", i)) { + return i; + } + } + return -1; } private static boolean onSameLineAsParent(LineMap lineMap, TreePath path) { @@ -479,9 +676,9 @@ private static boolean isEscaped(String input, int idx) { } /** - * Reflows the given source text, trying to split on word boundaries. + * Reflows the given source text, trying to split on word boundaries. Returns the pieces, one per line; see + * {@link #render}. * - * @param separator the line separator * @param columnLimit the number of columns to wrap at * @param trailing extra space to leave after the last line * @param components the text to reflow @@ -489,18 +686,22 @@ private static boolean isEscaped(String input, int idx) { * @param textStartColumn the column position of the beginning of the original text * @param firstLineStartColumn the column where the very first line starts (can be less than textStartColumn if text * follows variable declaration) + * @param firstLineMarkers the NON-NLS markers that trail the first line, with their leading blank, or empty + * @param otherLineMarkers the NON-NLS markers that trail every further line but the last, whose markers are part + * of {@code trailing} */ - private static String reflow( - String separator, + private static List reflow( int columnLimit, int trailing, ImmutableList components, boolean first0, int textStartColumn, - int firstLineStartColumn) { + int firstLineStartColumn, + String firstLineMarkers, + String otherLineMarkers) { // We have space between the start column and the limit to output the first line. - // Reserve two spaces for the quotes. - int width = columnLimit - textStartColumn - 2; + // Reserve two spaces for the quotes, and room for the markers of the line. + var width = columnLimit - textStartColumn - 2 - firstLineMarkers.length(); Deque input = new ArrayDeque<>(components); List lines = new ArrayList<>(); boolean first = first0; @@ -508,7 +709,8 @@ private static String reflow( int length = 0; List line = new ArrayList<>(); if (input.stream().mapToInt(String::length).sum() <= width) { - width -= trailing; + // The last line, probably: its markers are in the trailing text, not reserved on their own. + width -= trailing - (lines.isEmpty() ? 0 : otherLineMarkers.length()); } while (!input.isEmpty() && (length <= 4 || (length + input.peekFirst().length()) < width)) { @@ -535,13 +737,28 @@ private static String reflow( width += textStartColumn - firstLineStartColumn; first = false; } + if (lines.size() == 1) { + width += firstLineMarkers.length() - otherLineMarkers.length(); + } } + return lines; + } - return lines.stream() - .collect(joining( - "\"" + separator + " ".repeat(first0 ? firstLineStartColumn + 4 : textStartColumn - 2) + "+ \"", - "\"", - "\"")); + /** Renders reflowed pieces as literals in a chain of {@code +}, one per line, indented by {@code column}. */ + private static String render( + String separator, List pieces, int column, String firstLineMarkers, String otherLineMarkers) { + var continuation = separator + " ".repeat(column) + "+ "; + var result = new StringBuilder(); + for (var i = 0; i < pieces.size(); i++) { + if (i > 0) { + result.append(continuation); + } + result.append('"').append(pieces.get(i)).append('"'); + if (i < pieces.size() - 1) { + result.append(i == 0 ? firstLineMarkers : otherLineMarkers); + } + } + return result.toString(); } /** diff --git a/open-java-format/src/test/java/com/palantir/javaformat/doc/ReservedWidthTest.java b/open-java-format/src/test/java/com/palantir/javaformat/doc/ReservedWidthTest.java new file mode 100644 index 000000000..866c9d53a --- /dev/null +++ b/open-java-format/src/test/java/com/palantir/javaformat/doc/ReservedWidthTest.java @@ -0,0 +1,45 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.javaformat.doc; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.palantir.javaformat.CommentsHelper; +import org.junit.jupiter.api.Test; + +/** A {@link ReservedWidth} takes the room of a NON-NLS marker that is written to its line later, and nothing else. */ +final class ReservedWidthTest { + + private static final int WIDTH = " //$NON-NLS-1$".length(); + + @Test + void takesItsWidthOnTheLineButHasNoText() { + var reserved = ReservedWidth.make(WIDTH); + + assertThat(reserved.getWidth()).isEqualTo((float) WIDTH); + assertThat(reserved.getFlat()).isEmpty(); + assertThat(reserved.range().isEmpty()).isTrue(); + } + + @Test + void movesTheColumnByItsWidthWhenTheBreaksAreComputed() { + var before = State.startingState().withColumn(10); + CommentsHelper noRewrite = (tok, maxWidth, column0) -> tok.getOriginalText(); + + var after = ReservedWidth.make(WIDTH).computeBreaks(noRewrite, 120, before, Obs.createRoot(new NoopSink())); + + assertThat(after.column()).isEqualTo(10 + WIDTH); + } +} 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 9e4d24b3f..3f75b8ea8 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 @@ -399,6 +399,79 @@ void lineCommentGetsItsMissingSpaceExceptIdeMarkers() throws Exception { assertThat(Formatter.create().formatSource(input)).isEqualTo(expected); } + @Test + void trailingLineCommentGetsItsMissingSpaceExceptIdeMarkers() throws Exception { + // The same rule for a comment that trails a statement which fits on one line. Such a statement is written + // from its flat text, which skips the comment rewriting, so the IDE markers used to get the space too (#23). + var input = """ + class T { + void m() { + String a = "x"; //$NON-NLS-1$ + String b = "y"; //noinspection unchecked + String c = "z"; //foo + String d = "w"; ///foo + String e = "v"; // bar + String f = "u"; /// baz + } + } + """; + var expected = """ + class T { + void m() { + String a = "x"; //$NON-NLS-1$ + String b = "y"; //noinspection unchecked + String c = "z"; // foo + String d = "w"; /// foo + String e = "v"; // bar + String f = "u"; /// baz + } + } + """; + + var once = Formatter.create().formatSource(input); + assertThat(once).isEqualTo(expected); + assertThat(Formatter.create().formatSource(once)).isEqualTo(once); + } + + @Test + void nonNlsMarkersStayOnTheLineOfTheirLiteral() throws Exception { + // Eclipse numbers a marker by the position of its literal on the line. When a statement is wrapped, each + // marker moves to the line of its literal and is renumbered for that line; when a literal is reflowed, every + // piece gets one (#23). A marker whose number has no literal stays where it is. + var input = """ + class T { + void m() { + String s = Messages.format("first argument that is long enough to matter", "second argument that is also rather long", "third one"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + String reflowed = "a single string literal that is far too long to fit into one hundred and twenty columns, so the formatter reflows it"; //$NON-NLS-1$ + String n = Messages.format("first argument that is long enough to matter", "second argument that is also rather long", "third one"); //$NON-NLS-5$ + } + } + """; + var expected = """ + class T { + void m() { + String s = Messages.format( + "first argument that is long enough to matter", //$NON-NLS-1$ + "second argument that is also rather long", //$NON-NLS-1$ + "third one"); //$NON-NLS-1$ + String reflowed = + "a single string literal that is far too long to fit into one hundred and twenty" //$NON-NLS-1$ + + " columns, so the formatter reflows it"; //$NON-NLS-1$ + String n = Messages.format( + "first argument that is long enough to matter", + "second argument that is also rather long", + "third one"); //$NON-NLS-5$ + } + } + """; + + var formatter = Formatter.createFormatter( + JavaFormatterOptions.builder().style(Style.OJF).build()); + var once = formatter.formatSourceAndFixImports(input); + assertThat(once).isEqualTo(expected); + assertThat(formatter.formatSourceAndFixImports(once)).isEqualTo(once); + } + @Test void throwsFormatterException() throws Exception { assertThatThrownBy(() -> Formatter.create().formatSourceAndFixImports("package foo; public class {")) @@ -472,21 +545,6 @@ void wrapMarkdownDocstringRunKeepsTheSlashPrefix() throws Exception { assertThat(Formatter.create().formatSource(input)).isEqualTo(expected); } - @Test - void dontWrapMoeLineComments() throws Exception { - assertThat(Formatter.create() - .formatSource("class T {\n" - + " // MOE: one long incredibly" - + " unbroken sentence moving from topic to topic so that no-one had a" - + " chance to interrupt;\n" - + "}\n")) - .isEqualTo("class T {\n" - + " // MOE: one long incredibly" - + " unbroken sentence moving from topic to topic so that no-one had a" - + " chance to interrupt;\n" - + "}\n"); - } - @Test void canParse_java9_private_interface_methods() { assertThatCode(() -> Formatter.create() diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/JavaCommentsHelperTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/JavaCommentsHelperTest.java index f3d15f6cf..59e564baf 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/JavaCommentsHelperTest.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/JavaCommentsHelperTest.java @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat; import com.palantir.javaformat.java.JavaFormatterOptions.Style; +import java.util.List; import org.junit.jupiter.api.Test; /** @@ -111,4 +112,38 @@ void anOrdinaryLineCommentStillWraps() throws Exception { assertThat(rewritten.lines()).hasSize(2); assertThat(rewritten.lines()).allSatisfy(line -> assertThat(line.trim()).startsWith("// ")); } + + @Test + void aRunOfNonNlsMarkersIsNeverWrapped() throws Exception { + // Eclipse reads a marker only on the line of its literal, so a run that reaches past the limit stays as it is + // instead of being wrapped at a blank into a `// //$NON-NLS-3$` line of its own. + var run = "//$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$" + + " //$NON-NLS-8$ //$NON-NLS-9$"; + + assertThat(rewrite(run, Style.OJF)).isEqualTo(run); + } + + @Test + void addMissingSpacePutsABlankAfterTheSlashesWhateverTheirNumber() { + assertThat(JavaCommentsHelper.addMissingSpace("//foo")).isEqualTo("// foo"); + assertThat(JavaCommentsHelper.addMissingSpace("///foo")).isEqualTo("/// foo"); + assertThat(JavaCommentsHelper.addMissingSpace("//-- foo")).isEqualTo("// -- foo"); + } + + @Test + void addMissingSpaceLeavesSpacedCommentsAndIdeMarkersAlone() { + // A blank or a tab after the slashes is a space already; the IDE markers are recognised only without one. + var comments = List.of( + "// foo", + "// two spaces", + "//\tfoo", + "//", + "///", + "//$NON-NLS-1$ //$NON-NLS-2$", + "//noinspection unchecked"); + + for (var comment : comments) { + assertThat(JavaCommentsHelper.addMissingSpace(comment)).as(comment).isEqualTo(comment); + } + } } diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/NonNlsMarkersTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/NonNlsMarkersTest.java new file mode 100644 index 000000000..e53c363ec --- /dev/null +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/NonNlsMarkersTest.java @@ -0,0 +1,63 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.javaformat.java; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; +import org.junit.jupiter.api.Test; + +final class NonNlsMarkersTest { + + @Test + void aCommentOfMarkersAloneIsMarkersOnly() { + assertThat(NonNlsMarkers.isMarkersOnly("//$NON-NLS-1$")).isTrue(); + assertThat(NonNlsMarkers.isMarkersOnly("//$NON-NLS-1$ //$NON-NLS-2$")).isTrue(); + assertThat(NonNlsMarkers.isMarkersOnly("//$NON-NLS-1$\t//$NON-NLS-12$")).isTrue(); + } + + @Test + void anyOtherTextMakesACommentNotMarkersOnly() { + // A blank after the slashes, text next to a marker, or a marker without its number or its closing dollar. + var comments = List.of( + "// $NON-NLS-1$", + "//$NON-NLS-1$ note", + "// note //$NON-NLS-1$", + "//$NON-NLS-$", + "//$NON-NLS-1", + "// foo"); + + for (var comment : comments) { + assertThat(NonNlsMarkers.isMarkersOnly(comment)).as(comment).isFalse(); + } + } + + @Test + void numbersListsTheMarkersInTheirOrder() { + assertThat(NonNlsMarkers.numbers("//$NON-NLS-3$ //$NON-NLS-1$ //$NON-NLS-12$")).containsExactly(3, 1, 12); + } + + @Test + void numbersFindsMarkersAnywhereInAComment() { + assertThat(NonNlsMarkers.numbers("// note //$NON-NLS-2$")).containsExactly(2); + assertThat(NonNlsMarkers.numbers("// no markers here")).isEmpty(); + } + + @Test + void markerSpellsWhatEclipseReads() { + assertThat(NonNlsMarkers.marker(7)).isEqualTo("//$NON-NLS-7$"); + assertThat(NonNlsMarkers.numbers(NonNlsMarkers.marker(42))).containsExactly(42); + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/StringWrapperIntegrationTest/ojf-issue-23-marked-literals.input b/open-java-format/src/test/resources/com/palantir/javaformat/java/StringWrapperIntegrationTest/ojf-issue-23-marked-literals.input new file mode 100644 index 000000000..a8d077bea --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/StringWrapperIntegrationTest/ojf-issue-23-marked-literals.input @@ -0,0 +1,7 @@ +class T { + void f() { + String s = "one long incredibly unbroken sentence moving from topic to topic"; //$NON-NLS-1$ + foo("x", "a long unbroken sentence", "y"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + String t = "short" + " and one long sentence"; //$NON-NLS-1$ //$NON-NLS-2$ + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/StringWrapperIntegrationTest/ojf-issue-23-marked-literals.output b/open-java-format/src/test/resources/com/palantir/javaformat/java/StringWrapperIntegrationTest/ojf-issue-23-marked-literals.output new file mode 100644 index 000000000..8a1a7a10e --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/StringWrapperIntegrationTest/ojf-issue-23-marked-literals.output @@ -0,0 +1,20 @@ +class T { + void f() { + String s = "one long" //$NON-NLS-1$ + + " incredibly" //$NON-NLS-1$ + + " unbroken" //$NON-NLS-1$ + + " sentence" //$NON-NLS-1$ + + " moving from" //$NON-NLS-1$ + + " topic to" //$NON-NLS-1$ + + " topic"; //$NON-NLS-1$ + foo( + "x", //$NON-NLS-1$ + "a long" //$NON-NLS-1$ + + " unbroken" //$NON-NLS-1$ + + " sentence", //$NON-NLS-1$ + "y"); //$NON-NLS-1$ + String t = "short" //$NON-NLS-1$ + + " and one long" //$NON-NLS-1$ + + " sentence"; //$NON-NLS-1$ + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-non-nls-trailing-marker.input b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-non-nls-trailing-marker.input new file mode 100644 index 000000000..19ef37cda --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-non-nls-trailing-marker.input @@ -0,0 +1,20 @@ +class NonNlsTrailingMarker { + private static final String FIELD = "field"; //$NON-NLS-1$ + + String markers() { + String a = "x"; //$NON-NLS-1$ + String two = "x" + "y"; //$NON-NLS-1$ //$NON-NLS-2$ + log( "message" ); //$NON-NLS-1$ + return a; //noinspection ReturnOfNull + } + + void ownLine() { + //$NON-NLS-1$ + String b = "x"; + } + + void otherComments() { + String c = "x"; //foo + String d = "x"; // bar + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-non-nls-trailing-marker.output b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-non-nls-trailing-marker.output new file mode 100644 index 000000000..a566f2bed --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-non-nls-trailing-marker.output @@ -0,0 +1,20 @@ +class NonNlsTrailingMarker { + private static final String FIELD = "field"; //$NON-NLS-1$ + + String markers() { + String a = "x"; //$NON-NLS-1$ + String two = "x" + "y"; //$NON-NLS-1$ //$NON-NLS-2$ + log("message"); //$NON-NLS-1$ + return a; //noinspection ReturnOfNull + } + + void ownLine() { + //$NON-NLS-1$ + String b = "x"; + } + + void otherComments() { + String c = "x"; // foo + String d = "x"; // bar + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-reflowed-literal.input b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-reflowed-literal.input new file mode 100644 index 000000000..15c3eb9ce --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-reflowed-literal.input @@ -0,0 +1,9 @@ +class ReflowedLiteral { + void markers() { + String reflowed = "a single string literal that is far too long to fit into one hundred and twenty columns, so the formatter reflows it"; //$NON-NLS-1$ + String unmarked = "a single string literal that is far too long to fit into one hundred and twenty columns, so the formatter reflows it"; + log( "short", "a string literal as the second argument that is far too long to fit into one hundred and twenty columns", "after" ); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + log(KEY, "a string literal as the second argument that is far too long to fit into one hundred and twenty columns, so it reflows", VALUE); //$NON-NLS-1$ + String chain = "a string literal that is far too long to fit into one hundred and twenty columns, so it reflows" + "with another literal"; //$NON-NLS-1$ //$NON-NLS-2$ + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-reflowed-literal.output b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-reflowed-literal.output new file mode 100644 index 000000000..fdc8aaefa --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-reflowed-literal.output @@ -0,0 +1,24 @@ +class ReflowedLiteral { + void markers() { + String reflowed = + "a single string literal that is far too long to fit into one hundred and twenty" //$NON-NLS-1$ + + " columns, so the formatter reflows it"; //$NON-NLS-1$ + String unmarked = + "a single string literal that is far too long to fit into one hundred and twenty columns, so the" + + " formatter reflows it"; + log( + "short", //$NON-NLS-1$ + "a string literal as the second argument that is far too long to fit into one hundred" //$NON-NLS-1$ + + " and twenty columns", //$NON-NLS-1$ + "after"); //$NON-NLS-1$ + log( + KEY, + "a string literal as the second argument that is far too long to fit into one hundred" //$NON-NLS-1$ + + " and twenty columns, so it reflows", //$NON-NLS-1$ + VALUE); + String chain = + "a string literal that is far too long to fit into one hundred and twenty columns, so it" //$NON-NLS-1$ + + " reflows" //$NON-NLS-1$ + + "with another literal"; //$NON-NLS-1$ + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-wrapped-arguments.input b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-wrapped-arguments.input new file mode 100644 index 000000000..e13bb3e71 --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-wrapped-arguments.input @@ -0,0 +1,11 @@ +class WrappedArguments { + void markers() { + String message = Messages.format("first argument that is long enough to matter", "second argument that is also rather long", "third one"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + String second = Messages.format( "first argument that is long enough to matter", "second argument that is also rather long", "third one" ); //$NON-NLS-2$ + String twoOnALine = Messages.format("first argument that is long enough to matter", "a" + "b", "third argument that is also rather long"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ + String keyAndValue = Messages.format(KEY, "an argument that is long enough to matter", "second argument that is also rather long", VALUE); //$NON-NLS-1$ //$NON-NLS-2$ + String chained = builder.append("first argument that is long enough to matter").append("second argument that is also rather long").toString(); //$NON-NLS-1$ //$NON-NLS-2$ + String noSuchLiteral = Messages.format("first argument that is long enough to matter", "second argument that is also rather long", "third one"); //$NON-NLS-1$ //$NON-NLS-5$ + String fitsOnTheNextLine = Messages.format("first argument", "second one", "third"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-wrapped-arguments.output b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-wrapped-arguments.output new file mode 100644 index 000000000..b9d2836f9 --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ojf-issue-23-wrapped-arguments.output @@ -0,0 +1,30 @@ +class WrappedArguments { + void markers() { + String message = Messages.format( + "first argument that is long enough to matter", //$NON-NLS-1$ + "second argument that is also rather long", //$NON-NLS-1$ + "third one"); //$NON-NLS-1$ + String second = Messages.format( + "first argument that is long enough to matter", + "second argument that is also rather long", //$NON-NLS-1$ + "third one"); + String twoOnALine = Messages.format( + "first argument that is long enough to matter", //$NON-NLS-1$ + "a" + "b", //$NON-NLS-1$ //$NON-NLS-2$ + "third argument that is also rather long"); //$NON-NLS-1$ + String keyAndValue = Messages.format( + KEY, + "an argument that is long enough to matter", //$NON-NLS-1$ + "second argument that is also rather long", //$NON-NLS-1$ + VALUE); + String chained = builder.append("first argument that is long enough to matter") //$NON-NLS-1$ + .append("second argument that is also rather long") //$NON-NLS-1$ + .toString(); + String noSuchLiteral = Messages.format( + "first argument that is long enough to matter", //$NON-NLS-1$ + "second argument that is also rather long", + "third one"); //$NON-NLS-5$ + String fitsOnTheNextLine = + Messages.format("first argument", "second one", "third"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + } +}