IExtractMethodService and Preconditions

Question

Hello, I'm part of an undergraduate research team that is using Roslyn to learn and develop some essential refactoring tools.

1. We're using IRenameService but have not implemented any precondition checking. What would be the best way to go about that? For example, would I use a Compilation to search if a new name already exist for a local variable, global, and method?

2. Also, we noticed there is an IExtractMethodService. What exactly does it do? I'm guessing it just extracts the TextSpan parameter and uses it in the new method. What about the new method name? I assume all I need to check is if the TextSpan is a valid expression to be used in the new method?

3. Final question: what other services are there available? Is there some sort of list?

Thanks.

Answer 1

For #1: what sort of preconditions do you want to look for? The Roslyn rename engine has limited support for resolving name collisions that previously were invalid. For example, if you rename a local to the same name as a field, we will qualify the uses of the field with "this." to remove the disambiguation. In general, you can get yourself into some nasty cases where the only way to know if a given name will cause problems is to recompile the entire project and see if you introduced errors. Right now the IRenameService doesn't expose any information about those sorts of problems. If you can describe your particular scenario, I might be able to give a more concrete answer.

For #2: the IExtractMethodService implements the "Extract Method" refactoring in the IDE. The API is intended to be a resusable component to build larger refactorings with, as implementing it is very tricky. It doesn't go modifying any state, but simply returns a new syntax tree with the method rewritten. It's then up to you to use that syntax tree in whatever way you see fit. The new method name is picked automatically via series of heuristics, and it automatically makes sure the final method name doesn't conflict with another method name already in the method.

The ExtractMethodResult object the method returns gives you two tokens in the resulting tree: the name of the method at the declaration location and the token at the invocation. If you wanted to change the name, you could take the resulting tree and replace those two tokens with the name you desire. There isn't currently a way to specify the name you want before the rewrite.

As far as the TextSpan you pass in, the checking for what is and isn't a valid span is quite complex. When you call the service, the ExtractMethodResult object tells you whether the TextSpan was valid or not, and if it wasn't, why. Rather than implementing that logic yourself, I'd encourage you to just call the service and use what it gives you.

Answer 2

Thanks for the prompt reply!

I am having trouble getting the CodeAction for the IExtractMethodService to work. I am using a similar approach to importing it as the IRenameService.

Could you tell me what I'm doing wrong?

namespace ExtractMethod
{
    [ExportCodeRefactoringProvider("Extract Method", LanguageNames.CSharp)]
    class ExtractMethod : ICodeRefactoringProvider
    {
        private readonly ICodeActionEditFactory editFactory;

        [ImportingConstructor]
        public ExtractMethod(ICodeActionEditFactory editFactory)
        {
            this.editFactory = editFactory;
        }

        [Import]
        private IExtractMethodService extractMethodService = null;

        [Import]
        private IWorkspaceDiscoveryService workspaceDiscoveryService = null;

        public CodeRefactoring GetRefactoring(IDocument document, TextSpan textSpan, CancellationToken cancellationToken)
        {
            var syntaxTree = document.GetSyntaxTree(cancellationToken);
            var token = syntaxTree.Root.FindToken(textSpan.Start);

            if (token.Parent == null)
            {
                return null;
            }

            var workspace = workspaceDiscoveryService.GetWorkspace(document.GetText().Container);

            return new CodeRefactoring(
                new[] { new ExtractMethodAction(workspace, extractMethodService, document, textSpan) },
                token.Span);
        }
    }

public class ExtractMethodAction : ICodeAction
    {
        private IWorkspace workspace;
        private IExtractMethodService extractMethodService;
        private IDocument document;
        private TextSpan span;

        public ExtractMethodAction(IWorkspace workspace, IExtractMethodService extractMethodService,
            IDocument document, TextSpan span)
        {
            this.workspace = workspace;
            this.extractMethodService = extractMethodService;
            this.document = document;
            this.span = span;
        }

        public string Description
        {
            get { return "My Extract Method"; }
        }

        public ImageSource Icon
        {
            get { return null; }
        }

        public ICodeActionEdit GetEdit(CancellationToken cancellationToken = default(CancellationToken))
        {
            // Use a custom ICodeActionEdit instead.
            return new ExtractMethodEdit(workspace, extractMethodService, document, span);
        }
    }

public class ExtractMethodEdit : ICodeActionEdit
    {
        private IWorkspace workspace;
        private IExtractMethodService extractMethodService;
        private IDocument document;
        private TextSpan span;

