diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 1d5d8eeab5b..b0603c9591b 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -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."); @@ -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; @@ -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 { diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 79b4ff4b451..188d54977d2 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -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) { @@ -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); diff --git a/lib/errortypes.cpp b/lib/errortypes.cpp index 00345a59f3b..50622cb48f6 100644 --- a/lib/errortypes.cpp +++ b/lib/errortypes.cpp @@ -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()) diff --git a/test/cli/other_test.py b/test/cli/other_test.py index 529e6e8a390..7cdd3d9e841 100644 --- a/test/cli/other_test.py +++ b/test/cli/other_test.py @@ -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(""" @@ -1201,6 +1201,7 @@ def test_multiple_define_rules(tmpdir): error ruleId2 + define2 """) @@ -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(""" + + define + DEF_. + +""") + + 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(""" + + int + +""") + + 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 \ No newline at end of file + +def test_rule_file_raw(tmpdir): + rule_file = os.path.join(tmpdir, 'rule_file.xml') + with open(rule_file, 'wt') as f: + f.write(""" + + raw + i32 + +""") + + 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) + ] \ No newline at end of file diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index 4a42d56bc18..ca187287dcd 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -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 @@ -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 @@ -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; @@ -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", + "\n" + ".+\n" + "\n" + "" + "\n" + "\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", + "\n" + ".+\n" + "\n" + "" + "\n" + "\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", + "\n" + ".+\n" + "\n" + "none" + "\n" + "\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;