feature: create a fluent api for Spoon Launcher

[surli]

Some of use are considering that the actual Launcher class for Spoon is tedious to use. So we plan to create a new Launcher class to replace it on the long term, which would be easier and more explicit.
I invite you to express here your use cases and how you would want to use such a class, giving some example, to help understand your needs and how we can build such a new class.

As a first example, I proposed this kind of solution:

CtModel model = new LauncherBuilder().useAutoimport()
         .useNoclasspath()
         .addInputSource("./myDirectory/")
         .addInputSource("./anotherOne")
         .setOutputDir("./target")
         .build();

Launcher usecases

(please update the list here - so the concept is at one place)
UC1) build AST model from defined set of sources and appropriate config parameters
UC2) print sources from current AST model using appropriate config parameters
UC3) check that AST is consistent
UC4) print sources from current AST model and compile it so class loader can use it
UC5) incremental update of already built AST model by compiling of files modified since last build
UC6) incremental print of sources of model elements modified since last print

So @tdurieux @pvojtechovsky @monperrus @msteinbeck what are your proposal?

[pvojtechovsky]

I have added basic use cases as I see them from my point of view. Feel free to add another use cases or change my definitions to have it more understandable.

[ghost]

We have a number of different projects. Each project has a separate directory. It would be convenient if we could provide a list of projects and generate spoon-transformed output in a directory relative to each project's source. This could resemble:

// Package containing all processors to run.
final Package processors = Main.class.getPackage();

// Paths to project source files, relative to this Java project.
final List<Project> projects = Arrays.asList(
        new Project( "../../ProjectA", "src" ),
        new Project( "../../ProjectB", "src" )
);

// A LauncherBuilder builds Launcher instances; a CtModelBuilder builds CtModels.
final Launcher launcher = new LauncherBuilder()
        .withProjects(projects)
        .withProcessors(processors)
        .withSourceOutputDirectory("spoon")
        .build();

launcher.run();

This would run each processor found in the Main program's package on every source file in all given projects and write to a spoon directory relative to the projects' source directory. Such as:

• Input. ../../ProjectA/src/com/company/stardom/Awesome.java
• Output. ../../ProjectA/spoon/com/company/stardom/Awesome.java.

To build CtModels, I'd recommend a slight variation to avoid confusion with how builders typically behave (following the principle of least astonishment):

final CtModel ctModel = new LauncherBuilder()
        .withProjects(projects)
        .withProcessors(processors)
        .withSourceOutputDirectory("spoon")
        .buildCtModel();

[surli]

Hi @DaveJarvis

welcome here and thanks for your contribution to this discussion :)

Some remarks concerning your proposal: I like the idea of using a concept of Project, especially in your case, but then I think it could be better to provide all info in the project like the output, and not guessing the output path as you suggest based on the project source directory.

Something like new Project("../../ProjectA","src","spoon", list); // where list contains classpath info

It seems rather weird for me to pass a Package in a method called withProcessors: I understand the idea behind it, but it seems more reasonable for me to provide an utility method somewhere to compute the list of processors from a package and then to pass that list to withProcessors.

WDYT?

Agreed with your final remark, in fact the current method in Launcher is called buildModel I think we can keep that name.

[ghost]

Separating "src" is nonsensical, in retrospect. Using defaults is trivial if readily documented:

// The default output directory is "../../ProjectA/spoon" (derived).
// The default classpath is ".:../../ProjectA/src".
final List<Project> projects = Arrays.asList( new Project( "../../ProjectA/src" ), ... );

// The output directory is "../../ProjectA/output" (relative path to "src").
// The default classpath is ".:../../ProjectA/src".
final List<Project> projects = Arrays.asList( new Project(
  "../../ProjectA/src", "output" ), ... );

// The output directory is "/home/java/projects/spoon" (absolute path).
// The default classpath is ".:../../ProjectA/src".
final List<Project> projects = Arrays.asList( new Project(
  "../../ProjectA/src", "/home/java/projects/spoon" ), ... );

