while: diagnose errors during condition evaluation

Add a policy to diagnose condition errors in a compatible way.

Fixes: #23296

15 files changed, 111 insertions, 7 deletions

diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst
index 0939a82..dfc5de6 100644
--- a/Help/manual/cmake-policies.7.rst
+++ b/Help/manual/cmake-policies.7.rst
@@ -52,6 +52,14 @@ to determine whether to report an error on use of deprecated macros or
 functions.
 
 
+Policies Introduced by CMake 3.24
+=================================
+
+.. toctree::
+   :maxdepth: 1
+
+   CMP0130: while() diagnoses condition evaluation errors. </policy/CMP0130>
+
 Policies Introduced by CMake 3.23
 =================================
 
diff --git a/Help/policy/CMP0130.rst b/Help/policy/CMP0130.rst
new file mode 100644
index 0000000..0dd5623
--- /dev/null
+++ b/Help/policy/CMP0130.rst
@@ -0,0 +1,32 @@
+CMP0130
+-------
+
+.. versionadded:: 3.24
+
+:command:`while` diagnoses condition evaluation errors.
+
+CMake 3.23 and below accidentally tolerated errors encountered while
+evaluating the condition passed to the :command:`while` command
+(but not the :command:`if` command).  For example, the code
+
+.. code-block:: cmake
+
+  set(paren "(")
+  while(${paren})
+  endwhile()
+
+creates an unbalanced parenthesis during condition evaluation.
+
+CMake 3.24 and above prefer to diagnose such errors.  This policy
+provides compatibility for projects that have not been updated to
+fix their condition errors.
+
+The ``OLD`` behavior for this policy is to ignore errors in
+:command:`while` conditions.  The ``NEW`` behavior for this
+policy is to diagnose errors in :command:`while` conditions.
+
+This policy was introduced in CMake version 3.24.  CMake version |release|
+warns when the policy is not set and uses ``OLD`` behavior.  Use the
+:command:`cmake_policy` command to set it to ``OLD`` or ``NEW`` explicitly.
+
+.. include:: DEPRECATED.txt
diff --git a/Help/release/dev/while-errors.rst b/Help/release/dev/while-errors.rst
new file mode 100644
index 0000000..c39e6e8
--- /dev/null
+++ b/Help/release/dev/while-errors.rst
@@ -0,0 +1,5 @@
+while-errors
+------------
+
+* The :command:`while` command now diagnoses errors during condition
+  evaluation.  See policy :policy:`CMP0130`.
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 99e2eb6..7d4012d 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -388,7 +388,9 @@ class cmMakefile;
          22, 0, cmPolicies::WARN)                                             \
   SELECT(POLICY, CMP0129,                                                     \
          "Compiler id for MCST LCC compilers is now LCC, not GNU.", 3, 23, 0, \
-         cmPolicies::WARN)
+         cmPolicies::WARN)                                                    \
+  SELECT(POLICY, CMP0130, "while() diagnoses condition evaluation errors.",   \
+         3, 24, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \
diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx
index a93a81f..68d5a9a 100644
--- a/Source/cmWhileCommand.cxx
+++ b/Source/cmWhileCommand.cxx
@@ -17,6 +17,8 @@
 #include "cmMakefile.h"
 #include "cmMessageType.h"
 #include "cmOutputConverter.h"
+#include "cmPolicies.h"
+#include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
 #include "cmake.h"
 
@@ -79,9 +81,8 @@ bool cmWhileFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
     return out;
   };
 
-  // FIXME(#23296): For compatibility with older versions of CMake, we
-  // tolerate condition errors that evaluate to false.  We should add
-  // a policy to enforce such errors.
+  // For compatibility with projects that do not set CMP0130 to NEW,
+  // we tolerate condition errors that evaluate to false.
   bool enforceError = true;
   std::string errorString;
   MessageType messageType;
@@ -110,14 +111,38 @@ bool cmWhileFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
     }
   }
 
