You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "laimis (via GitHub)" <gi...@apache.org> on 2023/04/13 18:20:06 UTC

[GitHub] [lucenenet] laimis commented on a diff in pull request #831: ComposedQuery fix for virtual call being made from constructor

laimis commented on code in PR #831:
URL: https://github.com/apache/lucenenet/pull/831#discussion_r1165881542


##########
src/Lucene.Net.QueryParser/Surround/Query/ComposedQuery.cs:
##########
@@ -28,13 +27,22 @@ namespace Lucene.Net.QueryParsers.Surround.Query
     /// </summary>
     public abstract class ComposedQuery : SrndQuery
     {
-        protected ComposedQuery(IList<SrndQuery> qs, bool operatorInfix, string opName) // LUCENENET: CA1012: Abstract types should not have constructors (marked protected)
+        // LUCENENET specific - provided protected parameterless constructor to allow subclasses
+        // avoid issues with virtual Recompose method
+        protected ComposedQuery(bool operatorInfix, string opName)
         {
-            Recompose(qs);
             this.operatorInfix = operatorInfix;
             this.m_opName = opName;
         }
 
+        [SuppressMessage("CodeQuality", "IDE0079:Remove unnecessary suppression", Justification = "This is a SonarCloud issue")]
+        [SuppressMessage("CodeQuality", "S1699:Constructors should only call non-overridable methods", Justification = "Required for continuity with Lucene's design")]
+        protected ComposedQuery(IList<SrndQuery> qs, bool operatorInfix, string opName) // LUCENENET: CA1012: Abstract types should not have constructors (marked protected)
+            : this(operatorInfix, opName)
+        {
+            Recompose(qs);
+        }
+
         protected virtual void Recompose(IList<SrndQuery> queries)

Review Comment:
   with the above change, the order of operations changes. We essentially will be first assigning the operatorInfix and opName and then calling Recompose. Where before we first called Recompose and assigned after. Looking at the Recompose logic, it seems a safe thing to do. But figured to highlight that.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org