From 823d7c99bc61e05e26ec68ede3c42231b6e2e62b Mon Sep 17 00:00:00 2001 From: google-java-format Team Date: Wed, 6 Mar 2024 00:24:37 -0800 Subject: [PATCH 1/8] Print all known values when an invalid range is given. For easier root cause analysis when an invalid request is passed. PiperOrigin-RevId: 613110312 --- .../googlejavaformat/java/JavaInput.java | 6 ++-- .../googlejavaformat/java/FormatterTest.java | 28 +++++++++++++++---- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java b/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java index 7b5eb841f..b5290ab10 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java @@ -573,9 +573,11 @@ Range characterRangeToTokenRange(Range characterRange) if (characterRange.upperEndpoint() > text.length()) { throw new FormatterException( String.format( - "error: invalid length %d, offset + length (%d) is outside the file", + "error: invalid offset (%d) or length (%d); offset + length (%d) > file length (%d)", + characterRange.lowerEndpoint(), characterRange.upperEndpoint() - characterRange.lowerEndpoint(), - characterRange.upperEndpoint())); + characterRange.upperEndpoint(), + text.length())); } // empty range stands for "format the line under the cursor" Range nonEmptyRange = diff --git a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java index 9bbca496a..b0d7b4030 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java @@ -64,7 +64,7 @@ public void testFormatAosp() throws Exception { Path tmpdir = testFolder.newFolder().toPath(); Path path = tmpdir.resolve("A.java"); - Files.write(path, input.getBytes(UTF_8)); + Files.writeString(path, input); StringWriter out = new StringWriter(); StringWriter err = new StringWriter(); @@ -116,7 +116,7 @@ public void testFormatLengthUpToEOF() throws Exception { Path tmpdir = testFolder.newFolder().toPath(); Path path = tmpdir.resolve("Foo.java"); - Files.write(path, input.getBytes(UTF_8)); + Files.writeString(path, input); StringWriter out = new StringWriter(); StringWriter err = new StringWriter(); @@ -133,7 +133,7 @@ public void testFormatLengthOutOfRange() throws Exception { Path tmpdir = testFolder.newFolder().toPath(); Path path = tmpdir.resolve("Foo.java"); - Files.write(path, input.getBytes(UTF_8)); + Files.writeString(path, input); StringWriter out = new StringWriter(); StringWriter err = new StringWriter(); @@ -142,7 +142,25 @@ public void testFormatLengthOutOfRange() throws Exception { String[] args = {"--offset", "0", "--length", "9999", path.toString()}; assertThat(main.format(args)).isEqualTo(1); assertThat(err.toString()) - .contains("error: invalid length 9999, offset + length (9999) is outside the file"); + .contains("error: invalid offset (0) or length (9999); offset + length (9999)"); + } + + @Test + public void testFormatOffsetOutOfRange() throws Exception { + String input = "class Foo{}\n"; + + Path tmpdir = testFolder.newFolder().toPath(); + Path path = tmpdir.resolve("Foo.java"); + Files.writeString(path, input); + + StringWriter out = new StringWriter(); + StringWriter err = new StringWriter(); + + Main main = new Main(new PrintWriter(out, true), new PrintWriter(err, true), System.in); + String[] args = {"--offset", "9998", "--length", "1", path.toString()}; + assertThat(main.format(args)).isEqualTo(1); + assertThat(err.toString()) + .contains("error: invalid offset (9998) or length (1); offset + length (9999)"); } @Test @@ -303,7 +321,7 @@ private void importOrdering(String sortArg, String outputResourceName) String inputResourceName = "com/google/googlejavaformat/java/testimports/A.input"; String input = getResource(inputResourceName); String expectedOutput = getResource(outputResourceName); - Files.write(path, input.getBytes(UTF_8)); + Files.writeString(path, input); StringWriter out = new StringWriter(); StringWriter err = new StringWriter(); From 8ee063ef78e16321fa4f815b14d246df783f3828 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Mon, 18 Mar 2024 09:08:28 -0700 Subject: [PATCH 2/8] Update tycho version from 3.0.0 to 3.0.5 I think this fixes crashes from the presubmit for unknown commit: ``` Error: Exception in thread "main" java.lang.NoSuchMethodError: 'java.lang.String org.eclipse.equinox.p2.core.IProvisioningAgent.getProperty(java.lang.String)' at org.eclipse.equinox.internal.p2.artifact.repository.simple.SimpleArtifactRepository.getAgentPropertyWithFallback(SimpleArtifactRepository.java:813) at org.eclipse.equinox.internal.p2.artifact.repository.simple.SimpleArtifactRepository.getMaximumThreads(SimpleArtifactRepository.java:1012) ``` I tested a PR with these changes and it was clean [1], so either this helps or it was a transient issue, but either way this shouldn't hurt. [1] https://github.com/google/google-java-format/actions/runs/8329274139 PiperOrigin-RevId: 616846344 --- eclipse_plugin/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eclipse_plugin/pom.xml b/eclipse_plugin/pom.xml index 9e6acdac0..b2c6e368a 100644 --- a/eclipse_plugin/pom.xml +++ b/eclipse_plugin/pom.xml @@ -32,7 +32,7 @@ UTF-8 - 3.0.0 + 3.0.5 From 71a755b4daf0846c867834b5a0d86297ac7d0667 Mon Sep 17 00:00:00 2001 From: Michael Plump Date: Mon, 18 Mar 2024 12:14:38 -0700 Subject: [PATCH 3/8] Update the IntelliJ plugin to gfj 1.21.0. PiperOrigin-RevId: 616908365 --- idea_plugin/build.gradle.kts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/idea_plugin/build.gradle.kts b/idea_plugin/build.gradle.kts index 0c3ae944e..05010fe3c 100644 --- a/idea_plugin/build.gradle.kts +++ b/idea_plugin/build.gradle.kts @@ -22,7 +22,7 @@ apply(plugin = "java") repositories { mavenCentral() } -val googleJavaFormatVersion = "1.20.0" +val googleJavaFormatVersion = "1.21.0" java { sourceCompatibility = JavaVersion.VERSION_11 @@ -62,5 +62,5 @@ tasks { dependencies { implementation("com.google.googlejavaformat:google-java-format:${googleJavaFormatVersion}") testImplementation("junit:junit:4.13.2") - testImplementation("com.google.truth:truth:1.4.1") + testImplementation("com.google.truth:truth:1.4.2") } From 3ee6e2a34029bc5e68a5f733749e2732c2bf7940 Mon Sep 17 00:00:00 2001 From: google-java-format Team Date: Thu, 28 Mar 2024 17:07:54 -0700 Subject: [PATCH 4/8] Java-format: format multiples files in parallel to improve speed PiperOrigin-RevId: 620100015 --- scripts/google-java-format-diff.py | 82 ++++++++++++++++++------------ 1 file changed, 49 insertions(+), 33 deletions(-) diff --git a/scripts/google-java-format-diff.py b/scripts/google-java-format-diff.py index 2c75edac7..7f52ed1ec 100755 --- a/scripts/google-java-format-diff.py +++ b/scripts/google-java-format-diff.py @@ -33,8 +33,34 @@ import subprocess import io import sys +from concurrent.futures import ThreadPoolExecutor,wait,FIRST_EXCEPTION from shutil import which +def _apply_format(filename, lines, base_command, args): + """Apply format on filename.""" + if args.i and args.verbose: + print('Formatting', filename) + + command = base_command[:] + command.extend(lines) + command.append(filename) + p = subprocess.Popen(command, stdout=subprocess.PIPE, + stderr=None, stdin=subprocess.PIPE) + stdout, _ = p.communicate() + if p.returncode != 0: + sys.exit(p.returncode) + + if not args.i: + with open(filename) as f: + code = f.readlines() + formatted_code = io.StringIO(stdout.decode('utf-8')).readlines() + diff = difflib.unified_diff(code, formatted_code, + filename, filename, + '(before formatting)', '(after formatting)') + diff_string = ''.join(diff) + if len(diff_string) > 0: + sys.stdout.write(diff_string) + def main(): parser = argparse.ArgumentParser(description= 'Reformat changed lines in diff. Without -i ' @@ -108,39 +134,29 @@ def main(): binary = which('google-java-format') or '/usr/bin/google-java-format' base_command = [binary] - # Reformat files containing changes in place. - for filename, lines in lines_by_file.items(): - if args.i and args.verbose: - print('Formatting', filename) - command = base_command[:] - if args.i: - command.append('-i') - if args.aosp: - command.append('--aosp') - if args.skip_sorting_imports: - command.append('--skip-sorting-imports') - if args.skip_removing_unused_imports: - command.append('--skip-removing-unused-imports') - if args.skip_javadoc_formatting: - command.append('--skip-javadoc-formatting') - command.extend(lines) - command.append(filename) - p = subprocess.Popen(command, stdout=subprocess.PIPE, - stderr=None, stdin=subprocess.PIPE) - stdout, stderr = p.communicate() - if p.returncode != 0: - sys.exit(p.returncode); - - if not args.i: - with open(filename) as f: - code = f.readlines() - formatted_code = io.StringIO(stdout.decode('utf-8')).readlines() - diff = difflib.unified_diff(code, formatted_code, - filename, filename, - '(before formatting)', '(after formatting)') - diff_string = ''.join(diff) - if len(diff_string) > 0: - sys.stdout.write(diff_string) + if args.i: + base_command.append('-i') + if args.aosp: + base_command.append('--aosp') + if args.skip_sorting_imports: + base_command.append('--skip-sorting-imports') + if args.skip_removing_unused_imports: + base_command.append('--skip-removing-unused-imports') + if args.skip_javadoc_formatting: + base_command.append('--skip-javadoc-formatting') + + with ThreadPoolExecutor() as executor: + format_futures = [] + for filename, lines in lines_by_file.items(): + format_futures.append( + executor.submit(_apply_format, filename, lines, base_command, args) + ) + + done, _ = wait(format_futures, return_when=FIRST_EXCEPTION) + for future in done: + if exception := future.exception(): + executor.shutdown(wait=True, cancel_futures=True) + sys.exit(exception.args[0]) if __name__ == '__main__': main() From 31678188c402f8ff4a0cd173f1c53f7d82fa1033 Mon Sep 17 00:00:00 2001 From: nickreid Date: Fri, 29 Mar 2024 19:08:52 -0700 Subject: [PATCH 5/8] Stop using floats to represent line widths, which must be integers. Float was used so that forced breaks could be modeled with POSITIVE_INFINISTY, but an arbitrary large int works just as well. PiperOrigin-RevId: 620395732 --- .../java/com/google/googlejavaformat/Doc.java | 63 +++++++++++-------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/Doc.java b/core/src/main/java/com/google/googlejavaformat/Doc.java index cab688558..01a75846e 100644 --- a/core/src/main/java/com/google/googlejavaformat/Doc.java +++ b/core/src/main/java/com/google/googlejavaformat/Doc.java @@ -63,6 +63,16 @@ public enum FillMode { FORCED } + /** + * The maximum supported line width. + * + *

