* Added NameValidator, to provide a reasonably flexible mechanism for

specifying/testing name validity.

* NameValidator is used by DefaultProjectBuilder for project and target
  names, and by DefaultTaskContext for Property names.

* Added ProjectException, thrown by ProjectBuilder.build().

* Tidy-up error messages for project building errors.

* Added a bunch of tests

Submitted by Darrell DeBoer [darrell@apache.org]


git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@271810 13f79535-47bb-0310-9956-ffa450edef68

proposal/myrmidon/etc/testcases/org/apache/antlib/core/property.ant

@@ -50,4 +50,16 @@
             <property-test-type value="value 3"/>
         </property>
     </target>
+
+    <!-- Test properties with invalid names -->
+    <target name="bad-prop-name1">
+        <property name="badname!" value="value"/>
+    </target>
+    <target name="bad-prop-name2">
+        <property name="bad name" value="value"/>
+    </target>
+    <target name="bad-prop-name3">
+        <property name="" value="value"/>
+    </target>
+
 </project>

proposal/myrmidon/etc/testcases/org/apache/myrmidon/components/builder/bad-project-name.ant

@@ -0,0 +1,4 @@
+<!-- Project with an invalid name -->
+<project version="2.0" name="!badname">
+    <target name="main"/>
+</project>

proposal/myrmidon/etc/testcases/org/apache/myrmidon/components/builder/bad-target-name.ant

@@ -0,0 +1,5 @@
+<!-- Target with an invalid name -->
+<project version="2.0" name="ok name">
+    <target name="main"/>
+    <target name="bad ^ name"/>
+</project>

proposal/myrmidon/src/java/org/apache/myrmidon/components/builder/DefaultProjectBuilder.java

@@ -8,6 +8,7 @@
 package org.apache.myrmidon.components.builder;
 
 import java.io.File;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -22,11 +23,13 @@ import org.apache.avalon.framework.configuration.Configuration;
 import org.apache.avalon.framework.configuration.ConfigurationException;
 import org.apache.avalon.framework.configuration.SAXConfigurationHandler;
 import org.apache.avalon.framework.logger.AbstractLogEnabled;
-import org.apache.myrmidon.framework.conditions.Condition;
 import org.apache.myrmidon.framework.conditions.AndCondition;
+import org.apache.myrmidon.framework.conditions.Condition;
 import org.apache.myrmidon.framework.conditions.IsSetCondition;
 import org.apache.myrmidon.framework.conditions.NotCondition;
 import org.apache.myrmidon.interfaces.builder.ProjectBuilder;
+import org.apache.myrmidon.interfaces.builder.ProjectException;
+import org.apache.myrmidon.interfaces.model.DefaultNameValidator;
 import org.apache.myrmidon.interfaces.model.Project;
 import org.apache.myrmidon.interfaces.model.Target;
 import org.apache.myrmidon.interfaces.model.TypeLib;
@@ -37,6 +40,7 @@ import org.xml.sax.XMLReader;
  *
  * @author <a href="mailto:peter@apache.org">Peter Donald</a>
  * @version $Revision$ $Date$
+ *
  * @ant:type type="project-builder" name="xml"
  * @ant:type type="project-builder" name="ant"
  */
@@ -54,36 +58,37 @@ public class DefaultProjectBuilder
     private final static int IMPLICIT_TASKS = 2;
     private final static int TARGETS = 3;
 
+    // Use a name validator with the default rules.
+    private DefaultNameValidator m_nameValidator = new DefaultNameValidator();
+
     /**
      * build a project from file.
      *
      * @param source the source
      * @return the constructed Project
-     * @exception Exception if an error occurs
+     * @exception ProjectException if an error occurs
      */
     public Project build( final String source )
-        throws Exception
+        throws ProjectException
     {
         final File file = new File( source );
         return build( file, new HashMap() );
     }
 
     private Project build( final File file, final HashMap projects )
-        throws Exception
+        throws ProjectException
     {
-        final URL systemID = file.toURL();
+        final URL systemID = extractURL( file );
         final Project result = (Project)projects.get( systemID.toString() );
         if( null != result )
         {
             return result;
         }
 
-        final SAXConfigurationHandler handler = new SAXConfigurationHandler();
-
-        process( systemID, handler );
-
-        final Configuration configuration = handler.getConfiguration();
+        // Parse the project file
+        final Configuration configuration = parseProject( systemID );
 
+        // Build the project model
         final DefaultProject project = buildProject( file, configuration );
 
         projects.put( systemID.toString(), project );
@@ -94,20 +99,33 @@ public class DefaultProjectBuilder
         return project;
     }
 
-    protected void process( final URL systemID,
-                            final SAXConfigurationHandler handler )
-        throws Exception
+    /**
+     * Parses the project.
+     */
+    private Configuration parseProject( final URL systemID )
+        throws ProjectException
     {
-        final SAXParserFactory saxParserFactory = SAXParserFactory.newInstance();
-        final SAXParser saxParser = saxParserFactory.newSAXParser();
-        final XMLReader parser = saxParser.getXMLReader();
-        parser.setFeature( "http://xml.org/sax/features/namespace-prefixes", false );
-        parser.setFeature( "http://xml.org/sax/features/namespaces", false );
-        //parser.setFeature( "http://xml.org/sax/features/validation", false );
-
-        parser.setContentHandler( handler );
-        parser.setErrorHandler( handler );
-        parser.parse( systemID.toString() );
+        try
+        {
+            final SAXConfigurationHandler handler = new SAXConfigurationHandler();
+            final SAXParserFactory saxParserFactory = SAXParserFactory.newInstance();
+            final SAXParser saxParser = saxParserFactory.newSAXParser();
+            final XMLReader parser = saxParser.getXMLReader();
+            parser.setFeature( "http://xml.org/sax/features/namespace-prefixes", false );
+            parser.setFeature( "http://xml.org/sax/features/namespaces", false );
+            //parser.setFeature( "http://xml.org/sax/features/validation", false );
+
+            parser.setContentHandler( handler );
+            parser.setErrorHandler( handler );
+            parser.parse( systemID.toString() );
+
+            return handler.getConfiguration();
+        }
+        catch( Exception e )
+        {
+            String message = REZ.getString( "ant.project-parse.error" );
+            throw new ProjectException( message, e );
+        }
     }
 
     /**
@@ -116,23 +134,20 @@ public class DefaultProjectBuilder
      * @param file the file from which configuration was loaded
      * @param configuration the configuration loaded
      * @return the created Project
-     * @exception Exception if an error occurs
-     * @exception Exception if an error occurs
-     * @exception ConfigurationException if an error occurs
+     * @exception ProjectException if an error occurs building the project
      */
     private DefaultProject buildProject( final File file,
                                          final Configuration configuration )
