GoogleTest: Fix escaping in test names

Due to add_command() being a macro it introduced excessive and
nonobvious escaping in different parts of the script. Because of
one of such places the resulting script would have an erroneous
${TEST_LIST} if the user data (in test parameters) had a semicolon.

To eliminate this non-obvious escaping, add_command() was converted
to function. Updated the escaping accordingly.

Fixes: #23059

8 files changed, 147 insertions, 24 deletions

diff --git a/Modules/GoogleTestAddTests.cmake b/Modules/GoogleTestAddTests.cmake
index cef2e8a..2bd0cc9 100644
--- a/Modules/GoogleTestAddTests.cmake
+++ b/Modules/GoogleTestAddTests.cmake
@@ -9,7 +9,7 @@ set(flush_tests_MODE WRITE)
 # Flushes script to ${_CTEST_FILE}
 macro(flush_script)
   file(${flush_tests_MODE} "${_CTEST_FILE}" "${script}")
-  set(flush_tests_MODE APPEND)
+  set(flush_tests_MODE APPEND PARENT_SCOPE)
 
   set(script "")
 endmacro()
@@ -20,24 +20,22 @@ macro(flush_tests_buffer)
   set(tests_buffer "")
 endmacro()
 
-macro(add_command NAME TEST_NAME)
-  set(_args "")
-  foreach(_arg ${ARGN})
-    if(_arg MATCHES "[^-./:a-zA-Z0-9_]")
-      string(APPEND _args " [==[${_arg}]==]")
+function(add_command NAME TEST_NAME)
+  set(args "")
+  foreach(arg ${ARGN})
+    if(arg MATCHES "[^-./:a-zA-Z0-9_]")
+      string(APPEND args " [==[${arg}]==]")
     else()
-      string(APPEND _args " ${_arg}")
+      string(APPEND args " ${arg}")
     endif()
   endforeach()
-  string(APPEND script "${NAME}(${TEST_NAME} ${_args})\n")
-  string(LENGTH "${script}" _script_len)
-  if(${_script_len} GREATER "50000")
+  string(APPEND script "${NAME}(${TEST_NAME} ${args})\n")
+  string(LENGTH "${script}" script_len)
+  if(${script_len} GREATER "50000")
     flush_script()
   endif()
-  # Unsets macro local variables to prevent leakage outside of this macro.
-  unset(_args)
-  unset(_script_len)
-endmacro()
+  set(script "${script}" PARENT_SCOPE)
+endfunction()
 
 function(generate_testname_guards OUTPUT OPEN_GUARD_VAR CLOSE_GUARD_VAR)
   set(open_guard "[=[")
@@ -164,7 +162,6 @@ function(gtest_discover_tests_impl)
         endif()
 
         string(CONFIGURE "${test_name_template}" testname)
-        # sanitize test name for further processing downstream
         # unescape []
         if(open_sb)
           string(REPLACE "${open_sb}" "[" testname "${testname}")
@@ -172,13 +169,9 @@ function(gtest_discover_tests_impl)
         if(close_sb)
           string(REPLACE "${close_sb}" "]" testname "${testname}")
         endif()
-        # escape \
-        string(REPLACE [[\]] [[\\]] testname "${testname}")
-        # escape $
-        string(REPLACE [[$]] [[\$]] testname "${testname}")
         set(guarded_testname "${open_guard}${testname}${close_guard}")
 
-        # ...and add to script
+        # add to script
         add_command(add_test
           "${guarded_testname}"
           ${_TEST_EXECUTOR}
@@ -199,14 +192,14 @@ function(gtest_discover_tests_impl)
           "${guarded_testname}"
           PROPERTIES
           WORKING_DIRECTORY "${_TEST_WORKING_DIR}"
-          SKIP_REGULAR_EXPRESSION "\\\\[  SKIPPED \\\\]"
+          SKIP_REGULAR_EXPRESSION "\\[  SKIPPED \\]"
           ${properties}
         )
 
-        # possibly unbalanced square brackets render lists invalid so skip such tests in _TEST_LIST
+        # possibly unbalanced square brackets render lists invalid so skip such tests in ${_TEST_LIST}
         if(NOT "${testname}" MATCHES [=[(\[|\])]=])
           # escape ;
