Module Extract File

[rnveach]

At the request of @romani ,
This is broken off from #19 (comment) to discuss producing and maintaining a physical file for regression-tool to use to determine what are modules, properties, etc.
This can also be used to discuss who should own this file creator, Checkstyle or regression-tool.

What items we need:

-- Module Information

Package
Class Name
Class hierarchy: Can contain multiple classes. If AbstractSuperCheck is modified, we need to determine that SuperCloneCheck and SuperFinalizeCheck need regression. Must contain full class path.
Interfaces: Same reason as class heirarchy.
Parent Module: Checker or TreeWalker
List of properties
List of messages

-- Property Information

Name
Type
Default Value
Acceptable Tokens (only for Java and Javadoc checks)

-- Message Information

Key name

Layout of XML file:

    <module>
        <package></package>
        <name></name>
        <parent></parent>
        <heirarchy>
            <class></class>
            <class></class>
        </heirarchy>
        <interfaces>
            <class></class>
            <class></class>
        </interfaces>
        <properties>
            <property>
                <name></name>
                <type></type>
                <acceptableTokens></acceptableTokens>
                <acceptableJavadocTokens></acceptableJavadocTokens>
                <default></default>
            </property>
        </properties>
        <messages>
            <message></message>
        </messages>
    </module>
</modules>

Example Check, FinalLocalVariableCheck: (http://checkstyle.sourceforge.net/config_coding.html#FinalLocalVariable)

<modules>
    <module>
        <package>com.puppycrawl.tools.checkstyle.checks.coding</package>
        <name>FinalLocalVariableCheck</name>
        <parent>TreeWalker</parent>
        <heirarchy>
            <class>com.puppycrawl.tools.checkstyle.api.AbstractCheck</class>
            <class>com.puppycrawl.tools.checkstyle.api.AbstractViolationReporter</class>
            <class>com.puppycrawl.tools.checkstyle.api.AutomaticBean</class>
        </heirarchy>
        <interfaces>
            <class>com.puppycrawl.tools.checkstyle.api.Configurable</class>
            <class>com.puppycrawl.tools.checkstyle.api.Contextualizable</class>
        </interfaces>
        <properties>
            <property>
                <name>validateEnhancedForLoopVariable</name>
                <type>boolean</type>
                <default>false</default>
            </property>
            <property>
                <name>tokens</name>
                <type>tokens</type>
                <acceptableTokens>VARIABLE_DEF, PARAMETER_DEF</acceptableTokens>
                <default>VARIABLE_DEF</default>
            </property>
        </properties>
        <messages>
            <message>final.variable</message>
        </messages>
    </module>
</modules>

@Luolc Let me know if you agree or think we need anything else.

[Luolc]

I haven't found out other information we need by now. LGTM.

I was wondering if we could use json instead of xml? I don't have much experience on xml serialization/deserialization in java, IMO, that is much harder than json serialization/deserialization. There could be simple one line serialization/deserialization of json by using some 3rd libraries(i.e. Google's Gson). I have searched on Google but fail to find such developer-friendly library for xml. :(

I wrote the serialization of config xml in previous PR: https://github.com/Luolc/regression-tool/blob/master/src/main/java/com/github/checkstyle/regression/generator/AbstractConfigGenerator.java, and thought it really ugly. :(

[rnveach]

I was wondering if we could use json instead of xml?

I'm not fully against it, it is just more common for non-web applications to pass XML instead of JSON. eclipse-cs and sonar plugins currently work fully with XML files.
Also, if we write this file generator in Checkstyle, we should try to avoid adding a new non-test dependency just for our small process.

[Luolc]

If there is a good XML serialization/deserialization library, I am fine with it. I am just not very familiar with XML before.

eclipse-cs and sonar plugins

Are they serialization/deserialization libraries? I would have a look. :)

[Luolc]

Also, if we write this file generator in Checkstyle, we should try to avoid adding a new non-test dependency just for our small process.

I have just created #33 , the hardcode info in that PR was created by running a UT in PackageObjectFactoryTest. I am not very familiar with maven lifecycle and don't know which step you expect to generate the module infos. But it seems that at least we have a workaround to do the generation in test scope so that we would only introduce test dependency to checkstyle.

[rnveach]

which step you expect to generate the module infos

If Checkstyle takes over the process, I would prefer it be a physical file in src/main and be controlled by tests to be required to be updated in the PR before it gets to us.
Having it as a test class, would still require us to run it and we would need to package/compile the PR before or during our tool.

[romani]

@rnveach ,

        <heirarchy>
            <class></class>
            <class></class>
        </heirarchy>
        <interfaces>
            <class></class>
            <class></class>
        </interfaces>

do we really need this info ?
non of plugins required such info , please see at current metada data requirements - links at post.
It is better to focus on cases/scenarios where we detect changes only final implementations of Checks.
may be some day in future when tool be smart to cover our most basic use cases, we could come to detection of changes in parent classes and testing all dependent.

I was wondering if we could use json instead of xml?

just a historical reason, such decisions were done, when XML was the only solution for that, and it was a standard.
while it is out of checkstyle main repo - we could do json, just for fun.
XML is better to use is we care:

• to make a schema on xml, to let users know structure when some thirdparty extension is going to be done, base on our solution.
• to allow keep comments inside a config (in json there are no comments), so yaml is probably better approach.
• no extra dependencies, but in java9 json parsing will be embedded, not sure about yaml . We can avoid extra dependency but as we place smth in json, other projects who need this file will have to have extra json dependency.

[rnveach]

Me and romani had some discussion offline.

do we really need this info ?

As discussed, regression needs this. None of the other 3rd party plugins do.
See my comment in first post:

If AbstractSuperCheck is modified, we need to determine that SuperCloneCheck and SuperFinalizeCheck need regression.

---

We have decided that this file will only be created and used by regression tool. We can do this however we see fit.

The main reason for not doing this in Checkstyle is we need to include descriptions in the extract file for modules and properties for 3rd parties. This would require parsing the Java files for javadocs which may be limited in getting descriptions from properties in abstract parents. The best solution is to use a combination of reflection and Java parsing. Regression tool will most likely do the reflection for it's use, and javadoc parsing may be implemented using https://groups.google.com/forum/?hl=en#!topic/checkstyle-devel/lsNF6Dj90lo . Once both areas are done, we will look into combining the 2.

@Luolc
I am going to close this and make a clean issue.