violations-lib icon indicating copy to clipboard operation
violations-lib copied to clipboard

Checkstyle errors reported multiple times by Error Prone parser

Open leinardi opened this issue 6 years ago • 6 comments

Currently the Error Prone parser is reporting also the checkstyle errors. This is a problem for 2 reasons:

  1. the checkstyle errors appear multiple times in my log (one per build variant that I am checking)
  2. the checkstyle errors are reported also by checkstyle parser

This mean that if I have 1 error and 2 variants I get 3 reports of the same error (and with 3 errors and 5 variants I get 18 reports where only 3 are unique errors and the rest are just duplicates).

Checkstyle build output:

:library:checkstyleDebug[ant:checkstyle] [ERROR] /home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/FabWithLabelView.java:207:9: '{' at column 9 should be on the previous line. [LeftCurly]
 FAILED
:library:checkstyleRelease[ant:checkstyle] [ERROR] /home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/FabWithLabelView.java:207:9: '{' at column 9 should be on the previous line. [LeftCurly]
 FAILED

Full build source: https://travis-ci.org/leinardi/FloatingActionButtonSpeedDial/builds/365085225?utm_source=github_status&utm_medium=notification

image

leinardi avatar Apr 11 '18 12:04 leinardi

I'm not really sure how this should be solved. If ErrorProne is giving this output, and the purpose of the ErrorProne parser is to parse that output. Then that is working as intended.

Same with Checkstyle. If that is the output, then the lib is doing what it should.

Is this not more of an issue about making Error Prone not print Checkstyle errors?

tomasbjerre avatar Apr 14 '18 07:04 tomasbjerre

Are you sure that ErrorProne is outputting the checkstyle errors? I think is just the checkstyle task (library:checkstyleDebug[ant:checkstyle]) that is doing the output and the violations-lib ErrorProne parser is catching them.

This is the full build log:

94.54s$ ./gradlew clean build :library:check --profile --continue 2>&1 | tee build.log
To honour the JVM settings for this build a new JVM will be forked. Please consider using the daemon: https://docs.gradle.org/4.6/userguide/gradle_daemon.html.
Daemon will be stopped at the end of the build stopping after processing
Parallel execution is an incubating feature.
:clean
:library:clean UP-TO-DATE
:clean UP-TO-DATE
:sample:clean UP-TO-DATE
:sample:preBuild
:library:preBuild UP-TO-DATE
:library:preDebugBuild UP-TO-DATE
:library:compileDebugAidl
:sample:preBuild UP-TO-DATE
:sample:prepareLintJar
:sample:mainApkListPersistenceDebug
:sample:generateDebugResValues
:sample:createDebugCompatibleScreenManifests
:sample:splitsDiscoveryTaskDebug
:library:compileDebugRenderscript
:sample:mergeDebugShaders
:library:checkDebugManifest
:library:generateDebugBuildConfig
:library:generateDebugResValues
:library:generateDebugResources
:library:packageDebugResources
:sample:compileDebugShaders
:sample:generateDebugAssets
:sample:mergeDebugJniLibFolders
:sample:validateSigningDebug
:library:platformAttrExtractor
:library:processDebugManifest
:library:generateDebugRFile
:library:prepareLintJar
:library:generateDebugSources
:library:javaPreCompileDebug
:sample:preDebugBuild
:sample:compileDebugAidl
:sample:checkDebugManifest
:sample:generateDebugBuildConfig
:sample:processDebugManifest
:sample:compileDebugNdk NO-SOURCE
:sample:processDebugJavaRes NO-SOURCE
:sample:mainApkListPersistenceRelease
:sample:generateReleaseResValues
:sample:createReleaseCompatibleScreenManifests
:sample:splitsDiscoveryTaskRelease
:sample:mergeReleaseShaders
:sample:compileReleaseShaders
:sample:generateReleaseAssets
:sample:mergeReleaseJniLibFolders
:sample:preDebugUnitTestBuild UP-TO-DATE
:sample:mockableAndroidJar
:library:compileDebugJavaWithJavac
:sample:processDebugUnitTestJavaRes NO-SOURCE
:sample:preReleaseUnitTestBuild UP-TO-DATE
:sample:processReleaseUnitTestJavaRes NO-SOURCE
/home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/UiUtils.java:233: warning: [deprecation] BitmapDrawable(Bitmap) in BitmapDrawable has been deprecated
            return new BitmapDrawable(bitmap);
                   ^
