Tests + cleanup + docs for ctx.actions.declare_{file,directory}

RELNOTES: None.
PiperOrigin-RevId: 160273273

3 files changed, 36 insertions, 21 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkActionFactory.java
index 535987f090..ecfff67656 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkActionFactory.java
@@ -82,7 +82,7 @@ public class SkylarkActionFactory implements SkylarkValue {
       }
   )
   public Artifact declareFile(String filename, Object sibling) throws EvalException {
-    context.checkMutable("actions.declareFile");
+    context.checkMutable("actions.declare_file");
     if (Runtime.NONE.equals(sibling)) {
       return ruleContext.getPackageRelativeArtifact(filename, newFileRoot());
     } else {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
index 8fee2dda42..89f35200ac 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
@@ -887,7 +887,8 @@ public final class SkylarkRuleContext implements SkylarkValue {
   @SkylarkCallable(
     name = "new_file",
     doc =
-        "Creates a file object with the given filename, in the current package. "
+        "DEPRECATED. Use <a href=\"actions.html#declare_file\">ctx.actions.declare_file</a>. <br>"
+            + "Creates a file object with the given filename, in the current package. "
             + DOC_NEW_FILE_TAIL,
     parameters = {
       @Param(
@@ -899,13 +900,7 @@ public final class SkylarkRuleContext implements SkylarkValue {
   )
   public Artifact newFile(String filename) throws EvalException {
     checkMutable("new_file");
-    return newFile(newFileRoot(), filename);
-  }
-
-  private Root newFileRoot() throws EvalException {
-    return isForAspect()
-        ? getConfiguration().getBinDirectory(ruleContext.getRule().getRepository())
-        : ruleContext.getBinOrGenfilesDirectory();
+    return actionFactory.declareFile(filename, Runtime.NONE);
   }
 
   // Kept for compatibility with old code.
@@ -935,9 +930,7 @@ public final class SkylarkRuleContext implements SkylarkValue {
     )
   public Artifact newFile(Artifact baseArtifact, String newBaseName) throws EvalException {
     checkMutable("new_file");
-    PathFragment original = baseArtifact.getRootRelativePath();
-    PathFragment fragment = original.replaceName(newBaseName);
-    return ruleContext.getDerivedArtifact(fragment, newFileRoot());
+    return actionFactory.declareFile(newBaseName, baseArtifact);
   }
 
   // Kept for compatibility with old code.
@@ -949,7 +942,6 @@ public final class SkylarkRuleContext implements SkylarkValue {
     return ruleContext.getDerivedArtifact(fragment, root);
   }
 
-  // TODO(b/36548861): Document this when it's ready to be made publicly available.
   @SkylarkCallable(
     name = "experimental_new_directory",
     documented = false,
@@ -966,13 +958,7 @@ public final class SkylarkRuleContext implements SkylarkValue {
   )
   public Artifact newDirectory(String name, Object siblingArtifactUnchecked) throws EvalException {
     checkMutable("experimental_new_directory");
-    if (siblingArtifactUnchecked == Runtime.NONE) {
-      return ruleContext.getPackageRelativeTreeArtifact(PathFragment.create(name), newFileRoot());
-    }
-    Artifact siblingArtifact = (Artifact) siblingArtifactUnchecked;
-    PathFragment original = siblingArtifact.getRootRelativePath();
-    PathFragment fragment = original.replaceName(name);
-    return ruleContext.getTreeArtifact(fragment, newFileRoot());
+    return actionFactory.declareDirectory(name, siblingArtifactUnchecked);
   }
 
   @SkylarkCallable(documented = false)
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 24cc4e0ef2..3333e6ba81 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -776,6 +776,18 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
   }
 
   @Test
+  public void testDeriveTreeArtifactType() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    Object result =
+        evalRuleContextCode(ruleContext,
+            "b = ruleContext.actions.declare_directory('a/b')\n"
+            + "type(b)");
+    assertThat(result).isInstanceOf(String.class);
+    assertThat(result).isEqualTo("File");
+  }
+
+
+  @Test
   public void testDeriveTreeArtifactNextToSibling() throws Exception {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
     Object result =
@@ -802,7 +814,7 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
   }
 
   @Test
-  public void testParamFileSuffix() throws Exception {
+  public void testParamFileSuffixLegacy() throws Exception {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
     Object result =
         evalRuleContextCode(
@@ -814,6 +826,19 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
   }
 
   @Test
+  public void testParamFileSuffix() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    Object result =
+        evalRuleContextCode(
+            ruleContext,
+            "ruleContext.actions.declare_file(ruleContext.files.tools[0].basename + '.params', "
+                + "sibling = ruleContext.files.tools[0])");
+    PathFragment fragment = ((Artifact) result).getRootRelativePath();
+    assertThat(fragment.getPathString()).isEqualTo("foo/t.exe.params");
+  }
+
+
+  @Test
   public void testLabelKeyedStringDictConvertsToTargetToStringMap() throws Exception {
     scratch.file(
         "my_rule.bzl",
@@ -1832,6 +1857,10 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
       "expand('foo', [], Label('//test:main'))",
       "new_file('foo.txt')",
       "new_file(file, 'foo.txt')",
+      "actions.declare_file('foo.txt')",
+      "actions.declare_file('foo.txt', sibling = file)",
+      "actions.declare_directory('foo.txt')",
+      "actions.declare_directory('foo.txt', sibling = file)",
       "check_placeholders('foo', [])",
       "action(command = 'foo', outputs = [file])",
       "file_action(file, 'foo')",