@Newify(classNamePattern=/[A-Z].*/) - Questions

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
MG
Reply | Threaded
Open this post in threaded view
|

@Newify(classNamePattern=/[A-Z].*/) - Questions

MG
I am working on an implementation of @Newify classNamePattern support, which should allow Groovy to optionally support object creation (ctor calls) without the need for the "new" keyword on a global level, as in Python or Kotlin (https://issues.apache.org/jira/browse/GROOVY-8490) without negatively impacting build performance.

Imported classes and classes defined inside a script are already working inside my tests, but I have the following questions:

(1) What would the best way to get the class nodes for the missing case of predefined classes. I saw that ClassHelper has private static member that seems to have the classes I am looking for:
    private static final Class[] classes = new Class[]{
            Object.class, Boolean.TYPE, Character.TYPE, Byte.TYPE, Short.TYPE,
            Integer.TYPE, Long.TYPE, Double.TYPE, Float.TYPE, Void.TYPE,
            Closure.class, GString.class, List.class, Map.class, Range.class,
            Pattern.class, Script.class, String.class, Boolean.class,
            Character.class, Byte.class, Short.class, Integer.class, Long.class,
            Double.class, Float.class, BigDecimal.class, BigInteger.class,
            Number.class, Void.class, Reference.class, Class.class, MetaClass.class,
            Iterator.class, GeneratedClosure.class, GeneratedLambda.class, GroovyObjectSupport.class
    };

If I could access these, I could get their corresponding ClassNode through ClassHelper.makeCached(predefinedClass).
I could introduce a static class-name-to-class map (in ClassHelper or NewifyASTTransformation), but that would break one source of truth - unless not all predefined classes should be supported in the NewifyASTTransformation classNamePattern execution paths (I would need to filter for interfaces (e.g. MetaClass, GeneratedClosure, GeneratedLambda, GroovyObjectSupport) and non-instantiable classes (e.g. Void) in any case...)



(2) The name-to-class-node lookup code for the already implemented cases currently looks as follows:

final List<ClassNode> unitClassNodes = source.getAST().getClasses();
for(ClassNode type : unitClassNodes) { // speed up through map ?
    if(type.getNameWithoutPackage().equals(methodName)) {
        return type;
    }
}

final ClassNode type = source.getAST().getImportType(methodName);
if(type != null) {
    return type;
}
The question here would be if there already exists a faster way to look up the unitClassNodes, or if I should cache them in a map myself ?


(3) If someone globally uses an inefficient pattern string for classNamePattern (e.g. /.*/), build performance will probably be negatively impacted, because now every method call will be unecessarily checked if it is a ctor call. So would it make sense to either
(a) Introduce a boolean parameter that supplies a standard sensible pattern (e.g. /[A-Z].*/)
(b) Introduce a boolean parameter that allows to override a then to-be-introduced check that enforces the first matched char must be uppercase (either in the pattern or in the code) ?
It's a bit "safe the programmer from himself", but I could see someone making the honest mistake to think that if he wants every class to be instantiable without "new", to just use /.*/  (even if the GroovyDoc will of course warn not do so ;-) ) ...


Cheers,
mg