1 warning
:library:extractDebugAnnotations
:library:mergeDebugConsumerProguardFiles
:library:mergeDebugShaders
:library:compileDebugShaders
:library:generateDebugAssets
:library:packageDebugAssets
:library:packageDebugRenderscript NO-SOURCE
:library:processDebugJavaRes NO-SOURCE
:library:transformResourcesWithMergeJavaResForDebug
:sample:compileDebugRenderscript
:sample:generateDebugResources
:sample:mergeDebugResources
:library:transformClassesAndResourcesWithSyncLibJarsForDebug
:library:compileDebugNdk NO-SOURCE
:library:mergeDebugJniLibFolders
:library:transformNativeLibsWithMergeJniLibsForDebug
:library:transformNativeLibsWithSyncJniLibsForDebug
:library:bundleDebug
:library:compileDebugSources
:library:assembleDebug
:library:preReleaseBuild UP-TO-DATE
:library:compileReleaseAidl
:library:compileReleaseRenderscript
:library:checkReleaseManifest
:library:generateReleaseBuildConfig
:library:generateReleaseResValues
:library:generateReleaseResources
:library:packageReleaseResources
:library:processReleaseManifest
:library:generateReleaseRFile
:library:generateReleaseSources
:library:javaPreCompileRelease
:library:compileReleaseJavaWithJavac
:sample:processDebugResources
:sample:generateDebugSources
:sample:mergeDebugAssets
:sample:preReleaseBuild
:sample:compileReleaseAidl
:sample:checkReleaseManifest
:sample:generateReleaseBuildConfig
:sample:processReleaseManifest
:sample:compileReleaseNdk NO-SOURCE
:sample:processReleaseJavaRes NO-SOURCE
/home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/UiUtils.java:233: warning: [deprecation] BitmapDrawable(Bitmap) in BitmapDrawable has been deprecated
            return new BitmapDrawable(bitmap);
                   ^