        public ExtractMethodEdit(IWorkspace workspace, IExtractMethodService extractMethodService,
            IDocument document, TextSpan span)
        {
            this.workspace = workspace;
            this.extractMethodService = extractMethodService;
            this.document = document;
            this.span = span;
        }

        public Task ApplyAsync(IWorkspace workspce,
            CancellationToken cancellationToken = default(CancellationToken))
        {
            return Task.Factory.StartNew(() =>
                    extractMethodService.ExtractMethod(document, span));
        }

        public object GetPreview(CancellationToken cancellationToken = default(CancellationToken))
        {
            // Return null since we don't have a way to preview the changes that the
            // extract method service will perform.
            return null;
        }
}

Answer 3

Hi roslynese - Could you please elabotate a bit i.e. what exactly is the problem you are running into with the above code?

---

Shyam Namboodiripad | Software Development Engineer in Test | Roslyn Compilers Team

Answer 4

With IRenameService, I get a small menu when I have my the text cursor on a variable identifier; how do I get that menu for a textspan I want to extract? The default VS Extract Method refactoring shows the small menu when a user highlights some text.

Shouldn't CodeAction be getting that menu?

It would be best if you could show me how you would normally use the service.

Answer 5

So IExtractMethodService you don't just MEF import like you do IRenameService. I would bet if you set a breakpoint in your ICodeRefactoringProvider constructor you'll see it never being called.

IExtractMethodService implements ILanguageService which is your hint that you need to get the service on a per-language basis. In your GetRefactoring method, do:

document.LanguageServices.GetService<IExtractMethodService>()

That should give you an instance of the IExtractMethodService for the given language which you can use from there.

Answer 6

That definitely helps. How exactly would I implement the CodeRefactoringProvider so that the user can invoke the refactoring action when the user highlights a piece of code or textspan?

I'm not able to see the small quick option with my extract method implementation.

Answer 7

I'm not sure I understand your question now. A CodeRefactoringProvider provides precisely that: it lets you provide a refactoring for when the user selects a piece of code. The existing extract method feature implements itself as a ICodeRefactoringProvider

Do you think your provider still isn't being called for some reason? Dropping a breakpoint in GetRefactoring can verify that.

Answer 8

I think I'm confused. So, I believe GetRefactoring() returns when the user has selected a certain piece of code under the logic of the method. For example, for the GetRefactoring() method that uses IRenameService, the user must "select" (has the text cursor on) a VariableDeclaratorSyntax:

public CodeRefactoring GetRefactoring(IDocument document, TextSpan textSpan, CancellationToken cancellationToken)
        {
            var syntaxTree = document.GetSyntaxTree(cancellationToken);
            var token = syntaxTree.Root.FindToken(textSpan.Start);
            if (token.Parent == null)
            {
                return null;
            }

            var declaration = token.Parent.FirstAncestorOrSelf<VariableDeclaratorSyntax>();
            var variable = (VariableDeclaratorSyntax)declaration;
            if (declaration == null)
            {
                return null;
            }

            var semanticModel = (SemanticModel)document.GetSemanticModel();
            var symbol = semanticModel.GetDeclaredSymbol(variable);
            var workspace = workspaceDiscoveryService.GetWorkspace(document.GetText().Container);

            return new CodeRefactoring(
                new[] { new RenameCodeAction(workspace, renameService, document, symbol) },
                    variable.Identifier.Span);
        }

The user then sees a small menu under variable identifier with the description of "My Rename Refactor" or something like that.

My problem is that I don't know how to get that small menu to show with the GetRefactoring() method for extract method:

