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/05 18:53:39 UTC

[GitHub] [lucenenet] laimis opened a new pull request, #796: Fix virtual method calls from constructor

laimis opened a new pull request, #796:
URL: https://github.com/apache/lucenenet/pull/796

   This PR fixes issues with virtual calls being made from constructors. The issue originally reported by SonarCloud scans: https://sonarcloud.io/project/issues?resolved=false&rules=csharpsquid%3AS1699&id=apache_lucenenet and referenced in this issue: #670 
   
   Starting with TernaryTree to get feedback if the approach is correct and then we can discuss further how to proceed (if to fix all occurrences of this in one shot or split it up in multiple PRs).


-- 
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


[GitHub] [lucenenet] laimis commented on pull request #796: Fix virtual method calls from constructor

Posted by "laimis (via GitHub)" <gi...@apache.org>.
laimis commented on PR #796:
URL: https://github.com/apache/lucenenet/pull/796#issuecomment-1498149412

   Thanks for that. I made the adjustments. I see that TernaryTree is being subclassed by HyphenationTree but in this case, it did not override Init and initialized its version of protected members in its own constructor.


-- 
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


[GitHub] [lucenenet] laimis commented on pull request #796: Fix virtual method calls from constructor

Posted by "laimis (via GitHub)" <gi...@apache.org>.
laimis commented on PR #796:
URL: https://github.com/apache/lucenenet/pull/796#issuecomment-1498333286

   Closing this and will reopen a PR that starts with simple fixes.


-- 
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


[GitHub] [lucenenet] laimis commented on a diff in pull request #796: Fix virtual method calls from constructor

Posted by "laimis (via GitHub)" <gi...@apache.org>.
laimis commented on code in PR #796:
URL: https://github.com/apache/lucenenet/pull/796#discussion_r1159022728


##########
src/Lucene.Net.Analysis.Common/Analysis/Shingle/ShingleFilter.cs:
##########
@@ -638,8 +638,11 @@ public virtual void Advance()
             /// <see cref="outputUnigrams"/> = true.
             /// </para>
             /// </summary>

Review Comment:
   ShingleFilter has a private CircularSequence class inside of it, so removing virtual designation is enough.



-- 
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


[GitHub] [lucenenet] laimis commented on a diff in pull request #796: Fix virtual method calls from constructor

Posted by "laimis (via GitHub)" <gi...@apache.org>.
laimis commented on code in PR #796:
URL: https://github.com/apache/lucenenet/pull/796#discussion_r1159023306


##########
src/Lucene.Net.Analysis.Common/Analysis/Shingle/ShingleFilter.cs:
##########
@@ -638,8 +638,11 @@ public virtual void Advance()
             /// <see cref="outputUnigrams"/> = true.
             /// </para>
             /// </summary>
-            public virtual void Reset()
+            public void Reset()
             {
+                // LUCENENET specific - S1699 - marked non-virtual because calling
+                // virtual members from the constructor is not a safe operation in .NET.
+                

Review Comment:
   ShingleFilter has a private CircularSequence class inside of it, so removing virtual designation is enough.
   
   



-- 
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


[GitHub] [lucenenet] laimis closed pull request #796: Fix virtual method calls from constructor

Posted by "laimis (via GitHub)" <gi...@apache.org>.
laimis closed pull request #796: Fix virtual method calls from constructor
URL: https://github.com/apache/lucenenet/pull/796


-- 
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