This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit ad0b9c8faa37e8746c92396f4c38114495ef8640
Author: Eric Milles <[email protected]>
AuthorDate: Tue Jan 21 16:33:41 2020 -0600

    minor edits
---
 .../apache/groovy/parser/antlr4/AstBuilder.java    |  98 +++++-------
 .../apache/groovy/parser/antlr4/TestUtils.groovy   | 178 +++++++++------------
 2 files changed, 114 insertions(+), 162 deletions(-)

diff --git 
a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
 
b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index a3b56c6..363a75e 100644
--- 
a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ 
b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -1079,94 +1079,82 @@ public class AstBuilder extends 
GroovyParserBaseVisitor<Object> {
 
     @Override
     public ClassNode visitClassDeclaration(ClassDeclarationContext ctx) {
-        String packageName = moduleNode.getPackageName();
-        packageName = null != packageName ? packageName : "";
+        String packageName = 
Optional.ofNullable(moduleNode.getPackageName()).orElse("");
+        String className = this.visitIdentifier(ctx.identifier());
+        if (VAR_STR.equals(className)) {
+            throw createParsingFailedException("var cannot be used for type 
declarations", ctx.identifier());
+        }
 
         List<ModifierNode> modifierNodeList = 
ctx.getNodeMetaData(TYPE_DECLARATION_MODIFIERS);
         Objects.requireNonNull(modifierNodeList, "modifierNodeList should not 
be null");
-
         ModifierManager modifierManager = new ModifierManager(this, 
modifierNodeList);
         int modifiers = modifierManager.getClassModifiersOpValue();
 
         boolean syntheticPublic = ((modifiers & Opcodes.ACC_SYNTHETIC) != 0);
         modifiers &= ~Opcodes.ACC_SYNTHETIC;
 
-        final ClassNode outerClass = classNodeStack.peek();
-        ClassNode classNode;
-        String className = this.visitIdentifier(ctx.identifier());
-
-        if (VAR_STR.equals(className)) {
-            throw createParsingFailedException("var cannot be used for type 
declarations", ctx.identifier());
-        }
-
+        ClassNode classNode, outerClass = classNodeStack.peek();
         if (asBoolean(ctx.ENUM())) {
-            classNode =
-                    EnumHelper.makeEnumNode(
-                            asBoolean(outerClass) ? className : packageName + 
className,
-                            modifiers, null, outerClass);
+            classNode = EnumHelper.makeEnumNode(
+                    asBoolean(outerClass) ? className : packageName + 
className,
+                    modifiers,
+                    null,
+                    outerClass
+            );
+        } else if (asBoolean(outerClass)) {
+            if (outerClass.isInterface()) modifiers |= Opcodes.ACC_STATIC;
+            classNode = new InnerClassNode(
+                    outerClass,
+                    outerClass.getName() + "$" + className,
+                    modifiers,
+                    ClassHelper.OBJECT_TYPE
+            );
         } else {
-            if (asBoolean(outerClass)) {
-                classNode =
-                        new InnerClassNode(
-                                outerClass,
-                                outerClass.getName() + "$" + className,
-                                modifiers | (outerClass.isInterface() ? 
Opcodes.ACC_STATIC : 0),
-                                ClassHelper.OBJECT_TYPE);
-            } else {
-                classNode =
-                        new ClassNode(
-                                packageName + className,
-                                modifiers,
-                                ClassHelper.OBJECT_TYPE);
-            }
+            classNode = new ClassNode(
+                    packageName + className,
+                    modifiers,
+                    ClassHelper.OBJECT_TYPE
+            );
         }
 
         configureAST(classNode, ctx);
-        classNode.putNodeMetaData(CLASS_NAME, className);
         classNode.setSyntheticPublic(syntheticPublic);
+        
classNode.setGenericsTypes(this.visitTypeParameters(ctx.typeParameters()));
+        boolean isInterface = (asBoolean(ctx.INTERFACE()) && 
!asBoolean(ctx.AT()));
+        boolean isInterfaceWithDefaultMethods = (isInterface && 
this.containsDefaultMethods(ctx));
 
-        if (asBoolean(ctx.TRAIT())) {
-            attachTraitAnnotation(classNode);
+        if (isInterfaceWithDefaultMethods || asBoolean(ctx.TRAIT())) {
+            classNode.addAnnotation(new 
AnnotationNode(ClassHelper.make("groovy.transform.Trait")));
         }
         classNode.addAnnotations(modifierManager.getAnnotations());
-        
classNode.setGenericsTypes(this.visitTypeParameters(ctx.typeParameters()));
 
-        boolean isInterface = asBoolean(ctx.INTERFACE()) && 
!asBoolean(ctx.AT());
-        boolean isInterfaceWithDefaultMethods = false;
-
-        // declaring interface with default method
-        if (isInterface && this.containsDefaultMethods(ctx)) {
-            isInterfaceWithDefaultMethods = true;
-            attachTraitAnnotation(classNode);
-            classNode.putNodeMetaData(IS_INTERFACE_WITH_DEFAULT_METHODS, true);
+        if (isInterfaceWithDefaultMethods) {
+            classNode.putNodeMetaData(IS_INTERFACE_WITH_DEFAULT_METHODS, 
Boolean.TRUE);
         }
+        classNode.putNodeMetaData(CLASS_NAME, className);
 
-        if (asBoolean(ctx.CLASS()) || asBoolean(ctx.TRAIT()) || 
isInterfaceWithDefaultMethods) { // class OR trait OR interface with default 
methods
+        if (asBoolean(ctx.CLASS()) || asBoolean(ctx.TRAIT()) || 
isInterfaceWithDefaultMethods) {
             classNode.setSuperClass(this.visitType(ctx.sc));
             classNode.setInterfaces(this.visitTypeList(ctx.is));
-
             this.initUsingGenerics(classNode);
-        } else if (isInterface) { // interface(NOT annotation)
-            classNode.setModifiers(classNode.getModifiers() | 
Opcodes.ACC_INTERFACE | Opcodes.ACC_ABSTRACT);
 
+        } else if (isInterface) {
+            classNode.setModifiers(classNode.getModifiers() | 
Opcodes.ACC_INTERFACE | Opcodes.ACC_ABSTRACT);
             classNode.setSuperClass(ClassHelper.OBJECT_TYPE);
             classNode.setInterfaces(this.visitTypeList(ctx.scs));
-
             this.initUsingGenerics(classNode);
-
             this.hackMixins(classNode);
-        } else if (asBoolean(ctx.ENUM())) { // enum
-            classNode.setModifiers(classNode.getModifiers() | Opcodes.ACC_ENUM 
| Opcodes.ACC_FINAL);
 
+        } else if (asBoolean(ctx.ENUM())) {
+            classNode.setModifiers(classNode.getModifiers() | Opcodes.ACC_ENUM 
| Opcodes.ACC_FINAL);
             classNode.setInterfaces(this.visitTypeList(ctx.is));
-
             this.initUsingGenerics(classNode);
-        } else if (asBoolean(ctx.AT())) { // annotation
-            classNode.setModifiers(classNode.getModifiers() | 
Opcodes.ACC_INTERFACE | Opcodes.ACC_ABSTRACT | Opcodes.ACC_ANNOTATION);
 
+        } else if (asBoolean(ctx.AT())) {
+            classNode.setModifiers(classNode.getModifiers() | 
Opcodes.ACC_INTERFACE | Opcodes.ACC_ABSTRACT | Opcodes.ACC_ANNOTATION);
             classNode.addInterface(ClassHelper.Annotation_TYPE);
-
             this.hackMixins(classNode);
+
         } else {
             throw createParsingFailedException("Unsupported class declaration: 
" + ctx.getText(), ctx);
         }
@@ -1193,10 +1181,6 @@ public class AstBuilder extends 
GroovyParserBaseVisitor<Object> {
         return classNode;
     }
 
-    private void attachTraitAnnotation(ClassNode classNode) {
-        classNode.addAnnotation(new 
AnnotationNode(ClassHelper.make("groovy.transform.Trait")));
-    }
-
     @SuppressWarnings("unchecked")
     private boolean containsDefaultMethods(ClassDeclarationContext ctx) {
         List<MethodDeclarationContext> methodDeclarationContextList =
diff --git 
a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/TestUtils.groovy
 
b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/TestUtils.groovy
index 655080d..c2d363f 100644
--- 
a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/TestUtils.groovy
+++ 
b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/TestUtils.groovy
@@ -18,6 +18,7 @@
  */
 package org.apache.groovy.parser.antlr4
 
+import groovy.transform.AutoFinal
 import groovy.transform.CompileDynamic
 import groovy.transform.CompileStatic
 import groovy.util.logging.Log
@@ -50,42 +51,35 @@ import org.codehaus.groovy.syntax.Token
 import java.util.zip.ZipEntry
 import java.util.zip.ZipFile
 
-/**
- * Utilities for test
- */
+@CompileStatic @AutoFinal @Log
+final class TestUtils {
 
-@CompileStatic
-@Log
-class TestUtils {
-    public static final String DEFAULT_RESOURCES_PATH = 
'subprojects/parser-antlr4/src/test/resources';
-    public static final String RESOURCES_PATH = new 
File(DEFAULT_RESOURCES_PATH).exists() ? DEFAULT_RESOURCES_PATH : 
'src/test/resources';
+    public static final String RESOURCES_PATH = 
Optional.of('subprojects/parser-antlr4/src/test/resources').filter(path -> new 
File(path).exists()).orElse('src/test/resources')
 
     static doTest(String path) {
-        return doTest(path, ASTComparatorCategory.DEFAULT_CONFIGURATION)
-    }
-
-    static doTest(String path, List ignoreClazzList) {
-        return doTest(path, addIgnore(ignoreClazzList, 
ASTComparatorCategory.LOCATION_IGNORE_LIST))
+        doTest(path, ASTComparatorCategory.DEFAULT_CONFIGURATION)
     }
 
     static doTest(String path, conf) {
         doTest(path, conf, new 
CompilerConfiguration(CompilerConfiguration.DEFAULT))
     }
 
+    static doTest(String path, Collection<Class> ignoreSourcePosition) {
+        doTest(path, addIgnore(ignoreSourcePosition, 
ASTComparatorCategory.LOCATION_IGNORE_LIST))
+    }
+
     @CompileDynamic
     static doTest(String path, conf, CompilerConfiguration 
compilerConfiguration) {
         AbstractParser antlr4Parser = new Antlr4Parser(compilerConfiguration)
         AbstractParser antlr2Parser = new Antlr2Parser()
 
-        File file = new File("$RESOURCES_PATH/$path");
+        File file = new File("$RESOURCES_PATH/$path")
         def (newAST, newElapsedTime) = profile { antlr4Parser.parse(file) }
         def (oldAST, oldElapsedTime) = profile { antlr2Parser.parse(file) }
 
+        assertAST(newAST, oldAST, conf)
 
-        assertAST(newAST, oldAST, conf);
-
-        long diffInMillis = newElapsedTime - oldElapsedTime;
-
+        long diffInMillis = newElapsedTime - oldElapsedTime
         if (diffInMillis >= 500) {
             log.warning "${path}\t\t\t\t\tdiff:${diffInMillis / 
1000}s,\tnew:${newElapsedTime / 1000}s,\told:${oldElapsedTime / 1000}s."
         }
@@ -93,106 +87,96 @@ class TestUtils {
         return [newAST, oldAST]
     }
 
-    /*
-    static unzipAndTest(String path, String entryName) {
-        unzipAndTest(path, entryName, 
ASTComparatorCategory.DEFAULT_CONFIGURATION)
+    static shouldFail(String path, boolean toCheckNewParserOnly = false) {
+        shouldFail(path, ASTComparatorCategory.DEFAULT_CONFIGURATION, 
toCheckNewParserOnly)
     }
-    */
 
-    /*
-    static unzipAndTest(String path, String entryName, List ignoreClazzList) {
-        unzipAndTest(path, entryName, addIgnore(ignoreClazzList, 
ASTComparatorCategory.LOCATION_IGNORE_LIST))
+    static shouldFail(String path, Collection<Class> ignoreSourcePosition, 
boolean toCheckNewParserOnly = false) {
+        shouldFail(path, addIgnore(ignoreSourcePosition, 
ASTComparatorCategory.LOCATION_IGNORE_LIST), toCheckNewParserOnly)
     }
-    */
 
     @CompileDynamic
-    static unzipAndTest(String path, String entryName, conf, Map<String, 
String> replacementsMap=[:]) {
+    static shouldFail(String path, conf, boolean toCheckNewParserOnly = false) 
{
         AbstractParser antlr4Parser = new Antlr4Parser()
         AbstractParser antlr2Parser = new Antlr2Parser()
 
-        String name = "$path!$entryName";
-        String text = readZipEntry(path, entryName);
+        File file = new File("$RESOURCES_PATH/$path")
+        def (newAST, newElapsedTime) = profile { antlr4Parser.parse(file) }
+        def (oldAST, oldElapsedTime) = profile { antlr2Parser.parse(file) }
 
-        replacementsMap?.each {k, v ->
-            text = text.replace(k, v);
+        assert (newAST == null || newAST.context.errorCollector.hasErrors())
+        if (!toCheckNewParserOnly) {
+            assert (oldAST == null || 
oldAST.context.errorCollector.hasErrors())
         }
 
-        def (newAST, newElapsedTime) = profile { antlr4Parser.parse(name, 
text) }
-        def (oldAST, oldElapsedTime) = profile { antlr2Parser.parse(name, 
text) }
-
-
-        assertAST(newAST, oldAST, conf);
-
-        long diffInMillis = newElapsedTime - oldElapsedTime;
-
+        long diffInMillis = newElapsedTime - oldElapsedTime
         if (diffInMillis >= 500) {
-            log.warning "${path}!${entryName}\t\t\t\t\tdiff:${diffInMillis / 
1000}s,\tnew:${newElapsedTime / 1000}s,\told:${oldElapsedTime / 1000}s."
+            log.warning "${path}\t\t\t\t\tdiff:${diffInMillis / 
1000}s,\tnew:${newElapsedTime / 1000}s,\told:${oldElapsedTime / 1000}s."
         }
     }
 
-
-    static shouldFail(String path, boolean toCheckNewParserOnly = false) {
-        shouldFail(path, ASTComparatorCategory.DEFAULT_CONFIGURATION, 
toCheckNewParserOnly)
+    /*
+    static void unzipAndTest(String path, String entryName) {
+        unzipAndTest(path, entryName, 
ASTComparatorCategory.DEFAULT_CONFIGURATION)
     }
 
-    static shouldFail(String path, List ignoreClazzList, boolean 
toCheckNewParserOnly = false) {
-        shouldFail(path, addIgnore(ignoreClazzList, 
ASTComparatorCategory.LOCATION_IGNORE_LIST), toCheckNewParserOnly)
+    static void unzipAndTest(String path, String entryName, Collection<Class> 
ignoreSourcePosition) {
+        unzipAndTest(path, entryName, addIgnore(ignoreSourcePosition, 
ASTComparatorCategory.LOCATION_IGNORE_LIST))
     }
 
     @CompileDynamic
-    static shouldFail(String path, conf, boolean toCheckNewParserOnly = false) 
{
+    static void unzipAndTest(String path, String entryName, conf, Map<String, 
String> replacementsMap = null) {
         AbstractParser antlr4Parser = new Antlr4Parser()
         AbstractParser antlr2Parser = new Antlr2Parser()
 
-        File file = new File("$RESOURCES_PATH/$path");
-        def (newAST, newElapsedTime) = profile { antlr4Parser.parse(file) }
-        def (oldAST, oldElapsedTime) = profile { antlr2Parser.parse(file) }
+        String name = "$path!$entryName"
+        String text = readZipEntry(path, entryName)
 
-        if (toCheckNewParserOnly) {
-            assert (newAST == null || 
newAST.context.errorCollector.hasErrors())
-        } else {
-            assert (newAST == null || 
newAST.context.errorCollector.hasErrors()) &&
-                    (oldAST == null || 
oldAST.context.errorCollector.hasErrors())
+        replacementsMap?.each { k, v ->
+            text = text.replace(k, v)
         }
 
-        long diffInMillis = newElapsedTime - oldElapsedTime;
+        def (newAST, newElapsedTime) = profile { antlr4Parser.parse(name, 
text) }
+        def (oldAST, oldElapsedTime) = profile { antlr2Parser.parse(name, 
text) }
+
+
+        assertAST(newAST, oldAST, conf)
 
+        long diffInMillis = newElapsedTime - oldElapsedTime
         if (diffInMillis >= 500) {
-            log.warning "${path}\t\t\t\t\tdiff:${diffInMillis / 
1000}s,\tnew:${newElapsedTime / 1000}s,\told:${oldElapsedTime / 1000}s."
+            log.warning "${path}!${entryName}\t\t\t\t\tdiff:${diffInMillis / 
1000}s,\tnew:${newElapsedTime / 1000}s,\told:${oldElapsedTime / 1000}s."
         }
     }
+    */
 
     @CompileDynamic
-    static unzipAndFail(String path, String entryName, conf, Map<String, 
String> replacementsMap=[:], boolean toCheckNewParserOnly = false) {
+    static unzipAndFail(String path, String entryName, conf, Map<String, 
String> replacementsMap = null, boolean toCheckNewParserOnly = false) {
         AbstractParser antlr4Parser = new Antlr4Parser()
         AbstractParser antlr2Parser = new Antlr2Parser()
 
-        String name = "$path!$entryName";
-        String text = readZipEntry(path, entryName);
+        String name = "$path!$entryName"
+        String text = readZipEntry(path, entryName)
 
-        replacementsMap?.each {k, v ->
-            text = text.replace(k, v);
+        replacementsMap?.each { k, v ->
+            text = text.replace(k, v)
         }
 
         def (newAST, newElapsedTime) = profile { antlr4Parser.parse(name, 
text) }
         def (oldAST, oldElapsedTime) = profile { antlr2Parser.parse(name, 
text) }
 
-        if (toCheckNewParserOnly) {
-            assert (newAST == null || 
newAST.context.errorCollector.hasErrors())
-        } else {
-            assert (newAST == null || 
newAST.context.errorCollector.hasErrors()) &&
-                    (oldAST == null || 
oldAST.context.errorCollector.hasErrors())
+        assert (newAST == null || newAST.context.errorCollector.hasErrors())
+        if (!toCheckNewParserOnly) {
+            assert (oldAST == null || 
oldAST.context.errorCollector.hasErrors())
         }
 
-        long diffInMillis = newElapsedTime - oldElapsedTime;
-
+        long diffInMillis = newElapsedTime - oldElapsedTime
         if (diffInMillis >= 500) {
             log.warning "${path}!${entryName}\t\t\t\t\tdiff:${diffInMillis / 
1000}s,\tnew:${newElapsedTime / 1000}s,\told:${oldElapsedTime / 1000}s."
         }
     }
 
     static assertAST(ModuleNode ast1, ModuleNode ast2, conf) {
-        assert null != ast1 && null != ast2
+        assert ast1 != null && ast2 != null
 
         ASTComparatorCategory.apply(conf) {
             assert ast1 == ast2
@@ -202,7 +186,7 @@ class TestUtils {
     }
 
     static genSrc(ModuleNode ast) {
-        return new AstDumper(ast).gen();
+        return new AstDumper(ast).gen()
     }
 
     static profile(Closure c) {
@@ -210,43 +194,29 @@ class TestUtils {
         def result = c.call()
         long end = System.currentTimeMillis()
 
-        return [result, end - begin];
-    }
-
-    static addIgnore(Class aClass, List<String> ignore, Map<Class, 
List<String>> c = null) {
-        c = c ?: new HashMap<>(ASTComparatorCategory.DEFAULT_CONFIGURATION) as 
Map<Class, List<String>>;
-        c[aClass].addAll(ignore)
-        return c
+        return [result, end - begin]
     }
 
-    static addIgnore(Collection<Class> aClass, List<String> ignore, Map<Class, 
List<String>> c = null) {
-        c = c ?: new HashMap<>(ASTComparatorCategory.DEFAULT_CONFIGURATION) as 
Map<Class, List<String>>;
-        aClass.each { c[it].addAll(ignore) }
-        return c
+    static addIgnore(Collection<Class> c, List<String> ignore, Map<Class, 
List<String>> map = new HashMap<>(ASTComparatorCategory.DEFAULT_CONFIGURATION)) 
{
+        c.each { map[it].addAll(ignore) }
+        return map
     }
 
     static readZipEntry(String path, String entryName) {
-        String result = "";
+        String result = ''
 
-        def zf = new ZipFile(new File(path));
-        try {
-            def is = new BufferedInputStream(zf.getInputStream(new 
ZipEntry(entryName)));
-            result = is.getText("UTF-8");
-        } catch (Exception e) {
-            log.severe(e.message);
-        } finally {
-            try {
-                zf.close();
-            } catch(Exception e) {
-                // IGNORED
-            }
+        try (zf = new ZipFile(new File(path))) {
+            def is = new BufferedInputStream(zf.getInputStream(new 
ZipEntry(entryName)))
+            result = is.getText('UTF-8')
+        } catch (e) {
+            log.severe(e.message)
         }
 
-        return result;
+        return result
     }
 
     static doRunAndShouldFail(String path) {
-        assert !executeScript(path);
+        assert !executeScript(path)
     }
 
     static doRunAndTest(String path) {
@@ -255,7 +225,7 @@ class TestUtils {
     }
 
     static doRunAndTestAntlr4(String path, CompilerConfiguration 
compilerConfiguration = new 
CompilerConfiguration(CompilerConfiguration.DEFAULT)) {
-        assert executeScript(path, compilerConfiguration);
+        assert executeScript(path, compilerConfiguration)
     }
 
     static doRunAndTestAntlr2(String path) {
@@ -267,16 +237,14 @@ class TestUtils {
     }
 
     static executeScript(GroovyShell gsh, String path) {
-        def file = new File(path);
-        def content = file.text;
-
+        def file = new File(path)
+        def content = file.text
         try {
-            gsh.evaluate(content);
-//            log.info("Evaluated $file")
-            return true;
+            gsh.evaluate(content)
+            return true
         } catch (Throwable t) {
-            log.severe("Failed $file: ${t.getMessage()}");
-            return false;
+            log.severe("Failed $file: ${t.getMessage()}")
+            return false
         }
     }
 

Reply via email to