Skip to content

Commit

Permalink
fix some warnings generated from klocwork static analyzer (#350)
Browse files Browse the repository at this point in the history 
* fix some warnings generated from klocwork static analyzer

* Some more visual studio static analyzer and clang-tidy fixes

* some formatting updates
  • Loading branch information
@phlptp @henryiii
phlptp authored and henryiii committed Nov 29, 2019
1 parent d621658 commit 5f69659
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 98 deletions.
5 changes: 3 additions & 2 deletions include/CLI/App.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -751,10 +751,11 @@ class App {
std::string flag_description = "") {

CLI::callback_t fun = [function](const CLI::results_t &res) {
bool trigger;
bool trigger{false};
auto result = CLI::detail::lexical_cast(res[0], trigger);
if(trigger)
if(result && trigger) {
function();
}
return result;
};
return _add_flag_internal(flag_name, std::move(fun), std::move(flag_description));
Expand Down
150 changes: 87 additions & 63 deletions include/CLI/TypeTools.hpp
View file

Large diffs are not rendered by default.

30 changes: 16 additions & 14 deletions include/CLI/Validators.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,17 +373,16 @@ class IPV4Validator : public Validator {
func_ = [](std::string &ip_addr) {
auto result = CLI::detail::split(ip_addr, '.');
if(result.size() != 4) {
return "Invalid IPV4 address must have four parts (" + ip_addr + ')';
return std::string("Invalid IPV4 address must have four parts (") + ip_addr + ')';
}
int num;
bool retval = true;
for(const auto &var : result) {
retval &= detail::lexical_cast(var, num);
bool retval = detail::lexical_cast(var, num);
if(!retval) {
return "Failed parsing number (" + var + ')';
return std::string("Failed parsing number (") + var + ')';
}
if(num < 0 || num > 255) {
return "Each IP number must be between 0 and 255 " + var;
return std::string("Each IP number must be between 0 and 255 ") + var;
}
}
return std::string();
Expand All @@ -398,10 +397,10 @@ class PositiveNumber : public Validator {
func_ = [](std::string &number_str) {
double number;
if(!detail::lexical_cast(number_str, number)) {
return "Failed parsing number: (" + number_str + ')';
return std::string("Failed parsing number: (") + number_str + ')';
}
if(number <= 0) {
return "Number less or equal to 0: (" + number_str + ')';
return std::string("Number less or equal to 0: (") + number_str + ')';
}
return std::string();
};
Expand All @@ -414,10 +413,10 @@ class NonNegativeNumber : public Validator {
func_ = [](std::string &number_str) {
double number;
if(!detail::lexical_cast(number_str, number)) {
return "Failed parsing number: (" + number_str + ')';
return std::string("Failed parsing number: (") + number_str + ')';
}
if(number < 0) {
return "Number less than 0: (" + number_str + ')';
return std::string("Number less than 0: (") + number_str + ')';
}
return std::string();
};
Expand All @@ -431,7 +430,7 @@ class Number : public Validator {
func_ = [](std::string &number_str) {
double number;
if(!detail::lexical_cast(number_str, number)) {
return "Failed parsing as a number (" + number_str + ')';
return std::string("Failed parsing as a number (") + number_str + ')';
}
return std::string();
};
Expand Down Expand Up @@ -482,7 +481,8 @@ class Range : public Validator {
T val;
bool converted = detail::lexical_cast(input, val);
if((!converted) || (val < min || val > max))
return "Value " + input + " not in range " + std::to_string(min) + " to " + std::to_string(max);
return std::string("Value ") + input + " not in range " + std::to_string(min) + " to " +
std::to_string(max);

return std::string();
};
Expand All @@ -508,7 +508,7 @@ class Bound : public Validator {
T val;
bool converted = detail::lexical_cast(input, val);
if(!converted) {
return "Value " + input + " could not be converted";
return std::string("Value ") + input + " could not be converted";
}
if(val < min)
input = detail::to_string(min);
Expand Down Expand Up @@ -943,7 +943,8 @@ class AsNumberWithUnit : public Validator {

bool converted = detail::lexical_cast(input, num);
if(!converted) {
throw ValidationError("Value " + input + " could not be converted to " + detail::type_name<Number>());
throw ValidationError(std::string("Value ") + input + " could not be converted to " +
detail::type_name<Number>());
}

if(unit.empty()) {
Expand Down Expand Up @@ -991,7 +992,8 @@ class AsNumberWithUnit : public Validator {
for(auto &kv : mapping) {
auto s = detail::to_lower(kv.first);
if(lower_mapping.count(s)) {
throw ValidationError("Several matching lowercase unit representations are found: " + s);
throw ValidationError(std::string("Several matching lowercase unit representations are found: ") +
s);
}
lower_mapping[detail::to_lower(kv.first)] = kv.second;
}
Expand Down
23 changes: 12 additions & 11 deletions tests/AppTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ TEST_F(TApp, OneStringEqualVersionSingleString) {
TEST_F(TApp, OneStringEqualVersionSingleStringQuoted) {
std::string str;
app.add_option("-s,--string", str);
app.parse("--string=\"this is my quoted string\"");
app.parse(R"raw(--string="this is my quoted string")raw");
EXPECT_EQ(1u, app.count("-s"));
EXPECT_EQ(1u, app.count("--string"));
EXPECT_EQ(str, "this is my quoted string");
Expand All @@ -305,7 +305,7 @@ TEST_F(TApp, OneStringEqualVersionSingleStringQuotedMultiple) {
app.add_option("-s,--string", str);
app.add_option("-t,--tstr", str2);
app.add_option("-m,--mstr", str3);
app.parse("--string=\"this is my quoted string\" -t 'qstring 2' -m=`\"quoted string\"`");
app.parse(R"raw(--string="this is my quoted string" -t 'qstring 2' -m=`"quoted string"`)raw");
EXPECT_EQ(str, "this is my quoted string");
EXPECT_EQ(str2, "qstring 2");
EXPECT_EQ(str3, "\"quoted string\"");
Expand All @@ -316,12 +316,12 @@ TEST_F(TApp, OneStringEqualVersionSingleStringEmbeddedEqual) {
app.add_option("-s,--string", str);
app.add_option("-t,--tstr", str2);
app.add_option("-m,--mstr", str3);
app.parse("--string=\"app=\\\"test1 b\\\" test2=\\\"frogs\\\"\" -t 'qstring 2' -m=`\"quoted string\"`");
app.parse(R"raw(--string="app=\"test1 b\" test2=\"frogs\"" -t 'qstring 2' -m=`"quoted string"`)raw");
EXPECT_EQ(str, "app=\"test1 b\" test2=\"frogs\"");
EXPECT_EQ(str2, "qstring 2");
EXPECT_EQ(str3, "\"quoted string\"");

app.parse("--string=\"app='test1 b' test2='frogs'\" -t 'qstring 2' -m=`\"quoted string\"`");
app.parse(R"raw(--string="app='test1 b' test2='frogs'" -t 'qstring 2' -m=`"quoted string"`)raw");
EXPECT_EQ(str, "app='test1 b' test2='frogs'");
EXPECT_EQ(str2, "qstring 2");
EXPECT_EQ(str3, "\"quoted string\"");
Expand All @@ -333,12 +333,12 @@ TEST_F(TApp, OneStringEqualVersionSingleStringEmbeddedEqualWindowsStyle) {
app.add_option("-t,--tstr", str2);
app.add_option("--mstr", str3);
app.allow_windows_style_options();
app.parse("/string:\"app:\\\"test1 b\\\" test2:\\\"frogs\\\"\" /t 'qstring 2' /mstr:`\"quoted string\"`");
app.parse(R"raw(/string:"app:\"test1 b\" test2:\"frogs\"" /t 'qstring 2' /mstr:`"quoted string"`)raw");
EXPECT_EQ(str, "app:\"test1 b\" test2:\"frogs\"");
EXPECT_EQ(str2, "qstring 2");
EXPECT_EQ(str3, "\"quoted string\"");

app.parse("/string:\"app:'test1 b' test2:'frogs'\" /t 'qstring 2' /mstr:`\"quoted string\"`");
app.parse(R"raw(/string:"app:'test1 b' test2:'frogs'" /t 'qstring 2' /mstr:`"quoted string"`)raw");
EXPECT_EQ(str, "app:'test1 b' test2:'frogs'");
EXPECT_EQ(str2, "qstring 2");
EXPECT_EQ(str3, "\"quoted string\"");
Expand All @@ -350,7 +350,7 @@ TEST_F(TApp, OneStringEqualVersionSingleStringQuotedMultipleMixedStyle) {
app.add_option("-t,--tstr", str2);
app.add_option("-m,--mstr", str3);
app.allow_windows_style_options();
app.parse("/string:\"this is my quoted string\" /t 'qstring 2' -m=`\"quoted string\"`");
app.parse(R"raw(/string:"this is my quoted string" /t 'qstring 2' -m=`"quoted string"`)raw");
EXPECT_EQ(str, "this is my quoted string");
EXPECT_EQ(str2, "qstring 2");
EXPECT_EQ(str3, "\"quoted string\"");
Expand All @@ -361,7 +361,7 @@ TEST_F(TApp, OneStringEqualVersionSingleStringQuotedMultipleInMiddle) {
app.add_option("-s,--string", str);
app.add_option("-t,--tstr", str2);
app.add_option("-m,--mstr", str3);
app.parse(R"raw(--string="this is my quoted string" -t "qst\"ring 2" -m=`"quoted string"`")raw");
app.parse(R"raw(--string="this is my quoted string" -t "qst\"ring 2" -m=`"quoted string"`)raw");
EXPECT_EQ(str, "this is my quoted string");
EXPECT_EQ(str2, "qst\"ring 2");
EXPECT_EQ(str3, "\"quoted string\"");
Expand All @@ -384,7 +384,7 @@ TEST_F(TApp, OneStringEqualVersionSingleStringQuotedMultipleWithEqual) {
app.add_option("-t,--tstr", str2);
app.add_option("-m,--mstr", str3);
app.add_option("-j,--jstr", str4);
app.parse("--string=\"this is my quoted string\" -t 'qstring 2' -m=`\"quoted string\"` --jstr=Unquoted");
app.parse(R"raw(--string="this is my quoted string" -t 'qstring 2' -m=`"quoted string"` --jstr=Unquoted)raw");
EXPECT_EQ(str, "this is my quoted string");
EXPECT_EQ(str2, "qstring 2");
EXPECT_EQ(str3, "\"quoted string\"");
Expand All @@ -397,8 +397,9 @@ TEST_F(TApp, OneStringEqualVersionSingleStringQuotedMultipleWithEqualAndProgram)
app.add_option("-t,--tstr", str2);
app.add_option("-m,--mstr", str3);
app.add_option("-j,--jstr", str4);
app.parse("program --string=\"this is my quoted string\" -t 'qstring 2' -m=`\"quoted string\"` --jstr=Unquoted",
true);
app.parse(
R"raw(program --string="this is my quoted string" -t 'qstring 2' -m=`"quoted string"` --jstr=Unquoted)raw",
true);
EXPECT_EQ(str, "this is my quoted string");
EXPECT_EQ(str2, "qstring 2");
EXPECT_EQ(str3, "\"quoted string\"");
Expand Down
5 changes: 3 additions & 2 deletions tests/CreationTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ class Unstreamable {
int x_ = -1;

public:
Unstreamable() {}
Unstreamable() = default;
int get_x() const { return x_; }
void set_x(int x) { x_ = x; }
};
Expand All @@ -719,7 +719,8 @@ std::istream &operator>>(std::istream &in, Unstreamable &value) {
return in;
}
// these need to be different classes otherwise the definitions conflict
static_assert(CLI::detail::is_istreamable<Unstreamable>::value, "Unstreamable type is still unstreamable");
static_assert(CLI::detail::is_istreamable<Unstreamable>::value,
"Unstreamable type is still unstreamable and it should be");

TEST_F(TApp, MakeUnstreamableOptions) {
Unstreamable value;
Expand Down
12 changes: 6 additions & 6 deletions tests/HelpersTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ TEST(Types, TypeName) {
EXPECT_EQ("FLOAT", vector_name);

static_assert(CLI::detail::classify_object<std::pair<int, std::string>>::value ==
CLI::detail::objCategory::tuple_value,
CLI::detail::object_category::tuple_value,
"pair<int,string> does not read like a tuple");

std::string pair_name = CLI::detail::type_name<std::vector<std::pair<int, std::string>>>();
Expand All @@ -869,16 +869,16 @@ TEST(Types, TypeName) {
EXPECT_EQ("UINT", vector_name);

auto vclass = CLI::detail::classify_object<std::vector<std::vector<unsigned char>>>::value;
EXPECT_EQ(vclass, CLI::detail::objCategory::vector_value);
EXPECT_EQ(vclass, CLI::detail::object_category::vector_value);

auto tclass = CLI::detail::classify_object<std::tuple<double>>::value;
EXPECT_EQ(tclass, CLI::detail::objCategory::number_constructible);
EXPECT_EQ(tclass, CLI::detail::object_category::number_constructible);

std::string tuple_name = CLI::detail::type_name<std::tuple<double>>();
EXPECT_EQ("FLOAT", tuple_name);

static_assert(CLI::detail::classify_object<std::tuple<int, std::string>>::value ==
CLI::detail::objCategory::tuple_value,
CLI::detail::object_category::tuple_value,
"tuple<int,string> does not read like a tuple");
tuple_name = CLI::detail::type_name<std::tuple<int, std::string>>();
EXPECT_EQ("[INT,TEXT]", tuple_name);
Expand Down Expand Up @@ -906,8 +906,8 @@ TEST(Types, TypeName) {
EXPECT_EQ("ENUM", enum_name);

vclass = CLI::detail::classify_object<std::tuple<test>>::value;
EXPECT_EQ(vclass, CLI::detail::objCategory::tuple_value);
static_assert(CLI::detail::classify_object<std::tuple<test>>::value == CLI::detail::objCategory::tuple_value,
EXPECT_EQ(vclass, CLI::detail::object_category::tuple_value);
static_assert(CLI::detail::classify_object<std::tuple<test>>::value == CLI::detail::object_category::tuple_value,
"tuple<test> does not classify as a tuple");
std::string enum_name2 = CLI::detail::type_name<std::tuple<test>>();
EXPECT_EQ("ENUM", enum_name2);
Expand Down
1 change: 1 addition & 0 deletions tests/NewParseTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ TEST_F(TApp, BuiltinComplexSingleImag) {
EXPECT_DOUBLE_EQ(0, comp.imag());
}

/// Simple class containing two strings useful for testing lexical cast and conversions
class spair {
public:
spair() = default;
Expand Down

0 comments on commit 5f69659

Please sign in to comment.