Prevent using reserved names for properties or aliases (Fix #16846) (#16902)

* Prevent naming properties with reserved words (Fix #16846)

* Prevent using reserved constant names for properties or aliases

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* App: Add unit test for isTokenAConstant

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Chris Hennes <chennes@pioneerlibrarysystem.org>

src/App/Expression.cpp

@@ -3855,6 +3855,13 @@ bool ExpressionParser::isTokenAnIndentifier(const std::string & str)
     return (status == 0 && (token == IDENTIFIER || token == CELLADDRESS));
 }
 
+bool ExpressionParser::isTokenAConstant(const std::string & str)
+{
+    int token{}, status{};
+    std::tie(token, status) = getTokenAndStatus(str);
+    return (status == 0 && token == CONSTANT);
+}
+
 bool ExpressionParser::isTokenAUnit(const std::string & str)
 {
     int token{}, status{};

src/App/ExpressionParser.h

@@ -500,6 +500,7 @@ AppExport Expression * parse(const App::DocumentObject *owner, const char *buffe
 AppExport UnitExpression * parseUnit(const App::DocumentObject *owner, const char *buffer);
 AppExport ObjectIdentifier parsePath(const App::DocumentObject *owner, const char* buffer);
 AppExport bool isTokenAnIndentifier(const std::string & str);
+AppExport bool isTokenAConstant(const std::string & str);
 AppExport bool isTokenAUnit(const std::string & str);
 AppExport std::vector<std::tuple<int, int, std::string> > tokenize(const std::string & str);
 

src/Gui/DlgAddProperty.cpp

@@ -28,6 +28,7 @@
 #include <App/Application.h>
 #include <App/Document.h>
 #include <App/DocumentObject.h>
+#include <App/ExpressionParser.h>
 #include <Base/Tools.h>
 
 #include "DlgAddProperty.h"
@@ -111,6 +112,13 @@ void DlgAddProperty::accept()
     if(ui->chkAppend->isChecked())
         name = group + "_" + name;
 
+    if (App::ExpressionParser::isTokenAUnit(name) || App::ExpressionParser::isTokenAConstant(name)) {
+        QMessageBox::critical(getMainWindow(),
+            QObject::tr("Invalid name"),
+            QObject::tr("The property name is a reserved word."));
+        return;
+    }
+
     for(auto c : containers) {
         auto prop = c->getPropertyByName(name.c_str());
         if(prop && prop->getContainer() == c) {

src/Gui/DlgAddPropertyVarSet.cpp

@@ -30,6 +30,7 @@
 #include <App/Application.h>
 #include <App/Document.h>
 #include <App/DocumentObject.h>
+#include <App/ExpressionParser.h>
 #include <App/PropertyUnits.h>
 #include <Base/Tools.h>
 
@@ -351,6 +352,14 @@ void DlgAddPropertyVarSet::checkName() {
         throw CreatePropertyException("Invalid name");
     }
 
+    if(App::ExpressionParser::isTokenAUnit(name) || App::ExpressionParser::isTokenAConstant(name)) {
+        QMessageBox::critical(getMainWindow(),
+                              QObject::tr("Invalid name"),
+                              QObject::tr("The property name is a reserved word."));
+        clearEditors(!CLEAR_NAME);
+        throw CreatePropertyException("Invalid name");
+    }
+
     if (namePropertyToAdd.empty()) {
         // we are adding a new property, check whether it doesn't already exist
         auto prop = varSet->getPropertyByName(name.c_str());

src/Gui/DlgExpressionInput.cpp

@@ -437,6 +437,11 @@ static bool isNamePropOk(const QString& nameProp, App::DocumentObject* obj,
         return false;
     }
 
+    if (ExpressionParser::isTokenAUnit(name) || ExpressionParser::isTokenAConstant(name)) {
+        message << name << " is a reserved word";
+        return false;
+    }
+
     auto prop = obj->getPropertyByName(name.c_str());
     if (prop && prop->getContainer() == obj) {
         message << name << " already exists";

src/Mod/Spreadsheet/App/PropertySheet.cpp

@@ -137,8 +137,9 @@ bool PropertySheet::isValidAlias(const std::string& candidate)
         return false;
     }
 
-    /* Check to make sure it doesn't clash with a predefined unit */
-    if (ExpressionParser::isTokenAUnit(candidate)) {
+    /* Check to make sure it doesn't clash with a reserved name */
+    if (ExpressionParser::isTokenAUnit(candidate)
+        || ExpressionParser::isTokenAConstant(candidate)) {
         return false;
     }
 

tests/src/App/ExpressionParser.cpp

@@ -102,4 +102,19 @@ TEST_F(ExpressionParserTest, functionPARSEQUANT)
 
 }
 
+TEST_F(ExpressionParserTest, isTokenAConstant)
+{
+    std::array<std::string, 7> constants {"pi", "e", "True", "False", "true", "false", "None"};
+    for (const auto & constant : constants) {
+        EXPECT_TRUE(App::ExpressionParser::isTokenAConstant(constant))
+          << "\"" << constant << "\" did not evaluate as a constant";
+    }
+
+    std::array<std::string, 6> notConstants {"PI", "E", "TRUE", "FALSE", "NONE", "none"};
+    for (const auto & nonConstant : notConstants) {
+        EXPECT_FALSE(App::ExpressionParser::isTokenAConstant(nonConstant))
+          << "\"" << nonConstant << "\" evaluated as a constant";
+    }
+}
+
 // clang-format on