C++: Give error instead of crash with wrong extensions

The code was crashing on preconditions when passing wrong extensions for srcs
and hdrs attribute in the C++ compile() API.

#4570

RELNOTES:none
PiperOrigin-RevId: 245925608

src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java

@@ -17,6 +17,7 @@
 import static com.google.common.base.StandardSystemProperty.LINE_SEPARATOR;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -66,6 +67,7 @@
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
 import com.google.devtools.build.lib.syntax.SkylarkType;
+import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.StringUtil;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -1484,6 +1486,21 @@ protected Tuple<Object> compile(
         convertFromNoneable(skylarkFeatureConfiguration, null);
     Label label = getCallerLabel(location, actions, name);
     FdoContext fdoContext = ccToolchainProvider.getFdoContext();
+    validateExtensions(
+        location,
+        "srcs",
+        sources,
+        CppFileTypes.ALL_C_CLASS_SOURCE,
+        FileType.of(
+            ImmutableList.<String>builder()
+                .addAll(CppFileTypes.CPP_SOURCE.getExtensions())
+                .addAll(CppFileTypes.C_SOURCE.getExtensions())
+                .build()));
+    validateExtensions(
+        location, "public_hdrs", publicHeaders, CppFileTypes.CPP_HEADER, CppFileTypes.CPP_HEADER);
+    validateExtensions(
+        location, "private_hdrs", privateHeaders, CppFileTypes.CPP_HEADER, CppFileTypes.CPP_HEADER);
+
     CcCompilationHelper helper =
         new CcCompilationHelper(
                 actions.asActionRegistry(location, actions),
@@ -1607,4 +1624,25 @@ protected CcLinkingOutputs link(
       throw new EvalException(location, e);
     }
   }
+
+  private void validateExtensions(
+      Location location,
+      String paramName,
+      List<Artifact> files,
+      FileType validFileType,
+      FileType fileTypeForErrorMessage)
+      throws EvalException {
+    for (Artifact file : files) {
+      if (!validFileType.matches(file)) {
+        throw new EvalException(
+            location,
+            String.format(
+                "'%s' has wrong extension. The list of possible extensions for '"
+                    + paramName
+                    + "' are: %s",
+                file.getExecPathString(),
+                Joiner.on(",").join(fileTypeForErrorMessage.getExtensions())));
+      }
+    }
+  }
 }

src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java

@@ -29,6 +29,14 @@ public final class CppFileTypes {
   public static final FileType C_SOURCE = FileType.of(".c");
   public static final FileType OBJC_SOURCE = FileType.of(".m");
   public static final FileType OBJCPP_SOURCE = FileType.of(".mm");
+  public static final FileType ALL_C_CLASS_SOURCE =
+      FileType.of(
+          FileTypeSet.of(
+                  CppFileTypes.CPP_SOURCE,
+                  CppFileTypes.C_SOURCE,
+                  CppFileTypes.OBJCPP_SOURCE,
+                  CppFileTypes.OBJC_SOURCE)
+              .getExtensions());
 
   // Filetypes that generate LLVM bitcode when -flto is specified.
   public static final FileTypeSet LTO_SOURCE =

src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java

@@ -4937,6 +4937,85 @@ public void testAdditionalInputs() throws Exception {
     assertThat(target.get(CcInfo.PROVIDER).getCcLinkingContext().getNonCodeInputs()).hasSize(1);
   }
 
+  @Test
+  public void testPossibleSrcsExtensions() throws Exception {
+    doTestPossibleExtensions("srcs", CppFileTypes.ALL_C_CLASS_SOURCE.getExtensions());
+  }
+
+  @Test
+  public void testPossiblePrivateHdrExtensions() throws Exception {
+    doTestPossibleExtensions("private_hdrs", CppFileTypes.CPP_HEADER.getExtensions());
+  }
+
+  @Test
+  public void testPossiblePublicHdrExtensions() throws Exception {
+    doTestPossibleExtensions("public_hdrs", CppFileTypes.CPP_HEADER.getExtensions());
+  }
+
+  private void doTestPossibleExtensions(String attrName, List<String> extensions) throws Exception {
+    createFiles(scratch, "tools/build_defs/foo");
+    reporter.removeHandler(failFastHandler);
+
+    for (String extension : extensions) {
+      scratch.deleteFile("bar/BUILD");
+      scratch.file(
+          "bar/BUILD",
+          "load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
+          "cc_skylark_library(",
+          "    name = 'skylark_lib',",
+          "    " + attrName + " = ['file" + extension + "'],",
+          ")");
+      getConfiguredTarget("//bar:skylark_lib");
+      assertNoEvents();
+    }
+  }
+
+  @Test
+  public void testWrongSrcsExtensionGivesError() throws Exception {
+    doTestWrongExtension("srcs");
+  }
+
+  @Test
+  public void testWrongPrivateHdrExtensionGivesError() throws Exception {
+    doTestWrongExtension("private_hdrs");
+  }
+
+  @Test
+  public void testWrongPublicHdrExtensionGivesError() throws Exception {
+    doTestWrongExtension("public_hdrs");
+  }
+
+  private void doTestWrongExtension(String attrName) throws Exception {
+    createFiles(scratch, "tools/build_defs/foo");
+    scratch.file(
+        "bar/BUILD",
+        "load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
+        "cc_skylark_library(",
+        "    name = 'skylark_lib',",
+        "    " + attrName + " = ['skylark_lib.cannotpossiblybevalid'],",
+        ")");
+    reporter.removeHandler(failFastHandler);
+    getConfiguredTarget("//bar:skylark_lib");
+    assertContainsEvent(
+        "has wrong extension. The list of possible extensions for '" + attrName + "'");
+  }
+
+  @Test
+  public void testWrongSrcExtensionGivesError() throws Exception {
+    createFiles(scratch, "tools/build_defs/foo");
+
+    scratch.file(
+        "bar/BUILD",
+        "load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
+        "cc_skylark_library(",
+        "    name = 'skylark_lib',",
+        "    srcs = ['skylark_lib.qweqwe'],",
+        ")");
+    reporter.removeHandler(failFastHandler);
+    getConfiguredTarget("//bar:skylark_lib");
+    assertContainsEvent("The list of possible extensions for 'srcs'");
+  }
+
   @Test
   public void testFlagWhitelist() throws Exception {
     setSkylarkSemanticsOptions("--experimental_cc_skylark_api_enabled_packages=\"\"");
@@ -5009,10 +5088,12 @@ private static void createFiles(
         "    files_to_build = []",
         "    files_to_build.extend(compilation_outputs.pic_objects)",
         "    files_to_build.extend(compilation_outputs.objects)",
-        "    library_to_link = linking_outputs.library_to_link",
-        "    if library_to_link.pic_static_library != None:",
-        "       files_to_build.append(library_to_link.pic_static_library)",
-        "    files_to_build.append(library_to_link.dynamic_library)",
+        "    library_to_link = None",
+        "    if len(ctx.files.srcs) > 0:",
+        "        library_to_link = linking_outputs.library_to_link",
+        "        if library_to_link.pic_static_library != None:",
+        "            files_to_build.append(library_to_link.pic_static_library)",
+        "        files_to_build.append(library_to_link.dynamic_library)",
         "    return [MyInfo(libraries=[library_to_link]),",
         "            DefaultInfo(files=depset(files_to_build)),",
         "            CcInfo(compilation_context=compilation_context,",