-          string(REPLACE [[;]] [[\;]] testname "${testname}")
+          string(REPLACE [[;]] [[\\;]] testname "${testname}")
           list(APPEND tests_buffer "${testname}")
           list(LENGTH tests_buffer tests_buffer_length)
           if(${tests_buffer_length} GREATER "250")
@@ -221,7 +214,7 @@ function(gtest_discover_tests_impl)
   # Create a list of all discovered tests, which users may use to e.g. set
   # properties on the tests
   flush_tests_buffer()
-  add_command(set "" ${_TEST_LIST} ${tests})
+  add_command(set "" ${_TEST_LIST} "${tests}")
 
   # Write CTest script
   flush_script()
diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-check-test-list.cmake b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-check-test-list.cmake
new file mode 100644
index 0000000..94169e7
--- /dev/null
+++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-check-test-list.cmake
@@ -0,0 +1,6 @@
+list(LENGTH test_list_test_TESTS LIST_SIZE)
+set(EXPECTED_SIZE 2)
+if(NOT LIST_SIZE EQUAL ${EXPECTED_SIZE})
+  message("TEST_LIST should have ${EXPECTED_SIZE} elements but it has ${LIST_SIZE}")
+  message("The unexpected list: [${test_list_test_TESTS}]")
+endif()
diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-flush-script-check-list.cmake b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-flush-script-check-list.cmake
new file mode 100644
index 0000000..1ae222f
--- /dev/null
+++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-flush-script-check-list.cmake
@@ -0,0 +1,5 @@
+list(LENGTH flush_script_test_TESTS LIST_SIZE)
+set(EXPECTED_LIST_SIZE 4)
+if(NOT LIST_SIZE EQUAL ${EXPECTED_LIST_SIZE})
+  message("TEST_LIST should have ${EXPECTED_LIST_SIZE} elements but it has ${LIST_SIZE}")
+endif()
diff --git a/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryFlushScript.cmake b/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryFlushScript.cmake
new file mode 100644
index 0000000..2c138c7
--- /dev/null
+++ b/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryFlushScript.cmake
@@ -0,0 +1,14 @@
+enable_language(CXX)
+include(GoogleTest)
+
+enable_testing()
+
+include(xcode_sign_adhoc.cmake)
+
+add_executable(flush_script_test flush_script_test.cpp)
+xcode_sign_adhoc(flush_script_test)
+gtest_discover_tests(
+  flush_script_test
+)
+set_property(DIRECTORY APPEND PROPERTY TEST_INCLUDE_FILES
+  ${CMAKE_CURRENT_SOURCE_DIR}/GoogleTest-discovery-flush-script-check-list.cmake)
diff --git a/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTestList.cmake b/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTestList.cmake
new file mode 100644
index 0000000..5f4f859
--- /dev/null
+++ b/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTestList.cmake
@@ -0,0 +1,14 @@
+enable_language(CXX)
+include(GoogleTest)
+
+enable_testing()
+
+include(xcode_sign_adhoc.cmake)
+
+add_executable(test_list_test test_list_test.cpp)
+xcode_sign_adhoc(test_list_test)
+gtest_discover_tests(
+  test_list_test
+)
+set_property(DIRECTORY APPEND PROPERTY TEST_INCLUDE_FILES
+  ${CMAKE_CURRENT_SOURCE_DIR}/GoogleTest-discovery-check-test-list.cmake)
diff --git a/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake b/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake
index 33a4b43..695f562 100644
--- a/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake
+++ b/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake
@@ -236,6 +236,58 @@ function(run_GoogleTest_discovery_multi_config)
 
 endfunction()
 
