Generated code for refined methods with generic type declarations is missing return type

[Zimon Kuhs]

When refining a method with generic types, the generated code is wrong.

With this aspect

aspect Test {
    public class Klass {}
    private static <T extends List<?>> T Klass.method() { return new ArrayList<String>(); }
    refine Test private static T Klass.method() { return refined(); }
}

the following code is generated (note that the return type T is appended to the method name of the method being refined instead of being a return type):

public class Klass extends java.lang.Object {
    private static <T extends List<?>> refined_Test_Klass_T method() { return new ArrayList<String>(); }
    private static T method() { return refined_Test_Klass_T(); }
}

which of course leads to the following errors:

error: cannot find symbol
    private static <T extends List<?>> refined_Test_Klass_T method() { return new ArrayList<String>(); }
                                       ^
  symbol:   class refined_Test_Klass_T
  location: class Klass
error: cannot find symbol
      private static T method() { return refined_Test_Klass_T(); }
                     ^
  symbol:   class T
  location: class Klass
error: cannot find symbol
      private static T method() { return refined_Test_Klass_T(); }
                                         ^
  symbol:   method refined_Test_Klass_T()
  location: class Klass

As a side note, JastAdd 2's reference documentation does not specify whether or not the type declaration is required in the refine-statement or not.

[Jesper Öqvist]

Thank you for reporting this issue, and thank you for providing the test case! I can confirm that the test case fails in JastAdd2 2.2.2-39-g25b8f35.

I added the test to the regression test suite as refine/generic_method_01p.

The refine feature in JastAdd is one of the least polished features, in my opinion. The code that performs refinement is very complicated, and it does some really hacky stuff:

• it accesses values in the parse tree by index in many places, and,
• directly replaces literals in the parse tree to perform a refinement.

This refinement process is very fragile to small changes in the parse tree. In the current issue, the name of the method to be refined is taken by indexing a specific node in the parse tree. It seems like the type parameter part of the original declaration (<T extends List<?>>) has shifted the node containing the method name so that instead the return type T is taken as the method name.

Here is the relevant part of src/jastadd/ast/Attributes.jrag, in TypeDecl.refineWith(ClassBodyObject,ClassBodyObject):

    } else if (node instanceof org.jastadd.jrag.AST.ASTAspectMethodDeclaration
        || node instanceof org.jastadd.jrag.AST.ASTAspectRefineMethodDeclaration) {
      // Retrieve methodName.
      String idDecl = ((org.jastadd.jrag.AST.SimpleNode)decl.node.jjtGetChild(1)).firstToken.image;
      String methodName = idDecl.trim();

      // Add prefix refined_aspectName_.
      idDecl = idDecl.replaceAll(methodName,
          "refined_" + decl.aspectName() + "_" + name() + "_" + methodName);
      ((org.jastadd.jrag.AST.SimpleNode)decl.node.jjtGetChild(1)).firstToken.image = idDecl;

[Jesper Öqvist]

Fix error in generic method refinement

Fixed an error in how refined method names are computed and modified by the method refinement process. See JastAdd issue #277.

The parsing production for aspect method refinements has been updated to include optional type parameters.

Added note in the reference manual about modifiers/type parameters/return type for method refinements.

fixes #277 (bitbucket)

→ <<cset 4fed86ac9c59>>

[Jesper Öqvist]

@zimonkuhs_modelon I had to change some things in your test case to make it compile after the fix:

// Refining works for generic methods.
// https://bitbucket.org/jastadd/jastadd2/issues/277/generated-code-for-refined-methods-with
// .grammar: { R; }
// .result: COMPILE_PASS
import java.util.*;

aspect Test {
  public class Klass {}

  public static <T extends List<?>> T Klass.method() {
    return null;
  }

  refine Test public static <T extends List<?>> T Klass.method() {
    return refined();
  }

  public void R.foo() {
    // Simple use of the method to test that it was actually generated.
    Klass.method();
  }
}

Changes:

• Added type parameters in the refine statement.
• Removed new ArrayList<String>().
• Added import statement.

[Zimon Kuhs]

@joqvist it makes sense to put the type declaration explicitly in the refine statement anyway. Well done, and thanks a lot for updating the reference manual. It will help new guys like me down the line!

I hate to be a nitpicker but I noticed a spelling error in your commit's documentation update (those buggers can live forever), line 833 in doc/reference-manual.md, "paramgers": https://bitbucket.org/jastadd/jastadd2/commits/4fed86ac9c59f107e1278b58e09f9350a28aece0

[Jesper Öqvist]

@zimonkuhs_modelon Whoops, thanks for spotting the typo!