Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a WKTParser::grammarErrorList() method so that proj_create_from_wkt() can behave as documented #4108

Merged
merged 1 commit into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/proj/io.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,7 @@ class PROJ_GCC_DLL WKTParser {

PROJ_DLL WKTParser &setStrict(bool strict);
PROJ_DLL std::list<std::string> warningList() const;
PROJ_DLL std::list<std::string> grammarErrorList() const;

PROJ_DLL WKTParser &setUnsetIdentifiersIfIncompatibleDef(bool unset);

Expand Down
1 change: 1 addition & 0 deletions scripts/reference_exported_symbols.txt
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ osgeo::proj::io::WKTNode::~WKTNode()
osgeo::proj::io::WKTNode::WKTNode(std::string const&)
osgeo::proj::io::WKTParser::attachDatabaseContext(std::shared_ptr<osgeo::proj::io::DatabaseContext> const&)
osgeo::proj::io::WKTParser::createFromWKT(std::string const&)
osgeo::proj::io::WKTParser::grammarErrorList() const
osgeo::proj::io::WKTParser::guessDialect(std::string const&)
osgeo::proj::io::WKTParser::setStrict(bool)
osgeo::proj::io::WKTParser::setUnsetIdentifiersIfIncompatibleDef(bool)
Expand Down
8 changes: 7 additions & 1 deletion src/apps/projinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,12 +319,18 @@ static BaseObjectNNPtr buildObject(
wktParser.attachDatabaseContext(dbContext);
obj = wktParser.createFromWKT(l_user_string).as_nullable();
if (!quiet) {
auto warnings = wktParser.warningList();
const auto warnings = wktParser.warningList();
if (!warnings.empty()) {
for (const auto &str : warnings) {
std::cerr << "Warning: " << str << std::endl;
}
}
const auto grammarErrorList = wktParser.grammarErrorList();
if (!grammarErrorList.empty()) {
for (const auto &str : grammarErrorList) {
std::cerr << "Grammar error: " << str << std::endl;
}
}
}
} else if (dbContext && !kind.empty() && kind != "crs" &&
l_user_string.find(':') == std::string::npos) {
Expand Down
45 changes: 21 additions & 24 deletions src/iso19111/c_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,12 @@ PJ *proj_create(PJ_CONTEXT *ctx, const char *text) {
* The returned object must be unreferenced with proj_destroy() after use.
* It should be used by at most one thread at a time.
*
* The distinction between warnings and grammar errors is somewhat artificial
* and does not tell much about the real criticity of the non-compliance.
* Some warnings may be more concerning than some grammar errors. Human
* expertise (or, by the time this comment will be read, specialized AI) is
* generally needed to perform that assessment.
*
* @param ctx PROJ context, or NULL for default context
* @param wkt WKT string (must not be NULL)
* @param options null-terminated list of options, or NULL. Currently
Expand All @@ -632,8 +638,8 @@ PJ *proj_create(PJ_CONTEXT *ctx, const char *text) {
* </ul>
* @param out_warnings Pointer to a PROJ_STRING_LIST object, or NULL.
* If provided, *out_warnings will contain a list of warnings, typically for
* non recognized projection method or parameters. It must be freed with
* proj_string_list_destroy().
* non recognized projection method or parameters, or other issues found during
* WKT analys. It must be freed with proj_string_list_destroy().
* @param out_grammar_errors Pointer to a PROJ_STRING_LIST object, or NULL.
* If provided, *out_grammar_errors will contain a list of errors regarding the
* WKT grammar. It must be freed with proj_string_list_destroy().
Expand Down Expand Up @@ -682,42 +688,33 @@ PJ *proj_create_from_wkt(PJ_CONTEXT *ctx, const char *wkt,
}
auto obj = parser.createFromWKT(wkt);

std::vector<std::string> warningsFromParsing;
if (out_grammar_errors) {
auto rawWarnings = parser.warningList();
std::vector<std::string> grammarWarnings;
for (const auto &msg : rawWarnings) {
if (msg.find("Default it to") != std::string::npos) {
warningsFromParsing.push_back(msg);
} else {
grammarWarnings.push_back(msg);
}
}
if (!grammarWarnings.empty()) {
*out_grammar_errors = to_string_list(grammarWarnings);
auto grammarErrors = parser.grammarErrorList();
if (!grammarErrors.empty()) {
*out_grammar_errors = to_string_list(grammarErrors);
}
}

if (out_warnings) {
auto warnings = parser.warningList();
auto derivedCRS = dynamic_cast<const crs::DerivedCRS *>(obj.get());
if (derivedCRS) {
auto warnings =
auto extraWarnings =
derivedCRS->derivingConversionRef()->validateParameters();
warnings.insert(warnings.end(), warningsFromParsing.begin(),
warningsFromParsing.end());
if (!warnings.empty()) {
*out_warnings = to_string_list(warnings);
}
warnings.insert(warnings.end(), extraWarnings.begin(),
extraWarnings.end());
} else {
auto singleOp =
dynamic_cast<const operation::SingleOperation *>(obj.get());
if (singleOp) {
auto warnings = singleOp->validateParameters();
if (!warnings.empty()) {
*out_warnings = to_string_list(warnings);
}
auto extraWarnings = singleOp->validateParameters();
warnings.insert(warnings.end(), extraWarnings.begin(),
extraWarnings.end());
}
}
if (!warnings.empty()) {
*out_warnings = to_string_list(warnings);
}
}

return pj_obj_create(ctx, NN_NO_CHECK(obj));
Expand Down
33 changes: 30 additions & 3 deletions src/iso19111/io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1266,6 +1266,7 @@ struct WKTParser::Private {
bool strict_ = true;
bool unsetIdentifiersIfIncompatibleDef_ = true;
std::list<std::string> warningList_{};
std::list<std::string> grammarErrorList_{};
std::vector<double> toWGS84Parameters_{};
std::string datumPROJ4Grids_{};
bool esriStyle_ = false;
Expand All @@ -1288,7 +1289,8 @@ struct WKTParser::Private {
Private(const Private &) = delete;
Private &operator=(const Private &) = delete;

void emitRecoverableWarning(const std::string &errorMsg);
void emitRecoverableWarning(const std::string &warningMsg);
void emitGrammarError(const std::string &errorMsg);
void emitRecoverableMissingUNIT(const std::string &parentNodeName,
const UnitOfMeasure &fallbackUnit);

Expand Down Expand Up @@ -1517,6 +1519,20 @@ std::list<std::string> WKTParser::warningList() const {

// ---------------------------------------------------------------------------

/** \brief Return the list of grammar errors found during parsing.
*
* Grammar errors are non-compliance issues with respect to the WKT grammar.
*
* \note The list might be non-empty only is setStrict(false) has been called.
*
* @since PROJ 9.5
*/
std::list<std::string> WKTParser::grammarErrorList() const {
return d->grammarErrorList_;
}

// ---------------------------------------------------------------------------

//! @cond Doxygen_Suppress
void WKTParser::Private::emitRecoverableWarning(const std::string &errorMsg) {
if (strict_) {
Expand All @@ -1528,6 +1544,17 @@ void WKTParser::Private::emitRecoverableWarning(const std::string &errorMsg) {

// ---------------------------------------------------------------------------

//! @cond Doxygen_Suppress
void WKTParser::Private::emitGrammarError(const std::string &errorMsg) {
if (strict_) {
throw ParsingException(errorMsg);
} else {
grammarErrorList_.push_back(errorMsg);
}
}

// ---------------------------------------------------------------------------

static double asDouble(const std::string &val) { return c_locale_stod(val); }

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -8136,13 +8163,13 @@ BaseObjectNNPtr WKTParser::createFromWKT(const std::string &wkt) {
dialect == WKTGuessedDialect::WKT1_ESRI) {
auto errorMsg = pj_wkt1_parse(wkt);
if (!errorMsg.empty()) {
d->emitRecoverableWarning(errorMsg);
d->emitGrammarError(errorMsg);
}
} else if (dialect == WKTGuessedDialect::WKT2_2015 ||
dialect == WKTGuessedDialect::WKT2_2019) {
auto errorMsg = pj_wkt2_parse(wkt);
if (!errorMsg.empty()) {
d->emitRecoverableWarning(errorMsg);
d->emitGrammarError(errorMsg);
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/cli/testprojinfo_out.dist
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,7 @@ PROJCRS["Monte Mario (Rome) / Italy zone 1",

Testing non compliant WKT1
Warning: GEOGCS should have a PRIMEM node
Warning: Parsing error : syntax error, unexpected UNIT, expecting PRIMEM. Error occurred around:
Grammar error: Parsing error : syntax error, unexpected UNIT, expecting PRIMEM. Error occurred around:
HEROID["WGS 84",6378137,298.257223563]],UNIT["degree",0.0174532925199433]]
^
PROJ.4 string:
Expand Down
37 changes: 35 additions & 2 deletions test/unit/test_c_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,39 @@ TEST_F(CApi, proj_create_from_wkt) {
EXPECT_EQ(errorList, nullptr);
proj_string_list_destroy(errorList);
}
{
PROJ_STRING_LIST warningList = nullptr;
PROJ_STRING_LIST errorList = nullptr;
const char *const options[] = {
"UNSET_IDENTIFIERS_IF_INCOMPATIBLE_DEF=YES", nullptr};
auto wkt = "PROJCS[\"Merchich / Nord Maroc\","
" GEOGCS[\"Merchich\","
" DATUM[\"Merchich\","
" SPHEROID[\"Clarke 1880 (IGN)\","
"6378249.2,293.466021293627]],"
" PRIMEM[\"Greenwich\",0],"
" UNIT[\"grad\",0.015707963267949,"
" AUTHORITY[\"EPSG\",\"9105\"]],"
" AUTHORITY[\"EPSG\",\"4261\"]],"
" PROJECTION[\"Lambert_Conformal_Conic_1SP\"],"
" PARAMETER[\"latitude_of_origin\",37],"
" PARAMETER[\"central_meridian\",-6],"
" PARAMETER[\"scale_factor\",0.999625769],"
" PARAMETER[\"false_easting\",500000],"
" PARAMETER[\"false_northing\",300000],"
" UNIT[\"metre\",1,"
" AUTHORITY[\"EPSG\",\"9001\"]],"
" AXIS[\"Easting\",EAST],"
" AXIS[\"Northing\",NORTH]]";
auto obj = proj_create_from_wkt(m_ctxt, wkt, options, &warningList,
&errorList);
ObjectKeeper keeper(obj);
EXPECT_NE(obj, nullptr);
EXPECT_NE(warningList, nullptr);
proj_string_list_destroy(warningList);
EXPECT_EQ(errorList, nullptr);
proj_string_list_destroy(errorList);
}
{
PROJ_STRING_LIST warningList = nullptr;
PROJ_STRING_LIST errorList = nullptr;
Expand Down Expand Up @@ -4222,8 +4255,8 @@ TEST_F(CApi, proj_get_celestial_body_list_from_database) {
{ proj_celestial_body_list_destroy(nullptr); }

{
auto list =
proj_get_celestial_body_list_from_database(nullptr, nullptr, nullptr);
auto list = proj_get_celestial_body_list_from_database(nullptr, nullptr,
nullptr);
ASSERT_NE(list, nullptr);
ASSERT_NE(list[0], nullptr);
ASSERT_NE(list[0]->auth_name, nullptr);
Expand Down