From abff1a2489ea8af10e1bc0a335551262d22f44e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20K=C3=BChne?= Date: Tue, 19 Nov 2002 20:21:46 +0000 Subject: [PATCH] Made TreeWalker a FileSetCheck and moved the processing logic from Checker to TreeWalker. Checker is now only concerned with managing FileSetChecks, although the setup is still hardcoded in the constructor, c.f. TODO comments in Checker constructor. Uncommenting the addFileSetCheck calls for real is left until we have better control our configuration. --- .../puppycrawl/tools/checkstyle/Checker.java | 112 ++++++------------ .../tools/checkstyle/TreeWalker.java | 88 +++++++++++++- .../tools/checkstyle/TreeWalkerTest.java | 5 +- 3 files changed, 125 insertions(+), 80 deletions(-) diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Checker.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Checker.java index 50211de63..25c2ffa1b 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Checker.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Checker.java @@ -19,20 +19,15 @@ package com.puppycrawl.tools.checkstyle; import java.io.File; -import java.io.FileNotFoundException; -import java.io.IOException; import java.util.ArrayList; import java.util.Iterator; import java.util.Locale; -import antlr.RecognitionException; -import antlr.TokenStreamException; -import com.puppycrawl.tools.checkstyle.api.DetailAST; -import com.puppycrawl.tools.checkstyle.api.FileContents; import com.puppycrawl.tools.checkstyle.api.LocalizedMessage; -import com.puppycrawl.tools.checkstyle.api.LocalizedMessages; -import com.puppycrawl.tools.checkstyle.api.Utils; import com.puppycrawl.tools.checkstyle.api.MessageDispatcher; +import com.puppycrawl.tools.checkstyle.api.FileSetCheck; +import com.puppycrawl.tools.checkstyle.checks.TranslationCheck; +import com.puppycrawl.tools.checkstyle.checks.PackageHtmlCheck; /** * This class provides the functionality to check a set of files. @@ -99,17 +94,10 @@ public class Checker /** configuration */ private final GlobalProperties mConfig; - /** cache file **/ - private final PropertyCacheFile mCache; - /** vector of listeners */ private final ArrayList mListeners = new ArrayList(); - /** used to collect messages TODO: delete */ - private final LocalizedMessages mMessages = new LocalizedMessages(); - - /** used to walk an AST and notify the checks */ - private final TreeWalker mWalker; + private final ArrayList mFileSetChecks = new ArrayList(); /** * Creates a new Checker instance. @@ -134,10 +122,27 @@ public class Checker throws CheckstyleException { mConfig = aConfig; - mCache = new PropertyCacheFile(aConfig); LocalizedMessage.setLocale(new Locale(mConfig.getLocaleLanguage(), mConfig.getLocaleCountry())); - mWalker = new TreeWalker(mMessages, mConfig.getTabWidth()); + + // TODO: create, configure and register the FileSetChecks from config + // file instead of hardcoding it here in the Checker constructor. + // Probably the addFileSetCheck mthod must be called from outside + // the checker, just like the TreeWalker is not concerned with + // finding all the checks it has to execute (IOC principle). + + // TODO: uncommenting the addFileSetCheck calls breaks the tests + // because the packageHtml check is always executed and yields + // additional errors that are not expected in the current test code + // (which should stay like it currently is!) + + FileSetCheck translationCheck = new TranslationCheck(); + // addFileSetCheck(translationCheck); + + FileSetCheck packageHtmlCheck = new PackageHtmlCheck(); + // addFileSetCheck(packageHtmlCheck); + + TreeWalker mWalker = new TreeWalker(mConfig); // TODO: improve the error handing for (int i = 0; i < aConfigs.length; i++) { final CheckConfiguration config = aConfigs[i]; @@ -147,13 +152,24 @@ public class Checker config.createInstance(this.getClass().getClassLoader()), config); } + addFileSetCheck(mWalker); + this.addListener(mCounter); } + /** + * Adds a FileSetCheck to the list of FileSetChecks that is executed in process(). + * @param aFileSetCheck the additional FileSetCheck + */ + public void addFileSetCheck(FileSetCheck aFileSetCheck) + { + aFileSetCheck.setMessageDispatcher(this); + mFileSetChecks.add(aFileSetCheck); + } + /** Cleans up the object **/ public void destroy() { - mCache.destroy(); mListeners.clear(); } @@ -167,7 +183,7 @@ public class Checker } /** - * Processes a set of files. + * Processes a set of files with all FileSetChecks. * Once this is done, it is highly recommended to call for * the destroy method to close and remove the listeners. * @param aFiles the list of files to be audited. @@ -177,65 +193,15 @@ public class Checker public int process(File[] aFiles) { fireAuditStarted(); - for (int i = 0; i < aFiles.length; i++) { - process(aFiles[i]); + for (int i = 0; i < mFileSetChecks.size(); i++) { + FileSetCheck fileSetCheck = (FileSetCheck) mFileSetChecks.get(i); + fileSetCheck.process(aFiles); } int errorCount = mCounter.getCount(); fireAuditFinished(); return errorCount; } - /** - * Processes a specified file and reports all errors found. - * @param aFile the file to process - **/ - private void process(File aFile) - { - // check if already checked and passed the file - final String fileName = aFile.getPath(); - final long timestamp = aFile.lastModified(); - if (mCache.alreadyChecked(fileName, timestamp)) { - return; - } - - mMessages.reset(); - try { - fireFileStarted(fileName); - final String[] lines = Utils.getLines(fileName); - final FileContents contents = new FileContents(fileName, lines); - final DetailAST rootAST = TreeWalker.parse(contents); - mWalker.walk(rootAST, contents, mConfig.getClassLoader()); - } - catch (FileNotFoundException fnfe) { - mMessages.add(new LocalizedMessage(0, Defn.CHECKSTYLE_BUNDLE, - "general.fileNotFound", null)); - } - catch (IOException ioe) { - mMessages.add(new LocalizedMessage( - 0, Defn.CHECKSTYLE_BUNDLE, - "general.exception", - new String[] {ioe.getMessage()})); - } - catch (RecognitionException re) { - mMessages.add(new LocalizedMessage(0, Defn.CHECKSTYLE_BUNDLE, - "general.exception", - new String[] {re.getMessage()})); - } - catch (TokenStreamException te) { - mMessages.add(new LocalizedMessage(0, Defn.CHECKSTYLE_BUNDLE, - "general.exception", - new String[] {te.getMessage()})); - } - - if (mMessages.size() == 0) { - mCache.checkedOk(fileName, timestamp); - } - else { - fireErrors(fileName, mMessages.getMessages()); - } - - fireFileFinished(fileName); - } /** * Create a stripped down version of a filename. diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/TreeWalker.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/TreeWalker.java index 2a69d9bf5..79d3a0078 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/TreeWalker.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/TreeWalker.java @@ -23,6 +23,10 @@ import com.puppycrawl.tools.checkstyle.api.DetailAST; import com.puppycrawl.tools.checkstyle.api.LocalizedMessages; import com.puppycrawl.tools.checkstyle.api.TokenTypes; import com.puppycrawl.tools.checkstyle.api.FileContents; +import com.puppycrawl.tools.checkstyle.api.Utils; +import com.puppycrawl.tools.checkstyle.api.LocalizedMessage; +import com.puppycrawl.tools.checkstyle.api.FileSetCheck; +import com.puppycrawl.tools.checkstyle.checks.AbstractFileSetCheck; import java.util.ArrayList; import java.util.HashMap; @@ -32,6 +36,9 @@ import java.util.Map; import java.util.Set; import java.util.Arrays; import java.io.Reader; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; import antlr.RecognitionException; import antlr.TokenStreamException; @@ -44,6 +51,7 @@ import antlr.TokenStreamException; * @version 1.0 */ public final class TreeWalker + extends AbstractFileSetCheck { /** * Overrides ANTLR error reporting so we completely control @@ -100,17 +108,73 @@ public final class TreeWalker private final LocalizedMessages mMessages; /** the tab width for error reporting */ private final int mTabWidth; + /** cache file **/ + private final PropertyCacheFile mCache; + private final GlobalProperties mConfig; /** * Creates a new TreeWalker instance. * - * @param aMessages used to collect messages - * @param aTabWidth the tabwidth to use + * @param aConfig the configuration to use */ - public TreeWalker(LocalizedMessages aMessages, int aTabWidth) + public TreeWalker(GlobalProperties aConfig) { - mMessages = aMessages; - mTabWidth = aTabWidth; + mMessages = new LocalizedMessages(); + mConfig = aConfig; + mTabWidth = aConfig.getTabWidth(); + mCache = new PropertyCacheFile(aConfig); + } + + /** + * Processes a specified file and reports all errors found. + * @param aFile the file to process + **/ + private void process(File aFile) + { + // check if already checked and passed the file + final String fileName = aFile.getPath(); + final long timestamp = aFile.lastModified(); + if (mCache.alreadyChecked(fileName, timestamp)) { + return; + } + + mMessages.reset(); + try { + getMessageDispatcher().fireFileStarted(fileName); + final String[] lines = Utils.getLines(fileName); + final FileContents contents = new FileContents(fileName, lines); + final DetailAST rootAST = TreeWalker.parse(contents); + walk(rootAST, contents, mConfig.getClassLoader()); + } + catch (FileNotFoundException fnfe) { + mMessages.add(new LocalizedMessage(0, Defn.CHECKSTYLE_BUNDLE, + "general.fileNotFound", null)); + } + catch (IOException ioe) { + mMessages.add(new LocalizedMessage( + 0, Defn.CHECKSTYLE_BUNDLE, + "general.exception", + new String[] {ioe.getMessage()})); + } + catch (RecognitionException re) { + mMessages.add(new LocalizedMessage(0, Defn.CHECKSTYLE_BUNDLE, + "general.exception", + new String[] {re.getMessage()})); + } + catch (TokenStreamException te) { + mMessages.add(new LocalizedMessage(0, Defn.CHECKSTYLE_BUNDLE, + "general.exception", + new String[] {te.getMessage()})); + } + + if (mMessages.size() == 0) { + mCache.checkedOk(fileName, timestamp); + } + else { + getMessageDispatcher().fireErrors(fileName, mMessages.getMessages()); + } + + getMessageDispatcher().fireFileFinished(fileName); } /** @@ -333,4 +397,18 @@ public final class TreeWalker } return rootAST; } + + /** @see FileSetCheck */ + public void process(File[] aFiles) + { + for (int i = 0; i < aFiles.length; i++) { + process(aFiles[i]); + } + } + + public void destroy() + { + super.destroy(); + mCache.destroy(); + } } diff --git a/src/tests/com/puppycrawl/tools/checkstyle/TreeWalkerTest.java b/src/tests/com/puppycrawl/tools/checkstyle/TreeWalkerTest.java index 9a86d9505..aa6db6240 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/TreeWalkerTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/TreeWalkerTest.java @@ -9,14 +9,15 @@ package com.puppycrawl.tools.checkstyle; import junit.framework.TestCase; -import com.puppycrawl.tools.checkstyle.api.LocalizedMessages; public class TreeWalkerTest extends TestCase { public void testCreate() { - new TreeWalker(new LocalizedMessages(), 8); + // TODO: This test does not make sense, what does it actually test? + GlobalProperties props = new GlobalProperties(); + new TreeWalker(props); assertTrue(true); } }