Merge pull request #1295 from hsorby/analyse-node-tidy

Tidy analyseNode
cellml · Nov 11, 2024 · 4357d2b · 4357d2b
2 parents 5882718 + 95d180a
commit 4357d2b
Show file tree

Hide file tree

Showing 73 changed files with 2,820 additions and 2,821 deletions.
diff --git a/src/analyser.cpp b/src/analyser.cpp
@@ -742,36 +742,34 @@ void Analyser::AnalyserImpl::analyseNode(const XmlNodePtr &node,
         //                 +-------------+
 
         auto childCount = mathmlChildCount(node);
+        AnalyserEquationAstPtr tempAst;
+        AnalyserEquationAstPtr astRightChild;
 
-        analyseNode(mathmlChildNode(node, 0), ast, astParent, component, equation);
-
-        if (childCount >= 2) {
-            analyseNode(mathmlChildNode(node, 1), ast->mPimpl->mOwnedLeftChild, ast, component, equation);
-
-            if (childCount >= 3) {
-                AnalyserEquationAstPtr astRightChild;
-                AnalyserEquationAstPtr tempAst;
-
-                analyseNode(mathmlChildNode(node, childCount - 1), astRightChild, nullptr, component, equation);
-
-                for (auto i = childCount - 2; i > 1; --i) {
-                    tempAst = AnalyserEquationAst::create();
-
-                    analyseNode(mathmlChildNode(node, 0), tempAst, nullptr, component, equation);
-                    analyseNode(mathmlChildNode(node, i), tempAst->mPimpl->mOwnedLeftChild, tempAst, component, equation);
+        for (size_t i = childCount - 1; i > 0; --i) {
+            astRightChild = tempAst;
+            tempAst = AnalyserEquationAst::create();
 
+            if (astRightChild != nullptr) {
+                if (i == childCount - 2) {
+                    astRightChild->swapLeftAndRightChildren();
+                    tempAst = astRightChild;
+                } else {
                     astRightChild->mPimpl->mParent = tempAst;
-
                     tempAst->mPimpl->mOwnedRightChild = astRightChild;
-                    astRightChild = tempAst;
                 }
+            }
 
-                astRightChild->mPimpl->mParent = ast;
-
-                ast->mPimpl->mOwnedRightChild = astRightChild;
+            if (i != childCount - 2) {
+                analyseNode(mathmlChildNode(node, 0), tempAst, nullptr, component, equation);
             }
+
+            analyseNode(mathmlChildNode(node, i), tempAst->mPimpl->mOwnedLeftChild, tempAst, component, equation);
         }
 
+        analyseNode(mathmlChildNode(node, 0), tempAst, astParent, component, equation);
+
+        ast = tempAst;
+
         // Relational and logical operators.
 
     } else if (node->isMathmlElement("eq")) {

diff --git a/tests/analyser/analyser.cpp b/tests/analyser/analyser.cpp
@@ -898,8 +898,8 @@ TEST(Analyser, algebraicSystemWithThreeLinkedUnknownsWithOneExternalVariable)
     EXPECT_EQ(size_t(0), parser->issueCount());
 
     const std::vector<std::string> expectedIssues = {
-        "Variable 'y' in component 'my_algebraic_system' is computed more than once.",
         "Variable 'z' in component 'my_algebraic_system' is computed more than once.",
+        "Variable 'y' in component 'my_algebraic_system' is computed more than once.",
     };
 
     auto analyser = libcellml::Analyser::create();
@@ -954,8 +954,8 @@ TEST(Analyser, overconstrainedNlaSystem)
     EXPECT_EQ(size_t(0), parser->issueCount());
 
     const std::vector<std::string> expectedIssues = {
-        "Variable 'x' in component 'my_algebraic_system' is computed more than once.",
         "Variable 'y' in component 'my_algebraic_system' is computed more than once.",
+        "Variable 'x' in component 'my_algebraic_system' is computed more than once.",
     };
 
     auto analyser = libcellml::Analyser::create();

diff --git a/tests/analyser/analyserunits.cpp b/tests/analyser/analyserunits.cpp
@@ -883,8 +883,8 @@ TEST(AnalyserUnits, powerValues)
         "The units in 'eqnPi = pow(x, pi)' in component 'my_component' are not equivalent. 'eqnPi' is in 'second' while 'pow(x, pi)' is in 'second^3.14159'.",
         "The units in 'eqnInfinity = pow(x, infinity)' in component 'my_component' are not equivalent. 'eqnInfinity' is in 'second' while 'pow(x, infinity)' is in 'second^inf' (i.e. '10^nan x second^inf').",
         "The units in 'eqnNotanumber = pow(x, notanumber)' in component 'my_component' are not equivalent. 'eqnNotanumber' is in 'second' while 'pow(x, notanumber)' is in 'second^nan' (i.e. '10^nan x second^nan').",
-        "The type of variable 'eqnCoverage' in component 'my_component' is unknown.",
         "The type of variable 'u' in component 'my_component' is unknown.",
+        "The type of variable 'eqnCoverage' in component 'my_component' is unknown.",
         "The type of variable 'eqnCoverage2' in component 'my_component' is unknown.",
         "The type of variable 'eqnCoverage3' in component 'my_component' is unknown.",
     };

diff --git a/tests/bindings/javascript/analysermodel.test.js b/tests/bindings/javascript/analysermodel.test.js
@@ -64,7 +64,7 @@ describe("Analyser Model tests", () => {
     test('Checking Analyser Model variables related API.', () => {
         expect(am.variableCount()).toBe(18)
         expect(am.variables().size()).toBe(18)
-        expect(am.variable(2).variable().name()).toBe("i_K")
+        expect(am.variable(2).variable().name()).toBe("i_L")
     });
     test('Checking Analyser Model need* API.', () => {
         expect(am.needEqFunction()).toBe(false)

diff --git a/tests/bindings/python/test_analyser.py b/tests/bindings/python/test_analyser.py
@@ -152,12 +152,12 @@ def test_coverage(self):
 
         # Ensure coverage for AnalyserVariable.
 
-        av = am.variable(3)
+        av = am.variable(4)
 
         self.assertEqual(AnalyserVariable.Type.CONSTANT, av.type())
         self.assertEqual("constant", AnalyserVariable.typeAsString(av.type()))
         self.assertEqual("constant", AnalyserVariable_typeAsString(av.type()))
-        self.assertEqual(3, av.index())
+        self.assertEqual(4, av.index())
         self.assertIsNotNone(av.initialisingVariable())
         self.assertIsNotNone(av.variable())
         self.assertEqual(1, av.equationCount())

diff --git a/tests/coverage/coverage.cpp b/tests/coverage/coverage.cpp
@@ -623,8 +623,9 @@ TEST(Coverage, generator)
         EXPECT_NE(nullptr, analyserModel->state(i)->initialisingVariable());
     }
 
+    std::vector<size_t> nonNull = {0, 1, 5, 17, 178, 179, 181, 205, 206};
     for (size_t i = 0; i < analyserModel->variableCount(); ++i) {
-        if ((i == 1) || (i == 2) || (i == 6) || (i == 18) || (i == 179) || (i == 180) || (i == 182) || (i == 205) || (i == 206)) {
+        if (std::find(nonNull.begin(), nonNull.end(), i) != nonNull.end()) {
             EXPECT_TRUE(analyserModel->variable(i)->initialisingVariable() != nullptr);
         }
     }