Merge pull request #4108 from rouault/grammarErrorList

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

include/proj/io.hpp

@@ -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);
 

scripts/reference_exported_symbols.txt

@@ -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)

src/apps/projinfo.cpp

@@ -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) {

src/iso19111/c_api.cpp

@@ -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
@@ -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().
@@ -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));

src/iso19111/io.cpp

@@ -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;
@@ -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);
 
@@ -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_) {
@@ -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); }
 
 // ---------------------------------------------------------------------------
@@ -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);
         }
     }
 

test/cli/testprojinfo_out.dist

@@ -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:

test/unit/test_c_api.cpp

@@ -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;
@@ -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);