Fix Gradle task annotations

[vlsi]

This fixes #153

[uschindler]

Hi,
this PR may fix the warning, but it breaks uptodate checking, so I won't apply it.

It might be OK, to apply @Ignore to some of the properties which are "duplicates", but not to all properties.

Explanation: If classpath changes, the task must re-run, so it MUST be declared as @Input. The same applies for patterns.

[vlsi]

but it breaks uptodate checking

Why do you think so, can you please clarify?

[vlsi]

Explanation: If classpath changes, the task must re-run, so it MUST be declared as @input. The same applies for patterns.

The patterns are not really important on their own.
What is important, is the set of classes for the check.
In other words, if the user adds a useless pattern, then it does not make sense to re-run the task. So the patternset should be marked as @Internal.

[vlsi]

Explanation: If classpath changes, the task must re-run,

This is already handled by existing getClassFiles(). Adding @Input to the patternset, includes, and excludes adds nothing but it results in false positives (re-execution of the task even in the case the actual set of class files is the same).

forbidden-apis/src/main/java/de/thetaphi/forbiddenapis/gradle/CheckForbiddenApis.java

Lines 486 to 489 in dd13fcf

	@InputFiles
	@SkipWhenEmpty
	public FileTree getClassFiles() {
	return getClassesDirs().getAsFileTree().matching(getPatternSet());

[uschindler]

This is already handled by existing getClassFiles().

The reason why it is declared as @Output is, because forbiddenapis does not have an output at all. For following tasks to execute correctly an output must be declared.

The "correct" workaround would be to place a fake "output" file that updates its timestamp after every execution and declare this on as output.

Adding @input to the patternset, includes, and excludes adds nothing but it results in false positives (re-execution of the task even in the case the actual set of class files is the same).

I agree with that one, in Gradle's SourceTask it's the same:
https://github.com/gradle/gradle/blob/eb2878c5520a1cffe424745c7083b714a333a07e/subprojects/core/src/main/java/org/gradle/api/tasks/SourceTask.java#L174

[uschindler]

Explanation: If classpath changes, the task must re-run, so it MUST be declared as @input. The same applies for patterns.

Sorry for that. It's marked with @Output. The reason for this is a hack (see above).

[vlsi]

No output is fine. In that case, you configure the task with outputs.upToDateWhen { true }, and it would work incrementally just fine.

I have a Gradle task here: https://github.com/autostyle/autostyle/blob/3edcb79a2ddd0c8e6a988d5b6d8fd3ee2f9a729a/plugin-gradle/src/main/kotlin/com/github/autostyle/gradle/AutostyleCheckTask.kt

It produces no outputs, and it works incrementally.
Here's the test that verifies check task is executed incrementally: https://github.com/autostyle/autostyle/blob/3edcb79a2ddd0c8e6a988d5b6d8fd3ee2f9a729a/plugin-gradle/src/test/java/com/github/autostyle/gradle/GradleIncrementalResolutionTest.java

[vlsi]

Sorry for that. It's marked with @Output. The reason for this is a hack (see above).

Note: gradle clean removes all the @Output files. It might remove files unexpectedly as you never know what users configure for the classes dir.

For instance, if the user configures classesDirs = "src/main/java" (e.g. by mistake), then gradle clean would wipe all the java files.

[uschindler]

I am working on a solution for this. There is another issue open. The idea is to just create a text file with all violations found. This one should be declared as @Output.

I will later merge your PR, excluding the changes for @Output and fix those separately.

[uschindler]

The outputs.uptoDataWhen was already discussed in the linked issue. The problem is that the minimum Gradle version does not support this yet.

[vlsi]

The problem is that the minimum Gradle version does not support this yet.

:(
Does it really make sense to "support" ancient versions?

I recognize it takes time to maintain the library (and I recognize Gradle task is not the only way to apply the check).
However, it looks like you aim to support very old Gradle versions (of course it is you who decide which versions do you want to support), however, it is sad when it impacts the users who use recent Gradle versions (==warnings or errors in modern Gradle versions caused by "can't fix because we want to support Gradle 1.0").

---

By the way, outputs.upToDateWhen is there in the Javadoc even for Gradle 2.3: https://docs.gradle.org/2.3/javadoc/org/gradle/api/tasks/TaskOutputs.html
Are you sure minimum Gradle version does not support this yet?

[uschindler]

Hi,
there was some strange behaviour without any outputs in previous Gradle versions. I have to check what exactly the issue was.

In short: The correct fix is to supply some output, which also helps people that suppress any output in Gradle (I prefer to always run it with --info, but I am pro-verbose output somehow). This is not in the scope of this PR, so I'd like to delay that to another issue.

The support for older Gradle versions also has to do with Java-6-Support. This will go away soon.

[uschindler]

Hi,
your PR does not even compile. It says:

    [javac] warning: [options] bootstrap class path not set in conjunction with -source 1.6
    [javac] <hidden>/forbidden-apis\src\main\java\de\thetaphi\forbiddenapis\gradle\CheckForbiddenApis.java:47: error: cannot find symbol
    [javac] import org.gradle.api.tasks.Internal;
    [javac]                            ^
    [javac]   symbol:   class Internal
    [javac]   location: package org.gradle.api.tasks
    [javac] <hidden>/forbidden-apis\src\main\java\de\thetaphi\forbiddenapis\gradle\CheckForbiddenApis.java:127: error: cannot find symbol
    [javac]   @Internal
    [javac]    ^
    [javac]   symbol:   class Internal
    [javac]   location: class CheckForbiddenApis
    [javac] 2 errors
    [javac] 1 warning

How did this compile for you?

[vlsi]

How did this compile for you?

I have not tried to compile it

[uschindler]

Ok, thanks so this needs to wait a bit, until minimum Java version was upgraded (see above).

[uschindler]

Hi,
I am working on updating to Java 7 (was on TODO list anyways) - see #160. This implies forbiddenapis new major version 3.0. This allows to merge #155, which changes minimum Gradle version to 3.2; that should make this PR compile.

Thanks for the PR anyways, I wam working on adding it. :-)