Skip to content

Commit

Permalink
improved errorhandling of --rule= and --rule-file= / some related…
Browse files Browse the repository at this point in the history 
… cleanups (#6212)
  • Loading branch information
@firewave
firewave authored Apr 10, 2024
1 parent 9e7578e commit f450285
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 18 deletions.
16 changes: 16 additions & 0 deletions cli/cmdlineparser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,12 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
#ifdef HAVE_RULES
Settings::Rule rule;
rule.pattern = 7 + argv[i];

if (rule.pattern.empty()) {
mLogger.printError("no rule pattern provided.");
return Result::Fail;
}

mSettings.rules.emplace_back(std::move(rule));
#else
mLogger.printError("Option --rule cannot be used as Cppcheck has not been built with rules support.");
Expand Down Expand Up @@ -1134,6 +1140,11 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
return Result::Fail;
}

if (rule.id.empty()) {
mLogger.printError("unable to load rule-file '" + ruleFile + "' - a rule is lacking an id.");
return Result::Fail;
}

if (rule.tokenlist.empty()) {
mLogger.printError("unable to load rule-file '" + ruleFile + "' - a rule is lacking a tokenlist.");
return Result::Fail;
Expand All @@ -1144,6 +1155,11 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
return Result::Fail;
}

if (rule.severity == Severity::none) {
mLogger.printError("unable to load rule-file '" + ruleFile + "' - a rule has an invalid severity.");
return Result::Fail;
}

