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

Separate tasks mode #43

Open
vladimirfx opened this issue Sep 5, 2019 · 9 comments
Open

Separate tasks mode #43

vladimirfx opened this issue Sep 5, 2019 · 9 comments

Comments

@vladimirfx
Copy link

Please provide option to run CheckerFramework in separate tasks. This will solve many issues with interaction with other processors (#20) and make separation of assemble/check phases possible.

Currently we use this task description:

afterEvaluate {
    sourceSets.forEach { sourceSet ->
        if (!sourceSet.name.contains("test", ignoreCase = true)) {
            val newTask = tasks.register("checkerFramework${sourceSet.name.capitalize()}", JavaCompile::class) {
                source = sourceSet.java
                destinationDir = sourceSet.java.outputDir
                classpath = sourceSet.compileClasspath + sourceSet.output.classesDirs
                options.annotationProcessorPath = checkerFramework
                options.compilerArgs.addAll(
                    listOf(
                        "-proc:only",
                        "-processor",
                        "org.checkerframework.checker.nullness.NullnessChecker," +
                                "org.checkerframework.checker.signature.SignatureChecker," +
                                "org.checkerframework.checker.optional.OptionalChecker," +
                                "org.checkerframework.checker.index.IndexChecker," +
                                "org.checkerframework.checker.regex.RegexChecker," +
                                "org.checkerframework.checker.formatter.FormatterChecker," +
                                "org.checkerframework.checker.propkey.PropertyKeyChecker",
                        "-AwarnUnneededSuppressions",
                        "-Xmaxerrs", "10000",
                        "-Xmaxwarns", "10000",
                        "-Xbootclasspath/p:${checkerFrameworkAnnotatedJDK.asPath}"
                    )
                )
                if (ignoreStaticAnalysisFailures) {
                    //ignore CheckerFramework bugs
                    options.isFailOnError = false
                    options.compilerArgs.add("-Awarns")// turns Checker Framework errors into warnings
                }
            }
            tasks.check {
                dependsOn(newTask)
            }
        }
    }
}
@msridhar
Copy link
Collaborator

msridhar commented Sep 5, 2019

This makes sense but I don't think it mitigates #20. If there are other annotation processors that generate code, like AutoValue, then those processors still need to run during the separate Checker Framework task. Otherwise, the Checker Framework task will fail with compilation errors.

@vladimirfx
Copy link
Author

vladimirfx commented Sep 6, 2019

Actually it is no need to run processors within Checker task but generated code must be included as Checker task inputs. We use this technique in Android projects (Dagger, Autvalue etc.). In mixed Java+Kotlin tasks we add kapt output as input to Checker task.
With such approach there is conflicts with other processors (and code generation) in principle. Checker task just consume all code that should be compiled. Ideally generated code issues should be suppressed automatically by file path.
Plus separate typed task make various customizations much easier. As example - we need to add some additional classpath entries from Kotlin stub compiler (all Kotlin binaries carefully annotated).

@msridhar
Copy link
Collaborator

msridhar commented Sep 6, 2019

@vladimirfx you have a good point. It would be great if the plugin supported separate tasks, where the Checker task could just consume all the generated code from the annotation processors.

@kelloggm
Copy link
Owner

kelloggm commented Sep 9, 2019

One possible workaround for now: you can guard application of this plugin behind a Gradle property, and only supply the Gradle property when you want to run in "CI Mode". That would look something like the code snippet below. With that code, you can run ./gradlew build -PisCI=true on your CI server, but running ./gradlew build on your dev box (or elsewhere) won't run the Checker Framework.

plugins {
  ...
  // To do Checker Framework pluggable type-checking (and other CI only tasks), run:
  // ./gradlew compileJava -PisCI=true
  id 'org.checkerframework' version '0.3.30' apply false
}

if (!project.hasProperty("isCI")) {
    ext.isCI = "false"
}
if ("true".equals(project.ext.isCI)) {
  apply plugin: 'org.checkerframework'
}

if ("true".equals(project.ext.isCI)) {
  checkerFramework {
    checkers = [ "org.checkerframework.checker.nullness.NullnessChecker",
                           "org.checkerframework.checker.signature.SignatureChecker",
                            "org.checkerframework.checker.optional.OptionalChecker",
                            "org.checkerframework.checker.index.IndexChecker",
                            "org.checkerframework.checker.regex.RegexChecker",
                            "org.checkerframework.checker.formatter.FormatterChecker",
                            "org.checkerframework.checker.propkey.PropertyKeyChecker"
    ]
  }
}

@vladimirfx
Copy link
Author

@kelloggm CI mode is good if developers do not check code locally. In our case devs do code cleanup by running ./gradlew check locally...

@msridhar
Copy link
Collaborator

msridhar commented Nov 5, 2019

@vladimirfx you could use the Gradle property trick locally as well. Can you say what is insufficient about use of a Gradle property?

@vladimirfx
Copy link
Author

vladimirfx commented Nov 7, 2019

@msridhar as workaround it's possible, but requires some additional knowledge/documentation of build process.
Besides inconvenience there always be issues such as incompatibility with other tools that hooks in compilation process such as Error Prone, non javac compilers (ajc, eclipse).
Because Checker Framework use javac (and only javac) as code parser separate task can be configured with separate configuration with proper javac included. So release 2.11.x requires Java 9+ javac to build but supports code check only for Java 8. IMO very confusing requirements.
In contrast with Error Prone Checker Framework is slow and sophisticated quality checking tool that in practice can't be run on each compilation. Moreover it's restrict java compiler type and version (in very strange fashion). For such tools Gradle has separate task category with well established conventions.

@mernst
Copy link
Collaborator

mernst commented Nov 7, 2019

@vladimirfx Regarding your comment about 2.11.x, it's true that it works only on Java 8. Version 3.0.0 works on Java 8 and Java 11, so upgrading to 3.0.0 resolves that issue.

@msridhar
Copy link
Collaborator

msridhar commented Nov 8, 2019

@vladimirfx I agree that in many scenarios teams will want to run Checker Framework separately from their main build. Gradle flags provide one way to do that. I am not a Gradle expert, but I suspect one could make a separate task that just sets a flag before the normal build task is run? This way the flag would not need to be passed on the command line.

You also have requirements around fine-grained control of source paths, to add kapt output, etc. For this scenario, do your custom Gradle scripts still work? They might need to be tweaked to stick Error Prone javac in the bootclasspath, but we can probably help with that and document that better.

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

4 participants