+  if (!errorString.empty() && !enforceError) {
+    // This error should only be enforced if CMP0130 is NEW.
+    switch (mf.GetPolicyStatus(cmPolicies::CMP0130)) {
+      case cmPolicies::WARN:
+        // Convert the error to a warning and enforce it.
+        messageType = MessageType::AUTHOR_WARNING;
+        enforceError = true;
+        break;
+      case cmPolicies::OLD:
+        // OLD behavior is to silently ignore the error.
+        break;
+      case cmPolicies::REQUIRED_ALWAYS:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::NEW:
+        // NEW behavior is to enforce the error.
+        enforceError = true;
+        break;
+    }
+  }
+
   if (!errorString.empty() && enforceError) {
-    std::string err = "had incorrect arguments:\n ";
+    std::string err = "while() given incorrect arguments:\n ";
     for (auto const& i : expandedArguments) {
       err += " ";
       err += cmOutputConverter::EscapeForCMake(i.GetValue());
     }
     err += "\n";
     err += errorString;
+    if (mf.GetPolicyStatus(cmPolicies::CMP0130) == cmPolicies::WARN) {
+      err =
+        cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0130), '\n', err);
+    }
     mf.GetCMakeInstance()->IssueMessage(messageType, err, whileBT);
     if (messageType == MessageType::FATAL_ERROR) {
       cmSystemTools::SetFatalErrorOccured();
diff --git a/Tests/RunCMake/while/CMP0130-NEW-result.txt b/Tests/RunCMake/while/CMP0130-NEW-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/while/CMP0130-NEW-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/while/CMP0130-NEW-stderr.txt b/Tests/RunCMake/while/CMP0130-NEW-stderr.txt
new file mode 100644
index 0000000..df492d5
--- /dev/null
+++ b/Tests/RunCMake/while/CMP0130-NEW-stderr.txt
@@ -0,0 +1,9 @@
+^CMake Error at CMP0130-common.cmake:[0-9]+ \(while\):
+  while\(\) given incorrect arguments:
+
+    "\("
+
+  mismatched parenthesis in condition
+Call Stack \(most recent call first\):
+  CMP0130-NEW.cmake:[0-9]+ \(include\)
+  CMakeLists.txt:[0-9]+ \(include\)$
diff --git a/Tests/RunCMake/while/CMP0130-NEW.cmake b/Tests/RunCMake/while/CMP0130-NEW.cmake
new file mode 100644
index 0000000..3cc5027
--- /dev/null
+++ b/Tests/RunCMake/while/CMP0130-NEW.cmake
@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0130 NEW)
+include(CMP0130-common.cmake)
diff --git a/Tests/RunCMake/while/unbalanced-parenthesis-stdout.txt b/Tests/RunCMake/while/CMP0130-OLD-stdout.txt
index d45e194..d45e194 100644
--- a/Tests/RunCMake/while/unbalanced-parenthesis-stdout.txt
+++ b/Tests/RunCMake/while/CMP0130-OLD-stdout.txt
diff --git a/Tests/RunCMake/while/CMP0130-OLD.cmake b/Tests/RunCMake/while/CMP0130-OLD.cmake
new file mode 100644
index 0000000..15a4290
--- /dev/null
+++ b/Tests/RunCMake/while/CMP0130-OLD.cmake
@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0130 OLD)
+include(CMP0130-common.cmake)
diff --git a/Tests/RunCMake/while/CMP0130-WARN-stderr.txt b/Tests/RunCMake/while/CMP0130-WARN-stderr.txt
new file mode 100644
index 0000000..bc88c5e
--- /dev/null
+++ b/Tests/RunCMake/while/CMP0130-WARN-stderr.txt
@@ -0,0 +1,14 @@
+^CMake Warning \(dev\) at CMP0130-common.cmake:[0-9]+ \(while\):
+  Policy CMP0130 is not set: while\(\) diagnoses condition evaluation errors.
+  Run "cmake --help-policy CMP0130" for policy details.  Use the cmake_policy
+  command to set the policy and suppress this warning.
+
+  while\(\) given incorrect arguments:
+
+    "\("
+
+  mismatched parenthesis in condition
+Call Stack \(most recent call first\):
+  CMP0130-WARN.cmake:[0-9]+ \(include\)
+  CMakeLists.txt:[0-9]+ \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.$
diff --git a/Tests/RunCMake/while/CMP0130-WARN-stdout.txt b/Tests/RunCMake/while/CMP0130-WARN-stdout.txt
new file mode 100644
index 0000000..d45e194
--- /dev/null
+++ b/Tests/RunCMake/while/CMP0130-WARN-stdout.txt
@@ -0,0 +1 @@
+-- Code incorrectly accepted
diff --git a/Tests/RunCMake/while/CMP0130-WARN.cmake b/Tests/RunCMake/while/CMP0130-WARN.cmake
new file mode 100644
index 0000000..562c25d
--- /dev/null
+++ b/Tests/RunCMake/while/CMP0130-WARN.cmake
@@ -0,0 +1,2 @@
+# CMP0130 left unset
+include(CMP0130-common.cmake)
diff --git a/Tests/RunCMake/while/unbalanced-parenthesis.cmake b/Tests/RunCMake/while/CMP0130-common.cmake
index 39d736b..15322e0 100644
--- a/Tests/RunCMake/while/unbalanced-parenthesis.cmake
+++ b/Tests/RunCMake/while/CMP0130-common.cmake
@@ -3,5 +3,4 @@ while(${paren})
   message(STATUS "Condition incorrectly true")
   break()
 endwhile()
-# FIXME(#23296): The above condition error is tolerated for compatibility.
 message(STATUS "Code incorrectly accepted")
diff --git a/Tests/RunCMake/while/RunCMakeTest.cmake b/Tests/RunCMake/while/RunCMakeTest.cmake
index bb9b991..d018b16 100644
--- a/Tests/RunCMake/while/RunCMakeTest.cmake
+++ b/Tests/RunCMake/while/RunCMakeTest.cmake
@@ -6,4 +6,6 @@ run_cmake(EndMismatch)
 run_cmake(EndAlone)
 run_cmake(EndAloneArgs)
 
-run_cmake(unbalanced-parenthesis)
+run_cmake(CMP0130-OLD)
+run_cmake(CMP0130-WARN)
+run_cmake(CMP0130-NEW)