#260 Fix grammar build process

release_3.3
closed
None
5
2012-10-10
2003-10-21
Dale King
No

The build process of the grammar has an additional step
after the grammars are compiled by antlr in the
com.puppycrawl.tools.checkstyle package. Because
TokenTypes in the com.puppycrawl.tools.checkstyle.api
package needs the token information generated, the
GeneratedJava14TokenTypes.java file is copied to the
api directory and the package name changed on the
way. This was introduced by patch 757846.

The stated reason for doing it this way instead of just
having TokenTypes get it directly from the
com.puppycrawl.tools.checkstyle package is to avoid a
circular package dependency.

This is a terrible hack and needs to be eliminated. For
one thing it prevents a simple build to be done in
Eclipse (without using Ant). And it can be done very
easily.

Here is a step-by-step approach that puts the grammar
files into their own package and that package is not
dependent on any other checkstyle package.

Currently the grammar files are dependent on only two
files from the Checkstyle distribution: DetailAST and
FileContents.

Step 1:
The dependency on DetailAST can be eliminated
entirely. The grammar does not need to know that it
working with a DetailAST. The only methods it invokes
are those specified by the AST interface. I will submit a
patch tonight that eliminates any reference to DetailAST.

Step 2:
The only dependency from the grammar to FileContents
is for reporting comments. So a CommentListener
interface should be created in a newly created
com.puppycrawl.tools.checkstyle.grammar package. I
do not like the way that the methods in FileContents are
language-specific, since I hope we can have grammars
for other languages as well. I propose that instead of
reportCComment and reportCppComment that instead
there should be these language-neutral methods:

void reportSingleLineComment( String type, int line, int
col );
void reportMultiLineComment( String type, int startLine,
int startCol, int endLine, int endCol );

For Java, the type would be "//" for single line comment
and "/" or possibly "/*" for multi-line comments. A
properties file parser would only use single line
comment with a type of "#".

Step 3:
Have FileContents implement this CommentListener
interface. It would check the comment type and forward
the request on to existing reportCComment and
reportCppComment.

Step 4:
Change grammar to use the CommentListener interface
instead of the concrete FileContents class. The property
would be changed from FileContents to
CommentListener including changing the getter and
setter. Comment reporting would change to the new
language-neutral methods in the CommentListener
interface.

Step 5:
Move java.g and java14.g to the
com.puppycrawl.tools.checkstyle.grammar package.
This requires making the grammar public instead of
having package access. Imports need to be added to
TreeWalker, build.xml needs to change to reflect it, and
probably some unit test changes.

Step 6:
Remove the copying of the Generated14TokenTypes to
the API package and add an import to TokenTypes for
this class in the grammar package.

Discussion

  • Dale King

    Dale King - 2003-10-22

    Patch to remove references to DetailAST from the grammar

     
  • Dale King

    Dale King - 2003-10-22

    Logged In: YES
    user_id=130378

    I am attaching a full patch that does everything EXCEPT
    physically moving java.g and java14.g files to the grammars
    subdirectory. There is no good way to express moving files in
    a diif (or in CVS for that matter).

     
  • Dale King

    Dale King - 2003-10-22

    Patch that does all the steps except moving grammar files

     
  • Oliver Burn

    Oliver Burn - 2003-10-22

    Logged In: YES
    user_id=218824

    Dale, thanks for taking the time to put together such a
    detailed analysis and patch.

    We are in a code freeze until we get the 3.2 release out the
    door. Then we will consider incorporating your patch.

     
  • Oleg Sukhodolsky

    Logged In: YES
    user_id=746148

    Committed to CVS for 3.3.
    Thank you for the patch.

     

Log in to post a comment.