-        throws Exception
+        throws ProjectException
     {
         if( !configuration.getName().equals( "project" ) )
         {
             final String message = REZ.getString( "ant.no-project-element.error" );
-            throw new Exception( message );
+            throw new ProjectException( message );
         }
 
         //get project-level attributes
-        final String projectName = configuration.getAttribute( "name",
-                                                               FileUtil.removeExtension( file.getName() ) );
+        final String projectName = getProjectName( configuration, file );
         final String baseDirectoryName = configuration.getAttribute( "basedir", null );
         final String defaultTarget = configuration.getAttribute( "default", "main" );
         final Version version = getVersion( configuration );
@@ -141,7 +156,7 @@ public class DefaultProjectBuilder
         {
             final String message =
                 REZ.getString( "ant.bad-version.error", VERSION, version );
-            throw new Exception( message );
+            throw new ProjectException( message );
         }
 
         //determine base directory for project.  Use the directory containing
@@ -168,12 +183,52 @@ public class DefaultProjectBuilder
         return project;
     }
 
+    /**
+     * Get the project name from the configuration, or create a default name if none
+     * was supplied.
+     */
+    private String getProjectName( final Configuration configuration, final File file )
+        throws ProjectException
+    {
+        String projectName = configuration.getAttribute( "name", null );
+
+        if( projectName == null )
+        {
+            // Create a name based on the file name.
+            String fileNameBase = FileUtil.removeExtension( file.getName() );
+            try
+            {
+                projectName = m_nameValidator.makeValidName( fileNameBase );
+            }
+            catch( Exception e )
+            {
+                String message = REZ.getString( "ant.project-create-name.error" );
+                throw new ProjectException( message, e );
+            }
+        }
+        else
+        {
+            // Make sure the supplied name is valid.
+            try
+            {
+                m_nameValidator.validate( projectName );
+            }
+            catch( Exception e )
+            {
+                String message = REZ.getString( "ant.project-bad-name.error" );
+                throw new ProjectException( message, e );
+            }
+        }
+        return projectName;
+
+    }
+
     /**
      * Retrieve the version attribute from the specified configuration element.
      * Throw exceptions with meaningful errors if malformed or missing.
      */
     private Version getVersion( final Configuration configuration )