// For consistency, perhaps expose a builder pattern?
// Explicit classpath.
final Project project = new ProjectBuilder()
  .withInputDirectory( "../../ProjectA/src" )
  // Relative to "src" (last directory of input directory)
  .withOutputDirectory( "spoon" )
  // Also takes a List<String> ...
  .withClasspath( ".", "../../lib" )
  .build();

provide an utility method

Static utilities may not be all that object-oriented. Using strings for package names is a minor violation of DRY because package names end up encoded twice: once by virtue of having a class definition and once in the string. Refactoring the code using IDE tools (e.g., moving the processors into another package) will cause code breakage using literal strings such as "com.company.analysis.processors" because such strings will not be detected by IDEs.

Here's code that obtains the package names dynamically and won't break after refactoring:

    private List<String> getProcessorClassNames() {
        final Collection<Class<? extends Object>> classes = getProcessorClasses();
        final List<String> classNames = new ArrayList(classes.size());

        for (final Class c : classes) {
            classNames.add(c.getName());
        }

        return classNames;
    }

    // ... additional methods to find the classes (see below) ...

    private Package getProcessorPackage() {
        final Package p = EmptyCatchProcessor.class.getPackage();

        return p;
    }

That said, hiding the implementation details of how the processors are obtained (via reflection?), based on a collection of Packages is quite convenient. Such as the following code used to derive the package name for a single class (easily extended to multiple packages):

    private Collection<Class<? extends Object>> getProcessorClasses() {
        final Reflections reflections = createReflections();
        final Set<Class<? extends Object>> result
                = reflections.getSubTypesOf(Object.class);

        return result;
    }

    private Reflections createReflections() {
        final String packageName = getProcessorPackageName();
        final List<ClassLoader> classLoadersList = new LinkedList<>();

        classLoadersList.add(ClasspathHelper.contextClassLoader());
        classLoadersList.add(ClasspathHelper.staticClassLoader());

        final Reflections result = new Reflections(new ConfigurationBuilder()
                .setScanners(new SubTypesScanner(false), new ResourcesScanner())
                .setUrls(ClasspathHelper.forClassLoader(classLoadersList.toArray(new ClassLoader[0])))
                .filterInputsBy(new FilterBuilder().include(FilterBuilder.prefix(packageName))));

        return result;
    }

Providing individual processor classes, as implemented in the current code base, is flexible.

[pvojtechovsky]

Thank You Dave for contribution of ideas!

I have just small note here: There is one thing, which might be actually problem: You suggest to register individual output directory for each source directory. I guess Spoon actually accepts only one output directory. It is not able to output files into directories which are relative to origin source directories of these files. Or am I wrong?

May be spoon might support that in future. But do you really need it? Are the projects your are printing independent (each may have own Launcher) or they have some dependencies - so they have to be loaded into one Spoon model for all of them?

[ghost]

May be spoon might support that in future. But do you really need it? Are the projects your are printing independent (each may have own Launcher) or they have some dependencies - so they have to be loaded into one Spoon model for all of them?

The dependency relationship between the thirty different projects is nearly labyrinthine in its complexity. Some of them are independent, while others are interdependent, and all of them have their own set of third-party libraries sourced from a common directory.

That Spoon accepts a single output directory is limiting, but can be worked around by instantiating different Launchers.

[tdurieux]

I say a problem to have several outputs directory, what would be the behavior of spoon if a class is created. What is the output directory for this new class?

[ghost]

What is the output directory for this new class?

Some options:

• The current working directory.
• A "spoon" subdirectory in the current working directory.
• Force using a Project instance and require the Project instance to have a directory.
• Force using a Project instance and pick a default output directory.

Lots of possibilities and no problems. Clearly documented behaviour and examples will help.

[pvojtechovsky]

May be there shouldn't be configured global output directory at Launcher creation time. May be the output directory should be a mandatory parameter of functions, which prints model to output. Like this:

void printModel(File outputDirectory)
and
void printModel(File outputDirectory, Filter<CtType> typeFilter)

[ghost]

May be there shouldn't be configured global output directory at Launcher creation time

Making exceptions is the bane of software development:

• For multiple projects, use Projects.setOutputDirectory.
• For a single project, use Launcher.printModel.

