[GitHub] groovy pull request #748: GROOVY-8628: Groovydoc fails to parse Java static ...

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #748: GROOVY-8628: Groovydoc fails to parse Java static ...

asfgit
GitHub user av-m opened a pull request:

    https://github.com/apache/groovy/pull/748

    GROOVY-8628: Groovydoc fails to parse Java static nested classes with…

    … diamond operator
   
    java.g grammar file:
    * Added support for Java static nested class with diamond operator,
      e.g. new Class.Nested<>()
    * Simplified classOrInterfaceType with typeArgumentsOrDiamond
   
    groovydoc test:
    * Added groovydoc tool test for Java static nested class with diamond
      operator

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/av-m/groovy GROOVY-8628

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/groovy/pull/748.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #748
   
----
commit 02c7d8e0d911ca4fb4a7c51f348183dd5a061fd8
Author: Marco-Antonio Avallone <marco.avallone@...>
Date:   2018-06-03T20:36:17Z

    GROOVY-8628: Groovydoc fails to parse Java static nested classes with diamond operator
   
    java.g grammar file:
    * Added support for Java static nested class with diamond operator,
      e.g. new Class.Nested<>()
    * Simplified classOrInterfaceType with typeArgumentsOrDiamond
   
    groovydoc test:
    * Added groovydoc tool test for Java static nested class with diamond
      operator

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #748: GROOVY-8628: Groovydoc fails to parse Java static ...

asfgit
Github user danielsun1106 commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/748#discussion_r192608515
 
    --- Diff: subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/testfiles/JavaStaticNestedClassWithDiamond.java ---
    @@ -0,0 +1,33 @@
    +/*
    + *  Licensed to the Apache Software Foundation (ASF) under one
    + *  or more contributor license agreements.  See the NOTICE file
    + *  distributed with this work for additional information
    + *  regarding copyright ownership.  The ASF licenses this file
    + *  to you under the Apache License, Version 2.0 (the
    + *  "License"); you may not use this file except in compliance
    + *  with the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *  Unless required by applicable law or agreed to in writing,
    + *  software distributed under the License is distributed on an
    + *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + *  KIND, either express or implied.  See the License for the
    + *  specific language governing permissions and limitations
    + *  under the License.
    + */
    +package org.codehaus.groovy.tools.groovydoc.testfiles;
    +
    +public abstract class JavaStaticNestedClassWithDiamond<E> {
    +
    +    JavaStaticNestedClassWithDiamond() {
    +    }
    +
    +    static class Nested<E> extends JavaStaticNestedClassWithDiamond<E> {
    --- End diff --
   
    Would you mind add some javadoc to this inner class and test whether its javadoc can be extracted properly?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #748: GROOVY-8628: Groovydoc fails to parse Java static ...

asfgit
In reply to this post by asfgit
Github user av-m commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/748#discussion_r193147784
 
    --- Diff: subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/testfiles/JavaStaticNestedClassWithDiamond.java ---
    @@ -0,0 +1,33 @@
    +/*
    + *  Licensed to the Apache Software Foundation (ASF) under one
    + *  or more contributor license agreements.  See the NOTICE file
    + *  distributed with this work for additional information
    + *  regarding copyright ownership.  The ASF licenses this file
    + *  to you under the Apache License, Version 2.0 (the
    + *  "License"); you may not use this file except in compliance
    + *  with the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *  Unless required by applicable law or agreed to in writing,
    + *  software distributed under the License is distributed on an
    + *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + *  KIND, either express or implied.  See the License for the
    + *  specific language governing permissions and limitations
    + *  under the License.
    + */
    +package org.codehaus.groovy.tools.groovydoc.testfiles;
    +
    +public abstract class JavaStaticNestedClassWithDiamond<E> {
    +
    +    JavaStaticNestedClassWithDiamond() {
    +    }
    +
    +    static class Nested<E> extends JavaStaticNestedClassWithDiamond<E> {
    --- End diff --
   
    Sure, please see the changes made in ff2d499


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #748: GROOVY-8628: Groovydoc fails to parse Java static ...

asfgit
In reply to this post by asfgit
Github user danielsun1106 commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/748#discussion_r193270629
 
    --- Diff: subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/testfiles/JavaStaticNestedClassWithDiamond.java ---
    @@ -18,12 +18,17 @@
      */
     package org.codehaus.groovy.tools.groovydoc.testfiles;
     
    -public abstract class JavaStaticNestedClassWithDiamond<E> {
    +public class JavaStaticNestedClassWithDiamond<E> {
    --- End diff --
   
    Why should `abstract` be removed?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #748: GROOVY-8628: Groovydoc fails to parse Java static ...

asfgit
In reply to this post by asfgit
Github user av-m commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/748#discussion_r193308528
 
    --- Diff: subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/testfiles/JavaStaticNestedClassWithDiamond.java ---
    @@ -18,12 +18,17 @@
      */
     package org.codehaus.groovy.tools.groovydoc.testfiles;
     
    -public abstract class JavaStaticNestedClassWithDiamond<E> {
    +public class JavaStaticNestedClassWithDiamond<E> {
    --- End diff --
   
    It did not see any reason for the test class being abstract. Please let me know if I should revert the line in question.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #748: GROOVY-8628: Groovydoc fails to parse Java static ...

asfgit
In reply to this post by asfgit
Github user danielsun1106 commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/748#discussion_r193360554
 
    --- Diff: src/main/antlr2/org/codehaus/groovy/antlr/java/java.g ---
    @@ -402,10 +402,9 @@ wildcardType
      (("extends" | "super")=> typeArgumentBounds)?
      ;
     
    -typeArgumentsDiamond
    -{Token first = LT(1);}
    +typeArgumentsOrDiamond
         :   LT! GT!
    -    {#typeArgumentsDiamond = #(create(TYPE_ARGUMENTS, "TYPE_ARGUMENTS",first,LT(1)), #typeArgumentsDiamond);}
    --- End diff --
   
    Could you explain the reason why the line should be removed?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #748: GROOVY-8628: Groovydoc fails to parse Java static ...

asfgit
In reply to this post by asfgit
Github user danielsun1106 commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/748#discussion_r193360798
 
    --- Diff: subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/testfiles/JavaStaticNestedClassWithDiamond.java ---
    @@ -18,12 +18,17 @@
      */
     package org.codehaus.groovy.tools.groovydoc.testfiles;
     
    -public abstract class JavaStaticNestedClassWithDiamond<E> {
    +public class JavaStaticNestedClassWithDiamond<E> {
    --- End diff --
   
    OK. I see. It does not matter IMO.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #748: GROOVY-8628: Groovydoc fails to parse Java static ...

asfgit
In reply to this post by asfgit
Github user av-m commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/748#discussion_r193378890
 
    --- Diff: src/main/antlr2/org/codehaus/groovy/antlr/java/java.g ---
    @@ -402,10 +402,9 @@ wildcardType
      (("extends" | "super")=> typeArgumentBounds)?
      ;
     
    -typeArgumentsDiamond
    -{Token first = LT(1);}
    +typeArgumentsOrDiamond
         :   LT! GT!
    -    {#typeArgumentsDiamond = #(create(TYPE_ARGUMENTS, "TYPE_ARGUMENTS",first,LT(1)), #typeArgumentsDiamond);}
    --- End diff --
   
    Since this concerns diamond operators I could not think of any case where actual type arguments could be extracted.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] groovy pull request #748: GROOVY-8628: Groovydoc fails to parse Java static ...

asfgit
In reply to this post by asfgit
Github user asfgit closed the pull request at:

    https://github.com/apache/groovy/pull/748


---