-        throws Exception
+        throws ProjectException
     {
         try
         {
@@ -183,7 +238,7 @@ public class DefaultProjectBuilder
         catch( final ConfigurationException ce )
         {
             final String message = REZ.getString( "ant.version-missing.error" );
-            throw new ConfigurationException( message, ce );
+            throw new ProjectException( message, ce );
         }
     }
 
@@ -191,7 +246,7 @@ public class DefaultProjectBuilder
      * Utility function to extract version
      */
     private Version parseVersion( final String versionString )
-        throws Exception
+        throws ProjectException
     {
 
         try
@@ -202,8 +257,7 @@ public class DefaultProjectBuilder
         {
             final String message =
                 REZ.getString( "ant.malformed.version", versionString );
-            getLogger().warn( message );
-            throw new ConfigurationException( message, e );
+            throw new ProjectException( message, e );
         }
     }
 
@@ -212,12 +266,12 @@ public class DefaultProjectBuilder
      *
      * @param project the project
      * @param configuration the Configuration
-     * @exception Exception if an error occurs
+     * @exception ProjectException if an error occurs
      */
     private void buildTopLevelProject( final DefaultProject project,
                                        final Configuration configuration,
                                        final HashMap projects )
-        throws Exception
+        throws ProjectException
     {
         final ArrayList implicitTaskList = new ArrayList();
         final Configuration[] children = configuration.getChildren();
@@ -277,7 +331,7 @@ public class DefaultProjectBuilder
             {
                 final String message =
                     REZ.getString( "ant.unknown-toplevel-element.error", name, element.getLocation() );
-                throw new Exception( message );
+                throw new ProjectException( message );
             }
         }
 
@@ -291,7 +345,7 @@ public class DefaultProjectBuilder
     private void buildProjectRef( final DefaultProject project,
                                   final Configuration element,
                                   final HashMap projects )
-        throws Exception
+        throws ProjectException
     {
         final String name = element.getAttribute( "name", null );
         final String location = element.getAttribute( "location", null );
@@ -300,27 +354,31 @@ public class DefaultProjectBuilder
         {
             final String message =
                 REZ.getString( "ant.projectref-no-name.error", element.getLocation() );
-            throw new Exception( message );
+            throw new ProjectException( message );
         }
 
-        if( !validName( name ) )
+        try
+        {
+            m_nameValidator.validate( name );
+        }
+        catch( Exception e )
         {
             final String message =
                 REZ.getString( "ant.projectref-bad-name.error", element.getLocation() );
-            throw new Exception( message );
+            throw new ProjectException( message, e );
         }
 
         if( null == location )
         {
             final String message =
                 REZ.getString( "ant.projectref-no-location.error", element.getLocation() );
-            throw new Exception( message );
+            throw new ProjectException( message );
         }
 
         // Build the URL of the referenced projects
         final File baseDirectory = project.getBaseDirectory();
         final File file = FileUtil.resolveFile( baseDirectory, location );
-        final String systemID = file.toURL().toString();
+        final String systemID = extractURL( file ).toString();
 
         // Locate the referenced project, building it if necessary
         Project other = (Project)projects.get( systemID );
@@ -333,9 +391,22 @@ public class DefaultProjectBuilder
         project.addProject( name, other );
     }
 
+    private URL extractURL( final File file ) throws ProjectException
+    {
+        try
+        {
+            return file.toURL();
+        }
+        catch( MalformedURLException e )
+        {
+            final String message = REZ.getString( "ant.project-unexpected.error" );
+            throw new ProjectException( message, e );
+        }
+    }
+
     private void buildTypeLib( final DefaultProject project,
                                final Configuration element )
-        throws Exception
+        throws ProjectException
     {
         final String library = element.getAttribute( "library", null );
         final String name = element.getAttribute( "name", null );
@@ -345,7 +416,7 @@ public class DefaultProjectBuilder
         {
             final String message =
                 REZ.getString( "ant.import-no-library.error", element.getLocation() );
-            throw new Exception( message );
+            throw new ProjectException( message );
         }
 
         if( null == name || null == type )
@@ -354,7 +425,7 @@ public class DefaultProjectBuilder
             {
                 final String message =
                     REZ.getString( "ant.import-malformed.error", element.getLocation() );
-                throw new Exception( message );
+                throw new ProjectException( message );
             }
         }
 
@@ -368,14 +439,14 @@ public class DefaultProjectBuilder
      * @param target the Configuration
      */
     private void buildTarget( final DefaultProject project, final Configuration target )
-        throws Exception
+        throws ProjectException
     {
         final String name = target.getAttribute( "name", null );
         final String depends = target.getAttribute( "depends", null );
         final String ifCondition = target.getAttribute( "if", null );
         final String unlessCondition = target.getAttribute( "unless", null );
 
-        verifyName( name, target );
+        verifyTargetName( name, target );
 
         if( getLogger().isDebugEnabled() )
         {
@@ -392,24 +463,30 @@ public class DefaultProjectBuilder
         project.addTarget( name, defaultTarget );
     }
 
-    private void verifyName( final String name, final Configuration target ) throws Exception
+    private void verifyTargetName( final String name, final Configuration target )
+        throws ProjectException
     {
         if( null == name )
         {
             final String message =
                 REZ.getString( "ant.target-noname.error", target.getLocation() );
-            throw new Exception( message );
+            throw new ProjectException( message );
         }
 
-        if( !validName( name ) )
+        try
+        {
+            m_nameValidator.validate( name );
+        }
+        catch( Exception e )
         {
             final String message =
                 REZ.getString( "ant.target-bad-name.error", target.getLocation() );
-            throw new Exception( message );
+            throw new ProjectException( message, e );
         }
     }
 
-    private String[] buildDependsList( final String depends, final Configuration target ) throws Exception
+    private String[] buildDependsList( final String depends, final Configuration target )
+        throws ProjectException
     {
         String[] dependencies = null;
 
@@ -428,7 +505,7 @@ public class DefaultProjectBuilder
                     final String message = REZ.getString( "ant.target-bad-dependency.error",
                                                           target.getName(),
                                                           target.getLocation() );
-                    throw new Exception( message );
+                    throw new ProjectException( message );
                 }
 
                 if( getLogger().isDebugEnabled() )
@@ -447,7 +524,6 @@ public class DefaultProjectBuilder
 
     private Condition buildCondition( final String ifCondition,
                                       final String unlessCondition )
-        throws Exception
     {
         final AndCondition condition = new AndCondition();
 
@@ -475,16 +551,4 @@ public class DefaultProjectBuilder
 
         return condition;
     }
-
-    protected boolean validName( final String name )
-    {
-        if( -1 != name.indexOf( "->" ) )
-        {
-            return false;
-        }
-        else
-        {
-            return true;
-        }
-    }
 }

proposal/myrmidon/src/java/org/apache/myrmidon/components/builder/Resources.properties

@@ -10,10 +10,14 @@ ati.attribue-unquoted.error=Expecting the value of attribute {0} to be enclosed
 ant.project-banner.notice=Project {0} base directory: {1}.
 ant.target-parse.notice=Parsing target: {0}.
 ant.target-if.notice=Target if condition: {0}
-ant.target- unless.notice=Target unless condition: {0}
+ant.target-unless.notice=Target unless condition: {0}
 ant.target-dependency.notice=Target dependency: {0}
+ant.project-unexpected.error=Unexpected error building project.
+ant.project-parse.error=Error parsing project.
 ant.no-project-element.error=Project file must be enclosed in project element.
 ant.unknown-toplevel-element.error=Unknown top-level element {0} at {1}.
+ant.project-bad-name.error=Invalid project name.
+ant.project-create-name.error=Could not create a name for this project.
 ant.projectref-no-name.error=Malformed projectref without a name attribute at {0}.
 ant.projectref-bad-name.error=Projectref with an invalid name attribute at {0}.
 ant.projectref-no-location.error=Malformed projectref without a location attribute at {0}.

proposal/myrmidon/src/java/org/apache/myrmidon/components/workspace/DefaultTaskContext.java

@@ -20,6 +20,7 @@ import org.apache.avalon.framework.service.ServiceException;
 import org.apache.avalon.framework.service.ServiceManager;
 import org.apache.myrmidon.api.TaskContext;
 import org.apache.myrmidon.api.TaskException;
+import org.apache.myrmidon.interfaces.model.DefaultNameValidator;
 import org.apache.myrmidon.interfaces.workspace.PropertyResolver;
 
 /**
@@ -34,6 +35,12 @@ public class DefaultTaskContext
     private final static Resources REZ =
         ResourceManager.getPackageResources( DefaultTaskContext.class );
 
+    // Property name validator allows digits, but no internal whitespace.
+    private static DefaultNameValidator m_propertyNameValidator = new DefaultNameValidator();
+    static {
+        m_propertyNameValidator.setAllowInternalWhitespace( false );
+    }
+
     private final Map m_contextData = new Hashtable();
     private final TaskContext m_parent;
     private ServiceManager m_serviceManager;
@@ -199,6 +206,7 @@ public class DefaultTaskContext
     public void setProperty( final String name, final Object value )
         throws TaskException
     {
+        checkPropertyName( name );
         checkPropertyValid( name, value );
         m_contextData.put( name, value );
     }
@@ -362,6 +370,22 @@ public class DefaultTaskContext
         return value;
     }
 
+    /**
+     * Checks that the supplied property name is valid.
+     */
+    private void checkPropertyName( final String name ) throws TaskException
+    {
+        try
+        {
+            m_propertyNameValidator.validate( name );
+        }
+        catch( Exception e )
+        {
+            String message = REZ.getString( "bad-property-name.error" );
+            throw new TaskException( message, e );
+        }
+    }
+
     /**
      * Make sure property is valid if it is one of the "magic" properties.
      *

proposal/myrmidon/src/java/org/apache/myrmidon/components/workspace/Resources.properties

@@ -13,6 +13,7 @@ exec-task.notice=Executing task {0}.
 #DefaultTaskContext
 unknown-prop.error=Unknown property {0}.
 bad-property.error=Property {0} must have a value of type {1}.
+bad-property-name.error=Invalid property name.
 null-resolved-value.error=Value "{0}" resolved to null.
 bad-resolve.error=Unable to resolve value "{0}".
 bad-find-service.error=Could not find service "{0}".

proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/builder/ProjectBuilder.java

@@ -25,8 +25,8 @@ public interface ProjectBuilder
      *
      * @param source the source
      * @return the constructed Project
-     * @exception Exception if an error occurs
+     * @exception ProjectException if an error occurs
      */
     Project build( String source )
-        throws Exception;
+        throws ProjectException;
 }

proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/builder/ProjectException.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) The Apache Software Foundation. All rights reserved.
+ *
+ * This software is published under the terms of the Apache Software License
+ * version 1.1, a copy of which has been included with this distribution in
+ * the LICENSE.txt file.
+ */
+package org.apache.myrmidon.interfaces.builder;
+
+/**
+ * A cascading exception thrown on a problem constructing a Project model.
+ *
+ * @author <a href="mailto:darrell@apache.org">Darrell DeBoer</a>
+ * @version $Revision$ $Date$
+ */
+public class ProjectException
+    extends Exception
+{
+    /**
+     * If this exception is cascaded, the cause of this exception.
+     */
+    private final Throwable m_throwable;
+
+    /**
+     * Constructs an non-cascaded exception with a message
+     *
+     * @param message the message
+     */
+    public ProjectException( final String message )
+    {
+        this( message, null );
+    }
+
+    /**
+     * Constructs a cascaded exception with the supplied message, which links the
+     * Throwable provided.
+     *
+     * @param message the message
+     * @param throwable the throwable that caused this exception
+     */
+    public ProjectException( final String message, final Throwable throwable )
+    {
+        super( message );
+        m_throwable = throwable;
+    }
+
+    /**
+     * Retrieve root cause of the exception.
+     *
+     * @return the root cause
+     */
+    public final Throwable getCause()
+    {
+        return m_throwable;
+    }
+}

proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/DefaultNameValidator.java