That introduces extra complexity and usage inconsistency. A consistent API would make no difference between one project and multiple projects.

[pvojtechovsky]

I do not suggest to make exceptions. I suggest to provide output directory always together with request to output something.
It would be very simple API. And note that outputDirectory makes sense only with connection to printing feature. But it is just idea. I personally use one output directory so the current global value is sufficient for me.

[ghost]

one output directory so the current global value is sufficient

Use a façade to expose a "global value" that creates a single Project instance on behalf of the calling code. This enables a simple API for people who want to create a global output directory while eliminating any exception case. To be concrete:

final Launcher launcher = new LauncherBuilder()
  .withOutputDirectory( "output" )
  .build();

Is equivalent to:

final List<Project> projects = Arrays.asList( new Project( ".", "output" ) );

final Launcher launcher = new LauncherBuilder()
  .withProjects( projects )
  .build();

It is easy to see that the withOutputDirectory façade method would resemble:

public LauncherBuilder withOutputDirectory( final String path ) {
  final List<Project> projects = Arrays.asList( new Project( ".", path ) );

  final LauncherBuilder builder = withProjects( projects );
  return builder;
}

At this point there is no more "global" setting, merely a default Project that is created via the façade. Other parameters and configuration settings can be likewise coerced. Having both "global settings" and "individual projects" is what I mean by having exceptions -- eliminate the concept of global settings altogether and hide them using defaults.

[ghost]

On a similar note, the Environment interface would require fewer lines of code for development purposes if setSourceClasspath also accepted a Collection<String>.

[pvojtechovsky]

I have added new use cases into first description of this issue

UC5) incremental update of already built AST model by compiling of files modified since last build
UC6) incremental print of sources of model elements modified since last print

[surli]

I have added new use cases into first description of this issue

Thanks for adding them, but IMHO the last use cases are not only related to the design of a new launcher: they are features which would impact deeply Spoon.

[pvojtechovsky]

yes, UC5 and UC6 are probably new features of Spoon, which are either not implemented at all or not implemented correctly.

[monperrus]

See "Generating a Generic Fluent API in Java" https://arxiv.org/pdf/2002.06179

[MartinWitt]

After reading this interesting feature request, there are still some open questions for me.

Is the target creating a new launcher class, that combines all 3(?) launchers into one with a fluent API?
Or is it simply creating a fluent API for the "normal" launcher and ignoring maven and incremental launcher?
The second option could be done with a new class as wrapper.
The new launcher could hide all getEnvironment() calls for setting options with new methods like this.

    public FluentLauncher tabulationSize(int size) {
     launcher.getEnvironment().setTabulationSize(size);
     return this;
    }

[surli]

Is the target creating a new launcher class, that combines all 3(?) launchers into one with a fluent API?
Or is it simply creating a fluent API for the "normal" launcher and ignoring maven and incremental launcher?

IMO it would be already a good start to already have a fluent API for the normal "launcher", I guess best would be to allow it to be extensible in the future to maybe use it with incremental or maven launcher.

[MartinWitt]

I created some code to showcase an idea. See pr #3258. One of the problems is, that the fluent API dependents heavy on launcher and changes in environment class needs changes in the FluentLauncher class.

[surli]

One of the problems is, that the fluent API dependents heavy on launcher and changes in environment class needs changes in the FluentLauncher class.

I'd say that's ok if they are tightly coupled since we still need the old-way of using Spoon and the new way (FluentLauncher) aims at only being a wrapper for it.

Maybe on the future we could start thinking on deprecating some stuff and making the FluentLauncher more central, but it's a second step.

[ghost]

Another possibility to consider is using an inner class to indicate that the tight coupling is intentional. For example:

Builder b = new SpoonModel.Builder().setting( value ).build();

See also the following pattern for implementing builders: https://stackoverflow.com/a/23816160/59087

[surli]

Another possibility to consider is using an inner class to indicate that the tight coupling is intentional.

Well, in that case it would be a:

new Launcher.Builder().... 

and IMO the Launcher class is already too heavy (800+LOC) so I wouldn't put something more in it :)

[monperrus]

Closed per #3258