This can be used as a sentinel/threshold for {@code Doc}s that break unconditionally. + * + *

The value was selected to be obviously too large for any practical line, but small enough to + * prevent accidental overflow. + */ + public static final int MAX_LINE_WIDTH = 1000; + /** State for writing. */ public static final class State { final int lastIndent; @@ -103,8 +113,7 @@ public String toString() { private static final Range EMPTY_RANGE = Range.closedOpen(-1, -1); private static final DiscreteDomain INTEGERS = DiscreteDomain.integers(); - // Memoized width; Float.POSITIVE_INFINITY if contains forced breaks. - private final Supplier width = Suppliers.memoize(this::computeWidth); + private final Supplier width = Suppliers.memoize(this::computeWidth); // Memoized flat; not defined (and never computed) if contains forced breaks. private final Supplier flat = Suppliers.memoize(this::computeFlat); @@ -113,16 +122,16 @@ public String toString() { private final Supplier> range = Suppliers.memoize(this::computeRange); /** - * Return the width of a {@code Doc}, or {@code Float.POSITIVE_INFINITY} if it must be broken. + * Return the width of a {@code Doc}. * * @return the width */ - final float getWidth() { + final int getWidth() { return width.get(); } /** - * Return a {@code Doc}'s flat-string value; not defined (and never called) if the (@code Doc} + * Return a {@code Doc}'s flat-string value; not defined (and never called) if the {@code Doc} * contains forced breaks. * * @return the flat-string value @@ -143,9 +152,9 @@ final Range range() { /** * Compute the {@code Doc}'s width. * - * @return the width, or {@code Float.POSITIVE_INFINITY} if it must be broken + * @return the width */ - abstract float computeWidth(); + abstract int computeWidth(); /** * Compute the {@code Doc}'s flat value. Not defined (and never called) if contains forced breaks. @@ -202,12 +211,8 @@ void add(Doc doc) { } @Override - float computeWidth() { - float thisWidth = 0.0F; - for (Doc doc : docs) { - thisWidth += doc.getWidth(); - } - return thisWidth; + int computeWidth() { + return getWidth(docs); } @Override @@ -246,10 +251,10 @@ Range computeRange() { @Override public State computeBreaks(CommentsHelper commentsHelper, int maxWidth, State state) { - float thisWidth = getWidth(); + int thisWidth = getWidth(); if (state.column + thisWidth <= maxWidth) { oneLine = true; - return state.withColumn(state.column + (int) thisWidth); + return state.withColumn(state.column + thisWidth); } State broken = computeBroken( @@ -295,8 +300,8 @@ private static State computeBreakAndSplit( State state, Optional optBreakDoc, List split) { - float breakWidth = optBreakDoc.isPresent() ? optBreakDoc.get().getWidth() : 0.0F; - float splitWidth = getWidth(split); + int breakWidth = optBreakDoc.isPresent() ? optBreakDoc.get().getWidth() : 0; + int splitWidth = getWidth(split); boolean shouldBreak = (optBreakDoc.isPresent() && optBreakDoc.get().fillMode == FillMode.UNIFIED) || state.mustBreak @@ -348,12 +353,16 @@ private void writeFilled(Output output) { * Get the width of a sequence of {@link Doc}s. * * @param docs the {@link Doc}s - * @return the width, or {@code Float.POSITIVE_INFINITY} if any {@link Doc} must be broken + * @return the width */ - static float getWidth(List docs) { - float width = 0.0F; + static int getWidth(List docs) { + int width = 0; for (Doc doc : docs) { width += doc.getWidth(); + + if (width >= MAX_LINE_WIDTH) { + return MAX_LINE_WIDTH; // Paranoid overflow protection + } } return width; } @@ -455,7 +464,7 @@ public void add(DocBuilder builder) { } @Override - float computeWidth() { + int computeWidth() { return token.getTok().length(); } @@ -512,8 +521,8 @@ public void add(DocBuilder builder) { } @Override - float computeWidth() { - return 1.0F; + int computeWidth() { + return 1; } @Override @@ -615,8 +624,8 @@ public void add(DocBuilder builder) { } @Override - float computeWidth() { - return isForced() ? Float.POSITIVE_INFINITY : (float) flat.length(); + int computeWidth() { + return isForced() ? MAX_LINE_WIDTH : flat.length(); } @Override @@ -705,7 +714,7 @@ public void add(DocBuilder builder) { } @Override - float computeWidth() { + int computeWidth() { int idx = Newlines.firstBreak(tok.getOriginalText()); // only count the first line of multi-line block comments if (tok.isComment()) { @@ -718,7 +727,7 @@ float computeWidth() { return reformatParameterComment(tok).map(String::length).orElse(tok.length()); } } - return idx != -1 ? Float.POSITIVE_INFINITY : (float) tok.length(); + return idx != -1 ? MAX_LINE_WIDTH : tok.length(); } @Override From 9bdae025b7b023d45636e707c9225d0cdd66eb0b Mon Sep 17 00:00:00 2001 From: nickreid Date: Mon, 1 Apr 2024 06:23:56 -0700 Subject: [PATCH 6/8] Support multiline tokens in GJF-core Cursory perf measurements show no detectable cost from scanning every token. If one should be discovered later, we can add guards to only scan potentially multiline tokens. PiperOrigin-RevId: 620833035 --- .../src/main/java/com/google/googlejavaformat/Doc.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/Doc.java b/core/src/main/java/com/google/googlejavaformat/Doc.java index 01a75846e..6414a3fb1 100644 --- a/core/src/main/java/com/google/googlejavaformat/Doc.java +++ b/core/src/main/java/com/google/googlejavaformat/Doc.java @@ -397,6 +397,10 @@ boolean isReal() { private final Indent plusIndentCommentsBefore; private final Optional breakAndIndentTrailingComment; + private Input.Tok tok() { + return token.getTok(); + } + private Token( Input.Token token, RealOrImaginary realOrImaginary, @@ -465,7 +469,8 @@ public void add(DocBuilder builder) { @Override int computeWidth() { - return token.getTok().length(); + int idx = Newlines.firstBreak(tok().getOriginalText()); + return (idx >= 0) ? MAX_LINE_WIDTH : tok().length(); } @Override @@ -480,8 +485,7 @@ Range computeRange() { @Override public State computeBreaks(CommentsHelper commentsHelper, int maxWidth, State state) { - String text = token.getTok().getOriginalText(); - return state.withColumn(state.column + text.length()); + return state.withColumn(state.column + computeWidth()); } @Override From 33bf757b184cecd5be0c5377cdd3afbc7908f96f Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Mon, 1 Apr 2024 13:17:34 -0700 Subject: [PATCH 7/8] Update text block formatting after nickreid's improvements in unknown commit Previously the formatting was adding a forced break at the beginning of text blocks, which caused issues like #1081. With the changes in the baseline CL it correctly handles the 'width' of text blocks containing newlines as infinity, instead of counting the number of characters and treating the newline as having width 1. Fixes https://github.com/google/google-java-format/issues/1081 PiperOrigin-RevId: 620933964 --- .../googlejavaformat/java/JavaInputAstVisitor.java | 3 --- .../googlejavaformat/java/testdata/RSLs.input | 11 +++++++++++ .../googlejavaformat/java/testdata/RSLs.output | 14 ++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java index 6dc82f40c..1e0675ffd 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java @@ -1667,9 +1667,6 @@ public Void visitMemberSelect(MemberSelectTree node, Void unused) { public Void visitLiteral(LiteralTree node, Void unused) { sync(node); String sourceForNode = getSourceForNode(node, getCurrentPath()); - if (sourceForNode.startsWith("\"\"\"")) { - builder.forcedBreak(); - } if (isUnaryMinusLiteral(sourceForNode)) { token("-"); sourceForNode = sourceForNode.substring(1).trim(); diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/RSLs.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/RSLs.input index f401285bc..22aa8f2b2 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/RSLs.input +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/RSLs.input @@ -40,4 +40,15 @@ class RSLs { lorem ipsum """; + { + f(""" + lorem + ipsum + """, 42); + + """ + hello %s + """ + .formatted("world"); + } } diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/RSLs.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/RSLs.output index c6051d1fd..5ca1fb8cc 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/RSLs.output +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/RSLs.output @@ -51,4 +51,18 @@ ipsum lorem ipsum """; + + { + f( + """ + lorem + ipsum + """, + 42); + + """ + hello %s + """ + .formatted("world"); + } } From 0a46e9e896aa41abcd307ce3cb2e21a2de184f6c Mon Sep 17 00:00:00 2001 From: cushon Date: Mon, 1 Apr 2024 20:48:58 +0000 Subject: [PATCH 8/8] Release google-java-format 1.22.0 --- core/pom.xml | 2 +- eclipse_plugin/META-INF/MANIFEST.MF | 2 +- eclipse_plugin/pom.xml | 2 +- pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index c3753a150..504c301fc 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -22,7 +22,7 @@ com.google.googlejavaformat google-java-format-parent - HEAD-SNAPSHOT + 1.22.0 google-java-format diff --git a/eclipse_plugin/META-INF/MANIFEST.MF b/eclipse_plugin/META-INF/MANIFEST.MF index 913245393..c91435450 100644 --- a/eclipse_plugin/META-INF/MANIFEST.MF +++ b/eclipse_plugin/META-INF/MANIFEST.MF @@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2 Bundle-Name: google-java-format Bundle-SymbolicName: google-java-format-eclipse-plugin;singleton:=true Bundle-Vendor: Google -Bundle-Version: 1.13.0 +Bundle-Version: 1.22.0 Bundle-RequiredExecutionEnvironment: JavaSE-11 Require-Bundle: org.eclipse.jdt.core;bundle-version="3.10.0", org.eclipse.jface, diff --git a/eclipse_plugin/pom.xml b/eclipse_plugin/pom.xml index b2c6e368a..38908670a 100644 --- a/eclipse_plugin/pom.xml +++ b/eclipse_plugin/pom.xml @@ -22,7 +22,7 @@ com.google.googlejavaformat google-java-format-eclipse-plugin eclipse-plugin - 1.13.0 + 1.22.0 Google Java Format Plugin for Eclipse 4.5+ diff --git a/pom.xml b/pom.xml index 152fdfde2..e5c98da2f 100644 --- a/pom.xml +++ b/pom.xml @@ -23,7 +23,7 @@ com.google.googlejavaformat google-java-format-parent pom - HEAD-SNAPSHOT + 1.22.0 core