@@ -0,0 +1,356 @@
+/*
+ * Copyright (C) The Apache Software Foundation. All rights reserved.
+ *
+ * This software is published under the terms of the Apache Software License
+ * version 1.1, a copy of which has been included  with this distribution in
+ * the LICENSE.txt file.
+ */
+package org.apache.myrmidon.interfaces.model;
+
+import org.apache.avalon.excalibur.i18n.ResourceManager;
+import org.apache.avalon.excalibur.i18n.Resources;
+
+/**
+ * Simple helper class which determines the validity of names used
+ * in ant projects.
+ *
+ * @author <a href="mailto:darrell@apache.org">Darrell DeBoer</a>
+ * @version $Revision$ $Date$
+ */
+public class DefaultNameValidator
+    implements NameValidator
+{
+    private final static Resources REZ =
+        ResourceManager.getPackageResources( DefaultNameValidator.class );
+
+    /**
+     * Determines whether the supplied name may include surrounding whitespace.
+     */
+    private boolean m_allowSurroundingWhitespace;
+
+    // Settings for initial characters.
+    private boolean m_allowInitialDigit;
+    private String m_additionalInitialCharacters;
+
+    // Settings for internal characters.
+    private boolean m_allowInternalDigits;
+    private boolean m_allowInternalWhitespace;
+    private String m_additionalInternalCharacters;
+
+    /**
+     * Construct a default name validator.
+     * Letters, digits and "_" are permitted as initial character.
+     * Letters, digits, whitespace and "_-." are permitted as internal characters.
+     * Surrounding whitespace is not permitted.
+     */
+    public DefaultNameValidator()
+    {
+        this( false, true, "_", true, true, "_-." );
+    }
+
+    /**
+     * Contstruct a NameValidator with the specified rules.
+     * @param allowSurroundingWhitespace
+     *          specified if names are trimmed before checking
+     * @param allowInitialDigit
+     *          specifies if digits are permitted as intial characters
+     * @param additionalInitialCharacters
+     *          extra characters to allow as initial characters.
+     * @param allowInternalDigits
+     *          specifies if digits are permitted as internal characters
+     * @param allowInternalWhitespace
+     *          specifies if whitespace is permitted internally in names
+     * @param additionalInternalCharacters
+     *          extra characters permitted in names
+     */
+    public DefaultNameValidator( final boolean allowSurroundingWhitespace,
+                                 final boolean allowInitialDigit,
+                                 final String additionalInitialCharacters,
+                                 final boolean allowInternalDigits,
+                                 final boolean allowInternalWhitespace,
+                                 final String additionalInternalCharacters )
+    {
+        setAllowSurroundingWhitespace( allowSurroundingWhitespace );
+        setAllowInitialDigit( allowInitialDigit );
+        setAdditionalInitialCharacters( additionalInitialCharacters );
+        setAllowInternalDigits( allowInternalDigits );
+        setAllowInternalWhitespace( allowInternalWhitespace );
+        setAdditionalInternalCharacters( additionalInternalCharacters );
+    }
+
+    /**
+     * Creates a valid name based on the supplied string value, removing invalid
+     * characters. If no valid characters are present, an exception is thrown.
+     * @param baseName the name used to construct the valid name
+     * @throws Exception if no valid name could be constructed.
+     */
+    public String makeValidName( final String baseName ) throws Exception
+    {
+        final StringBuffer buffer = new StringBuffer( baseName );
+        while( buffer.length() > 0 && !isValidInitialChar( buffer.charAt( 0 ) ) )
+        {
+            buffer.delete( 0, 1 );
+        }
+        if( buffer.length() == 0 )
+        {
+            final String message = REZ.getString( "name.could-not-create.error", baseName );
+            throw new Exception( message );
+        }
+
+        for( int i = 1; i < buffer.length(); )
+        {
+            if( !isValidInternalChar( buffer.charAt( i ) ) )
+            {
+                buffer.delete( i, i + 1 );
+            }
+            else
+            {
+                i++;
+            }
+        }
+
+        return buffer.toString();
+    }
+
+    /**
+     * Validates the supplied name, failing if it is not.
+     * @throws Exception is the supplied name is not valid.
+     */
+    public void validate( final String name ) throws Exception
+    {
+        String testName = name;
+
+        // If surrounding whitespace is allowed, trim it. Otherwise, check.
+        if( m_allowSurroundingWhitespace )
+        {
+            testName = testName.trim();
+        }
+        else
+        {
+            checkSurroundingWhitespace( testName );
+        }
+
+        // Zero-length name is invalid.
+        if( testName.length() == 0 )
+        {
+            final String message = REZ.getString( "name.zero-char-name.error" );
+            throw new Exception( message );
+        }
+
+        // Check first character.
+        final char initial = testName.charAt( 0 );
+        checkInitialCharacter( initial, testName );
+
+        // Check the rest of the characters.
+        for( int i = 1; i < testName.length(); i++ )
+        {
+            final char internal = testName.charAt( i );
+            checkInternalCharacter( internal, testName );
+        }
+    }
+
+    /**
+     * Checks if the supplied character is permitted as an internal character.
+     * @throws Exception if the character is not permitted
+     */
+    private void checkInternalCharacter( final char internal, final String name )
+        throws Exception
+    {
+        if( !isValidInternalChar( internal ) )
+        {
+            final String message = REZ.getString( "name.invalid-internal-char.error",
+                                                  name,
+                                                  describeValidInternalChars() );
+            throw new Exception( message );
+        }
+    }
+
+    /**
+     * Checks if the supplied character is permitted as an internal character.
+     * @throws Exception if the character is not permitted
+     */
+    private void checkInitialCharacter( final char initial, final String name )
+        throws Exception
+    {
+        if( !isValidInitialChar( initial ) )
+        {
+            final String message = REZ.getString( "name.invalid-initial-char.error",
+                                                  name,
+                                                  describeValidInitialChars() );
+            throw new Exception( message );
+        }
+    }
+
+    /**
+     * Checks the name for surrounding whitespace
+     * @throws Exception if surrounding whitespace is found
+     */
+    private void checkSurroundingWhitespace( final String testName )
+        throws Exception
+    {
+        if( testName.length() == 0 )
+        {
+            return;
+        }
+
+        if( Character.isWhitespace( testName.charAt( 0 ) ) ||
+            Character.isWhitespace( testName.charAt( testName.length() - 1 ) ) )
+        {
+            final String message =
+                REZ.getString( "name.enclosing-whitespace.error", testName );
+            throw new Exception( message );
+        }
+    }
+
+    /**
+     * Determines if a character is allowed as the first character in a name.
+     * Valid characters are Letters, Digits, and defined initial characters ("_").
+     * @param chr the character to be assessed
+     * @return <code>true</code> if the character can be the first character of a name
+     */
+    protected boolean isValidInitialChar( final char chr )
+    {
+        if( Character.isLetter( chr ) )
+        {
+            return true;
+        }
+
+        if( m_allowInitialDigit
+            && Character.isDigit( chr ) )
+        {
+            return true;
+        }
+
+        if( m_additionalInitialCharacters.indexOf( chr ) != -1 )
+        {
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * Determines if a character is allowed as a non-initial character in a name.
+     * Valid characters are Letters, Digits, whitespace, and defined
+     * internal characters ("_-.").
+     * @param chr the character to be assessed
+     * @return <code>true</code> if the character can be included in a name
+     */
+    protected boolean isValidInternalChar( final char chr )
+    {
+        if( Character.isLetter( chr ) )
+        {
+            return true;
+        }
+
+        if( m_allowInternalDigits
+            && Character.isDigit( chr ) )
+        {
+            return true;
+        }
+
+        if( m_allowInternalWhitespace
+            && Character.isWhitespace( chr ) )
+        {
+            return true;
+        }
+
+        if( m_additionalInternalCharacters.indexOf( chr ) != -1 )
+        {
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * Builds a message detailing the valid initial characters.
+     */
+    protected String describeValidInitialChars()
+    {
+        StringBuffer validChars = new StringBuffer( "letters" );
+        if( m_allowInitialDigit )
+        {
+            validChars.append( ", digits" );
+        }
+        validChars.append( ", and \"" );
+        validChars.append( m_additionalInitialCharacters );
+        validChars.append( "\"" );
+        return validChars.toString();
+    }
+
+    /**
+     * Builds a message detailing the valid internal characters.
+     */
+    protected String describeValidInternalChars()
+    {
+        StringBuffer validChars = new StringBuffer( "letters" );
+        if( m_allowInternalDigits )
+        {
+            validChars.append( ", digits" );
+        }
+        if( m_allowInternalWhitespace )
+        {
+            validChars.append( ", whitespace" );
+        }
+        validChars.append( ", and \"" );
+        validChars.append( m_additionalInternalCharacters );
+        validChars.append( "\"" );
+        return validChars.toString();
+    }
+
+    /**
+     * @param allowSurroundingWhitespace
+     *          specified if names are trimmed before checking
+     */
+    public void setAllowSurroundingWhitespace( boolean allowSurroundingWhitespace )
+    {
+        m_allowSurroundingWhitespace = allowSurroundingWhitespace;
+    }
+
+    /**
+     * @param allowInitialDigit
+     *          specifies if digits are permitted as intial characters
+     */
+    public void setAllowInitialDigit( boolean allowInitialDigit )
+    {
+        m_allowInitialDigit = allowInitialDigit;
+    }
+
+    /**
+     * @param additionalInitialCharacters
+     *          extra characters to allow as initial characters.
+     */
+    public void setAdditionalInitialCharacters( String additionalInitialCharacters )
+    {
+        m_additionalInitialCharacters = additionalInitialCharacters;
+    }
+
+    /**
+     * @param allowInternalDigits
+     *          specifies if digits are permitted as internal characters
+     */
+    public void setAllowInternalDigits( boolean allowInternalDigits )
+    {
+        m_allowInternalDigits = allowInternalDigits;
+    }
+
+    /**
+     * @param allowInternalWhitespace
+     *          specifies if whitespace is permitted internally in names
+     */
+    public void setAllowInternalWhitespace( boolean allowInternalWhitespace )
+    {
+        m_allowInternalWhitespace = allowInternalWhitespace;
+    }
+
+    /**
+     * @param additionalInternalCharacters
+     *          extra characters permitted in names
+     */
+    public void setAdditionalInternalCharacters( String additionalInternalCharacters )
+    {
+        m_additionalInternalCharacters = additionalInternalCharacters;
+    }
+
+}

proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/NameValidator.java

@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) The Apache Software Foundation. All rights reserved.
+ *
+ * This software is published under the terms of the Apache Software License
+ * version 1.1, a copy of which has been included  with this distribution in
+ * the LICENSE.txt file.
+ */
+package org.apache.myrmidon.interfaces.model;
+
+/**
+ * Determines the validity of names used in projects.
+ *
+ * @author <a href="mailto:darrell@apache.org">Darrell DeBoer</a>
+ * @version $Revision$ $Date$
+ */
+public interface NameValidator
+{
+    /**
+     * Validates the supplied name, failing if it is not.
+     * @throws Exception is the supplied name is not valid.
+     */
+    void validate( String name ) throws Exception;
+}

proposal/myrmidon/src/java/org/apache/myrmidon/interfaces/model/Resources.properties

@@ -0,0 +1,6 @@
+# Name validation
+name.zero-char-name.error=Name "" is invalid, as it contains no characters.
+name.enclosing-whitespace.error=Name "{0}" is invalid, as it contains enclosing whitespace.
+name.invalid-initial-char.error=Name "{0}" is invalid, as it begins with an illegal character. Names can start with {1}.
+name.invalid-internal-char.error=Name "{0}" is invalid, as it contains an illegal character. Permitted name characters are {1}.
+name.could-not-create.error=Could not valid name from "{0}".

proposal/myrmidon/src/test/org/apache/antlib/core/PropertyTest.java

@@ -12,6 +12,7 @@ import org.apache.avalon.excalibur.i18n.ResourceManager;
 import org.apache.avalon.excalibur.i18n.Resources;
 import org.apache.myrmidon.AbstractProjectTest;
 import org.apache.myrmidon.LogMessageTracker;
+import org.apache.myrmidon.components.workspace.DefaultTaskContext;
 
 /**
  * Test cases for <property> task.
@@ -91,4 +92,26 @@ public class PropertyTest
         executeTargetExpectError( projectFile, "too-many-values3", messages );
     }
 
+    /**
+     * Tests basic validation of property names.
+     */
+    public void testNameValidation() throws Exception
+    {
+        final File projectFile = getTestResource( "property.ant" );
+
+        final Resources contextResources
+            = ResourceManager.getPackageResources( DefaultTaskContext.class );
+
+        // Invalid names
+        String[] messages = new String[]
+        {
+            null,
+            contextResources.getString( "bad-property-name.error" ),
+            null
+        };
+        executeTargetExpectError( projectFile, "bad-prop-name1", messages );
+        executeTargetExpectError( projectFile, "bad-prop-name2", messages );
+        executeTargetExpectError( projectFile, "bad-prop-name3", messages );
+    }
+
 }

proposal/myrmidon/src/test/org/apache/myrmidon/components/builder/DefaultProjectBuilderTest.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) The Apache Software Foundation. All rights reserved.
+ *
+ * This software is published under the terms of the Apache Software License
+ * version 1.1, a copy of which has been included  with this distribution in
+ * the LICENSE.txt file.
+ */
+package org.apache.myrmidon.components.builder;
+
+import java.io.File;
+import org.apache.avalon.excalibur.i18n.ResourceManager;
+import org.apache.avalon.excalibur.i18n.Resources;
+import org.apache.myrmidon.AbstractMyrmidonTest;
+
+/**
+ * Test cases for {@link DefaultProjectBuilder}.
+ *
+ * @author <a href="mailto:darrell@apache.org">Darrell DeBoer</a>
+ * @version $Revision$ $Date$
+ */
+public class DefaultProjectBuilderTest
+    extends AbstractMyrmidonTest
+{
+    private final static Resources REZ
+        = ResourceManager.getPackageResources( DefaultProjectBuilder.class );
+
+    private DefaultProjectBuilder m_builder;
+
+    public DefaultProjectBuilderTest( String name )
+    {
+        super( name );
+    }
+
+    protected void setUp() throws Exception
+    {
+        super.setUp();
+        m_builder = new DefaultProjectBuilder();
+        m_builder.enableLogging( createLogger() );
+    }
+
+    /**
+     * Test validation of project and target names.
+     */
+    public void testNameValidation() throws Exception
+    {
+        // Check bad project name
+        final File badProjectFile = getTestResource( "bad-project-name.ant" );
+        try
+        {
+            m_builder.build( badProjectFile.getAbsolutePath() );
+            fail();
+        }
+        catch( Exception e )
+        {
+            assertSameMessage( REZ.getString( "ant.project-bad-name.error" ), e );
+        }
+
+        // Check bad target name
+        final File badTargetFile = getTestResource( "bad-target-name.ant" );
+        try
+        {
+            m_builder.build( badTargetFile.getAbsolutePath() );
+            fail();
+        }
+        catch( Exception e )
+        {
+            // TODO - check error message
+        }
+    }
+}

proposal/myrmidon/src/test/org/apache/myrmidon/interfaces/model/DefaultNameValidatorTest.java

@@ -0,0 +1,122 @@
+/*
+ * Copyright (C) The Apache Software Foundation. All rights reserved.
+ *
+ * This software is published under the terms of the Apache Software License
+ * version 1.1, a copy of which has been included  with this distribution in
+ * the LICENSE.txt file.
+ */
+package org.apache.myrmidon.interfaces.model;
+
+import org.apache.myrmidon.AbstractMyrmidonTest;
+
+/**
+ * TestCases for {@link DefaultNameValidator}.
+ *
+ * @author <a href="mailto:darrell@apache.org">Darrell DeBoer</a>
+ * @version $Revision$ $Date$
+ */
+public class DefaultNameValidatorTest
+    extends AbstractMyrmidonTest
+{
+    private DefaultNameValidator m_validator = new DefaultNameValidator();
+
+    public DefaultNameValidatorTest( String name )
+    {
+        super( name );
+    }
+
+    /**
+     * Test valid names for the default validator.
+     */
+    public void testValidNames() throws Exception
+    {
+        testValid( "aName" );
+        testValid( "123456" );
+        testValid( "s p     a ce s" );
+        testValid( "d-a-s-h-e-s-" );
+        testValid( "d.o.t.s." );
+        testValid( "_u_n_d_e_r_s_c_o_r_e_s" );
+        testValid( "a" );
+        testValid( "1" );
+        testValid( "_" );
+
+    }
+
+    /**
+     * Test invalid names for the default validator.
+     */
+    public void testInvalidNames() throws Exception
+    {
+        testInvalid( "" );
+        testInvalid( " " );
+        testInvalid( "           " );
+        testInvalid( " bad" );
+        testInvalid( "bad " );
+        testInvalid( " bad " );
+        testInvalid( "-dashfirst" );
+        testInvalid( ".dotfirst" );
+        testInvalid( "question?" );
+    }
+
+    /**
+     * Test that certain characters are disallowed in the default validator.
+     */
+    public void testReservedChars() throws Exception
+    {
+        String reserved = "!@#$%^&*()+=~`{}[]|\\/?<>,:;";
+
+        for( int pos = 0; pos < reserved.length(); pos++ )
+        {
+            char chr = reserved.charAt( pos );
+            testReservedChar( chr );
+        }
+    }
+
+    private void testReservedChar( char chr ) throws Exception
+    {
+        String test = "a" + String.valueOf( chr );
+        testInvalid( test );
+    }
+
+    /**
+     * Test validation using a restrictive set of validation rules.
+     */
+    public void testStrictNames() throws Exception
+    {
+        m_validator = new DefaultNameValidator( false, false, "", false, false, "." );
+
+        testValid( "name" );
+        testValid( "a" );
+        testValid( "yep.ok" );
+
+        testInvalid( "_nogood" );
+        testInvalid( "no_good" );
+        testInvalid( "nope1" );
+        testInvalid( "123" );
+        testInvalid( "not ok" );
+    }
+
+    private void testValid( String name )
+    {
+        try
+        {
+            m_validator.validate( name );
+        }
+        catch( Exception e )
+        {
+            fail( e.getMessage() );
+        }
+    }
+
+    private void testInvalid( String name )
+    {
+        try
+        {
+            m_validator.validate( name );
+            fail( "Name \"" + name + "\" should be invalid." );
+        }
+        catch( Exception e )
+        {
+        }
+    }
+}

proposal/myrmidon/src/testcases/org/apache/antlib/core/PropertyTest.java

@@ -12,6 +12,7 @@ import org.apache.avalon.excalibur.i18n.ResourceManager;
 import org.apache.avalon.excalibur.i18n.Resources;
 import org.apache.myrmidon.AbstractProjectTest;
 import org.apache.myrmidon.LogMessageTracker;
+import org.apache.myrmidon.components.workspace.DefaultTaskContext;
 
 /**
  * Test cases for <property> task.
@@ -91,4 +92,26 @@ public class PropertyTest
         executeTargetExpectError( projectFile, "too-many-values3", messages );
     }
 
+    /**
+     * Tests basic validation of property names.
+     */
+    public void testNameValidation() throws Exception
+    {
+        final File projectFile = getTestResource( "property.ant" );
+
+        final Resources contextResources
+            = ResourceManager.getPackageResources( DefaultTaskContext.class );
+
+        // Invalid names
+        String[] messages = new String[]
+        {
+            null,
+            contextResources.getString( "bad-property-name.error" ),
+            null
+        };
+        executeTargetExpectError( projectFile, "bad-prop-name1", messages );
+        executeTargetExpectError( projectFile, "bad-prop-name2", messages );
+        executeTargetExpectError( projectFile, "bad-prop-name3", messages );
+    }
+
 }

