Remove this call from a constructor to the overridable 'xxx' method/property #670
Labels
Comments
NightOwl888
added
up-for-grabs
NightOwl888
added a commit
to NightOwl888/lucenenet
that referenced
this issue
Oct 23, 2022
…erridable members in constructor (See apache#670). Added some missing guard clauses.
NightOwl888
added a commit
to NightOwl888/lucenenet
that referenced
this issue
Nov 17, 2022
…erridable members in constructor (See apache#670). Added some missing guard clauses.
NightOwl888
added a commit
to NightOwl888/lucenenet
that referenced
this issue
Nov 17, 2022
…erridable members in constructor (See apache#670). Added some missing guard clauses.
NightOwl888
added a commit
that referenced
this issue
Nov 17, 2022
…ionary) (#762) * BUG: Lucene.Net.Analysis.Util (CharArraySet + CharArrayMap): Fixed IsReadOnly flag to return the proper value reflecting read-only state. Added tests. * Lucene.Net.Analysis.Util (CharArraySet + CharArrayMap): Don't call overridable members in constructor (See #670). Added some missing guard clauses. * BREAKING: Lucene.Net.Analysis.Util (CharArraySet + CharArrayMap): Removed Remove(), IntersectWith(), ExceptWith(), SymmetricExceptWith(), Contains(KeyValuePair) from public APIs by implementing collection interfaces explicitly. * Lucene.Net.Analysis.Util (CharArraySet + CharArrayMap): Added ToCharArraySet() and ToCharArrayMap() methods and extension methods to make copying collections easier. * BREAKING: Lucene.Net.Analysis.Util.CharArrayMap: Renamed EntryIterator > Enumerator, removed HasNext, NextKey(), and NextKeyString(), added CurrentKey and CurrentKeyString properties. Removed EntrySet_ class, EntrySet() method. * BREAKING: Lucene.Net.Analysis.Util: Renamed CharArrayMap > CharArrayDictionary. * BREAKING: Lucene.Net.Analysis.Util.CharArrayDictionary: Removed EmptyMap() method and created public Empty static field to replace it. * BREAKING: Lucene.Net.Analysis.Util.CharArraySet: Renamed EMPTY_SET > Empty. * Lucene.Net.Analysis.Util.CharArrayDictionary: Added missing guard clauses and updated documentation. * PERFORMANCE: Lucene.Net.Analysis.Util.CharArrayMap: Use CurrentKeyString and CurrentKeyValue to avoid allocation of KeyValuePair * BUG: Lucene.Net.Analysis.Util.CharArrayMap::Equals(): Use JCG.EqualityComparer when comparing values for equality. * Lucene.Net.Analysis.Util.CharArraySet: Added missing guard clauses and added documentation. * PERFORMANCE/BUG: Lucene.Net.Analysis.Util.CharArrayDictionary: Added cases for string and ICharSequence to object overload of PutImpl, ContainsKey, Get, and TryGetValue. Convert unknown types to string in invariant context. * PERFORMANCE: Lucene.Net.Analysis.Util.CharArrayDictionary: Added optimized PutImpl(string, MapValue) implementation that only allocates a char[] if it is required. * PERFORMANCE: Lucene.Net.Analysis.Util.CharArrayDictionary: Added Set/SetImpl methods that don't look up the previous value for use in the CharArrayDictionary.this[] overloads. * Lucene.Net.Analysis.Util.CharArraySet: Changed signature of UnionWith(IEnumerable<string>) to return a bool instead of void. Added explicit interface implementation to adhere to the ISet<string> contract. * BREAKING: Lucene.Net.Analysis.Util.CharArraySet: Marked ContainsAll() overloads obsolete, since this is duplicate functionality of ISet<T>.IsSupersetOf(). * BREAKING: Lucene.Net.Analysis.Util.CharArrayDictionary: Moved OriginalKeySet property from the public API to explicitly defined on the ICharArrayDictionary interface. Previously it was hidden from Intellisense, so this will have minimal impact. * Lucene.Net.Analysis.Util (CharArrayDictionary + CharArraySet): Consolidated KeyCollection and UnmodifiableCharArraySet into a single KeyCollection enumerator. Migrated KeyCollection.KeyEnumerator to CharArraySet and renamed it Enumerator. Added ICharArrayDictionaryEnumerator so the necessary members are visible without using generics. * Lucene.Net.Analysis.Util.CharArrayDictionary: Added documentation to Put() methods that accept a value indicating that this[key] = value is more efficient if the return value is unused. * PERFORMANCE: Lucene.Net.Analysis.Common: Updated all classes that use CharArrayDictionary<TValue>.Put() and discard the value to use CharArrayDictionary<TValue>.this[] instead. * Lucene.Net.Analysis.CharArrayDictionary: Added Set() overloads to use to populate CharArraySet * BREAKING: Lucene.Net.Analysis.Util.CharArraySet::ctor(): Renamed parameter from c > collection. * Lucene.Net.Analysis.Util.CharArraySet: Added constructor overloads for ICollection<char[]> and ICollection<ICharSequence>. * Lucene.Net.Analysis.Util.CharArrayDictionary::Copy(): Use pattern matching when converting to CharArrayDictionary<TValue> * PERFORMANCE: Lucene.Net.Analysis.Util.CharArrayDictionary: Fixed enumerators to use CurrentKey and CurrentValue where appropriate to reduce allocations * Lucene.Net.Analysis.Util.CharArrayDictionary: Added KeyValuePair<char[], TValue> overload of CopyTo * Lucene.Net.Analysis.Util.CharArraySet: Fixed implementation of SetEquals() so it matches any IEnumerable<T> type with the same values (accounting for case sensitivity). Added overloads for IEnumerable<char[]>, IEnumerable<ICharSequence> and IEnumerable<T> (object). Added tests. * Lucene.Net.Analysis.Util.CharArrayDictionary<TValue>: Added error checking to ensure enumerator instances throw InvalidOperationException when the collection state is mutated, or the enumerator is positioned before or after the bounds of the collection. * Lucene.Net.Analysis.Util.CharArraySet: Added overloads for IEnumerable<char[]> and IEnumerable<ICharSequence> for IsSubset(), IsSuperset(), IsProperSubset() and IsProperSuperset() + tests. * Lucene.Net.Analysis.Util.CharArraySet: Added overloads of CopyTo() for IList<char[]> and ICharSequence[]. * Lucene.Net.Analysis.Util.CharArraySet: Added tests for IsSubset(), IsProperSubset(), IsSuperset(), IsProperSuperset(), Overlaps(), SetEquals() with null values in the comparison set. Added overloads of char[] and ICharSequence for Overlaps(). * Lucene.Net.Analysis.Util.CharArrayDictionary: Added CopyTo() overload for ICharSequence + added tests for all 3 overloads * BREAKING: Lucene.Net.Analysis.Util (CharArraySet + CharArrayDictionary<TValue>): Renamed constructor parameters c > collection, startSize > capacity. Changed type of collection from ICollection<T> to IEnumerable<T> to match .NET collections. * Lucene.Net.Analysis.Util.CharArrayDictionary<TValue>::ToString(): Print "null" when there is a null value to match Java. * BREAKING: Lucene.Net.Analysis.Util.CharArraySet: Changed parameter type of Copy() and CopySet() methods from ICollection<T> to IEnumerable<T>. Renamed the parameter from set > collection. * BUG: Lucene.Net.Analysis.Util.CharArrayDictionary<TValue>::ctor(): Don't call virtual methods in the constructor. * Lucene.Net.Analysis.Util (CharArraySet + CharArrayDictionary): Implemented standard interfaces for collections and enumerators. Consolidated error messages in new SR class. * BREAKING: Lucene.Net.Analysis.Util.CharArrayDictionary: Removed Get() overloads from the public API. Refactored them to throw KeyNotFoundException instead of returning default(TValue). Enabled nullable reference type support and fixed warnings. * BREAKING: Lucene.Net.Analysis.Util.CharArrayDictionary: Refactored Put() overloads allow for value types without requiring them to be made nullable. The signature was changed to return a bool and the previousValue (that was returned before) was made into an out parameter. * BREAKING: Lucene.Net.Analysis.Util (CharArraySet + CharArrayDictionary): Changed all object overloads of common methods from object to T to allow passing through value types without boxing. * Lucene.Net.Analysis.Util (CharArraySet + CharArrayDictionary): Reworked ConvertObjectToChars() and optimized ICharSequence paths. Fixed both to check ICharSequence.HasValue before using and throw ArgumentNullException if it is not true when setting keys. * BREAKING: Removed Lucene.Net.Analysis.Util.CharArraySetExtensions. These are edge cases that are now handled by ConvertObjectToChars(). * BREAKING: Removed Lucene.Net.Analysis.Util.CharArrayDictionaryExtensions. These are edge cases that are now handled by ConvertObjectToChars(). * PERFORMANCE: Lucene.Net.Analysis.Util.CharArrayDictionary::Copy(): Use Span<T> when copying the dictionary/set. * Lucene.Net.Analysis.Util.CharArrayDictionary.Enumerator::CurrentValue: Set to MaybeNull to match SetValue() * Lucene.Net.Analysis.Util (CharArraySet + CharArrayDictionary): Added documentation for public members * Lucene.Net.Analysis.Util.CharArrayDictionary: Renamed UnmodifiableCharArrayDictionary > ReadOnlyCharArrayDictionary to conform with .NET conventions. * BREAKING: Lucene.Net.Analysis.Util (CharArraySet + CharArrayDictionary): Renamed all method parameters to be consistently using text (instead of key) and startIndex (instead of offset) * BUG: Lucene.Net.Analysis.Util: Fixed this[char[], int, int] setter so it will correctly set an array representing startIndex and length as the key. Added overloads of Put() to allow setting slices of char[]. * Lucene.Net.Analysis.Util.CharArraySet: Added overloads of Add() to allow setting slices of char[]. * Lucene.Net.Analysis.Util.CharArrayDictionary: Fixed Set() and Add() overloads * BREAKING: Lucene.Net.Analysis.Util: Removed Put() overloads that don't accept a value from the public API. They were only intended to be used by CharArraySet. * BREAKING: Removed Add(KeyValuePair<string, TValue>) overload from the public API. * Lucene.Net.Analysis.Util.CharArrayDictionary: Added constructor overloads for char[] and ICharSequence. * BUG: Lucene.Net.Analysis.Util.CharArraySet::Empty: Changed empty generic closing type for the related CharArrayDictionary from string to object. * Lucene.Net.Analysis.Util: Changed extension methods to use IEnumerable<T> for better interop with LINQ.
This was referenced Apr 5, 2023
Merged
Merged
Merged
This was referenced Apr 12, 2023
Merged
|
All the issues that were reported by this rule have been addressed. Thank you for setting this up and reporting it! The fixing PRs have been linked to the issue and can be reviewed to see what was done. In some cases we were able to ignore the warning, in other cases we created private methods that can be called from the constructor, as well as the virtual methods where it was safe to do so. In publicly exposed classes that needed the functionality we introduced factory classes that are passed to the constructor and can be extended to introduce custom behavior. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Mentioned in #648
Issues found: https://sonarcloud.io/project/issues?resolved=false&rules=csharpsquid%3AS1699&id=apache_lucenenet
This is a difference in initialization order between Java and .NET. In Java it is safe to make a call to a virtual member from a constructor, but in .NET it is not. Same issue in #408. I still haven't found a good solution to this, but it would be nice to have a list of all of the places where it is a problem so we can consider what solution(s) will work.
The text was updated successfully, but these errors were encountered: