Trait interface and VariableAccess/MethodAccess implementation

[jkschneider]

What's changed?

Introduces a Trait API into OpenRewrite which encapsulates semantic information on top of syntax.

What's your motivation?

We had an initial implementation of Trait in rewrite-analysis written by @JLLeitschuh that really did a lot to help understand the shape of what is needed.

In this implementation, we've removed the dependency on functionaljava, reasoning that this dependency would be too steep of a learning curve for the average user. One of the key benefits of functionaljava was in support of Jonathan's enhanced Validation type which has also been removed here. I think Validation was mostly useful in the development of the trait, and where possible traits should build on other traits so that the logic of each is fairly comprehensible.

We are also adding some API, in particular take note of how Trait can be turned into a visitor. Check out the test for how that is used. I'm imagining a future where many recipes would start out with a Trait that quickly hones the recipe in on a point of interest in the code.

Example 1 (use the higher method to assert something about a tree's ancestors)

public class MicrometerNamesUseDots extends Recipe {
    @Override
    public String getDisplayName() {
        return "Micrometer meters should use dot separation";
    }

    @Override
    public String getDescription() {
        return "Micrometer naming convention automatically replaces dots with underscores," +
               "camel casing, or whatever is idiomatic depending on the monitoring system " +
               "that the metrics are going to.";
    }

    @Override
    public TreeVisitor<?, ExecutionContext> getVisitor() {
        return new JavaIsoVisitor<>() {
            final MethodAccess.Matcher matcher = new MethodAccess.Matcher("io.micrometer.core.instrument.MeterRegistry *(String, ..)")
              .returns(ret -> TypeUtils.isAssignableTo("io.micrometer.core.instrument.Meter", ret));

            @Override
            public J.Literal visitLiteral(J.Literal literal, ExecutionContext ctx) {
                if (matcher.higher(getCursor()).findAny().isPresent()) {
                    //noinspection DataFlowIssue
                    String value = literal.getValue().toString();
                    if (value.contains("_")) {
                        return literal.withValue(value.replace("_", "."));
                    }
                }
                return super.visitLiteral(literal, ctx);
            }
        };
    }
}

Example 2 (turn the trait into a visitor)

This is the same recipe as above, implemented differently.

public class MicrometerNamesUseDots2 extends Recipe {
    @Override
    public String getDisplayName() {
        return "Micrometer meters should use dot separation";
    }

    @Override
    public String getDescription() {
        return "Micrometer naming convention automatically replaces dots with underscores," +
               "camel casing, or whatever is idiomatic depending on the monitoring system " +
               "that the metrics are going to.";
    }

    @Override
    public TreeVisitor<?, ExecutionContext> getVisitor() {
        return new MethodAccess.Matcher("io.micrometer.core.instrument.MeterRegistry *(String, ..)")
          .returns(ret -> TypeUtils.isAssignableTo("io.micrometer.core.instrument.Meter", ret))
          .asVisitor(ma -> {
              MethodCall call = ma.getTree();
              Expression name = call.getArguments().get(0);
              if (name instanceof J.Literal) {
                  J.Literal nameLiteral = (J.Literal) name;
                  String nameStr = nameLiteral.getValue().toString();
                  if (nameStr.contains("_")) {
                      call = call.withArguments(ListUtils.mapFirst(call.getArguments(),
                        n -> nameLiteral.withValue(nameStr.replace("_", "."))));
                  }
              }
              return call;
          });
    }
}

[JLLeitschuh]

Do you want to add a test for the this and super cases?

[jkschneider]

More tests are certainly warranted, but expensive in time to create. Wanted to do a quick check on whether or not this approach was even close to reasonable.

[JLLeitschuh]

Very interesting to see this. I'm curious to see other cases too!

[JLLeitschuh]

Have you thought about tree navigation as well? VariableAccess -> VariableDeclaration for example? Or the other way around?

[jkschneider]

Have you thought about tree navigation as well? VariableAccess -> VariableDeclaration for example? Or the other way around?

I think the model you have in VarAccess is quite good. I don't see any reason why we can't build var access -> var declaration as either behavior or state of either.

[timtebeek]

First part of a review before calls & heading out. Mostly smaller items and not on the big design, but good to clear out before a merge.