Update to remove warnings generated by antcall due to last immutability tightening. Did this by adding a protected constructor to property.

Made sure that operations internal to Project could not generate these warnings by adding a new setPropertyInternal method that didn't do any checks.

Also made the getUserProperties and getProperties methods return copies of maps rather than direct references.

Updated WHATSNEW to warn about these issues.

Added some unit tests to verify that property immutability is enforced - where we actually do enforce it ;)

Submitted by: "Erik Hatcher" <jakarta-ant@ehatchersolutions.com>


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

WHATSNEW

@@ -17,7 +17,9 @@ Changes that could break older environments:
   instead.
 
 * Some loopholes in the immutability rule have been closed. It is no longer
-  possible to overwrite a property using tasks like <available> or <condition>.  
+  possible to overwrite a property using tasks like <checksum>, <condition>,
+  <exec>, <pathconvert>, or <tstamp>. In some exceptional cases it will 
+  generate a warning if you attempt to subvert property immutability.
   
 * Taskwriters please note: Whenever tasks had any overloaded set* methods,
   Ant's introspection mechanism would select the last overloaded method

src/etc/testcases/core/immutable.xml

@@ -0,0 +1,63 @@
+<?xml version="1.0"?>
+
+<project name="available-test" basedir="." default="test1">
+
+  <target name="test1">
+    <property name="test" value="original"/>
+    <available file="immutable.xml" property="test" value="override"/>
+  </target>
+  
+  <target name="test2">
+    <tstamp/>
+    <tstamp prefix="start"/>
+  </target>
+
+  <target name="test3">
+    <property name="DSTAMP" value="original"/>
+    <tstamp/>
+  </target>
+
+  <target name="test4">
+    <property name="test" value="original"/>
+    <condition property="test" value="override">
+      <equals arg1="1" arg2="1"/>
+    </condition>
+  </target>
+
+  <target name="test5">
+    <property name="test" value="original"/>
+    <checksum file="immutable.xml" verifyProperty="test"/>
+  </target>
+  
+  <target name="test6">
+    <property name="test1" value="original"/>
+    <property name="test2" value="original"/>
+    <!--  How to make this cross-platform? -->
+    <exec executable="cmd.exe" os="Windows 2000" outputproperty="test1" resultProperty="test2">
+      <arg line="/c dir"/>
+    </exec>
+  </target>
+
+  <target name="test7">
+    <property name="test" value="original"/>
+    <pathconvert targetos="unix" property="test" >
+      <path>
+        <pathelement location="/lib/weblogicaux.jar" />
+        <pathelement location="/classes" />
+        <pathelement location="/mssqlserver4/classes" />
+        <pathelement location="c:\winnt\System32" />
+      </path>
+    </pathconvert>
+  </target>
+
+  <target name="test8">
+    <antcall inheritAll="false" target="echo-target">
+      <param name="echo.value" value="Meep meep!" />
+    </antcall>
+  </target>
+
+  <target name="echo-target">
+    <echo message="Value of echo=${echo.value}"/>
+  </target>
+
+</project>

src/main/org/apache/tools/ant/Project.java

