Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkstyle errors reported multiple times by Error Prone parser #34

Open
leinardi opened this issue Apr 11, 2018 · 6 comments
Open

Checkstyle errors reported multiple times by Error Prone parser #34

leinardi opened this issue Apr 11, 2018 · 6 comments

Comments

@leinardi
Copy link

leinardi commented Apr 11, 2018

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

@tomasbjerre
Copy link
Owner

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?

@leinardi
Copy link
Author

leinardi commented Apr 14, 2018

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

@tomasbjerre
Copy link
Owner

Ok, I see. You are right.

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

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

@leinardi
Copy link
Author

Sure, I had already given a 👍 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?

@tomasbjerre
Copy link
Owner

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

@leinardi
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants