Make TaskReportContainer public to ease implementation of ReportContainer

[3flex]

Expected Behavior

3rd party code quality plugins can implement TaskReportContainer.

Current Behavior

3rd party code quality plugins cannot implement TaskReportContainer as it's part of the internal package namespace.

Context

The docs for Reporting say to implement the reporting interface just do the following:

class MyTask implements Reporting<MyReportContainer> {
    // implementation
}

But to implement the interface an implementation for approximately 40 methods is required.

Gradle's code quality plugins can take advantage of internal class TaskReportContainer which simplifies things greatly, but as it's part of the internal API it's not available to well-behaved 3rd party plugins.

Steps to Reproduce (for bugs)

N/A

Your Environment

Trying to implement ReportContainer in detekt.

[adammurdoch]

The Reporting interface isn't really intended to be implemented outside of the Gradle core plugins. We certainly could make it possible to implement it using only public types or methods. We probably wouldn't do this by making the TaskReportContainer type public but might instead provide factory methods for creating ReportContainer instances.

However, I think it might be better to reconsider the report APIs so that the reports generated by a task are described using the same pattern that we use to describe other kinds of inputs and outputs, rather than this entirely different pattern. For example, by introducing an @OutputReport annotation.

It might be better to change the title of this issue to something like "make it possible for tasks and plugins to generate custom reports".

[ldaley]

Tasks can already generate reports. The only thing that Reporting really provides is a semi-consistent mechanism for users to configure report generation for a task. There are potentially other useful things that Gradle could do with the knowledge of where a task produced its reports, but it doesn't today.

The title should probably be “make it possible for tasks to allow configuring their reporting in a consistent manner” or some such.

[3flex]

The Reporting interface isn't really intended to be implemented outside of the Gradle core plugins.

The documentation is misleading then, since it looks like Reporting, ReportContainer and Report are viable options to implement config DSL for tasks. Both Reporting and ReportContainer mention "implementation" leading readers like myself to think that's a sensible thing to do.

The only thing that Reporting really provides is a semi-consistent mechanism for users to configure report generation for a task.

There are incubating features like the Build Dashboard plugin which "aggregates the reports for all tasks that implement the Reporting interface". If Reporting is only intended to be implemented by the core plugins, that means the Build Dashboard is intended for core plugins only as well?

It's unfortunate that there's a config DSL used for core code quality plugins which is not practical to implement in external code quality plugins. Of course the reports.xml/reports.html DSL can be replicated in task implementations but features the ReportContainer offers like being a live, immutable collection of reports is useful but not practical to implement.

The title should probably be “make it possible for tasks to allow configuring their reporting in a consistent manner” or some such.

Happy for guidance but either documentation needs to be updated to make clear that these interfaces are not intended for 3rd party implementations, or an easier way to implement them should be available.

[ldaley]

@3flex apologies, I was more responding to @adammurdoch's comments.

If Reporting is only intended to be implemented by the core plugins, that means the Build Dashboard is intended for core plugins only as well?

Effectively, yes. I would be surprised if anyone is seriously using the build dashboard plugin.

I agree with you that the current situation is unfortunate and unkind. In your case, what is motivating you to use Reporting? Anything in particular? Or are you just trying to follow the lead of the core plugins?

[3flex]

I would be surprised if anyone is seriously using the build dashboard plugin.

I agree, just pointing out that the docs there also make it seem like a 3rd party code quality plugin would want to implement Reporting to show up in the dashboard, because why wouldn't other tools want closer integration with built-in Gradle features? (not being rude, just stating my thought process - reading things like that in the docs provide motivation for me to "do the right thing", because who knows which other features in future might also require Reporting to be implemented).

trying to follow the lead of the core plugins

Exactly this - guided by what looked like a recommendation from the docs. Seems it shouldn't have been too hard to have the custom task implement ReportContainer and expose config for a single xml and html report which only required customisation of their destinations based on this example.

Simplest thing for now would be to update the docs and remove references to implementations that aren't feasible, and possibly also state in docs that certain interfaces are documented publically but only intended to be implemented internally. Request to make TaskReportContainer public was a moonshot :)

A factory to create a ReportContainer would be welcome but I doubt it's valuable enough to justify building and maintaining.

[ldaley]

Thanks for clarifying.

Part of the problem here is that Reporting doesn't work very well. It's cumbersome to implement and is awkward for a bunch of uses. If it was really good, we'd be much more enthusiastic about opening it up because there is no good reason why explicit modelling of reports should be confined to core tasks and good reasons for having a comprehensible pattern here.

I'll take a look at the docs. In the meantime, I suggest modelling your reporting however best makes sense for your task.

[rpalcolea]

Hi @ldaley , are there any plans to provide another constructor for https://github.com/gradle/gradle/blob/master/subprojects/reporting/src/main/java/org/gradle/api/reporting/internal/TaskReportContainer.java#L29 that doesn’t rely on CollectionCallbackActionDecorator?

We know it is internal API but other OSS plugins depend on it, in this case spotbugs -> https://github.com/spotbugs/spotbugs-gradle-plugin/blob/master/src/main/java/com/github/spotbugs/internal/SpotBugsReportsImpl.java#L17

The maintainers didn’t like the idea of releasing a non-backwards compatible (spotbugs/spotbugs-gradle-plugin#75) which makes sense. However, there is no official guideline or recommendation to move these kind of implementations to use only public APIs.
Is there something out there that it could be used instead?

[ldaley]

@oehme 5.1 breaks the spotbugs plugin due to internal API usage. Given that these containers are effectively immutable, the callback decoration doesn't buy all that much here.

Options:

1. Do nothing, put the onus on the spotbugs plugin to fix by not using TaskReportContainer
2. Put back the original constructor by not decorating for TaskReportContainer
3. Put back the original constructor and leave the new one there (i.e. just don't decorate for third party usage)
4. Put back the original constructor and get the decorator via some side mechanism

I think we should do 2 for 5.1, given that we effectively endorse the spotbugs plugin.

[oehme]

I'd go for 3 and deprecate the old one. This is what we have done for high-profile usage of internal APIs previously.

[ldaley]

@oehme can you give some guidance on whether we do this for 5.1 or not please.

[oehme]

Yes I think we should fix this for 5.1

[ldaley]

OK, @breskeby is on it.

[breskeby]

I fixed this as suggested and added a smoketest in PR at #8076

[rpalcolea]

Thanks for the quick turnaround! I guess this will go on the next 5.1 nightly

[slavik112211]

@3flex "Request to make TaskReportContainer public was a moonshot :)"
For an idea how you can reuse TaskReportContainer in an external reporting plugin, see here:
https://github.com/handmadecode/quill/blob/development/src/main/groovy/org/myire/quill/report/MutableReportContainer.java

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. If you're interested in how we try to keep the backlog in a healthy state, please read our blog post on how we refine our backlog. If you feel this is something you could contribute, please have a look at our Contributor Guide. Thank you for your contribution.

[JLLeitschuh]

Same issue was raised against ktlint-gradle:
JLLeitschuh/ktlint-gradle#349

[baron1405]

Over five years since this issue was opened and still no solution for reporting from plugins. Over the years I just took the hit and implemented the ReportContainer interface. While it might not have been intended for clients to implement, I prefer the consistent integration with the built-in plugin report handling to waiting, and waiting, and waiting...

Here's my implementation in case it may be of use to someone, feedback welcomed.

import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.SortedMap;
import java.util.SortedSet;

import org.gradle.api.Action;
import org.gradle.api.DomainObjectCollection;
import org.gradle.api.NamedDomainObjectCollectionSchema;
import org.gradle.api.NamedDomainObjectProvider;
import org.gradle.api.NamedDomainObjectSet;
import org.gradle.api.Namer;
import org.gradle.api.Project;
import org.gradle.api.Rule;
import org.gradle.api.UnknownDomainObjectException;
import org.gradle.api.provider.Provider;
import org.gradle.api.reporting.Report;
import org.gradle.api.reporting.ReportContainer;
import org.gradle.api.specs.Spec;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import groovy.lang.Closure;


/**
 * Implements a report container.
 *
 * @param <T> Type of report
 */
public class Reports<T extends Report> implements ReportContainer<T> {

    private final NamedDomainObjectSet<T> reports;
    private final NamedDomainObjectSet<T> enabled;

    public Reports(final Project project, final Class<T> clazz) {
        this.reports = project.getObjects().namedDomainObjectSet(clazz);
        this.enabled = this.reports.matching(report -> report.getRequired().get());
    }

    @Override
    public NamedDomainObjectSet<T> getEnabled() {
        return this.enabled;
    }

    /**
     * Adds the specified report to the container.
     *
     * @param report Report to add to the container
     * @return {@code true} if the report was added, or {@code} false if a report with the same name already exists.
     */
    protected boolean addReport(final T report) {
        return this.reports.add(report);
    }

    @Override
    public boolean add(final T report) {
        throw new ImmutableViolationException();
    }

    @Override
    public boolean addAll(final Collection<? extends T> reps) {
        throw new ImmutableViolationException();
    }

    @Override
    public void addLater(final Provider<? extends T> provider) {
        throw new ImmutableViolationException();
    }

    @Override
    public void addAllLater(final Provider<? extends Iterable<T>> provider) {
        throw new ImmutableViolationException();
    }

    @Override
    public boolean remove(final Object report) {
        throw new ImmutableViolationException();
    }

    @Override
    public boolean removeAll(@NotNull final Collection<?> reps) {
        throw new ImmutableViolationException();
    }

    @Override
    public boolean retainAll(@NotNull final Collection<?> reps) {
        throw new ImmutableViolationException();
    }

    @Override
    public void clear() {
        throw new ImmutableViolationException();
    }

    @Override
    public boolean containsAll(@NotNull final Collection<?> reps) {
        return this.reports.containsAll(reps);
    }

    @Override
    public Namer<T> getNamer() {
        return Report::getName;
    }

    @Override
    public SortedMap<String, T> getAsMap() {
        return this.reports.getAsMap();
    }

    @Override
    public SortedSet<String> getNames() {
        return this.reports.getNames();
    }

    @Nullable
    @Override
    public T findByName(final String name) {
        return this.reports.findByName(name);
    }

    @Override
    public T getByName(final String name) throws UnknownDomainObjectException {
        return this.reports.getByName(name);
    }

    @Override
    @SuppressWarnings("rawtypes")
    public T getByName(final String name, final Closure configureClosure) throws UnknownDomainObjectException {
        return this.reports.getByName(name, configureClosure);
    }

    @Override
    public T getByName(final String name, final Action<? super T> configureAction) throws UnknownDomainObjectException {
        return this.reports.getByName(name, configureAction);
    }

    @Override
    public T getAt(final String name) throws UnknownDomainObjectException {
        return this.reports.getAt(name);
    }

    @Override
    public Rule addRule(final Rule rule) {
        return this.reports.addRule(rule);
    }

    @Override
    @SuppressWarnings("rawtypes")
    public Rule addRule(final String description, final Closure ruleAction) {
        return this.reports.addRule(description, ruleAction);
    }

    @Override
    public Rule addRule(final String description, final Action<String> ruleAction) {
        return this.reports.addRule(description, ruleAction);
    }

    @Override
    public List<Rule> getRules() {
        return this.reports.getRules();
    }

    @Override
    public int size() {
        return this.reports.size();
    }

    @Override
    public boolean isEmpty() {
        return this.reports.isEmpty();
    }

    @Override
    public boolean contains(final Object report) {
        return this.reports.contains(report);
    }

    @Override
    public Iterator<T> iterator() {
        return this.reports.iterator();
    }

    @Override
    public Object[] toArray() {
        return this.reports.toArray();
    }

    @Override
    @SuppressWarnings("unchecked")
    public <T1> T1[] toArray(@NotNull final T1[] arr) {
        return (T1[])this.reports.toArray((T[])arr);
    }

    @Override
    public Map<String, T> getEnabledReports() {
        return this.enabled.getAsMap();
    }

    @Override
    public <S extends T> NamedDomainObjectSet<S> withType(final Class<S> type) {
        return this.reports.withType(type);
    }

    @Override
    public <S extends T> DomainObjectCollection<S> withType(final Class<S> type,
                                                            final Action<? super S> configureAction) {
        return this.reports.withType(type, configureAction);
    }

    @Override
    @SuppressWarnings("rawtypes")
    public <S extends T> DomainObjectCollection<S> withType(final Class<S> type, final Closure configureClosure) {
        return this.reports.withType(type, configureClosure);
    }

    @Override
    public NamedDomainObjectSet<T> matching(final Spec<? super T> spec) {
        return this.reports.matching(spec);
    }

    @Override
    @SuppressWarnings("rawtypes")
    public NamedDomainObjectSet<T> matching(final Closure spec) {
        return this.reports.matching(spec);
    }

    @Override
    public Action<? super T> whenObjectAdded(final Action<? super T> action) {
        return this.reports.whenObjectAdded(action);
    }

    @Override
    @SuppressWarnings("rawtypes")
    public void whenObjectAdded(final Closure action) {
        this.reports.whenObjectAdded(action);
    }

    @Override
    public Action<? super T> whenObjectRemoved(final Action<? super T> action) {
        return this.reports.whenObjectRemoved(action);
    }

    @Override
    @SuppressWarnings("rawtypes")
    public void whenObjectRemoved(final Closure action) {
        this.reports.whenObjectRemoved(action);
    }

    @Override
    public void all(final Action<? super T> action) {
        this.reports.all(action);
    }

    @Override
    @SuppressWarnings("rawtypes")
    public void all(final Closure action) {
        this.reports.all(action);
    }

    @Override
    public void configureEach(final Action<? super T> action) {
        this.reports.configureEach(action);
    }

    @Override
    public NamedDomainObjectProvider<T> named(final String name) throws UnknownDomainObjectException {
        return this.reports.named(name);
    }

    @Override
    public NamedDomainObjectProvider<T> named(final String name,
                                              final Action<? super T> configurationAction) throws UnknownDomainObjectException {
        return this.reports.named(name, configurationAction);
    }

    @Override
    public <S extends T> NamedDomainObjectProvider<S> named(final String name,
                                                            final Class<S> type) throws UnknownDomainObjectException {
        return this.reports.named(name, type);
    }

    @Override
    public <S extends T> NamedDomainObjectProvider<S> named(final String name, final Class<S> type,
                                                            final Action<? super S> configurationAction) throws UnknownDomainObjectException {
        return this.reports.named(name, type, configurationAction);
    }

    @Override
    public NamedDomainObjectSet<T> named(final Spec<String> nameFilter) {
        return this.reports.named(nameFilter);
    }

    @Override
    public NamedDomainObjectCollectionSchema getCollectionSchema() {
        return this.reports.getCollectionSchema();
    }

    @Override
    @SuppressWarnings("rawtypes")
    public Set<T> findAll(final Closure spec) {
        return this.reports.findAll(spec);
    }

    @Override
    @SuppressWarnings("rawtypes")
    public ReportContainer<T> configure(final Closure closure) {
        final Closure cl = (Closure)closure.clone();
        cl.setResolveStrategy(Closure.DELEGATE_FIRST);
        cl.setDelegate(this);
        cl.call(this);
        return this;
    }
}