nikcio opened a new pull request, #746: URL: https://github.com/apache/lucenenet/pull/746
Related to #265 I've run through most of the issues reported in Sonarcloud related to the use of the IDisposable pattern (https://sonarcloud.io/project/issues?resolved=false&rules=csharpsquid%3AS3881&id=apache_lucenenet) This PR is meant to further expose which implementations of the pattern require more investigation while also removing the easiest areas where the pattern just needs a simple alignment. This means that if any of the following changes exposes an issue requiring much more effort this is best to be moved to comment on the existing issue and have a solution opened in another PR. I realize that a correct implementation of this pattern can be advanced and is related to at least a handful of issues opened in this repository which is why I want to take the approach stated 😄 . To help the review I've outlined the changes here in some categories: ## Issues from private classes were marked `sealed` Private classes don't have to uphold the same pattern as other classes if they are marked `sealed`. ## Basic implementation of pattern in some classes The following files had a class where the following template was implemented. ```csharp public void Dispose() { Dispose(true); GC.SuppressFinalize(this); } protected virtual void Dispose(bool disposing) { if (disposing) { // Cleanup } } ``` 1. BufferedCharFilter 2. MemoryDocValuesConsumer 3. PerFieldDocValuesFormat 4. PerFieldPostingsFormat 5. IndexWriter 6. TaskMergeScheduler ## `base.Dispose(disposing)` referencing code The `base.Dispose(disposing)` line in `BufferedCharFilter` references the dispose method in `Lucene.Net.Analysis.CharFilter` ## `base.Dispose(disposing)` referencing disposing methods in `System.IO` The changes in the following files have `base.Dispose(disposing)` methods which reference code in `System.IO` 1. SourceCodeSectionReader 2. SafeTextWriterWrapper ## Fixed `GC.SuppressFinalize` call In `PrefixCodedTerms` the call to `GC.SuppressFinalize` had the object parameter set to `true` instead of `this` ## `FSDirectory` There's a commented `base.Disposing(disposed)` change in `FSDirectory` which when activated causes at least 140 tests to fail (I didn't run it all the way through when I noticed the error). Please validate that the `Dispose(bool)` method isn't supposed to call its base. ## Other `base.Dispose(dispoing)` Other places that aren't part of the categories above reference an empty `Dispose(bool)` method in its base. Therefore, this shouldn't cause any side effects and only make future development more reliable as the classes will follow the same dispose pattern. ## Sidenote Assuming the comment around this `Dispose` method is correct this issue can be removed as intended either in SonarCloud or in source: https://sonarcloud.io/project/issues?issues=AYRH0T17_qq9ReJdi40Q&open=AYRH0T17_qq9ReJdi40Q&id=apache_lucenenet -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
