Skip to content

Enhance split_string to support splitting on white space [TG-2922] #2071

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 19, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ set_target_properties(
mmcc
pointer-analysis
solvers
string_utils
test-bigint
testing-utils
unit
Expand Down
1 change: 0 additions & 1 deletion unit/.gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Unit test binaries
miniBDD
sharing_node
string_utils
unit_tests
12 changes: 0 additions & 12 deletions unit/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ file(GLOB_RECURSE testing_utils "testing-utils/*.cpp" "testing-utils/*.h")
list(REMOVE_ITEM sources
# Used in executables
${CMAKE_CURRENT_SOURCE_DIR}/miniBDD.cpp
${CMAKE_CURRENT_SOURCE_DIR}/string_utils.cpp

# Don't build
${CMAKE_CURRENT_SOURCE_DIR}/sharing_map.cpp
Expand Down Expand Up @@ -63,14 +62,3 @@ target_include_directories(miniBDD
target_link_libraries(miniBDD solvers ansi-c)
add_test(NAME miniBDD COMMAND $<TARGET_FILE:miniBDD>)
set_tests_properties(miniBDD PROPERTIES LABELS "CORE;CBMC")

add_executable(string_utils string_utils.cpp)
target_include_directories(string_utils
PUBLIC
${CBMC_BINARY_DIR}
${CBMC_SOURCE_DIR}
${CMAKE_CURRENT_SOURCE_DIR}
)
target_link_libraries(string_utils solvers ansi-c)
add_test(NAME string_utils COMMAND $<TARGET_FILE:string_utils>)
set_tests_properties(string_utils PROPERTIES LABELS "CORE;CBMC")
11 changes: 3 additions & 8 deletions unit/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
# Source files for test utilities
SRC = unit_tests.cpp \
catch_example.cpp \
util/expr_iterator.cpp \
util/optional.cpp \
analyses/call_graph.cpp \
java_bytecode/java_bytecode_convert_class/convert_abstract_class.cpp \
# Empty last line

# Test source files
Expand Down Expand Up @@ -48,8 +44,11 @@ SRC += unit_tests.cpp \
util/irep.cpp \
util/irep_sharing.cpp \
util/message.cpp \
util/optional.cpp \
util/parameter_indices.cpp \
util/simplify_expr.cpp \
util/string_utils/split_string.cpp \
util/string_utils/strip_string.cpp \
util/symbol_table.cpp \
catch_example.cpp \
java_bytecode/java_virtual_functions/virtual_functions.cpp \
Expand Down Expand Up @@ -88,7 +87,6 @@ OBJ += $(CPROVER_LIBS) testing-utils/testing-utils$(LIBEXT)

TESTS = unit_tests$(EXEEXT) \
miniBDD$(EXEEXT) \
string_utils$(EXEEXT) \
# Empty last line

CLEANFILES = $(TESTS)
Expand All @@ -107,6 +105,3 @@ unit_tests$(EXEEXT): $(OBJ)

miniBDD$(EXEEXT): miniBDD$(OBJEXT) $(CPROVER_LIBS)
$(LINKBIN)

string_utils$(EXEEXT): string_utils$(OBJEXT) $(CPROVER_LIBS)
$(LINKBIN)
50 changes: 0 additions & 50 deletions unit/string_utils.cpp

This file was deleted.

158 changes: 158 additions & 0 deletions unit/util/string_utils/split_string.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/*
Author: Diffblue Ltd.
*/

/// \file
/// split_string Unit Tests

#include <testing-utils/catch.hpp>
#include <util/string_utils.h>

struct expected_resultst
{
std::vector<std::string> no_strip_no_remove;
std::vector<std::string> strip_no_remove;
std::vector<std::string> no_strip_remove_empty;
std::vector<std::string> strip_remove_empty;
};

/// For a given string and delimiter, use the split_string function with all
/// the possible combinations of stripping whitespace and removing empty
/// elements.
/// \param string: The string to split
/// \param delimiter: The delimter to split on
/// \param expected_results: The results expected for each of the versions of
/// the method
void run_on_all_variants(
std::string string,
char delimiter,
const expected_resultst &expected_results)
{
WHEN("Not stripping, not removing empty")
{
std::vector<std::string> result;
split_string(string, delimiter, result, false, false);

THEN("Should get expected vector")
{
REQUIRE_THAT(
result,
// NOLINTNEXTLINE(whitespace/braces)
Catch::Matchers::Vector::EqualsMatcher<std::string>{
expected_results.no_strip_no_remove});
}
}
WHEN("Not stripping, removing empty")
{
std::vector<std::string> result;
split_string(string, delimiter, result, false, true);

THEN("Should get expected vector")
{
REQUIRE_THAT(
result,
// NOLINTNEXTLINE(whitespace/braces)
Catch::Matchers::Vector::EqualsMatcher<std::string>{
expected_results.no_strip_remove_empty});
}
}
WHEN("Stripping, not removing empty")
{
std::vector<std::string> result;
split_string(string, delimiter, result, true, false);

THEN("Should get expected vector")
{
REQUIRE_THAT(
result,
// NOLINTNEXTLINE(whitespace/braces)
Catch::Matchers::Vector::EqualsMatcher<std::string>{
expected_results.strip_no_remove});
}
}
WHEN("Stripping and removing empty")
{
std::vector<std::string> result;
split_string(string, delimiter, result, true, true);

THEN("Should get expected vector")
{
REQUIRE_THAT(
result,
// NOLINTNEXTLINE(whitespace/braces)
Catch::Matchers::Vector::EqualsMatcher<std::string>{
expected_results.strip_remove_empty});
}
}
}

SCENARIO("split_string", "[core][utils][string_utils][split_string]")
{
GIVEN("A non whitespace delimited string with two elements")
{
const char delimiter = ',';
const std::string string = " a,, x , ,";

expected_resultst expected_results;
expected_results.no_strip_no_remove = {" a", "", " x ", " ", ""};
expected_results.strip_no_remove = {"a", "", "x", "", ""};
expected_results.no_strip_remove_empty = {" a", " x ", " "};
expected_results.strip_remove_empty = {"a", "x"};

run_on_all_variants(string, delimiter, expected_results);
}
GIVEN("An empty string")
{
std::string string = "";
WHEN("Splitting it")
{
expected_resultst expected_results;
expected_results.no_strip_no_remove = {""};
expected_results.strip_no_remove = {""};

// TODO(tkiley): This is probably wrong, since I'd expect removing empty
// TODO(tkiley): elements to return an empty vector here.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, we could change this to return an empty vector. I've looked at the places where split_string() is currently used, and they would also work when returning an empty vector (except an assertion assert(!result.empty()) would need to be removed in util/unwind.cpp). Also once changed, the current usages of split_string() can be slightly simplified as some of them have a check for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent. Since this PR is already a pre-req for another I will put together a separate PR to address this.

expected_results.no_strip_remove_empty = {""};
expected_results.strip_remove_empty = {""};

run_on_all_variants(string, ',', expected_results);
}
}
GIVEN("A whitespace only string")
{
std::string string = " ";
WHEN("Splitting it")
{
expected_resultst expected_results;
expected_results.no_strip_no_remove = {" "};
expected_results.strip_no_remove = {""};
expected_results.no_strip_remove_empty = {" "};
// TODO(tkiley): This is probably wrong, since I'd expect removing empty
// TODO(tkiley): elements to return an empty vector here.
expected_results.strip_remove_empty = {""};

run_on_all_variants(string, ',', expected_results);
}
}
}

SCENARIO("split_string into two", "[core][utils][string_utils][split_string]")
{
GIVEN("A string with one separator")
{
const char separator = ':';
std::string string = "a:b";

WHEN("Splitting into two strings")
{
std::string s1;
std::string s2;
split_string(string, separator, s1, s2, false);
THEN("The string should be split")
{
REQUIRE(s1 == "a");
REQUIRE(s2 == "b");
}
}
}
}
27 changes: 27 additions & 0 deletions unit/util/string_utils/strip_string.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
Author: Diffblue Ltd.
*/

/// \file
/// strip_string Unit Tests

#include <testing-utils/catch.hpp>
#include <util/string_utils.h>

SCENARIO("strip_string", "[core][utils][string_utils][strip_string]")
{
GIVEN("A string with some whitespace in it")
{
std::string string = " x y ";
WHEN("Using strip_string")
{
std::string result = strip_string(string);
THEN(
"Whitespace at either end should be removed, but not internal "
"whitespace")
{
REQUIRE(result == "x y");
}
}
}
}