proposal/myrmidon/src/testcases/org/apache/myrmidon/components/builder/DefaultProjectBuilderTest.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) The Apache Software Foundation. All rights reserved.
+ *
+ * This software is published under the terms of the Apache Software License
+ * version 1.1, a copy of which has been included  with this distribution in
+ * the LICENSE.txt file.
+ */
+package org.apache.myrmidon.components.builder;
+
+import java.io.File;
+import org.apache.avalon.excalibur.i18n.ResourceManager;
+import org.apache.avalon.excalibur.i18n.Resources;
+import org.apache.myrmidon.AbstractMyrmidonTest;
+
+/**
+ * Test cases for {@link DefaultProjectBuilder}.
+ *
+ * @author <a href="mailto:darrell@apache.org">Darrell DeBoer</a>
+ * @version $Revision$ $Date$
+ */
+public class DefaultProjectBuilderTest
+    extends AbstractMyrmidonTest
+{
+    private final static Resources REZ
+        = ResourceManager.getPackageResources( DefaultProjectBuilder.class );
+
+    private DefaultProjectBuilder m_builder;
+
+    public DefaultProjectBuilderTest( String name )
+    {
+        super( name );
+    }
+
+    protected void setUp() throws Exception
+    {
+        super.setUp();
+        m_builder = new DefaultProjectBuilder();
+        m_builder.enableLogging( createLogger() );
+    }
+
+    /**
+     * Test validation of project and target names.
+     */
+    public void testNameValidation() throws Exception
+    {
+        // Check bad project name
+        final File badProjectFile = getTestResource( "bad-project-name.ant" );
+        try
+        {
+            m_builder.build( badProjectFile.getAbsolutePath() );
+            fail();
+        }
+        catch( Exception e )
+        {
+            assertSameMessage( REZ.getString( "ant.project-bad-name.error" ), e );
+        }
+
+        // Check bad target name
+        final File badTargetFile = getTestResource( "bad-target-name.ant" );
+        try
+        {
+            m_builder.build( badTargetFile.getAbsolutePath() );
+            fail();
+        }
+        catch( Exception e )
+        {
+            // TODO - check error message
+        }
+    }
+}

