CMP0037: Extend policy to reserved names and custom targets

Teach add_custom_target to check the policy too.  Extend the policy to
disallow reserved target names that we use for builtin targets like
"all".

Extend the RunCMake.CMP0037 test to cover these cases.

7 files changed, 101 insertions, 13 deletions

diff --git a/Source/cmAddCustomTargetCommand.cxx b/Source/cmAddCustomTargetCommand.cxx
index 2a683a4..ef62523 100644
--- a/Source/cmAddCustomTargetCommand.cxx
+++ b/Source/cmAddCustomTargetCommand.cxx
@@ -11,6 +11,9 @@
 ============================================================================*/
 #include "cmAddCustomTargetCommand.h"
 
+#include "cmGeneratorExpression.h"
+#include "cmGlobalGenerator.h"
+
 // cmAddCustomTargetCommand
 bool cmAddCustomTargetCommand
 ::InitialPass(std::vector<std::string> const& args,
@@ -22,11 +25,13 @@ bool cmAddCustomTargetCommand
     return false;
     }
 
+  std::string targetName = args[0];
+
   // Check the target name.
-  if(args[0].find_first_of("/\\") != args[0].npos)
+  if(targetName.find_first_of("/\\") != targetName.npos)
     {
     cmOStringStream e;
-    e << "called with invalid target name \"" << args[0]
+    e << "called with invalid target name \"" << targetName
       << "\".  Target names may not contain a slash.  "
       << "Use ADD_CUSTOM_COMMAND to generate files.";
     this->SetError(e.str().c_str());
@@ -138,16 +143,59 @@ bool cmAddCustomTargetCommand
       }
     }
 
-  std::string::size_type pos = args[0].find_first_of("#<>");
-  if(pos != args[0].npos)
+  std::string::size_type pos = targetName.find_first_of("#<>");
+  if(pos != targetName.npos)
     {
     cmOStringStream msg;
-    msg << "called with target name containing a \"" << args[0][pos]
+    msg << "called with target name containing a \"" << targetName[pos]
         << "\".  This character is not allowed.";
     this->SetError(msg.str().c_str());
     return false;
     }
 
+  // Some requirements on custom target names already exist
+  // and have been checked at this point.
+  // The following restrictions overlap but depend on policy CMP0037.
+  bool nameOk = cmGeneratorExpression::IsValidTargetName(targetName) &&
+    !cmGlobalGenerator::IsReservedTarget(targetName);
+  if (nameOk)
+    {
+    nameOk = targetName.find(":") == std::string::npos;
+    }
+  if (!nameOk)
+    {
+    cmake::MessageType messageType = cmake::AUTHOR_WARNING;
+    bool issueMessage = false;
+    switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0037))
+      {
+      case cmPolicies::WARN:
+        issueMessage = true;
+      case cmPolicies::OLD:
+        break;
+      case cmPolicies::NEW:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::REQUIRED_ALWAYS:
+        issueMessage = true;
+        messageType = cmake::FATAL_ERROR;
+      }
+    if (issueMessage)
+      {
+      cmOStringStream e;
+      e << (this->Makefile->GetPolicies()
+           ->GetPolicyWarning(cmPolicies::CMP0037)) << "\n";
+      e << "The target name \"" << targetName <<
+          "\" is reserved or not valid for certain "
+          "CMake features, such as generator expressions, and may result "
+          "in undefined behavior.";
+      this->Makefile->IssueMessage(messageType, e.str().c_str());
+
+      if (messageType == cmake::FATAL_ERROR)
+        {
+        return false;
+        }
+      }
+    }
+
   // Store the last command line finished.
   if(!currentLine.empty())
     {
@@ -158,7 +206,7 @@ bool cmAddCustomTargetCommand
   // Enforce name uniqueness.
   {
   std::string msg;
-  if(!this->Makefile->EnforceUniqueName(args[0], msg, true))
+  if(!this->Makefile->EnforceUniqueName(targetName, msg, true))
     {
     this->SetError(msg.c_str());
     return false;
@@ -176,7 +224,7 @@ bool cmAddCustomTargetCommand
   // Add the utility target to the makefile.
   bool escapeOldStyle = !verbatim;
   cmTarget* target =
-    this->Makefile->AddUtilityCommand(args[0].c_str(), excludeFromAll,
+    this->Makefile->AddUtilityCommand(targetName.c_str(), excludeFromAll,
                                       working_directory.c_str(), depends,
                                       commandLines, escapeOldStyle, comment);
 
diff --git a/Source/cmAddExecutableCommand.cxx b/Source/cmAddExecutableCommand.cxx
index a93e834..a352be0 100644
--- a/Source/cmAddExecutableCommand.cxx
+++ b/Source/cmAddExecutableCommand.cxx
@@ -69,7 +69,9 @@ bool cmAddExecutableCommand
       }
     }
 
-  bool nameOk = cmGeneratorExpression::IsValidTargetName(exename);
+  bool nameOk = cmGeneratorExpression::IsValidTargetName(exename) &&
+    !cmGlobalGenerator::IsReservedTarget(exename);
+
   if (nameOk && !importTarget && !isAlias)
     {
     nameOk = exename.find(":") == std::string::npos;
@@ -95,7 +97,8 @@ bool cmAddExecutableCommand
       cmOStringStream e;
       e << (this->Makefile->GetPolicies()
             ->GetPolicyWarning(cmPolicies::CMP0037)) << "\n";
-      e << "The target name \"" << exename << "\" is not valid for certain "
+      e << "The target name \"" << exename <<
+          "\" is reserved or not valid for certain "
           "CMake features, such as generator expressions, and may result "
           "in undefined behavior.";
       this->Makefile->IssueMessage(messageType, e.str().c_str());
diff --git a/Source/cmAddLibraryCommand.cxx b/Source/cmAddLibraryCommand.cxx
index e9c5d6b..0f98f35 100644
--- a/Source/cmAddLibraryCommand.cxx
+++ b/Source/cmAddLibraryCommand.cxx
@@ -109,7 +109,9 @@ bool cmAddLibraryCommand
       }
     }
 
-  bool nameOk = cmGeneratorExpression::IsValidTargetName(libName);
+  bool nameOk = cmGeneratorExpression::IsValidTargetName(libName) &&
+    !cmGlobalGenerator::IsReservedTarget(libName);
+
   if (nameOk && !importTarget && !isAlias)
     {
     nameOk = libName.find(":") == std::string::npos;
@@ -135,7 +137,8 @@ bool cmAddLibraryCommand
       cmOStringStream e;
       e << (this->Makefile->GetPolicies()
             ->GetPolicyWarning(cmPolicies::CMP0037)) << "\n";
-      e << "The target name \"" << libName << "\" is not valid for certain "
+      e << "The target name \"" << libName <<
+          "\" is reserved or not valid for certain "
           "CMake features, such as generator expressions, and may result "
           "in undefined behavior.";
       this->Makefile->IssueMessage(messageType, e.str().c_str());
diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index b2a0ef7..b76b943 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -2466,6 +2466,37 @@ void cmGlobalGenerator::AddTarget(cmTarget* t)
     }
 }
 
