GwtIncompatibleStripper adds an extra newline to files with no `@GwtIncompatible` annotation

[niloc132]

Describe the bug .java files with no @GwtIncompatible annotation have an extra newline appended to the end after they are processed by the GwtIncompatibleStripper. I have not validated if files which do contain a @GwtIncompatible annotation are also affected in this way.

To Reproduce Method one, in J2CL itself, using Bazel to build:

• Check out J2CL.
• Examine a .java file in the j2cl JRE, for example jre/java/java/io/Serializable.java - as of d3835de97 this file has 44 lines.
• Invoke bazel build //jre/java:* to build the JRE.
• Examine the resulting file emitted by J2CL (note that this technically lets it also pass through J2CL, the file now appears to have 45 lines.

  [colin@runes j2cl ((d3835de...))]$ wc -l ./jre/java/java/io/Serializable.java
  44 ./jre/java/java/io/Serializable.java
  [colin@runes j2cl ((d3835de...))]$ bazel build //jre/java:*
  ...
  INFO: Elapsed time: 65.787s, Critical Path: 47.88s
  INFO: 392 processes: 32 internal, 308 local, 52 worker.
  INFO: Build completed successfully, 392 total actions
  [colin@runes j2cl ((d3835de...))]$ wc -l ./bazel-bin/jre/java/jre.js/java/io/Serializable.java
  45 ./bazel-bin/jre/java/jre.js/java/io/Serializable.java

Method two, running the preprocessor directly on a single file, without Bazel:

• Build J2CL's gwtincompatible stripper for external usage, bazel build //tools/java/com/google/j2cl/tools/gwtincompatible:GwtIncompatibleStripper_deploy.jar
• Create a simple file, as simplistic as package-info.java with a single line, package com.hello.world;. Verify the number of lines in the file.
• Run the deploy jar on this new file, java -jar bazel-bin/tools/java/com/google/j2cl/tools/gwtincompatible/GwtIncompatibleStripper_deploy.jar package-info.java (note that this will apparently update the file in place).
• Observe that the file now is one line longer.
• If repeated, the file will continue to grow each invocation, getting progressively longer.

Bazel version Please include version of Bazel that you are running J2CL with:

[colin@runes j2cl ((d3835de...))]$ bazel version
Bazelisk version: v1.7.4
Build label: 5.1.0
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Thu Mar 24 14:02:30 2022 (1648130550)
Build timestamp: 1648130550
Build timestamp as int: 1648130550

Expected behavior A clear and concise description of what you expected to happen. Files that require no changes by the preprocessor should have no changes made to them.

[gkdn]

Thanks for the detailed report. Do you already have a patch?

I'm little bit surprised. We have unit tests covering GwtIncompatibleStripperTest behavior. Are we missing something there?

[niloc132]

I haven't dug in to the code to see where it is coming from except to say that I can't see where it is coming from :). I definitely agree that the tests look like they should cover this case, so my hunch is that it is somehow IO related - the tests appear to validate the strings, and so perhaps OutputUtils is doing something it shouldn't? I'm afraid that's as far as I've gotten.

[gkdn]

I have a theory. Will send a quick lazy fix without trying to repro...

[gkdn]

(I'm pretty confident that this should fix it but pls re-open if it is not fixed)