You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2017/01/18 13:35:19 UTC

groovy git commit: Refine error messages for visibility modifiers

Repository: groovy
Updated Branches:
  refs/heads/parrot a7b90046d -> 30d6db060


Refine error messages for visibility modifiers


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/30d6db06
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/30d6db06
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/30d6db06

Branch: refs/heads/parrot
Commit: 30d6db0605c8def6bf9b3b3741eba357d2baf13e
Parents: a7b9004
Author: sunlan <su...@apache.org>
Authored: Wed Jan 18 21:35:08 2017 +0800
Committer: sunlan <su...@apache.org>
Committed: Wed Jan 18 21:35:08 2017 +0800

----------------------------------------------------------------------
 build.gradle                                    |  4 +--
 .../apache/groovy/parser/antlr4/AstBuilder.java | 34 ++++++++++++++------
 .../groovy/parser/antlr4/SyntaxErrorTest.groovy |  2 ++
 .../src/test/resources/fail/Modifier_04x.groovy |  3 ++
 .../src/test/resources/fail/Modifier_05x.groovy |  3 ++
 5 files changed, 34 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/30d6db06/build.gradle
----------------------------------------------------------------------
diff --git a/build.gradle b/build.gradle
index b3c6206..04e4193 100644
--- a/build.gradle
+++ b/build.gradle
@@ -471,10 +471,10 @@ compileTestGroovy {
     groovyOptions.fork(memoryMaximumSize: groovycTest_mx)
 }
 
-// TODO superfluous to check for JDK7 for Gradle version 3.2+ but leave for future?
+// TODO superfluous to check for JDK8 for Gradle version 3.2+ but leave for future?
 task checkCompatibility {
     doLast {
-        assert JavaVersion.current().java7Compatible
+        assert JavaVersion.current().java8Compatible
     }
 }
 

http://git-wip-us.apache.org/repos/asf/groovy/blob/30d6db06/subprojects/groovy-parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
----------------------------------------------------------------------
diff --git a/subprojects/groovy-parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/subprojects/groovy-parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index 539b6f2..b2f0a54 100644
--- a/subprojects/groovy-parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ b/subprojects/groovy-parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -217,7 +217,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> implements Groov
                 cfe = createParsingFailedException(t);
             }
 
-            LOGGER.log(Level.SEVERE, "Failed to build AST", cfe);
+//            LOGGER.log(Level.SEVERE, "Failed to build AST", cfe);
 
             throw cfe;
         }
@@ -4165,21 +4165,30 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> implements Groov
         private List<ModifierNode> modifierNodeList;
 
         public ModifierManager(List<ModifierNode> modifierNodeList) {
-            this.checkDuplicatedModifiers(modifierNodeList);
+            this.validate(modifierNodeList);
             this.modifierNodeList = Collections.unmodifiableList(asBoolean((Object) modifierNodeList) ? modifierNodeList : Collections.emptyList());
         }
 
-        public void checkDuplicatedModifiers(List<ModifierNode> modifierNodeList) {
-            Map<ModifierNode, Integer> modifierNodeCounter = new HashMap<>(modifierNodeList.size());
+        private void validate(List<ModifierNode> modifierNodeList) {
+            Map<ModifierNode, Integer> modifierNodeCounter = new LinkedHashMap<>(modifierNodeList.size());
+            int visibilityModifierCnt = 0;
 
             for (ModifierNode modifierNode : modifierNodeList) {
-               Integer cnt = modifierNodeCounter.get(modifierNode);
+                Integer cnt = modifierNodeCounter.get(modifierNode);
 
-               if (null == cnt) {
-                   modifierNodeCounter.put(modifierNode, 1);
-               } else if (1 == cnt && !modifierNode.isRepeatable()) {
-                   throw createParsingFailedException(modifierNode.getText() + " can not be duplicated", modifierNode);
-               }
+                if (null == cnt) {
+                    modifierNodeCounter.put(modifierNode, 1);
+                } else if (1 == cnt && !modifierNode.isRepeatable()) {
+                    throw createParsingFailedException("Cannot repeat modifier[" + modifierNode.getText() + "]", modifierNode);
+                }
+
+                if (modifierNode.isVisibilityModifier()) {
+                    visibilityModifierCnt++;
+
+                    if (visibilityModifierCnt > 1) {
+                        throw createParsingFailedException("Cannot specify modifier[" + modifierNode.getText() + "] when access scope has already been defined", modifierNode);
+                    }
+                }
             }
         }
 
@@ -4400,6 +4409,11 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> implements Groov
         public int hashCode() {
             return Objects.hash(type, text, annotationNode);
         }
+
+        @Override
+        public String toString() {
+            return this.text;
+        }
     }
 
     private final ModuleNode moduleNode;

http://git-wip-us.apache.org/repos/asf/groovy/blob/30d6db06/subprojects/groovy-parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
----------------------------------------------------------------------
diff --git a/subprojects/groovy-parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy b/subprojects/groovy-parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
index 680f146..787b313 100644
--- a/subprojects/groovy-parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
+++ b/subprojects/groovy-parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
@@ -129,6 +129,8 @@ class SyntaxErrorTest extends GroovyTestCase {
         TestUtils.doRunAndShouldFail('fail/Modifier_01x.groovy');
         TestUtils.doRunAndShouldFail('fail/Modifier_02x.groovy');
         TestUtils.doRunAndShouldFail('fail/Modifier_03x.groovy');
+        TestUtils.doRunAndShouldFail('fail/Modifier_04x.groovy');
+        TestUtils.doRunAndShouldFail('fail/Modifier_05x.groovy');
     }
 
     /**************************************/

http://git-wip-us.apache.org/repos/asf/groovy/blob/30d6db06/subprojects/groovy-parser-antlr4/src/test/resources/fail/Modifier_04x.groovy
----------------------------------------------------------------------
diff --git a/subprojects/groovy-parser-antlr4/src/test/resources/fail/Modifier_04x.groovy b/subprojects/groovy-parser-antlr4/src/test/resources/fail/Modifier_04x.groovy
new file mode 100644
index 0000000..fa56dec
--- /dev/null
+++ b/subprojects/groovy-parser-antlr4/src/test/resources/fail/Modifier_04x.groovy
@@ -0,0 +1,3 @@
+class A {
+    private public a
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/groovy/blob/30d6db06/subprojects/groovy-parser-antlr4/src/test/resources/fail/Modifier_05x.groovy
----------------------------------------------------------------------
diff --git a/subprojects/groovy-parser-antlr4/src/test/resources/fail/Modifier_05x.groovy b/subprojects/groovy-parser-antlr4/src/test/resources/fail/Modifier_05x.groovy
new file mode 100644
index 0000000..4f05f32
--- /dev/null
+++ b/subprojects/groovy-parser-antlr4/src/test/resources/fail/Modifier_05x.groovy
@@ -0,0 +1,3 @@
+class A {
+    protected public a
+}
\ No newline at end of file