Code Fixes/refactorings should add using statements if necessary and possible

[YairHalberstadt]

For example, when pull members up to base class is used, and the base class is in a separate document without a suitable using statement, the following happens:

interface Interface
{
    public System.Threading.Tasks.Task PulledUp(System.Guid guid);
}

Instead I think it would be best if pull members up tried to preserve the names as they appeared in the original document. This means that it should add using statements if doing so will not cause conflicts.

Thus it should result in:

using System;
using System.Threading.Tasks;

interface Interface
{
    public Task PulledUp(Guid guid);
}

Similiar applies to Implement members, generate variable, etc.

[sharwell]

I was recently hoping the code fix APIs supported an annotation like Simplifier.Annotation that does this, but it doesn't seem to be in place yet.

[CyrusNajmabadi]

We investigated this at one point. Main problem was changes in semantics. We could try an approach that complexifies/simplifies in the presence of the new usings though. I think our work predated having that system.

[YairHalberstadt]

I believe resharper does this, so it should be possible in theory at least.

[CyrusNajmabadi]

I believe resharper does this, so it should be possible in theory at least.

Our last investigation (which admittedly was years ago) showed resharper producing far more bugs in these sorts of cases. That was def something i wanted to avoid as a constant theme we were hearing from people was "we want to be able to trust these refactorings". It's def possible the landscape has changed though (i.e. the tools have gotten better, or people are more willing to just accept the more desirable result, even if it causes problems a higher percentage of times).

[YairHalberstadt]

The naive approach would be to choose which namespace you want to add, build a hashset of all types mentioned in the document, and check if any members of the namespace are in the hashset of types in the document. If not, you add the using, if yes, you don't.

That would be overly conservative, but significantly better than the current approach.

I have no idea what the performance impact would be.

[CyrusNajmabadi]

It's unfortunately more complex than this. For example, because of extension methods. Also (and this is really crazy, but does happen in practice), but adding/removing theses usings can still break things because it can affect error paths in lambdas. Meaning that the types from the usings might not even be used in teh final compiled code, but touching them can change meaning.

Note: that's if you want to ensure no changed semantics. I'd be ok saying that the benefit here is good enough that we can accept minor breaks in edge cases.

[YairHalberstadt]

The lambdas could be solved by looking for type names at the syntactic level, not the symbol level. So any expression which can be a type you put the name of in the hashset.

Extension methods, especially for things like deconstructs etc. is more complex.

For pull members up it may be possible to try a completely different approach. Just try it, and then diff the operation tree for all other methods to see if it changed them. I have no idea if that would work though.

[CyrusNajmabadi]

see if it changed them

This also gets complex, and usually requires custom logic. i.e. by definition the code is now different (things like the 'this' reference in the method being an obvious case). So you need to write and define what equality means.

note: none of this is meant to try to say this can't be done. just outlining the size of hte task :)

Extension methods, especially for things like deconstructs etc. is more complex.

Definitely. And i imagine it will get worse in later language versions. i.e. if we do shapes, and if witness types and the like are brought in through usings, etc. etc. :)

[sharwell]

Design review conclusion:

• We already have a service to add missing using directives, but the service does not automatically run during the post-processing of code fixes.
• We need a new public-facing annotation which means "allow simplifying this name by adding a using directive". The annotation would not be applied by default in the syntax factory APIs, but could be applied manually (or possibly in the generator/editor APIs) during a code fix computation.
• The code fix post-processing steps would consider this annotation during the simplification steps, leveraging existing services for adding using directives.