proposal/myrmidon/src/testcases/org/apache/myrmidon/interfaces/model/DefaultNameValidatorTest.java

@@ -0,0 +1,122 @@
+/*
+ * Copyright (C) The Apache Software Foundation. All rights reserved.
+ *
+ * This software is published under the terms of the Apache Software License
+ * version 1.1, a copy of which has been included  with this distribution in
+ * the LICENSE.txt file.
+ */
+package org.apache.myrmidon.interfaces.model;
+
+import org.apache.myrmidon.AbstractMyrmidonTest;
+
+/**
+ * TestCases for {@link DefaultNameValidator}.
+ *
+ * @author <a href="mailto:darrell@apache.org">Darrell DeBoer</a>
+ * @version $Revision$ $Date$
+ */
+public class DefaultNameValidatorTest
+    extends AbstractMyrmidonTest
+{
+    private DefaultNameValidator m_validator = new DefaultNameValidator();
+
+    public DefaultNameValidatorTest( String name )
+    {
+        super( name );
+    }
+
+    /**
+     * Test valid names for the default validator.
+     */
+    public void testValidNames() throws Exception
+    {
+        testValid( "aName" );
+        testValid( "123456" );
+        testValid( "s p     a ce s" );
+        testValid( "d-a-s-h-e-s-" );
+        testValid( "d.o.t.s." );
+        testValid( "_u_n_d_e_r_s_c_o_r_e_s" );
+        testValid( "a" );
+        testValid( "1" );
+        testValid( "_" );
+
+    }
+
+    /**
+     * Test invalid names for the default validator.
+     */
+    public void testInvalidNames() throws Exception
+    {
+        testInvalid( "" );
+        testInvalid( " " );
+        testInvalid( "           " );
+        testInvalid( " bad" );
+        testInvalid( "bad " );
+        testInvalid( " bad " );
+        testInvalid( "-dashfirst" );
+        testInvalid( ".dotfirst" );
+        testInvalid( "question?" );
+    }
+
+    /**
+     * Test that certain characters are disallowed in the default validator.
+     */
+    public void testReservedChars() throws Exception
+    {
+        String reserved = "!@#$%^&*()+=~`{}[]|\\/?<>,:;";
+
+        for( int pos = 0; pos < reserved.length(); pos++ )
+        {
+            char chr = reserved.charAt( pos );
+            testReservedChar( chr );
+        }
+    }
+
+    private void testReservedChar( char chr ) throws Exception
+    {
+        String test = "a" + String.valueOf( chr );
+        testInvalid( test );
+    }
+
+    /**
+     * Test validation using a restrictive set of validation rules.
+     */
+    public void testStrictNames() throws Exception
+    {
+        m_validator = new DefaultNameValidator( false, false, "", false, false, "." );
+
+        testValid( "name" );
+        testValid( "a" );
+        testValid( "yep.ok" );
+
+        testInvalid( "_nogood" );
+        testInvalid( "no_good" );
+        testInvalid( "nope1" );
+        testInvalid( "123" );
+        testInvalid( "not ok" );
+    }
+
+    private void testValid( String name )
+    {
+        try
+        {
+            m_validator.validate( name );
+        }
+        catch( Exception e )
+        {
+            fail( e.getMessage() );
+        }
+    }
+
+    private void testInvalid( String name )
+    {
+        try
+        {
+            m_validator.validate( name );
+            fail( "Name \"" + name + "\" should be invalid." );
+        }
+        catch( Exception e )
+        {
+        }
+    }
+}

proposal/myrmidon/src/xdocs/todo.xml

@@ -209,12 +209,6 @@
                     <li>Fire ProjectListener events projectStarted() and projectFinished()
                     events on start and finish of referenced projects, adding indicator methods
                     to ProjectEvent.</li>
-                    <li>Validate project and target names in DefaultProjectBuilder - reject dodgy
-                    names like "," or "", or "  ".  Probably want to reject names that start or
-                    end with white-space (though internal whitespace is probably fine).  We also
-                    want to reserve certain punctuation characters like , : ? $ [ ] { } &lt; &gt;, etc for
-                    future use.</li>
-                    <li>Similarly, validate property names, using the same rules.</li>
                     <li>Detect duplicate type names.</li>
                     <li>Add fully qualified type names, based on antlib name and type shorthand name.
                     Allow these to be used in build files in addition to the shorthand names.</li>