Dissallow <import> within targets

remove some commented out code in ImportTask


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

src/etc/testcases/taskdefs/import/subdir/importinsequential-inner.xml

@@ -0,0 +1,8 @@
+<project>
+  <target name="within-imported">
+    <property name="foo" value="bar"/>
+    <path id="baz">
+      <pathelement location="."/>
+    </path>
+  </target>
+</project>

src/etc/testcases/taskdefs/import/subdir/importinsequential.xml

@@ -0,0 +1,5 @@
+<project>
+  <sequential>
+    <import file="importinsequential-inner.xml"/>
+  </sequential>
+</project>

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

@@ -116,20 +116,20 @@ public class ImportTask extends Task {
         if (file == null) {
             throw new BuildException("import requires file attribute");
         }
-
+        if (getOwningTarget() == null
+            || !"".equals(getOwningTarget().getName())) {
+            throw new BuildException("import only allowed as a top-level task");
+        }
+        
         ProjectHelper helper =
                 (ProjectHelper) getProject().getReference("ant.projectHelper");
         Vector importStack = helper.getImportStack();
+
         if (importStack.size() == 0) {
             // this happens if ant is used with a project
             // helper that doesn't set the import.
             throw new BuildException("import requires support in ProjectHelper");
         }
-//        ProjectHelper2.AntXmlContext context;
-//        context=(ProjectHelper2.AntXmlContext)project.getReference("ant.parsing.context");
-
-//        File buildFile=context.buildFile;
-//        File buildFileParent=context.buildFileParent;
 
         if (getLocation() == null || getLocation().getFileName() == null) {
             throw new BuildException("Unable to get location of import task");
@@ -137,7 +137,7 @@ public class ImportTask extends Task {
 
         File buildFile = new File(getLocation().getFileName());
         buildFile = new File(buildFile.getAbsolutePath());
-        //System.out.println("Importing from " + currentSource);
+
         File buildFileParent = new File(buildFile.getParent());
 
         getProject().log("Importing file " + file + " from "
@@ -165,26 +165,6 @@ public class ImportTask extends Task {
             return;
         }
 
-//        // Add parent build file to the map to avoid cycles...
-//        String parentFilename = getPath(buildFile);
-//        if (!context.importedFiles.containsKey(parentFilename)) {
-//            context.importedFiles.put(parentFilename, buildFile);
-//        }
-//
-//        // Make sure we import the file only once
-//        String importedFilename = getPath(importedFile);
-//        if (context.importedFiles.containsKey(importedFilename)) {
-//            project.log("\nSkipped already imported file:\n   "+
-//                    importedFilename+"\n",Project.MSG_WARN);
-//            return;
-//        } else {
-//            context.importedFiles.put(importedFilename, importedFile);
-//        }
-
-//        context.ignoreProjectTag=true;
-//        context.helper.parse(project, importedFile,
-//                new ProjectHelper2.RootHandler(context));
-
         helper.parse(getProject(), importedFile);
     }
 

src/testcases/org/apache/tools/ant/taskdefs/ImportTest.java

@@ -91,16 +91,33 @@ public class ImportTest extends BuildFileTest {
             "Unnamed2.xmlUnnamed1.xmlSkipped already imported file");
     }
 
+    // allow this as imported in targets are only tested when a target is run
     public void testImportInTargetNoEffect() {
         configureProject("src/etc/testcases/taskdefs/import/subdir/importintarget.xml");
         expectPropertyUnset("no-import", "foo");
         assertTrue(null == getProject().getReference("baz"));
     }
 
-    public void testImportInTargetWithEffect() {
+    // deactivate this test as imports within targets are not allowed
+    public void notTestImportInTargetWithEffect() {
         configureProject("src/etc/testcases/taskdefs/import/subdir/importintarget.xml");
         expectPropertySet("do-import", "foo", "bar");
         assertNotNull(getProject().getReference("baz"));
     }
+    
+    public void testImportInTargetNotAllowed() {
+        configureProject(
+            "src/etc/testcases/taskdefs/import/subdir/importintarget.xml");
+        expectBuildExceptionContaining(
+            "do-import", "not a top level task",
+            "import only allowed as a top-level task");
+    }
+
+    public void testImportInSequential() {
+        configureProject(
+            "src/etc/testcases/taskdefs/import/subdir/importinsequential.xml");
+        expectPropertySet("within-imported", "foo", "bar");
+        assertNotNull(getProject().getReference("baz"));
+    }
 }