@@ -159,7 +159,7 @@ public class Project {
     public Project() {
         fileUtils = FileUtils.newFileUtils();
     }
-
+    
     /**
      * Initialise the project.
      *
@@ -353,6 +353,18 @@ public class Project {
         userProperties.put(name, value);
         properties.put(name, value);
     }
+    
+    /**
+     * Allows Project and subclasses to set a property unless its
+     * already defined as a user property. There are a few cases 
+     * internally to Project that need to do this currently.
+     */
+    private void setPropertyInternal(String name, String value) {
+        if (null != userProperties.get(name)) {
+            return;
+        }
+        properties.put(name, value);
+    }
 
     /**
      * query a property.
@@ -377,19 +389,37 @@ public class Project {
     }
 
     /**
-     * get the property hashtable
+     * get a copy of the property hashtable
      * @return the hashtable containing all properties, user included
      */
     public Hashtable getProperties() {
-        return properties;
+        Hashtable propertiesCopy = new Hashtable();
+        
+        Enumeration e = properties.keys();
+        while (e.hasMoreElements()) {
+            Object name = e.nextElement();
+            Object value = properties.get(name);
+            propertiesCopy.put(name, value);
+        }
+        
+        return propertiesCopy;
     }
 
     /**
-     * get the user property hashtable
+     * get a copy of the user property hashtable
      * @return the hashtable user properties only
      */
     public Hashtable getUserProperties() {
-        return userProperties;
+        Hashtable propertiesCopy = new Hashtable();
+        
+        Enumeration e = userProperties.keys();
+        while (e.hasMoreElements()) {
+            Object name = e.nextElement();
+            Object value = properties.get(name);
+            propertiesCopy.put(name, value);
+        }
+        
+        return propertiesCopy;
     }
 
     /**
@@ -486,7 +516,7 @@ public class Project {
         if (!baseDir.isDirectory()) 
             throw new BuildException("Basedir " + baseDir.getAbsolutePath() + " is not a directory");
         this.baseDir = baseDir;
-        setProperty( "basedir", this.baseDir.getPath());
+        setPropertyInternal( "basedir", this.baseDir.getPath());
         String msg = "Project base dir set to: " + this.baseDir;
         log(msg, MSG_VERBOSE);
     }
@@ -521,7 +551,7 @@ public class Project {
      * @throws BuildException if this Java version is not supported
      */
     public void setJavaVersionProperty() throws BuildException {
-        setProperty("ant.java.version", javaVersion);
+        setPropertyInternal("ant.java.version", javaVersion);
 
         // sanity check
         if (javaVersion == JAVA_1_0) {
@@ -543,7 +573,7 @@ public class Project {
         while (e.hasMoreElements()) {
             Object name = e.nextElement();
             String value = systemP.get(name).toString();
-            this.setProperty(name.toString(), value);
+            this.setPropertyInternal(name.toString(), value);
         }
     }
 

src/main/org/apache/tools/ant/taskdefs/Ant.java

@@ -330,8 +330,9 @@ public class Ant extends Task {
         if (newProject == null) {
             reinit();
         }
-        Property p=(Property)newProject.createTask("property");
-        p.setUserProperty(true);
+        Property p = new Property(true);
+        p.setProject(newProject);
+        p.setTaskName("property");
         properties.addElement( p );
         return p;
     }

src/main/org/apache/tools/ant/taskdefs/Property.java

@@ -85,10 +85,18 @@ public class Property extends Task {
     protected String resource;
     protected Path classpath;
     protected String env;
-    protected Reference ref = null;
-
-    protected boolean userProperty=false; // set read-only properties
+    protected Reference ref;
 
+    protected boolean userProperty; // set read-only properties
+    
+    public Property() {
+        super();
+    }
+    
+    protected Property(boolean userProperty) {
+        this.userProperty = userProperty;
+    }
+    
     public void setName(String name) {
         this.name = name;
     }
@@ -161,10 +169,11 @@ public class Property extends Task {
     }
 
     /**
-    * @deprecated
-    */
+     * @deprecated This was never a supported feature and has been deprecated without replacement
+     */
     public void setUserProperty(boolean userProperty) {
-        this.userProperty = userProperty;
+        log("DEPRECATED: Ignoring request to set user property in Property task.", 
+            Project.MSG_WARN);
     }
 
     public String toString() {
@@ -287,12 +296,11 @@ public class Property extends Task {
 
     protected void addProperty(String n, String v) {
         if( userProperty ) {
-            log("DEPRECATED - Setting user properties through the Property task has been deprecated.");
             if (project.getUserProperty(n) == null) {
                 project.setUserProperty(n, v);
             } else {
                 log("Override ignored for " + n, Project.MSG_VERBOSE);
-            } 
+            }
         } else {
             project.setNewProperty(n, v);
         }

src/testcases/org/apache/tools/ant/ImmutableTest.java

@@ -0,0 +1,115 @@
+/*
+ * The Apache Software License, Version 1.1
+ *
+ * Copyright (c) 2000 The Apache Software Foundation.  All rights
+ * reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * 3. The end-user documentation included with the redistribution, if
+ *    any, must include the following acknowlegement:
+ *       "This product includes software developed by the
+ *        Apache Software Foundation (http://www.apache.org/)."
+ *    Alternately, this acknowlegement may appear in the software itself,
+ *    if and wherever such third-party acknowlegements normally appear.
+ *
+ * 4. The names "The Jakarta Project", "Ant", and "Apache Software
+ *    Foundation" must not be used to endorse or promote products derived
+ *    from this software without prior written permission. For written
+ *    permission, please contact apache@apache.org.
+ *
+ * 5. Products derived from this software may not be called "Apache"
+ *    nor may "Apache" appear in their names without prior written
+ *    permission of the Apache Group.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ */
+
+package org.apache.tools.ant;
+
+import org.apache.tools.ant.BuildFileTest;
+
+/**
+ * @author Erik Hatcher
+ */
+public class ImmutableTest extends BuildFileTest { 
+
+    public ImmutableTest(String name) { 
+        super(name);
+    }
+    
+    public void setUp() { 
+        configureProject("src/etc/testcases/core/immutable.xml");
+    }
+    
+    // override allowed on <available>
+    public void test1() { 
+        executeTarget("test1");
+        assertEquals("override", project.getProperty("test"));
+    }
+    
+    // ensure <tstamp>'s new prefix attribute is working
+    public void test2() {
+        executeTarget("test2");
+        assertNotNull(project.getProperty("DSTAMP"));
+        assertNotNull(project.getProperty("start.DSTAMP"));
+    }
+
+    // ensure <tstamp> follows the immutability rule
+    public void test3() {
+        executeTarget("test3");
+        assertEquals("original", project.getProperty("DSTAMP"));
+    }
+
+    // ensure <condition> follows the immutability rule
+    public void test4() {
+        executeTarget("test4");
+        assertEquals("original", project.getProperty("test"));
+    }
+    // ensure <checksum> follows the immutability rule
+    public void test5() {
+        executeTarget("test5");
+        assertEquals("original", project.getProperty("test"));
+    }
+
+    // ensure <exec> follows the immutability rule
+    public void test6() {
+        executeTarget("test6");
+        assertEquals("original", project.getProperty("test1"));
+        assertEquals("original", project.getProperty("test2"));
+    }
+
+    // ensure <pathconvert> follows the immutability rule
+    public void test7() {
+        executeTarget("test7");
+        assertEquals("original", project.getProperty("test"));
+    }
+}
+