1 warning
:library:extractReleaseAnnotations
:library:mergeReleaseConsumerProguardFiles
:library:mergeReleaseShaders
:library:compileReleaseShaders
:library:generateReleaseAssets
:library:packageReleaseAssets
:library:packageReleaseRenderscript
:sample:mergeReleaseAssets
:library:packageReleaseRenderscript NO-SOURCE
:library:processReleaseJavaRes
:sample:compileReleaseRenderscript
:library:processReleaseJavaRes NO-SOURCE
:library:transformResourcesWithMergeJavaResForRelease
:sample:generateReleaseResources
:sample:mergeReleaseResources
:library:transformClassesAndResourcesWithSyncLibJarsForRelease
:library:compileReleaseNdk NO-SOURCE
:library:mergeReleaseJniLibFolders
:library:transformNativeLibsWithMergeJniLibsForRelease
:library:transformNativeLibsWithSyncJniLibsForRelease
:library:bundleRelease
:library:compileReleaseSources
:library:mergeReleaseResources
:library:verifyReleaseResources
:sample:processReleaseResources
:library:assembleRelease
:library:javadoc
:sample:generateReleaseSources
/home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/SpeedDialView.java:394: warning: no @return
    public boolean isOpen() {
                   ^
/home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/UiUtils.java:198: warning: no @param for drawable
    public static Bitmap getBitmapFromDrawable(@Nullable Drawable drawable) {
                         ^
/home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/UiUtils.java:198: warning: no @return
    public static Bitmap getBitmapFromDrawable(@Nullable Drawable drawable) {
                         ^
/home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/UiUtils.java:229: warning: no @param for bitmap
    public static Drawable getDrawableFromBitmap(@Nullable Bitmap bitmap) {
                           ^
/home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/UiUtils.java:229: warning: no @return
    public static Drawable getDrawableFromBitmap(@Nullable Bitmap bitmap) {
                           ^
5 warnings
:library:javadocJar
:library:sourcesJar
:library:assemble
:library:checkstyleDebug[ant:checkstyle] [ERROR] /home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/FabWithLabelView.java:207:9: '{' at column 9 should be on the previous line. [LeftCurly]
 FAILED
:library:checkstyleRelease[ant:checkstyle] [ERROR] /home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/FabWithLabelView.java:207:9: '{' at column 9 should be on the previous line. [LeftCurly]
 FAILED
:library:lint
Ran lint on variant release: 2 issues found
Ran lint on variant debug: 2 issues found
Wrote HTML report to file:///home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/build/reports/lint-results.html
Wrote XML report to file:///home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/build/reports/lint-results.xml
:library:preDebugUnitTestBuild UP-TO-DATE
:library:javaPreCompileDebugUnitTest
:library:compileDebugUnitTestJavaWithJavac NO-SOURCE
:library:mockableAndroidJar
:library:processDebugUnitTestJavaRes NO-SOURCE
:library:transformClassesAndResourcesWithPrepareIntermediateJarsForDebug
:library:testDebugUnitTest NO-SOURCE
:library:preReleaseUnitTestBuild UP-TO-DATE
:library:javaPreCompileReleaseUnitTest
:sample:javaPreCompileDebug
:library:compileReleaseUnitTestJavaWithJavac NO-SOURCE
:library:processReleaseUnitTestJavaRes NO-SOURCE
:library:transformClassesAndResourcesWithPrepareIntermediateJarsForRelease
:sample:compileDebugJavaWithJavac
:library:testReleaseUnitTest NO-SOURCE
:library:test UP-TO-DATE
:library:transformNativeLibsWithIntermediateJniLibsForDebug
:library:transformNativeLibsWithIntermediateJniLibsForRelease
:sample:compileDebugSources
:sample:transformClassesWithDexBuilderForDebug
:sample:transformDexArchiveWithExternalLibsDexMergerForDebug
:sample:transformDexArchiveWithDexMergerForDebug
:sample:transformNativeLibsWithMergeJniLibsForDebug
:sample:transformResourcesWithMergeJavaResForDebug
:sample:packageDebug
:sample:assembleDebug
:sample:javaPreCompileRelease
:sample:compileReleaseJavaWithJavac
:sample:compileReleaseSources
:sample:lintVitalRelease SKIPPED
:sample:transformClassesWithDexBuilderForRelease
:sample:transformDexArchiveWithExternalLibsDexMergerForRelease
:sample:transformDexArchiveWithDexMergerForRelease
:sample:transformNativeLibsWithMergeJniLibsForRelease
:sample:transformResourcesWithMergeJavaResForRelease
:sample:packageRelease
:sample:assembleRelease
:sample:assemble
:sample:checkstyleDebug
:sample:checkstyleRelease
:sample:lint
Ran lint on variant debug: 130 issues found
Ran lint on variant release: 130 issues found
Wrote HTML report to file:///home/travis/build/leinardi/FloatingActionButtonSpeedDial/sample/build/reports/lint-results.html
Wrote XML report to file:///home/travis/build/leinardi/FloatingActionButtonSpeedDial/sample/build/reports/lint-results.xml
:sample:javaPreCompileDebugUnitTest
:sample:compileDebugUnitTestJavaWithJavac NO-SOURCE
:sample:testDebugUnitTest NO-SOURCE
:sample:javaPreCompileReleaseUnitTest
:sample:compileReleaseUnitTestJavaWithJavac NO-SOURCE
:sample:testReleaseUnitTest NO-SOURCE
:sample:test UP-TO-DATE
:sample:check
:sample:build
FAILURE: Build completed with 2 failures.

https://travis-ci.org/leinardi/FloatingActionButtonSpeedDial/builds/365085225?utm_source=github_status&utm_medium=notification

leinardi avatar Apr 14 '18 08:04 leinardi

Ok, I see. You are right.

But the Error Prone parser is fragile. The correct way of fixing this would be to solve: https://github.com/google/error-prone/issues/444

You can up-vote or comment that one if you like =)

tomasbjerre avatar Apr 14 '18 08:04 tomasbjerre

Sure, I had already given a :+1: to your message, but I just added also a comment to make it more clear.

Regarding this issue, what about ignoring all the errors that contain string "checkstyle" in them? I don't think this string should appear in any ErrorProne error, unless you are building some code that has it in the source itself, and should not be that common imho. What to you think?

leinardi avatar Apr 14 '18 09:04 leinardi

It feels like a bumpy road to go down...

Perhaps you can replace it yourself before parsing the output with the lib? https://stackoverflow.com/questions/5410757/delete-lines-in-a-text-file-that-contain-a-specific-string

tomasbjerre avatar Apr 14 '18 09:04 tomasbjerre

Thanks, this could be a workaround for me :)

But consider maybe the opposite approach of what I have suggested: check if the error don't contain the string "errorprone.info" on them. All the official ErrorProne errors have them at the end:

error: [CollectionIncompatibleType] Argument 'i - 1' should not be passed to this method;
its type int is not compatible with its collection's type argument Short
      s.remove(i - 1);
              ^
    (see http://errorprone.info/bugpattern/CollectionIncompatibleType)
1 error 

And with this logic you can support error prone plugins just checking for other keywords. For example for NullAway it will be "uber.com/nullaway":

/home/travis/build/leinardi/FloatingActionButtonSpeedDial/library/src/main/java/com/leinardi/android/speeddial/UiUtils.java:103: error: [NullAway] dereferenced expression x is @Nullable
         x.hashCode();
          ^
    (see http://t.uber.com/nullaway )
1 error

leinardi avatar Apr 14 '18 09:04 leinardi