Expressions: preserve brackets for right-associative operators

Fixes #11475

python/core/qgsexpression.sip

@@ -379,7 +379,6 @@ class QgsExpression
 
         int precedence() const;
         bool leftAssociative() const;
-        bool rightAssociative() const;
     };
 
     class NodeInOperator : QgsExpression::Node

src/core/qgsexpression.cpp

@@ -2576,51 +2576,24 @@ bool QgsExpression::NodeBinaryOperator::leftAssociative() const
   return -1;
 }
 
-bool QgsExpression::NodeBinaryOperator::rightAssociative() const
-{
-  // see left/right in qgsexpressionparser.yy
-  switch ( mOp )
-  {
-    case boOr:
-    case boAnd:
-    case boEQ:
-    case boNE:
-    case boLE:
-    case boGE:
-    case boLT:
-    case boGT:
-    case boRegexp:
-    case boLike:
-    case boILike:
-    case boNotLike:
-    case boNotILike:
-    case boIs:
-    case boIsNot:
-    case boPlus:
-    case boMul:
-    case boMod:
-    case boConcat:
-    case boPow:
-      return true;
-
-    case boMinus:
-    case boDiv:
-    case boIntDiv:
-      return false;
-  }
-  Q_ASSERT( 0 && "unexpected binary operator" );
-  return -1;
-}
-
 QString QgsExpression::NodeBinaryOperator::dump() const
 {
   QgsExpression::NodeBinaryOperator *lOp = dynamic_cast<QgsExpression::NodeBinaryOperator *>( mOpLeft );
   QgsExpression::NodeBinaryOperator *rOp = dynamic_cast<QgsExpression::NodeBinaryOperator *>( mOpRight );
 
   QString fmt;
-  fmt += lOp && ( lOp->precedence() < precedence() || !lOp->leftAssociative() ) ? "(%1)" : "%1";
-  fmt += " %2 ";
-  fmt += rOp && ( rOp->precedence() < precedence() || !rOp->rightAssociative() ) ? "(%3)" : "%3";
+  if ( leftAssociative() )
+  {
+    fmt += lOp && ( lOp->precedence() < precedence() ) ? "(%1)" : "%1";
+    fmt += " %2 ";
+    fmt += rOp && ( rOp->precedence() <= precedence() ) ? "(%3)" : "%3";
+  }
+  else
+  {
+    fmt += lOp && ( lOp->precedence() <= precedence() ) ? "(%1)" : "%1";
+    fmt += " %2 ";
+    fmt += rOp && ( rOp->precedence() < precedence() ) ? "(%3)" : "%3";
+  }
 
   return fmt.arg( mOpLeft->dump() ).arg( BinaryOperatorText[mOp] ).arg( mOpRight->dump() );
 }

src/core/qgsexpression.h

@@ -514,7 +514,6 @@ class CORE_EXPORT QgsExpression
 
         int precedence() const;
         bool leftAssociative() const;
-        bool rightAssociative() const;
 
       protected:
         bool compare( double diff );

tests/src/core/testqgsexpression.cpp

@@ -435,6 +435,12 @@ class TestQgsExpression: public QObject
       QTest::newRow( "color hsva" ) << "color_hsva(40,100,100,200)" << false << QVariant( "255,170,0,200" );
       QTest::newRow( "color cmyk" ) << "color_cmyk(100,50,33,10)" << false << QVariant( "0,115,154" );
       QTest::newRow( "color cmyka" ) << "color_cmyka(50,25,90,60,200)" << false << QVariant( "51,76,10,200" );
+
+      // Precedence and associativity
+      QTest::newRow( "multiplication first" ) << "1+2*3" << false << QVariant( 7 );
+      QTest::newRow( "brackets first" ) << "(1+2)*(3+4)" << false << QVariant( 21 );
+      QTest::newRow( "right associativity" ) << "(2^3)^2" << false << QVariant( 64. );
+      QTest::newRow( "left associativity" ) << "1-(2-1)" << false << QVariant( 0 );
     }
 
     void run_evaluation_test( QgsExpression& exp, bool evalError, QVariant& result )
@@ -515,17 +521,6 @@ class TestQgsExpression: public QObject
       QCOMPARE( QgsExpression::BinaryOperatorText[QgsExpression::boDiv], "/" );
       QCOMPARE( QgsExpression::BinaryOperatorText[QgsExpression::boConcat], "||" );
 
-      QgsExpression e0( "1+2*3" );
-      QCOMPARE( e0.evaluate().toInt(), 7 );
-
-      QgsExpression e1( "(1+2)*(3+4)" );
-      QCOMPARE( e1.evaluate().toInt(), 21 );
-
-      QgsExpression e2( e1.dump() );
-      QCOMPARE( e2.evaluate().toInt(), 21 );
-
-      QgsExpression e3( "(2^3)^2" );
-      QCOMPARE( QgsExpression( e3.dump() ).evaluate().toInt(), 64 );
     }
 
     void eval_columns()