dotnet / roslyn-analyzers

MIT License
1.59k stars 465 forks source link

CA2000, yet another false positive #3422

Open tamlin-mike opened 4 years ago

tamlin-mike commented 4 years ago

Not sure if this specific scenario has already been mentioned. We didn't find anything standing out among the many issues for this analyzer. Should this be a dupe, feel free to close this and link to main issue.

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v2.9.8 (Latest)

Diagnostic ID

CA2000

Repro steps

Have a method create a Stream. Use that stream as a parameter to multiple helper functions performing I/O on it, called f.ex. sequentially. For the sake of example say for reading.

Helper functions , for this example, follow the pattern:

void Helper(Stream stream) {
  var br = new BinaryReader(stream);
  // use 'br'
}

Expected behavior

No CA2000.

Actual behavior

All such uses in this scenario, in error gets tagged with CA2000.

Since the BinaryReader interface is obviously incomplete, it is impossible to place it in a using but detach from an existing stream before Dispose, and it lacks a constructor taking the bool leaveOpen without also providing an (in our case) unwanted Encoding, meaning we have to add suppressions for all these uses. Ugh.

Would it make sense for this warning to check if the stream is not created in the same scope (say, same method), and if so not provide its "helpful" suggestion to introduce bugs?

If not, we believe it needs to do DFA, look at its caller, and see if the stream continues to be used after the helper returns. Lacking that ability to do DFA (f.ex. is this is through an interface, where it could be called by an external assembly) we believe this CA2000 needs to be disabled (for this scenario).

Evangelink commented 4 years ago

@tamlin-mike The overload with only the Stream sets the encoding to UTF-8 so you could easily use the overload with the bool leaveOpen parameter. If you do so then you can dispose the BinaryReader and so there is no issue.

Tagging @mavasani

Evangelink commented 4 years ago

Ping @mavasani