Output "Changes were written" only if changes were written, and just once

[mernst]

Description of the problem / feature request:

Please make Error Prone output its "Changes were written to ..." message only if changes were written, and at most once.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Here is how to reproduce the problem.

Setup:

DIR=$HOME/tmp/errorprone-suggested-replacements
rm -rf $DIR
mkdir -p $DIR
cd $DIR
wget -N https://repo1.maven.org/maven2/com/google/errorprone/error_prone_core/2.3.4/error_prone_core-2.3.4-with-dependencies.jar
wget -N https://repo1.maven.org/maven2/org/checkerframework/dataflow/2.5.7/dataflow-2.5.7.jar
wget -N https://repo1.maven.org/maven2/org/checkerframework/javacutil/2.5.7/javacutil-2.5.7.jar
wget -N https://repo1.maven.org/maven2/com/google/code/findbugs/jFormatString/3.0.0/jFormatString-3.0.0.jar
wget -N https://repo1.maven.org/maven2/com/github/ben-manes/caffeine/caffeine/2.8.1/caffeine-2.8.1.jar
wget -N https://repo1.maven.org/maven2/com/googlecode/java-diff-utils/diffutils/1.3.0/diffutils-1.3.0.jar

cat > File1.java <<END
public class File1 {
  void m(java.util.Set<Short> s, int i) {
    s.remove(i-1);
  }
}
END

cat > File2.java <<END
public class File2 {
  void m(java.util.Set<Short> s, int i) {
    s.remove(i-1);
  }
}
END

This command creates patches for problems in the two files:

rm -f error-prone.patch
javac \
  -XDcompilePolicy=simple \
  -processorpath error_prone_core-2.3.4-with-dependencies.jar:dataflow-2.5.7.jar:javacutil-2.5.7.jar:jFormatString-3.0.0.jar:caffeine-2.8.1.jar:diffutils-1.3.0.jar \
  "-Xplugin:ErrorProne -XepDisableAllChecks -XepPatchChecks:CollectionIncompatibleType -XepPatchLocation:$DIR" \
  File1.java File2.java

A problem is that it outputs the same message twice, which is unnecessary and verbose:

Changes were written to /home/mernst/tmp/errorprone-suggested-replacements/error-prone.patch. Please inspect the file and apply with: patch -p0 -u -i error-prone.patch
Changes were written to /home/mernst/tmp/errorprone-suggested-replacements/error-prone.patch. Please inspect the file and apply with: patch -p0 -u -i error-prone.patch

This command tries to create patches, for a problem that is not present in the two files:

rm -f error-prone.patch
javac \
  -XDcompilePolicy=simple \
  -processorpath error_prone_core-2.3.4-with-dependencies.jar:dataflow-2.5.7.jar:javacutil-2.5.7.jar:jFormatString-3.0.0.jar:caffeine-2.8.1.jar:diffutils-1.3.0.jar \
  "-Xplugin:ErrorProne -XepDisableAllChecks -XepPatchChecks:UnnecessaryParentheses -XepPatchLocation:$DIR" \
  File1.java File2.java

However, it still outputs the message (twice), even though the file was never created:

Changes were written to /home/mernst/tmp/errorprone-suggested-replacements/error-prone.patch. Please inspect the file and apply with: patch -p0 -u -i error-prone.patch
Changes were written to /home/mernst/tmp/errorprone-suggested-replacements/error-prone.patch. Please inspect the file and apply with: patch -p0 -u -i error-prone.patch

What version of Error Prone are you using?

2.3.4

Have you found anything relevant by searching the web?

• GitHub issues: https://github.com/google/error-prone/issues
• email threads on https://groups.google.com/forum/#!forum/error-prone-discuss

Searched both, found nothing.

[TimvdLippe]

I am trying out patching with ErrorProne for the first time using the -XepPatchLocation and can confirm the spam in my console. Even more, it was confusing as first, because the files weren't on my filesystem and I thought ErrorProne was broken. Then I figured out that it would always output this line, regardless if it generated any diff.

Using find . -iname 'error-prone.patch' I eventually found all the files that were actually generated. Would be great if we can get this sorted 😄

[Stephan202]

I filed #3236, which should improve the situation. (It doesn't resolve the duplicate Changes were written to path/error-prone.patch logging, as that would require more work. At Picnic we use IN_PLACE replacement, in which case each log statement is distinct anyway (because then each statement references another output file).)

[TimvdLippe]

@Stephan202 That's amazing, thanks for the quick fix!