You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by GitBox <gi...@apache.org> on 2020/09/08 19:58:41 UTC

[GitHub] [groovy] eric-milles opened a new pull request #1362: GROOVY-9712: type name may start with more than A-Z (CapitalizedIdentifier)

eric-milles opened a new pull request #1362:
URL: https://github.com/apache/groovy/pull/1362


   https://issues.apache.org/jira/browse/GROOVY-9712
   
   **Note:** The parser tests that use `TestUtils#doTest` are silently swallowing all parser errors since there is no longer comparison against any antlr2 result.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [groovy] eric-milles commented on pull request #1362: GROOVY-9712: type name may start with more than A-Z (CapitalizedIdentifier)

Posted by GitBox <gi...@apache.org>.
eric-milles commented on pull request #1362:
URL: https://github.com/apache/groovy/pull/1362#issuecomment-689161300


   From the issue:
   >Another way of looking at this is that field/property uses the generalized-typename rule and a method's return type uses standard-typename rule.  The later requires a capital letter (A-Z) as the first name character.  The former has allowances for lowercase names.  I'm not sure why this distinction exists in the parser.
   ```
   generalClassOrInterfaceType
   options { baseContext = classOrInterfaceType; }
       :   qualifiedClassName typeArguments?
       ;
   
   standardClassOrInterfaceType
   options { baseContext = classOrInterfaceType; }
       :   qualifiedStandardClassName typeArguments?
       ;
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [groovy] paulk-asert commented on a change in pull request #1362: GROOVY-9712: type name may start with more than A-Z (CapitalizedIdentifier)

Posted by GitBox <gi...@apache.org>.
paulk-asert commented on a change in pull request #1362:
URL: https://github.com/apache/groovy/pull/1362#discussion_r485372609



##########
File path: src/antlr/GroovyLexer.g4
##########
@@ -882,7 +882,7 @@ ELVIS_ASSIGN    : '?=';
 
 // §3.8 Identifiers (must appear after all keywords in the grammar)
 CapitalizedIdentifier
-    :   [A-Z] JavaLetterOrDigit*
+    :   JavaLetter {Character.isUpperCase(getInputStream().LA(-1))}? JavaLetterOrDigit*

Review comment:
       LGTM. I'll merge and include this change.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [groovy] paulk-asert commented on a change in pull request #1362: GROOVY-9712: type name may start with more than A-Z (CapitalizedIdentifier)

Posted by GitBox <gi...@apache.org>.
paulk-asert commented on a change in pull request #1362:
URL: https://github.com/apache/groovy/pull/1362#discussion_r485372966



##########
File path: subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/TestUtils.groovy
##########
@@ -87,17 +81,15 @@ final class TestUtils {
         return [newAST, null]
     }
 
-    static void shouldFail(String path, boolean toCheckNewParserOnly = false) {

Review comment:
       I'll add back in the `(String, boolean)` variant for binary compatibility




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [groovy] asfgit closed pull request #1362: GROOVY-9712: type name may start with more than A-Z (CapitalizedIdentifier)

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1362:
URL: https://github.com/apache/groovy/pull/1362


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [groovy] danielsun1106 commented on a change in pull request #1362: GROOVY-9712: type name may start with more than A-Z (CapitalizedIdentifier)

Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on a change in pull request #1362:
URL: https://github.com/apache/groovy/pull/1362#discussion_r485344093



##########
File path: src/antlr/GroovyLexer.g4
##########
@@ -882,7 +882,7 @@ ELVIS_ASSIGN    : '?=';
 
 // §3.8 Identifiers (must appear after all keywords in the grammar)
 CapitalizedIdentifier
-    :   [A-Z] JavaLetterOrDigit*
+    :   JavaLetter {Character.isUpperCase(getInputStream().LA(-1))}? JavaLetterOrDigit*

Review comment:
       `getInputStream().LA(-1)` could be simplified as  `_input.LA(-1)`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org