+function(run_GoogleTest_discovery_test_list DISCOVERY_MODE)
+  # Use a single build tree for a few tests without cleaning.
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/GoogleTest-discovery-test-list-build)
+  set(RunCMake_TEST_NO_CLEAN 1)
+  if(NOT RunCMake_GENERATOR_IS_MULTI_CONFIG)
+    set(RunCMake_TEST_OPTIONS -DCMAKE_BUILD_TYPE=Debug)
+  endif()
+  file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}")
+  file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}")
+
+  run_cmake_with_options(GoogleTestDiscoveryTestList -DCMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE=${DISCOVERY_MODE})
+
+  run_cmake_command(GoogleTest-discovery-test-list-build
+    ${CMAKE_COMMAND}
+    --build .
+    --config Debug
+    --target test_list_test
+  )
+
+  run_cmake_command(GoogleTest-discovery-test-list-test
+    ${CMAKE_CTEST_COMMAND}
+    -C Debug
+    --no-label-summary
+  )
+endfunction()
+
+function(run_GoogleTest_discovery_flush_script DISCOVERY_MODE)
+  # Use a single build tree for a few tests without cleaning.
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/GoogleTest-discovery-flush-script-build)
+  set(RunCMake_TEST_NO_CLEAN 1)
+  if(NOT RunCMake_GENERATOR_IS_MULTI_CONFIG)
+    set(RunCMake_TEST_OPTIONS -DCMAKE_BUILD_TYPE=Debug)
+  endif()
+  file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}")
+  file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}")
+
+  run_cmake_with_options(GoogleTestDiscoveryFlushScript -DCMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE=${DISCOVERY_MODE})
+
+  run_cmake_command(GoogleTest-discovery-flush-script-build
+    ${CMAKE_COMMAND}
+    --build .
+    --config Debug
+    --target flush_script_test
+  )
+
+  run_cmake_command(GoogleTest-discovery-flush-script-test
+    ${CMAKE_CTEST_COMMAND}
+    -C Debug
+    --no-label-summary
+  )
+endfunction()
+
 foreach(DISCOVERY_MODE POST_BUILD PRE_TEST)
   message("Testing ${DISCOVERY_MODE} discovery mode via CMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE global override...")
   run_GoogleTest(${DISCOVERY_MODE})
@@ -246,6 +298,8 @@ foreach(DISCOVERY_MODE POST_BUILD PRE_TEST)
       NOT "${DISCOVERY_MODE};${RunCMake_GENERATOR}" MATCHES "^POST_BUILD;Visual Studio 9")
     run_GoogleTest_discovery_arg_change(${DISCOVERY_MODE})
   endif()
+  run_GoogleTest_discovery_test_list(${DISCOVERY_MODE})
+  run_GoogleTest_discovery_flush_script(${DISCOVERY_MODE})
 endforeach()
 
 if(RunCMake_GENERATOR_IS_MULTI_CONFIG)
diff --git a/Tests/RunCMake/GoogleTest/flush_script_test.cpp b/Tests/RunCMake/GoogleTest/flush_script_test.cpp
new file mode 100644
index 0000000..9473bb5
--- /dev/null
+++ b/Tests/RunCMake/GoogleTest/flush_script_test.cpp
@@ -0,0 +1,19 @@
+#include <iostream>
+#include <string>
+
+int main(int argc, char** argv)
+{
+  // Note: GoogleTest.cmake doesn't actually depend on Google Test as such;
+  // it only requires that we produces output in the expected format when
+  // invoked with --gtest_list_tests. Thus, we fake that here. This allows us
+  // to test the module without actually needing Google Test.
+  if (argc > 1 && std::string(argv[1]) == "--gtest_list_tests") {
+    std::cout << "flush_script_test.\n";
+    const size_t flushThreshold = 50000;
+    const size_t testCaseNum = 4;
+    std::string testName(flushThreshold / (testCaseNum - 1), 'T');
+    for (size_t i = 0; i < testCaseNum; ++i)
+      std::cout << "  " << testName.c_str() << "\n";
+  }
+  return 0;
+}
diff --git a/Tests/RunCMake/GoogleTest/test_list_test.cpp b/Tests/RunCMake/GoogleTest/test_list_test.cpp
new file mode 100644
index 0000000..c9f9512
--- /dev/null
+++ b/Tests/RunCMake/GoogleTest/test_list_test.cpp
@@ -0,0 +1,18 @@
+#include <iostream>
+#include <string>
+
+int main(int argc, char** argv)
+{
+  // Note: GoogleTest.cmake doesn't actually depend on Google Test as such;
+  // it only requires that we produces output in the expected format when
+  // invoked with --gtest_list_tests. Thus, we fake that here. This allows us
+  // to test the module without actually needing Google Test.
+  if (argc > 1 && std::string(argv[1]) == "--gtest_list_tests") {
+    std::cout << "test_list_test/test.\n";
+    std::cout << "  case/0  # GetParam() = \"semicolon;\"\n";
+    std::cout << "  case/1  # GetParam() = 'osb['\n";
+    std::cout << "  case/2  # GetParam() = 'csb]'\n";
+    std::cout << "  case/3  # GetParam() = 'S p a c e s'\n";
+  }
+  return 0;
+}