From 86f345d7236013b9d8eb2deb39ee7acf0ec2c4ca Mon Sep 17 00:00:00 2001 From: paulk Date: Wed, 31 Aug 2016 23:41:29 +1000 Subject: [PATCH] GROOVY-7497: GroovyDoc reports only last method of a script (closes #405) --- .../SimpleGroovyClassDocAssembler.java | 91 +++++++++++++------ .../tools/groovydoc/SimpleGroovyDoc.java | 1 + .../classLevel/classDocName.html | 3 +- .../classLevel/classDocStructuredData.xml | 14 +++ .../tools/groovydoc/GroovyDocToolTest.java | 25 +++++ .../tools/groovydoc/testfiles/Script.groovy | 44 +++++++++ 6 files changed, 147 insertions(+), 31 deletions(-) create mode 100644 subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/testfiles/Script.groovy diff --git a/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyClassDocAssembler.java b/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyClassDocAssembler.java index 6b503e120dc..810e69484a0 100644 --- a/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyClassDocAssembler.java +++ b/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyClassDocAssembler.java @@ -244,20 +244,7 @@ public void visitMethodDef(GroovySourceAST t, int visit) { if (currentClassDoc == null) { // assume we have a script if ("true".equals(properties.getProperty("processScripts", "true"))) { - currentClassDoc = new SimpleGroovyClassDoc(importedClassesAndPackages, aliases, className, links); - currentClassDoc.setFullPathName(packagePath + FS + className); - currentClassDoc.setPublic(true); - currentClassDoc.setScript(true); - currentClassDoc.setGroovy(isGroovy); - currentClassDoc.setSuperClassName("groovy/lang/Script"); - if ("true".equals(properties.getProperty("includeMainForScripts", "true"))) { - currentClassDoc.add(createMainMethod(currentClassDoc)); - } - classDocs.put(currentClassDoc.getFullPathName(), currentClassDoc); - if (foundClasses == null) { - foundClasses = new HashMap(); - } - foundClasses.put(className, currentClassDoc); + currentClassDoc = getOrMakeScriptClassDoc(); } else { return; } @@ -267,6 +254,29 @@ public void visitMethodDef(GroovySourceAST t, int visit) { } } + private SimpleGroovyClassDoc getOrMakeScriptClassDoc() { + SimpleGroovyClassDoc currentClassDoc; + if (foundClasses != null && foundClasses.containsKey(className)) { + currentClassDoc = foundClasses.get(className); + } else { + currentClassDoc = new SimpleGroovyClassDoc(importedClassesAndPackages, aliases, className, links); + currentClassDoc.setFullPathName(packagePath + FS + className); + currentClassDoc.setPublic(true); + currentClassDoc.setScript(true); + currentClassDoc.setGroovy(isGroovy); + currentClassDoc.setSuperClassName("groovy/lang/Script"); + if ("true".equals(properties.getProperty("includeMainForScripts", "true"))) { + currentClassDoc.add(createMainMethod(currentClassDoc)); + } + classDocs.put(currentClassDoc.getFullPathName(), currentClassDoc); + if (foundClasses == null) { + foundClasses = new HashMap(); + } + foundClasses.put(className, currentClassDoc); + } + return currentClassDoc; + } + private void processPropertiesFromGetterSetter(SimpleGroovyMethodDoc currentMethodDoc) { String methodName = currentMethodDoc.name(); int len = methodName.length(); @@ -402,21 +412,41 @@ public void visitEnumConstantDef(GroovySourceAST t, int visit) { @Override public void visitVariableDef(GroovySourceAST t, int visit) { - if (visit == OPENING_VISIT && !insideAnonymousInnerClass() && isFieldDefinition()) { + if (visit == OPENING_VISIT && !insideAnonymousInnerClass()) { + boolean validField = true; SimpleGroovyClassDoc currentClassDoc = getCurrentClassDoc(); - if (currentClassDoc != null) { - String fieldName = getIdentFor(t); - currentFieldDoc = new SimpleGroovyFieldDoc(fieldName, currentClassDoc); - currentFieldDoc.setRawCommentText(getJavaDocCommentsBeforeNode(t)); - boolean isProp = processModifiers(t, currentFieldDoc); - currentFieldDoc.setType(new SimpleGroovyType(getTypeOrDefault(t))); - processAnnotations(t, currentFieldDoc); - if (isProp) { - currentClassDoc.addProperty(currentFieldDoc); + if (currentClassDoc == null) { + // assume we have a script (and it may have a @Field) + if ("true".equals(properties.getProperty("processScripts", "true"))) { + currentClassDoc = getOrMakeScriptClassDoc(); + validField = false; } else { - currentClassDoc.add(currentFieldDoc); + return; + } + } else if (!isFieldDefinition()) { + return; + } + String fieldName = getIdentFor(t); + if (fieldName.isEmpty()) return; // multi-assignment + currentFieldDoc = new SimpleGroovyFieldDoc(fieldName, currentClassDoc); + currentFieldDoc.setRawCommentText(getJavaDocCommentsBeforeNode(t)); + boolean isProp = processModifiers(t, currentFieldDoc); + currentFieldDoc.setType(new SimpleGroovyType(getTypeOrDefault(t))); + processAnnotations(t, currentFieldDoc); + if (!validField) { // look for @Field + for (GroovyAnnotationRef ref : currentFieldDoc.annotations()) { + if ("Field".equals(ref.name()) || "groovy/transform/Field".equals(ref.name())) { + validField = true; + break; + } } } + if (!validField) return; + if (isProp) { + currentClassDoc.addProperty(currentFieldDoc); + } else { + currentClassDoc.add(currentFieldDoc); + } } } @@ -488,10 +518,10 @@ private static void adjustForAutomaticEnumMethods(SimpleGroovyClassDoc currentCl } private String extractImportPath(GroovySourceAST t) { - return recurseDownImportBranch(getImportPathDotType(t)); + return recurseDownImportBranch(getPackageDotType(t)); } - private static GroovySourceAST getImportPathDotType(GroovySourceAST t) { + private static GroovySourceAST getPackageDotType(GroovySourceAST t) { GroovySourceAST child = t.childOfType(DOT); if (child == null) { child = t.childOfType(IDENT); @@ -517,14 +547,14 @@ private String recurseDownImportBranch(GroovySourceAST t) { } private void addAnnotationRef(SimpleGroovyProgramElementDoc node, GroovySourceAST t) { - GroovySourceAST classNode = t.childOfType(IDENT); + GroovySourceAST classNode = getPackageDotType(t); if (classNode != null) { node.addAnnotationRef(new SimpleGroovyAnnotationRef(extractName(classNode), getChildTextFromSource(t).trim())); } } private void addAnnotationRef(SimpleGroovyParameter node, GroovySourceAST t) { - GroovySourceAST classNode = t.childOfType(IDENT); + GroovySourceAST classNode = getPackageDotType(t); if (classNode != null) { node.addAnnotationRef(new SimpleGroovyAnnotationRef(extractName(classNode), getChildTextFromSource(t).trim())); } @@ -952,7 +982,8 @@ private SimpleGroovyClassDoc getCurrentClassDoc() { } private static String getIdentFor(GroovySourceAST gpn) { - return gpn.childOfType(IDENT).getText(); + GroovySourceAST ident = gpn.childOfType(IDENT); + return ident == null ? "" : ident.getText(); } private String getIdentPlusTypeArgsFor(GroovySourceAST gpn) { diff --git a/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyDoc.java b/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyDoc.java index 8a52fef83b5..6e9abd567fc 100644 --- a/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyDoc.java +++ b/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyDoc.java @@ -159,6 +159,7 @@ public String getTypeDescription() { if (isTrait()) return "Trait"; if (isAnnotationType()) return "Annotation Type"; if (isEnum()) return "Enum"; + if (isScript()) return "Script"; return "Class"; } diff --git a/subprojects/groovy-groovydoc/src/main/resources/org/codehaus/groovy/tools/groovydoc/gstringTemplates/classLevel/classDocName.html b/subprojects/groovy-groovydoc/src/main/resources/org/codehaus/groovy/tools/groovydoc/gstringTemplates/classLevel/classDocName.html index 01041b739d8..a4812c3a335 100644 --- a/subprojects/groovy-groovydoc/src/main/resources/org/codehaus/groovy/tools/groovydoc/gstringTemplates/classLevel/classDocName.html +++ b/subprojects/groovy-groovydoc/src/main/resources/org/codehaus/groovy/tools/groovydoc/gstringTemplates/classLevel/classDocName.html @@ -177,7 +177,8 @@
<% def pkg = classDoc.containingPackage().nameWithDots() -String classDesc = "${classDoc.isGroovy() ? "[Groovy]" : "[Java]"} ${classDoc.typeDescription} ${org.codehaus.groovy.tools.groovydoc.SimpleGroovyClassDoc.encodeAngleBrackets(classDoc.getNameWithTypeArgs())}" +def name = classDoc.getNameWithTypeArgs() ?: classDoc.name() +String classDesc = "${classDoc.isGroovy() ? "[Groovy]" : "[Java]"} ${classDoc.typeDescription} ${org.codehaus.groovy.tools.groovydoc.SimpleGroovyClassDoc.encodeAngleBrackets(name)}" if (pkg != "DefaultPackage") { %>
Package: ${pkg}
diff --git a/subprojects/groovy-groovydoc/src/main/resources/org/codehaus/groovy/tools/groovydoc/gstringTemplates/classLevel/classDocStructuredData.xml b/subprojects/groovy-groovydoc/src/main/resources/org/codehaus/groovy/tools/groovydoc/gstringTemplates/classLevel/classDocStructuredData.xml index 596d9a4c142..7fd237823bc 100644 --- a/subprojects/groovy-groovydoc/src/main/resources/org/codehaus/groovy/tools/groovydoc/gstringTemplates/classLevel/classDocStructuredData.xml +++ b/subprojects/groovy-groovydoc/src/main/resources/org/codehaus/groovy/tools/groovydoc/gstringTemplates/classLevel/classDocStructuredData.xml @@ -45,4 +45,18 @@ <% } %> + + <% for (property in classDoc.properties()) { %> + + ${property.commentText()} + +<% } %> + + + <% for (field in classDoc.fields()) { %> + + ${field.commentText()} + +<% } %> + diff --git a/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java b/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java index 2b6900bb395..9326c85cfad 100644 --- a/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java +++ b/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java @@ -646,6 +646,31 @@ public void testClassAliasing() throws Exception { assertEquals("There has to be a reference to class ArrayList", "ArrayList", m.group(2)); } + public void testScript() throws Exception { + List srcList = new ArrayList(); + srcList.add("org/codehaus/groovy/tools/groovydoc/testfiles/Script.groovy"); + xmlTool.add(srcList); + + MockOutputTool output = new MockOutputTool(); + xmlTool.renderToOutput(output, MOCK_DIR); + String scriptDoc = output.getText(MOCK_DIR + "/org/codehaus/groovy/tools/groovydoc/testfiles/Script.html"); + assertTrue("There should be a reference to method sayHello", containsTagWithName(scriptDoc, "method", "sayHello")); + assertTrue(scriptDoc, scriptDoc.contains("Use this to say Hello")); + + assertTrue("There should be a reference to method sayGoodbye", containsTagWithName(scriptDoc, "method", "sayGoodbye")); + assertTrue(scriptDoc, scriptDoc.contains("Use this to bid farewell")); + + assertTrue("There should be a reference to property instanceProp", containsTagWithName(scriptDoc, "property", "instanceProp")); + + assertTrue("There should be a reference to field staticField", containsTagWithName(scriptDoc, "field", "staticField")); + + assertFalse("Script local variables should not appear in groovydoc output", scriptDoc.contains("localVar")); + } + + private boolean containsTagWithName(String text, String tagname, String name) { + return text.matches("(?s).*<"+ tagname + "[^>]* name=\""+ name + "\".*"); + } + private GroovyClassDoc getGroovyClassDocByName(GroovyRootDoc root, String name) { GroovyClassDoc[] classes = root.classes(); diff --git a/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/testfiles/Script.groovy b/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/testfiles/Script.groovy new file mode 100644 index 00000000000..bca24cc4ee4 --- /dev/null +++ b/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/testfiles/Script.groovy @@ -0,0 +1,44 @@ +/* + * 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 + +import groovy.transform.Field + +@Field public static Integer staticField = 2 // should appear as a field +@groovy.transform.Field Integer instanceProp // a property (annotation with fqn ok) +def localVar // should not appear +def localVarWithInit = 3 // should not appear +def (foo, bar) = [1, 2] // should not appear + +/** + * Use this to say Hello + */ +void sayHello() { + println 'hello' +} + +/** + * Use this to bid farewell + */ +void sayGoodbye() { + println 'goodbye' +} + +sayHello() +sayGoodbye()