PVS and Clang-tidy modify

[dota17]

Fix some remaining lower priority warnings in issue #747:

• V127. An overflow of the 32-bit variable is possible inside a long cycle which utilizes a memsize-type loop counter. (int --> size_t)

Because that PVS Studio report is old, I use clang-tidy tools to scan jsoncpp recently like #1033 #1048 and find two warnings:

warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
warning: use 'using' instead of 'typedef' [modernize-use-using]

[coveralls]

Coverage increased (+0.003%) to 92.138% when pulling 7be1cef on dota17:PVSandClangtidyModify into a0bd9ad on open-source-parsers:master.

[cdunn2001]

These are all good changes, but I'd really rather see the test-coverage go up before we worry about cosmetic improvements.

[dota17]

These are all good changes, but I'd really rather see the test-coverage go up before we worry about cosmetic improvements.

Hi @cdunn2001
Currently, what's our goal of the test-coverage? Is there a minimal threshold? At the present, we have gone up to 90%+ coverage on the whole.

Since the latest version is only a pre-release version, and some users feedback that there is a little problem with it, so will we need to release a stable version after the coverge reach the goal?

[BillyDonahue]

All this buggy fuss.
Meanwhile this whole function is conceptually 4 easy lines. :)

static Json::String readInputTestFile(const char* path) {
    std::ifstream in(path, in.binary);
    std::stringstream os;
    os << in.rdbuf();
    return Json::String(os.str());
}

[dota17]

Yes, that's great! but this sould be in another PR.

[BillyDonahue]

We could try doing it an easier way.

// order by map size, then by map contents.
auto tieFields = [](const Value& v) { return std::tie(v.map_->size(), *v.map_); };
return tieFields(*this) < tieFields(other);

[cdunn2001]

Currently, what's our goal of the test-coverage? Is there a minimal threshold? At the present, we have gone up to 90%+ coverage on the whole.

Well, I thought someone said the jsontestrunner used the old Reader, not the new CharReader. If that's true, we are missing a lot of behavioral coverage. But maybe I misunderstood that.