Annotations for cases the analyzer can't figure out.

[JohanLarsson]

public interface IWithAnnotations
{
    [return: MustDispose]
    IDisposable Create();

    bool TryCreate([MustDispose]out IDisposable disposable);

    [return: DoNotDispose]
    IDisposable GetOrCreate();

    bool TryGet([DoNotDispose]out IDisposable disposable);

    void Add([TransferOwnership] IDisposable disposable);
}

Not sure what attributes are needed and how we should word things. Also should probably not require a dependency on a nuget with attributes. Does JetBrains.Annotations have something for this?

[JohanLarsson]

An alternative is to rely on doc comments, or both. Ownership of disposables is a reasonable thing to document.

[bclothier]

IMO, having the ability to annotate at class? interface? level is good to have to provide a default behavior for all of its members, and thus avoid having to annotate every member that returns a IDisposable.

I would probably flag the lack of comment separately. That way we get reminder to document our methods and you get to avoid the complexity of parsing the doc comments to decide which behavior to apply.

[bclothier]

After thinking about it more - I'm convinced that I'd rather have those annotations be on the interfaces and its members rather than classes and its members. We do not want this scenario:

interface IFoo {
  IDisposable Bar();
}

class Goo : IFoo
{
  [returns:DisposedByCaller]
  IDisposable Bar() { ... }
}

class Hoo : IFoo
{
  [returns:Cached]
  IDisposable Bar() { ... }
}

Requiring the class to implement an interface to get the use of annotation seems preferable than to run into this messy scenario. This also conveniently fixes the problem of where analyzer cannot reason about the implementation due to us passing around interfaces.

[JohanLarsson]

About the scenario: annotation must be on the interface I think. That is what you wrote yes. Also for different implementations the analyzer can check it.

Do you have suggestions for what an annotation ~API using attributes could look like? The stuff I wrote in the issues start was just a quick suggestion without much thought. Just keep ideas coming and we can do some voting with 👍 and 👎 and see if we can agree. It should definitely not be me deciding. Also @jeremyVignelles @milleniumbug & everyone.

[bclothier]

I think the return decorations covers by far the large majority of the problems we would encounter with resolving the ownership.

Regarding the parameter annotation, TBH this feels funky to me to be calling a method that will dispose a passed-in object. I'd rather encourage the design where the caller will dispose the object rather than the callee.

Also, as I mentioned in issue #127, it might be useful to know that a method must dispose those that weren't returned while leaving up to the caller to dispose the returned.

[jeremyVignelles]

@bclothier : IIRC, this is what StreamReader does with the stream, right?
You can also have CompositeDisposable, that takes ownership of several disposables. In this case, this is explicit

[bclothier]

@jeremyVignelles The thing is that it's for a constructor, not for a method. I think that's the crucial difference, isn't it? You can't have a constructor on an interface; only an abstract class.

Are you referring to this CompositeDisposable? It says it's going to be deprecated?

[jeremyVignelles]

What's the difference between a class and an interface from the analyzer's point of view?
In the CompositeDisposable's example, you have an Add method that would need this kind of decoration

[bclothier]

As mentioned earlier, we should not have annotations on a concrete class which may be provided as interface or we fall in the situation where class A caches but class B doesnt. The consumers using an interface implemented by both is now screwed. Enforcing the annotation on only interfaces or abstract classes(?) ensures that all implementations must be consistent in how they will handle the disposables.

[jeremyVignelles]

I didn't read the full conversation yet.
There are cases where you don't have interfaces. I'd say "Don't specify what should be done with the Disposable in an override". This rule applies to interface implementations, abstract method overrides as well as virtual overrides. (does it apply to new methods?)
This doesn't mean you can't put an annotation on the constructor or in a specific method

[milleniumbug]

You need to enforce whether implementing/overriding method doesn't introduce extra requirements in the contract (in the LSP sense).

[JohanLarsson]

Made a PR with draft names, please review & comment.