gapi document generation ignores access modifiers set on groovydoc

Yes, package and protected scopes are supported as options but currently ignored.

org.codehaus.groovy.ant.Groovydoc has a little weird way of getting the access modifier set - through private / protected / public / package properties.

It's weird because if someone sets private = true and public = false, it will be meaningless - unless if you change other properties when one changes.

Ant Javadoc task on the other hand provides something cleaner through just one property called "access" that can be set to protected/public/private/package - so no chance of conflicting access modifier from being set.

Not sure how freely we can change the weird but public API of org.codehaus.groovy.ant.Groovydoc!

Looks like Ant Javadoc supports 'Private/Public/Protected/Package' as well as 'access'. I am not sure about the case sensitivity of those attributes. I thought Ant was case insensitive in which case we have the former. I think it is worth changing to support both. We probably don't do any error checking at the moment. Again we should align with whatever the Java implementation does.

Yes, Ant Javadoc seems to be supporting 'Private/Public/Protected/Package' as well as 'access'. All it does is that it passes all the options, which are set to true, to the javadoc command, which then handles them.

It seems javadoc rejects if more than one modifier is passed, as:

javadoc: error - More than one of -public, -private, -package, or -protected specified.

That makes life simpler. We can do the same.

Handling of modifiers in groovydoc seems a little messed up.

Paul, any idea why Groovifier removes "public" modifier in its visitDefault() when it converts java AST to groovy AST?

It becomes a problem later on in modifiers handling during the documentation generation (in SimpleGroovyClassDocAssembler#processModifiers()) because you don't know whether "public" was never specified or cleared. It goes by AST node's type here, which Groovifier messes up when it clears "public".

Yes, that is mostly historical. The Groovifier is used both for GroovyDoc's purposes and for the Java2Groovy conversion program.
For the latter case, when converting a Java program into Groovy you normally want to get rid of 'public' for methods and classes as that is the default for Groovy and is only added noise. However, when treating Groovyified Java programs for GroovyDoc's purposes it is unwanted behavior. There now is an option to turn off that behavior.

It still needs lots more tidying up and refactoring but it mostly supports the access scopes now. The visibility checking needs to be pushed into the GroovyDoc classes not spread all over the templates. We also need to create an extra boolean in SimpleGroovyClassDoc to remember whether it is representing Java or Groovy. That would allow modifier visibility and representation to be fixed a little better still from what we have right now.

Thanks for getting back, Paul. That will help me move forward. I may have to come back again in case of doubts, because, unlike in regular code, documentation generation regression issues, if introduced, will not come out so easily.

So, I will introduce a flag on GroovyVerifier so that it knows whether it is coming from Java2Groovy or from groovydoc - so that it can do noise reduction for Java2Groovy and not come in the way for groovydoc.

I think isGroovy flag is generally in place - I will pass it on to SimpleGroovyClassDoc and related classes. That will surely be required when dealing with modifiers because absence of any modifier means package-private for Java classes and public for groovy classes.

You might want to check my recent commit. It has some of this in place already. The isGroovy change you are suggesting to SimpleGroovyClassDoc is the next logical thing to look at. I probably won't get time again for a few days and will be very happy for someone else to help with the necessary refactoring.

Oh, you have made some changes already! I didn't get that earlier. I will look at something else then

I wanted to check one more thing. The grammar merge that Guillaume is working on - that can also have impact on the groovydoc generation, right? If yes, I am not sure whether that part is being tested in the grammar merging.

Sure. I will try taking the scope related changes forward.

The grammar changes will require quite some work so we can integrate that in Groovy, so it's not for today.

Yes, minimal error handling of the strange cases is in place, e.g. multiple settings. Plus the access scope is mostly respected - so the recently discussed classes like NodeChildren will be gone.

There are some things which perhaps need further discussion. For Groovy classes I suspect people will be expecting 'public' to be missing. At the moment we still leave public on at the top most class level (perhaps we should turn that off - e.g. look at CliBuilder) but off for e.g. methods. Currently Java is done the same way but perhaps people expect the methods to be public as per normal Java conventions (e.g. look at a method in Expando). The isGroovy flag will help with this (but we also need a reference between members and the defining class - there is an incomplete hook for that already).

Just on overall design, I think there are two approaches we can take as we evolve Groovydoc further. One path we can take is to make the Java follow Groovy conventions. After all, why should I know or even care about what the base language is and won't getting rid of some of the noise assist making the Java easier to read? Perhaps, but it gives us no way to distinguish Java specific things like package private.

So, the other approach is to treat the two languages differently, which is mostly what we do now. For this to really work better, I think we need some more visual clues so that I can tell whether I am in Groovy and have something public or in Java and have package private. At the moment, it would be hard to tell. So, I think we need to work on that too. Perhaps some refinement of the CSS colors or perhaps a Java vs Groovy icon or some other indication on the page itself.

Also, on the grammar changes. They will mostly improve the ability to process the content inside comments. E.g. extracting the @author tags easily rather than regex scrapping the information and keeping such info in its own data structure - helpful for smart @link's etc. The other processing, e.g. modifiers, method processing etc. won't be affected so much. There was also discussion about a doclet approach but that is also a long way off at the moment.

The isGroovy flag is now in SimpleGroovyClassDoc and there is a visibility indicator [Groovy] or [Java] before each class declaration in the GroovyDoc. This will enable us to start correcting want the temples do, e.g. no 'public' in front of a normal Groovy class but retain for a Java class. And so forth for methods etc.