 public CodeRefactoring GetRefactoring(IDocument document, TextSpan textSpan, CancellationToken cancellationToken)
        {
            var syntaxTree = document.GetSyntaxTree(cancellationToken);
            var token = syntaxTree.Root.FindToken(textSpan.Start);

            if (token.Parent == null)
            {
                return null;
            }

            var workspace = workspaceDiscoveryService.GetWorkspace(document.GetText().Container);

            return new CodeRefactoring(
                new[] { new ExtractMethodAction(workspace, extractMethodService, document, textSpan) },
                token.Span);
        }

I set a breakpoint on the method but I'm not getting anything while debugging, "No Source Available." I've only debugged console apps and XNA Game Studio apps, is it the same for debugging a Roslyn extension?

Answer 9

So if you're not getting a breakpoint being hit, that means you're not properly exporting your provider via MEF. It has nothing to do with the code inside GetRefactoring. MEF has a behavior that if you import something that it can't satisfy, it silently rejects your extension and so stuff won't be called.

Can I see the code you currently have in your provider? You can omit GetRefactoring, since it's not the problem here.

Answer 10

Yes, my provider code is similar to above: 

 [ExportCodeRefactoringProvider("Extract Method", LanguageNames.CSharp)]
    class ExtractMethod : ICodeRefactoringProvider
    {
        private readonly ICodeActionEditFactory editFactory;

        [ImportingConstructor]
        public ExtractMethod(ICodeActionEditFactory editFactory)
        {
            this.editFactory = editFactory;
        }

        [Import]
        private IExtractMethodService extractMethodService = null;

        [Import]
        private IWorkspaceDiscoveryService workspaceDiscoveryService = null;

        public CodeRefactoring GetRefactoring(IDocument document, TextSpan textSpan, CancellationToken cancellationToken)
        {
            extractMethodService = document.LanguageServices.GetService<IExtractMethodService>();
            var syntaxTree = document.GetSyntaxTree(cancellationToken);
            var token = syntaxTree.Root.FindToken(textSpan.Start);

            if (token.Parent == null)
            {
                return null;
            }

            var workspace = workspaceDiscoveryService.GetWorkspace(document.GetText().Container);

            return new CodeRefactoring(
                new[] { new ExtractMethodAction(workspace, extractMethodService, document, textSpan) },
                token.Span);
        }
    }

Answer 11

Ah, what I suspected: remove your extractMethodService field and the [Import] attribute for it. Since that's not imported that way, MEF couldn't find it, and so wasn't exporting your provider at all. Thus it was never being called.

The one way you can debug this is look for CompositionExceptions during the load of VS, but even then it's still really hard to find.

Answer 12

Ok! Now, we're getting somewhere. My problem now is a NullReferenceException in GetEdit() of my CodeAction:

        public ICodeActionEdit GetEdit(CancellationToken cancellationToken = default(CancellationToken))
        {
            var tree = (SyntaxTree)document.GetSyntaxTree(cancellationToken);
            ExtractMethodResult extractResult = extractMethodService.ExtractMethod(document, span); // NullReferenceException here
            var newRoot = extractResult.ResultingTree;
            return editFactory.CreateTreeTransformEdit(document.Project.Solution, tree, newRoot);
        }

Not sure if the span is null. (Don't think it is the span) Does extractMethod() check the span parameter for any preconditions?

I really appreciate your patience and help!

Answer 13

ExtractMethod will verify the span is a valid span, but it shouldn't be throwing a null ref. TextSpan can't be null, as it's a struct.

Do you have a call stack for the exception?

Answer 14

Yes:

>    ExtractMethod.dll!ExtractMethod.ExtractMethodAction.GetEdit(System.Threading.CancellationToken cancellationToken) Line 53 + 0x4c bytes    C#

What happens when the span is not valid?

Also, just in case:

+        document    {Roslyn.Services.Document}    Roslyn.Services.IDocument {Roslyn.Services.Document}
+        extractMethodService    {Roslyn.Services.Editor.CSharp.ExtractMethod.CSharpExtractMethodService}    Roslyn.Services.Editor.IExtractMethodService {Roslyn.Services.Editor.CSharp.ExtractMethod.CSharpExtractMethodService}
        extractResult    null    Roslyn.Services.Editor.ExtractMethodResult
+        span    {[256..315)}    Roslyn.Compilers.TextSpan
+        this    {ExtractMethod.ExtractMethodAction}    ExtractMethod.ExtractMethodAction

Answer 15

extract method shouldn't return Null. in a case where extract method can't be performed, it should return a result with "Succeeded" property set to "false". and all its other property set to default(type).

...

if it returned null, that probably is a bug. can you give us some repro code?

thank you

- heejae

---

HeeJae Chang

Answer 16

Sorry for taking so long to reply the code you requested:

namespace ExtractMethod
{
    [ExportCodeRefactoringProvider("ExtractMethod", LanguageNames.CSharp)]
    class CodeRefactoringProvider : ICodeRefactoringProvider
    {
        private readonly ICodeActionEditFactory editFactory;

        [ImportingConstructor]
        public CodeRefactoringProvider(ICodeActionEditFactory editFactory)
        {
            this.editFactory = editFactory;
        }

        [Import]
        private IWorkspaceDiscoveryService workspaceDiscoveryService = null;

        public CodeRefactoring GetRefactoring(IDocument document, TextSpan textSpan, CancellationToken cancellationToken)
        {
            var extractMethodService = document.LanguageServices.GetService<IExtractMethodService>();
            var syntaxTree = document.GetSyntaxTree(cancellationToken);
            var token = syntaxTree.Root.FindToken(textSpan.Start);

            if (token.Parent == null)
            {
                return null;
            }

            var declaration = token.Parent.FirstAncestorOrSelf<StatementSyntax>();
            var variable = (StatementSyntax)declaration;
            if (declaration == null)
            {
                return null;
            }

            var semanticModel = (SemanticModel)document.GetSemanticModel();
            //var symbol = semanticModel.GetDeclaredSymbol(variable);
            var workspace = workspaceDiscoveryService.GetWorkspace(document.GetText().Container);

            return new CodeRefactoring(
                new[] { new ExtractMethodAction(workspace, extractMethodService, document, variable.Span, editFactory) },
                    variable.Span);
        }
    }

public class ExtractMethodAction : ICodeAction
    {
        private IWorkspace workspace;
        private IExtractMethodService extractMethodService;
        private IDocument document;
        private TextSpan span;
        private ICodeActionEditFactory editFactory;

        public ExtractMethodAction(IWorkspace workspace, IExtractMethodService extractMethodService,
            IDocument document, TextSpan span, ICodeActionEditFactory editFactory)
        {
            this.workspace = workspace;
            this.extractMethodService = extractMethodService;
            this.document = document;
            this.span = span;
            this.editFactory = editFactory;
        }

        public string Description
        {
            get { return "Rfactor Extract Method"; }
        }

        public ImageSource Icon
        {
            get { return null; }
        }

        public ICodeActionEdit GetEdit(CancellationToken cancellationToken = default(CancellationToken))
        {
            var tree = (SyntaxTree)document.GetSyntaxTree(cancellationToken);
            System.IO.File.WriteAllText(@"../../Span.txt", span.ToString());

            var extractResult = extractMethodService.ExtractMethod(document, span);
            var newRoot = extractResult.ResultingTree;
            return editFactory.CreateTreeTransformEdit(document.Project.Solution, tree, newRoot);
            
            // Use a custom ICodeActionEdit instead.
            //return new ExtractMethodEdit(workspace, extractMethodService, document, span, editFactory);
        }
    }

public class ExtractMethodEdit : ICodeActionEdit
    {
        private IWorkspace workspace;
        private IExtractMethodService extractMethodService;
        private IDocument document;
        private TextSpan span;
        private ICodeActionEditFactory editFactory;

        public ExtractMethodEdit(IWorkspace workspace, IExtractMethodService extractMethodService,
            IDocument document, TextSpan span, ICodeActionEditFactory editFactory)
        {
            this.workspace = workspace;
            this.extractMethodService = extractMethodService;
            this.document = document;
            this.span = span;
            this.editFactory = editFactory;
        }

        public Task ApplyAsync(IWorkspace workspce,
            CancellationToken cancellationToken = default(CancellationToken))
        {
            var tree = (SyntaxTree)document.GetSyntaxTree(cancellationToken);
            ExtractMethodResult e = extractMethodService.ExtractMethod(document, span);
            var newRoot = e.ResultingTree;
            //return editFactory.CreateTreeTransformEdit(document.Project.Solution, tree, newRoot);
            //return Task.Factory.StartNew(() =>
            //        extractMethodService.ExtractMethod(document, span));
            return null;
        }

        public object GetPreview(CancellationToken cancellationToken = default(CancellationToken))
        {
            // Return null since we don't have a way to preview the changes that the
            // extract method service will perform.
            return null;
        }

        public void CancelRequest() { }
    }
}