Skip to content

Commit

Permalink
AddonInfo: const-ified loading and improved errorhandling (danmar#5834)
Browse files Browse the repository at this point in the history
  • Loading branch information
@firewave
firewave authored Jan 5, 2024
1 parent c7cd091 commit 42c3aeb
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 51 deletions.
86 changes: 56 additions & 30 deletions lib/addoninfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,47 +52,73 @@ static std::string parseAddonInfo(AddonInfo& addoninfo, const picojson::value &j
return "Loading " + fileName + " failed. " + json_error;
}
if (!json.is<picojson::object>())
return "Loading " + fileName + " failed. Bad json.";
picojson::object obj = json.get<picojson::object>();
if (obj.count("args")) {
if (!obj["args"].is<picojson::array>())
return "Loading " + fileName + " failed. args must be array.";
for (const picojson::value &v : obj["args"].get<picojson::array>())
addoninfo.args += " " + v.get<std::string>();
}
return "Loading " + fileName + " failed. JSON is not an object.";

if (obj.count("ctu")) {
// ctu is specified in the config file
if (!obj["ctu"].is<bool>())
return "Loading " + fileName + " failed. ctu must be boolean.";
addoninfo.ctu = obj["ctu"].get<bool>();
} else {
addoninfo.ctu = false;
// TODO: remove/complete default value handling for missing fields
const picojson::object& obj = json.get<picojson::object>();
{
const auto it = obj.find("args");
if (it != obj.cend()) {
const auto& val = it->second;
if (!val.is<picojson::array>())
return "Loading " + fileName + " failed. 'args' must be an array.";
for (const picojson::value &v : val.get<picojson::array>()) {
if (!v.is<std::string>())
return "Loading " + fileName + " failed. 'args' entry is not a string.";
addoninfo.args += " " + v.get<std::string>();
}
}
}

if (obj.count("python")) {
// Python was defined in the config file
if (obj["python"].is<picojson::array>()) {
return "Loading " + fileName +" failed. python must not be an array.";
{
const auto it = obj.find("ctu");
if (it != obj.cend()) {
const auto& val = it->second;
// ctu is specified in the config file
if (!val.is<bool>())
return "Loading " + fileName + " failed. 'ctu' must be a boolean.";
addoninfo.ctu = val.get<bool>();
}
else {
addoninfo.ctu = false;
}
addoninfo.python = obj["python"].get<std::string>();
} else {
addoninfo.python = "";
}

if (obj.count("executable")) {
if (!obj["executable"].is<std::string>())
return "Loading " + fileName + " failed. executable must be a string.";
addoninfo.executable = getFullPath(obj["executable"].get<std::string>(), fileName);
return "";
{
const auto it = obj.find("python");
if (it != obj.cend()) {
const auto& val = it->second;
// Python was defined in the config file
if (!val.is<std::string>()) {
return "Loading " + fileName +" failed. 'python' must be a string.";
}
addoninfo.python = val.get<std::string>();
}
else {
addoninfo.python = "";
}
}

if (!obj.count("script") || !obj["script"].is<std::string>())
{
return "Loading " + fileName + " failed. script must be set to a string value.";
const auto it = obj.find("executable");
if (it != obj.cend()) {
const auto& val = it->second;
if (!val.is<std::string>())
return "Loading " + fileName + " failed. 'executable' must be a string.";
addoninfo.executable = getFullPath(val.get<std::string>(), fileName);
return ""; // TODO: why bail out?
}
}

return addoninfo.getAddonInfo(obj["script"].get<std::string>(), exename);
const auto it = obj.find("script");
if (it == obj.cend())
return "Loading " + fileName + " failed. 'script' is missing.";

const auto& val = it->second;
if (!val.is<std::string>())
return "Loading " + fileName + " failed. 'script' must be a string.";

return addoninfo.getAddonInfo(val.get<std::string>(), exename);
}

std::string AddonInfo::getAddonInfo(const std::string &fileName, const std::string &exename) {
Expand Down
78 changes: 57 additions & 21 deletions test/cli/test-other.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
import sys
import pytest
import json

from testutils import cppcheck, assert_cppcheck

Expand Down Expand Up @@ -278,22 +279,6 @@ def test_addon_threadsafety(tmpdir):
assert stderr == '{}:4:12: warning: strerror is MT-unsafe [threadsafety-unsafe-call]\n'.format(test_file)


def test_addon_invalidjson(tmpdir):
addon_file = os.path.join(tmpdir, 'invalid.json')
with open(addon_file, 'wt') as f:
f.write("""
{
"Script": "addons/something.py"
}
""")

args = ['--addon={}'.format(addon_file), '--enable=all', 'nonexistent.cpp']

exitcode, stdout, stderr = cppcheck(args)
assert exitcode != 0
assert stdout == 'Loading {} failed. script must be set to a string value.\n'.format(addon_file)


def test_addon_naming(tmpdir):
# the addon does nothing without a config
addon_file = os.path.join(tmpdir, 'naming1.json')
Expand Down Expand Up @@ -644,12 +629,10 @@ def test_invalid_addon_json(tmpdir):
""")

test_file = os.path.join(tmpdir, 'file.cpp')
with open(test_file, 'wt') as f:
f.write("""
typedef int MISRA_5_6_VIOLATION;
""")
with open(test_file, 'wt'):
pass

args = ['--addon={}'.format(addon_file), '--enable=all', test_file]
args = ['--addon={}'.format(addon_file), test_file]

exitcode, stdout, stderr = cppcheck(args)
assert exitcode == 1
Expand Down Expand Up @@ -1124,3 +1107,56 @@ def test_build_dir_j_memleak(tmpdir): #12111
]

assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=out_lines)


def __test_addon_json_invalid(tmpdir, addon_json, expected):
addon_file = os.path.join(tmpdir, 'invalid.json')
with open(addon_file, 'wt') as f:
f.write(addon_json)

test_file = os.path.join(tmpdir, 'file.cpp')
with open(test_file, 'wt'):
pass

args = ['--addon={}'.format(addon_file), test_file]

exitcode, stdout, stderr = cppcheck(args)
assert exitcode == 1
lines = stdout.splitlines()
assert len(lines) == 1
assert lines == [
'Loading {} failed. {}'.format(addon_file, expected)
]
assert stderr == ''


def test_addon_json_invalid_no_obj(tmpdir):
__test_addon_json_invalid(tmpdir, json.dumps([]), "JSON is not an object.")


def test_addon_json_invalid_args_1(tmpdir):
__test_addon_json_invalid(tmpdir, json.dumps({'args':0}), "'args' must be an array.")


def test_addon_json_invalid_args_2(tmpdir):
__test_addon_json_invalid(tmpdir, json.dumps({'args':[0]}), "'args' entry is not a string.")


def test_addon_json_invalid_ctu(tmpdir):
__test_addon_json_invalid(tmpdir, json.dumps({'ctu':0}), "'ctu' must be a boolean.")


def test_addon_json_invalid_python(tmpdir):
__test_addon_json_invalid(tmpdir, json.dumps({'python':0}), "'python' must be a string.")


def test_addon_json_invalid_executable(tmpdir):
__test_addon_json_invalid(tmpdir, json.dumps({'executable':0}), "'executable' must be a string.")


def test_addon_json_invalid_script_1(tmpdir):
__test_addon_json_invalid(tmpdir, json.dumps({'Script':''}), "'script' is missing.")


def test_addon_json_invalid_script_2(tmpdir):
__test_addon_json_invalid(tmpdir, json.dumps({'script':0}), "'script' must be a string.")

0 comments on commit 42c3aeb

Please sign in to comment.