mSettings.rules.emplace_back(std::move(rule));
}
} else {
Expand Down
29 changes: 16 additions & 13 deletions lib/cppcheck.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1298,13 +1298,14 @@ void CppCheck::executeRules(const std::string &tokenlist, const TokenList &list)
return;

// Write all tokens in a string that can be parsed by pcre
std::ostringstream ostr;
for (const Token *tok = list.front(); tok; tok = tok->next())
ostr << " " << tok->str();
const std::string str(ostr.str());
std::string str;
for (const Token *tok = list.front(); tok; tok = tok->next()) {
str += " ";
str += tok->str();
}

for (const Settings::Rule &rule : mSettings.rules) {
if (rule.pattern.empty() || rule.id.empty() || rule.severity == Severity::none || rule.tokenlist != tokenlist)
if (rule.tokenlist != tokenlist)
continue;

if (!mSettings.quiet) {
Expand Down Expand Up @@ -1379,28 +1380,30 @@ void CppCheck::executeRules(const std::string &tokenlist, const TokenList &list)
pos = (int)pos2;

// determine location..
std::string file = list.getSourceFilePath();
int fileIndex = 0;
int line = 0;

std::size_t len = 0;
for (const Token *tok = list.front(); tok; tok = tok->next()) {
len = len + 1U + tok->str().size();
if (len > pos1) {
file = list.getFiles().at(tok->fileIndex());
fileIndex = tok->fileIndex();
line = tok->linenr();
break;
}
}

const std::string& file = list.getFiles()[fileIndex];

ErrorMessage::FileLocation loc(file, line, 0);

// Create error message
std::string summary;
if (rule.summary.empty())
summary = "found '" + str.substr(pos1, pos2 - pos1) + "'";
else
summary = rule.summary;
const ErrorMessage errmsg({std::move(loc)}, list.getSourceFilePath(), rule.severity, summary, rule.id, Certainty::normal);
const ErrorMessage errmsg({std::move(loc)},
list.getSourceFilePath(),
rule.severity,
!rule.summary.empty() ? rule.summary : "found '" + str.substr(pos1, pos2 - pos1) + "'",
rule.id,
Certainty::normal);

// Report error
reportErr(errmsg);
Expand Down
1 change: 1 addition & 0 deletions lib/errortypes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ std::string severityToString(Severity severity)
throw InternalError(nullptr, "Unknown severity");
}

// TODO: bail out on invalid severity
Severity severityFromString(const std::string& severity)
{
if (severity.empty())
Expand Down
129 changes: 124 additions & 5 deletions test/cli/other_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,7 @@ def test_unknown_extension(tmpdir):
assert stderr == ''


def test_multiple_define_rules(tmpdir):
def test_rule_file_define_multiple(tmpdir):
rule_file = os.path.join(tmpdir, 'rule_file.xml')
with open(rule_file, 'wt') as f:
f.write("""
Expand All @@ -1201,6 +1201,7 @@ def test_multiple_define_rules(tmpdir):
<message>
<severity>error</severity>
<id>ruleId2</id>
<summary>define2</summary>
</message>
</rule>
</rules>""")
Expand All @@ -1213,18 +1214,136 @@ def test_multiple_define_rules(tmpdir):
void f() { }
''')

exitcode, stdout, stderr = cppcheck(['--template=simple', '--rule-file={}'.format(rule_file), test_file])
exitcode, stdout, stderr = cppcheck(['--template=simple', '--rule-file={}'.format(rule_file), '-DDEF_3', test_file])
assert exitcode == 0, stderr
lines = stdout.splitlines()
assert lines == [
'Checking {} ...'.format(test_file),
'Processing rule: DEF_1',
'Processing rule: DEF_2'
'Processing rule: DEF_2',
'Checking {}: DEF_3=1...'.format(test_file)
]
lines = stderr.splitlines()
assert lines == [
"{}:2:0: error: found 'DEF_1' [ruleId1]".format(test_file),
"{}:3:0: error: found 'DEF_2' [ruleId2]".format(test_file)
"{}:3:0: error: define2 [ruleId2]".format(test_file)
]


def test_rule_file_define(tmpdir):
rule_file = os.path.join(tmpdir, 'rule_file.xml')
with open(rule_file, 'wt') as f:
f.write("""
<rule>
<tokenlist>define</tokenlist>
<pattern>DEF_.</pattern>
</rule>
""")

test_file = os.path.join(tmpdir, 'test.c')
with open(test_file, 'wt') as f:
f.write('''
#define DEF_1
#define DEF_2
void f() { }
''')

exitcode, stdout, stderr = cppcheck(['--template=simple', '--rule-file={}'.format(rule_file), '-DDEF_3', test_file])
assert exitcode == 0, stdout
lines = stdout.splitlines()
assert lines == [
'Checking {} ...'.format(test_file),
'Processing rule: DEF_.',
'Checking {}: DEF_3=1...'.format(test_file)
]
lines = stderr.splitlines()
assert lines == [
"{}:2:0: style: found 'DEF_1' [rule]".format(test_file),
"{}:3:0: style: found 'DEF_2' [rule]".format(test_file)
]


def test_rule_file_normal(tmpdir):
rule_file = os.path.join(tmpdir, 'rule_file.xml')
with open(rule_file, 'wt') as f:
f.write("""
<rule>
<pattern>int</pattern>
</rule>
""")

test_file = os.path.join(tmpdir, 'test.c')
with open(test_file, 'wt') as f:
f.write('''
#define DEF_1
#define DEF_2
typedef int i32;
void f(i32) { }
''')

exitcode, stdout, stderr = cppcheck(['--template=simple', '--rule-file={}'.format(rule_file), test_file])
assert exitcode == 0, stdout
lines = stdout.splitlines()
assert lines == [
'Checking {} ...'.format(test_file),
'Processing rule: int',
]
lines = stderr.splitlines()
assert lines == [
"{}:5:0: style: found 'int' [rule]".format(test_file)
]

# TODO: test "raw" and "normal" rules

def test_rule_file_raw(tmpdir):
rule_file = os.path.join(tmpdir, 'rule_file.xml')
with open(rule_file, 'wt') as f:
f.write("""
<rule>
<tokenlist>raw</tokenlist>
<pattern>i32</pattern>
</rule>
""")

test_file = os.path.join(tmpdir, 'test.c')
with open(test_file, 'wt') as f:
f.write('''
#define DEF_1
#define DEF_2
typedef int i32;
void f(i32) { }
''')

exitcode, stdout, stderr = cppcheck(['--template=simple', '--rule-file={}'.format(rule_file), test_file])
assert exitcode == 0, stdout
lines = stdout.splitlines()
assert lines == [
'Checking {} ...'.format(test_file),
'Processing rule: i32',
]
lines = stderr.splitlines()
assert lines == [
"{}:4:0: style: found 'i32' [rule]".format(test_file),
"{}:5:0: style: found 'i32' [rule]".format(test_file)
]


def test_rule(tmpdir):
test_file = os.path.join(tmpdir, 'test.c')
with open(test_file, 'wt') as f:
f.write('''
#define DEF_1
#define DEF_2
void f() { }
''')

exitcode, stdout, stderr = cppcheck(['--template=simple', '--rule=f', test_file])
assert exitcode == 0, stdout
lines = stdout.splitlines()
assert lines == [
'Checking {} ...'.format(test_file),
'Processing rule: f',
]
lines = stderr.splitlines()
assert lines == [
"{}:4:0: style: found 'f' [rule]".format(test_file)
]
53 changes: 53 additions & 0 deletions test/testcmdlineparser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ class TestCmdlineParser : public TestFixture {
TEST_CASE(addonMissing);
#ifdef HAVE_RULES
TEST_CASE(rule);
TEST_CASE(ruleMissingPattern);
#else
TEST_CASE(ruleNotSupported);
#endif
Expand All @@ -349,6 +350,9 @@ class TestCmdlineParser : public TestFixture {
TEST_CASE(ruleFileUnknownElement2);
TEST_CASE(ruleFileMissingTokenList);
TEST_CASE(ruleFileUnknownTokenList);
TEST_CASE(ruleFileMissingId);
TEST_CASE(ruleFileInvalidSeverity1);
TEST_CASE(ruleFileInvalidSeverity2);
#else
TEST_CASE(ruleFileNotSupported);
#endif
Expand Down Expand Up @@ -2177,6 +2181,13 @@ class TestCmdlineParser : public TestFixture {
auto it = settings->rules.cbegin();
ASSERT_EQUALS(".+", it->pattern);
}

void ruleMissingPattern() {
REDIRECT;
const char * const argv[] = {"cppcheck", "--rule=", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
ASSERT_EQUALS("cppcheck: error: no rule pattern provided.\n", logger->str());
}
#else
void ruleNotSupported() {
REDIRECT;
Expand Down Expand Up @@ -2359,6 +2370,48 @@ class TestCmdlineParser : public TestFixture {
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' - a rule is using the unsupported tokenlist 'simple'.\n", logger->str());
}

void ruleFileMissingId() {
REDIRECT;
ScopedFile file("rule.xml",
"<rule>\n"
"<pattern>.+</pattern>\n"
"<message>\n"
"<id/>"
"</message>\n"
"</rule>\n");
const char * const argv[] = {"cppcheck", "--rule-file=rule.xml", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' - a rule is lacking an id.\n", logger->str());
}

void ruleFileInvalidSeverity1() {
REDIRECT;
ScopedFile file("rule.xml",
"<rule>\n"
"<pattern>.+</pattern>\n"
"<message>\n"
"<severity/>"
"</message>\n"
"</rule>\n");
const char * const argv[] = {"cppcheck", "--rule-file=rule.xml", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' - a rule has an invalid severity.\n", logger->str());
}

void ruleFileInvalidSeverity2() {
REDIRECT;
ScopedFile file("rule.xml",
"<rule>\n"
"<pattern>.+</pattern>\n"
"<message>\n"
"<severity>none</severity>"
"</message>\n"
"</rule>\n");
const char * const argv[] = {"cppcheck", "--rule-file=rule.xml", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' - a rule has an invalid severity.\n", logger->str());
}
#else
void ruleFileNotSupported() {
REDIRECT;
Expand Down

0 comments on commit f450285

Please sign in to comment.