+bool cmGlobalGenerator::IsReservedTarget(std::string const& name)
+{
+  // The following is a list of targets reserved
+  // by one or more of the cmake generators.
+
+  // Adding additional targets to this list will require a policy!
+  const char* reservedTargets[] =
+  {
+    "all", "ALL_BUILD",
+    "help",
+    "install", "INSTALL",
+    "preinstall",
+    "clean",
+    "edit_cache",
+    "rebuild_cache",
+    "test", "RUN_TESTS",
+    "package", "PACKAGE",
+    "package_source",
+    "ZERO_CHECK",
+    0
+  };
+
+  for(const char** reservedTarget = reservedTargets;
+    *reservedTarget; ++reservedTarget)
+    {
+    if(name == *reservedTarget) return true;
+    }
+
+  return false;
+}
+
 void cmGlobalGenerator::SetExternalMakefileProjectGenerator(
                             cmExternalMakefileProjectGenerator *extraGenerator)
 {
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index 2761158..8aebae8 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -239,6 +239,8 @@ public:
 
   void AddTarget(cmTarget* t);
 
+  static bool IsReservedTarget(std::string const& name);
+
   virtual const char* GetAllTargetName()         const { return "ALL_BUILD"; }
   virtual const char* GetInstallTargetName()       const { return "INSTALL"; }
   virtual const char* GetInstallLocalTargetName()  const { return 0; }
diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx
index 7b80d14..a18fc16 100644
--- a/Source/cmPolicies.cxx
+++ b/Source/cmPolicies.cxx
@@ -289,7 +289,7 @@ cmPolicies::cmPolicies()
 
   this->DefinePolicy(
     CMP0037, "CMP0037",
-    "Target names should match a validity pattern.",
+    "Target names should not be reserved and should match a validity pattern.",
     3,0,0,0, cmPolicies::WARN);
 
   this->DefinePolicy(
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index fc239d4..361d820 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -88,7 +88,8 @@ public:
     CMP0034, ///< Disallow command: utility_source
     CMP0035, ///< Disallow command: variable_requires
     CMP0036, ///< Disallow command: build_name
-    CMP0037, ///< Target names should match a validity pattern.
+    CMP0037, ///< Target names should not be reserved and
+    /// should match a validity pattern.
     CMP0038, ///< Targets may not link directly to themselves
     CMP0039, ///< Utility targets may not have link dependencies