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 2022/04/23 17:31:37 UTC

[GitHub] [groovy] stalb opened a new pull request, #1716: GROOVY-10593: replace '.' by '/' in imports

stalb opened a new pull request, #1716:
URL: https://github.com/apache/groovy/pull/1716

   Fixes imports that were not rewritten to replace dots with slashes. 


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

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


[GitHub] [groovy] paulk-asert commented on pull request #1716: GROOVY-10593: fixes groovydoc sometimes links to incorrect classes

Posted by GitBox <gi...@apache.org>.
paulk-asert commented on PR #1716:
URL: https://github.com/apache/groovy/pull/1716#issuecomment-1115989007

   Merged, thanks!


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

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


[GitHub] [groovy] stalb commented on a diff in pull request #1716: GROOVY-10593: replace '.' by '/' in imports

Posted by GitBox <gi...@apache.org>.
stalb commented on code in PR #1716:
URL: https://github.com/apache/groovy/pull/1716#discussion_r857152026


##########
subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyClassDoc.java:
##########
@@ -682,11 +686,24 @@ private static boolean isPrimitiveType(String name) {
     private GroovyClassDoc resolveInternalClassDocFromImport(GroovyRootDoc rootDoc, String baseName) {
         if (isPrimitiveType(baseName)) return null;
         for (String importName : importedClassesAndPackages) {
-            if (importName.endsWith("/" + baseName)) {
-                GroovyClassDoc doc = ((SimpleGroovyRootDoc)rootDoc).classNamedExact(importName);
-                if (doc != null) return doc;
+            String targetClassName = null;
+            if (aliases.containsKey(baseName)) {
+                targetClassName = aliases.get(baseName);
+            } else if (importName.endsWith("/" + baseName)) {
+                targetClassName = importName;
             } else if (importName.endsWith("/*")) {
-                GroovyClassDoc doc = ((SimpleGroovyRootDoc)rootDoc).classNamedExact(importName.substring(0, importName.length() - 2) + baseName);
+                targetClassName = importName.substring(0, importName.length() - 1) + baseName;
+            }
+            // need this for correct resolution of static imports
+            if (targetClassName != null){
+                GroovyClassDoc doc = null;
+                Optional<Name>  name = new JavaParser().parseName(targetClassName.replace('/','.')).getResult();
+                String staticPart = "";
+                while (doc == null && name.isPresent()) {               
+                    doc = ((SimpleGroovyRootDoc)rootDoc).classNamedExact(name.get().asString().replace('.','/')+staticPart);
+                    staticPart = "."+name.get().getIdentifier()+staticPart;
+                    name = name.get().getQualifier();
+                }

Review Comment:
   _SimpleGroovyRootDoc.classNamedExact_ is used at several other places in this class. 
   Thus I can't exclude that the same problem occurs elsewhere. 
   
   Perhaps it  would be better to change the method itself.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

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


[GitHub] [groovy] paulk-asert merged pull request #1716: GROOVY-10593: fixes groovydoc sometimes links to incorrect classes

Posted by GitBox <gi...@apache.org>.
paulk-asert merged PR #1716:
URL: https://github.com/apache/groovy/pull/1716


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

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


[GitHub] [groovy] paulk-asert commented on a diff in pull request #1716: GROOVY-10593: fixes groovydoc sometimes links to incorrect classes

Posted by GitBox <gi...@apache.org>.
paulk-asert commented on code in PR #1716:
URL: https://github.com/apache/groovy/pull/1716#discussion_r863679483


##########
subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyClassDoc.java:
##########
@@ -682,11 +686,24 @@ private static boolean isPrimitiveType(String name) {
     private GroovyClassDoc resolveInternalClassDocFromImport(GroovyRootDoc rootDoc, String baseName) {
         if (isPrimitiveType(baseName)) return null;
         for (String importName : importedClassesAndPackages) {
-            if (importName.endsWith("/" + baseName)) {
-                GroovyClassDoc doc = ((SimpleGroovyRootDoc)rootDoc).classNamedExact(importName);
-                if (doc != null) return doc;
+            String targetClassName = null;
+            if (aliases.containsKey(baseName)) {
+                targetClassName = aliases.get(baseName);
+            } else if (importName.endsWith("/" + baseName)) {
+                targetClassName = importName;
             } else if (importName.endsWith("/*")) {
-                GroovyClassDoc doc = ((SimpleGroovyRootDoc)rootDoc).classNamedExact(importName.substring(0, importName.length() - 2) + baseName);
+                targetClassName = importName.substring(0, importName.length() - 1) + baseName;
+            }
+            // need this for correct resolution of static imports
+            if (targetClassName != null){
+                GroovyClassDoc doc = null;
+                Optional<Name>  name = new JavaParser().parseName(targetClassName.replace('/','.')).getResult();
+                String staticPart = "";
+                while (doc == null && name.isPresent()) {               
+                    doc = ((SimpleGroovyRootDoc)rootDoc).classNamedExact(name.get().asString().replace('.','/')+staticPart);
+                    staticPart = "."+name.get().getIdentifier()+staticPart;
+                    name = name.get().getQualifier();
+                }

Review Comment:
   I had a look at the other places which call that method. I am unsure if any are affected in the same way, so I've merged as is and we can always refine later if we find a case.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

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