From 9e1edf36f05b808f3638c4564ee23174a126ec04 Mon Sep 17 00:00:00 2001 From: Andrei Pozolotin Date: Thu, 1 Feb 2024 10:43:03 -0600 Subject: [PATCH] Resolve #11965 - no proper execute() for cross-property references Solution B: parsequant() function --- src/App/Expression.cpp | 9 ++ src/App/ExpressionParser.h | 1 + tests/src/App/CMakeLists.txt | 2 + tests/src/App/ExpressionParser.cpp | 105 +++++++++++++++++++++ tests/src/App/PropertyExpressionEngine.cpp | 93 ++++++++++++++++++ 5 files changed, 210 insertions(+) create mode 100644 tests/src/App/ExpressionParser.cpp create mode 100644 tests/src/App/PropertyExpressionEngine.cpp diff --git a/src/App/Expression.cpp b/src/App/Expression.cpp index 75ac7e2e37..183d6ace68 100644 --- a/src/App/Expression.cpp +++ b/src/App/Expression.cpp @@ -1754,6 +1754,7 @@ FunctionExpression::FunctionExpression(const DocumentObject *_owner, Function _f case SINH: case SQRT: case STR: + case PARSEQUANT: case TAN: case TANH: case TRUNC: @@ -2291,6 +2292,11 @@ Py::Object FunctionExpression::evaluate(const Expression *expr, int f, const std } case STR: return Py::String(args[0]->getPyValue().as_string()); + case PARSEQUANT: { + auto quantity_text = args[0]->getPyValue().as_string(); + auto quantity_object = Quantity::parse(QString::fromStdString(quantity_text)); + return Py::asObject(new QuantityPy(new Quantity(quantity_object))); + } case TRANSLATIONM: { if (args.size() != 1) break; // Break and proceed to 3 size version. @@ -2797,6 +2803,8 @@ void FunctionExpression::_toString(std::ostream &ss, bool persistent,int) const ss << "rotationz("; break;; case STR: ss << "str("; break;; + case PARSEQUANT: + ss << "parsequant("; break;; case TRANSLATIONM: ss << "translationm("; break;; case TUPLE: @@ -3679,6 +3687,7 @@ static void initParser(const App::DocumentObject *owner) registered_functions["rotationy"] = FunctionExpression::ROTATIONY; registered_functions["rotationz"] = FunctionExpression::ROTATIONZ; registered_functions["str"] = FunctionExpression::STR; + registered_functions["parsequant"] = FunctionExpression::PARSEQUANT; registered_functions["translationm"] = FunctionExpression::TRANSLATIONM; registered_functions["tuple"] = FunctionExpression::TUPLE; registered_functions["vector"] = FunctionExpression::VECTOR; diff --git a/src/App/ExpressionParser.h b/src/App/ExpressionParser.h index 3e3b282bbe..305df966bf 100644 --- a/src/App/ExpressionParser.h +++ b/src/App/ExpressionParser.h @@ -305,6 +305,7 @@ public: ROTATIONY, // Create y-axis rotation object. ROTATIONZ, // Create z-axis rotation object. STR, // stringify + PARSEQUANT, // parse string quantity TRANSLATIONM, // Create translation matrix object. TUPLE, // Create Python tuple. VECTOR, // Create vector object. diff --git a/tests/src/App/CMakeLists.txt b/tests/src/App/CMakeLists.txt index 14f763c1a8..c54c1d1343 100644 --- a/tests/src/App/CMakeLists.txt +++ b/tests/src/App/CMakeLists.txt @@ -6,6 +6,7 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/ComplexGeoData.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Document.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Expression.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/ExpressionParser.cpp ${CMAKE_CURRENT_SOURCE_DIR}/ElementMap.cpp ${CMAKE_CURRENT_SOURCE_DIR}/IndexedName.cpp ${CMAKE_CURRENT_SOURCE_DIR}/License.cpp @@ -13,5 +14,6 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/MappedName.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Metadata.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Property.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/PropertyExpressionEngine.cpp ${CMAKE_CURRENT_SOURCE_DIR}/StringHasher.cpp ) diff --git a/tests/src/App/ExpressionParser.cpp b/tests/src/App/ExpressionParser.cpp new file mode 100644 index 0000000000..fc95dafafe --- /dev/null +++ b/tests/src/App/ExpressionParser.cpp @@ -0,0 +1,105 @@ +#include "gtest/gtest.h" + +#include "Base/Quantity.h" + +#include "App/Application.h" +#include "App/Document.h" +#include "App/DocumentObject.h" +#include "App/Expression.h" +#include "App/ExpressionParser.h" + +#include "src/App/InitApplication.h" + +// clang-format off + +class ExpressionParserTest: public ::testing::Test +{ +protected: + static void SetUpTestSuite() + { + tests::initApplication(); + } + + void SetUp() override + { + _doc_name = App::GetApplication().getUniqueDocumentName("test"); + _this_doc = App::GetApplication().newDocument(_doc_name.c_str(), "testUser"); + _this_obj = _this_doc -> addObject("Sketcher::SketchObject"); + } + + void TearDown() override + { + App::GetApplication().closeDocument(_doc_name.c_str()); + } + + std::string doc_name() { return _doc_name; } + App::Document* this_doc() { return _this_doc; } + App::DocumentObject* this_obj() { return _this_obj; } + + Base::Quantity parse_expression_text_as_quantity(const char* expression_text) { + auto expression = App::ExpressionParser::parse(this_obj(), expression_text); + auto expression_value = expression->getValueAsAny(); + auto quantity_result = App::any_cast(expression_value); + return quantity_result; + } + + Base::Quantity parse_quantity_text_as_quantity(const char* quantity_text) { + auto quantity_qstr = QString::fromStdString(std::string(quantity_text)); + auto quantity_result = Base::Quantity::parse(quantity_qstr); + return quantity_result; + } + +private: + std::string _doc_name; + App::Document* _this_doc {}; + App::DocumentObject* _this_obj {}; +}; + +// https://github.com/FreeCAD/FreeCAD/issues/11965 +TEST_F(ExpressionParserTest, functionPARSEQUANT) +{ + + EXPECT_ANY_THROW(App::ExpressionParser::parse(this_obj(), "parsequant()")) << "should not parse empty"; + + EXPECT_NO_THROW(App::ExpressionParser::parse(this_obj(), "parsequant(1 mm)")) << "should parse simple quantity"; + EXPECT_NO_THROW(App::ExpressionParser::parse(this_obj(), "parsequant(<<(1 + 2) m>>)")) << "should parse literal quantity"; + EXPECT_NO_THROW(App::ExpressionParser::parse(this_obj(), "parsequant(str(1 m + 2 mm))")) << "should parse str-function quantity"; + + EXPECT_ANY_THROW(parse_quantity_text_as_quantity("parsequant(1 mm)")) << "should not treat parsequant-function as quantity"; + EXPECT_EQ(parse_quantity_text_as_quantity("1 mm"), parse_quantity_text_as_quantity("1 mm")) << "equality sanity check"; + EXPECT_NE(parse_quantity_text_as_quantity("1 mm"), parse_quantity_text_as_quantity("2 mm")) << "inequality sanity check"; + + std::array, 12> expression_vs_quantity_list = {{ + // length + { "1 mm", "1 mm" }, + { "parsequant(1 mm)", "1 mm" }, + { "parsequant(<<(1 + 2) m>>)", "3000 mm" }, + { "parsequant(str(1 m + 2 mm))", "1002 mm"}, + // angle + { "10 deg", "10 deg" }, + { "parsequant(10 deg)", "10 deg" }, + { "parsequant(<<(10 + 20) deg>>)", "30 deg" }, + { "parsequant(str(10 deg + 20 deg))", "30 deg" }, + // mass + { "10 g", "10 g" }, + { "parsequant(10 g)", "10 g" }, + { "parsequant(<<(10 + 20) kg>>)", "30000 g" }, + { "parsequant(str(10 kg + 20010 g))", "30.01 kg" }, + }}; + + for ( const auto& expression_vs_quantity_pair : expression_vs_quantity_list ) { + auto expression_text = expression_vs_quantity_pair.first; + auto quantity_text = expression_vs_quantity_pair.second; + auto expression_result = parse_expression_text_as_quantity(expression_text); + auto quantity_result = parse_quantity_text_as_quantity(quantity_text); + EXPECT_EQ(expression_result, quantity_result) << "mismatch:" + " expression_text='" + std::string(expression_text) + "'" + " quantity_text='" + std::string(quantity_text) + "'" + " expression_representation='" + expression_result.getUserString().toStdString() + "'" + " quantity_representation='" + quantity_result.getUserString().toStdString() + "'" + ; + } + +} + +// clang-format on diff --git a/tests/src/App/PropertyExpressionEngine.cpp b/tests/src/App/PropertyExpressionEngine.cpp new file mode 100644 index 0000000000..99d165dbce --- /dev/null +++ b/tests/src/App/PropertyExpressionEngine.cpp @@ -0,0 +1,93 @@ +#include "gtest/gtest.h" + +#include "Base/Quantity.h" + +#include "App/Application.h" +#include "App/Document.h" +#include "App/DocumentObject.h" +#include "App/Expression.h" +#include "App/ObjectIdentifier.h" +#include "App/PropertyExpressionEngine.h" + +#include "src/App/InitApplication.h" + +// clang-format off + +class PropertyExpressionEngineTest: public ::testing::Test +{ +protected: + static void SetUpTestSuite() + { + tests::initApplication(); + } + + void SetUp() override + { + _doc_name = App::GetApplication().getUniqueDocumentName("test"); + _this_doc = App::GetApplication().newDocument(_doc_name.c_str(), "testUser"); + _this_obj = _this_doc -> addObject("Sketcher::SketchObject"); + _source_name = std::string("this_origin"); + _source_prop = _this_obj -> addDynamicProperty("App::PropertyString", _source_name.c_str()); // property with length as string + _target_name = std::string("this_length"); + _target_prop = _this_obj -> addDynamicProperty("App::PropertyLength", _target_name.c_str()); // property with reference to source + } + + void TearDown() override + { + App::GetApplication().closeDocument(_doc_name.c_str()); + } + + std::string doc_name() { return _doc_name; } + App::Document* this_doc() { return _this_doc; } + App::DocumentObject* this_obj() { return _this_obj; } + std::string source_name() { return _source_name; } + App::Property* source_prop() { return _source_prop; } + std::string target_name() { return _target_name; } + App::Property* target_prop() { return _target_prop; } + +private: + std::string _doc_name; + App::Document* _this_doc {}; + App::DocumentObject* _this_obj {}; + std::string _source_name; + App::Property* _source_prop {}; + std::string _target_name; + App::Property* _target_prop {}; +}; + +// https://github.com/FreeCAD/FreeCAD/issues/11965 +TEST_F(PropertyExpressionEngineTest, executeCrossPropertyReference) +{ + auto source_text = std::string("1.5 m"); // provided in source + auto target_text = std::string("1500 mm"); // expected in target + + auto source_path = App::ObjectIdentifier::parse(this_obj(), source_name()); + source_prop()->setPathValue(source_path, source_text); + + auto target_expr = "parsequant(" + source_name() + ")"; // Solution B: parsequant() function + + auto target_path = App::ObjectIdentifier::parse(this_obj(), target_name()); + std::shared_ptr target_rule(App::Expression::parse(this_obj(), target_expr)); + this_obj()->setExpression(target_path, target_rule); + + this_obj() -> ExpressionEngine.execute(); + + auto source_entry = source_prop() -> getPathValue(source_path); + ASSERT_TRUE(source_entry.type() == typeid(std::string)); + auto source_value = App::any_cast(source_entry); + + auto target_entry = target_prop() -> getPathValue(target_path); + ASSERT_TRUE(target_entry.type() == typeid(Base::Quantity)); + auto target_quant = App::any_cast(target_entry); + auto target_value = target_quant.getValue(); + auto target_unit = target_quant.getUnit().getString().toStdString(); + + auto verify_quant = Base::Quantity::parse(QString::fromStdString(target_text)); + + EXPECT_EQ(target_quant, verify_quant) << "" + "expecting equal: source_text='" + source_text + "' target_text='" + target_text + "'" + "instead produced: target_value='" + std::to_string(target_value) + "' target_unit='" + target_unit + "'" + ; +} + +// clang-format on