Rename symbol in loaded csx throws NullReferenceException

[bjorkstromm]

OmniSharp-VSCode: 1.15.2 OmniSharp-Roslyn: 1.31.2-beta.73

Rename symbol, which should rename in loaded .csx throws NullReferenceException

Steps to reproduce:

1. Add following files test1.csx

   public class Foo {}

   test2.csx

   
   #load "test1.csx"
   var foo = new Foo();

2. Rename Foo in test2.csx
3. Exception is thrown. See request and response below.

[dbug]: OmniSharp.Stdio.Host
        ************ Request ************
{
  "Type": "request",
  "Seq": 920,
  "Command": "/rename",
  "Arguments": {
    "FileName": "c:\\Users\\mb\\src\\tmp\\test\\test2.csx",
    "Line": 3,
    "Column": 16,
    "WantsTextChanges": true,
    "RenameTo": "FooX"
  }
}
[dbug]: OmniSharp.Stdio.Host
        ************  Response ************ 
{
  "Request_seq": 920,
  "Command": "/rename",
  "Running": true,
  "Success": false,
  "Message": "\"System.NullReferenceException: Object reference not set to an instance of an object.\\r\\n   at Microsoft.CodeAnalysis.Rename.RenameLocations.ReferenceProcessing.<GetRenamableDefinitionLocationsAsync>d__6.MoveNext()\\r\\n--- End of stack trace from previous location where exception was thrown ---\\r\\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\\r\\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\\r\\n   at Microsoft.CodeAnalysis.Rename.RenameLocations.<AddLocationsReferenceSymbolsAsync>d__30.MoveNext()\\r\\n--- End of stack trace from previous location where exception was thrown ---\\r\\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\\r\\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\\r\\n   at Microsoft.CodeAnalysis.Rename.RenameLocations.<FindAsync>d__26.MoveNext()\\r\\n--- End of stack trace from previous location where exception was thrown ---\\r\\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\\r\\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\\r\\n   at Microsoft.CodeAnalysis.Rename.Renamer.<RenameSymbolAsync>d__4.MoveNext()\\r\\n--- End of stack trace from previous location where exception was thrown ---\\r\\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\\r\\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\\r\\n   at OmniSharp.Roslyn.CSharp.Services.Refactoring.RenameService.<Handle>d__2.MoveNext()\\r\\n--- End of stack trace from previous location where exception was thrown ---\\r\\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\\r\\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\\r\\n   at OmniSharp.Endpoint.EndpointHandler`2.<HandleSingleRequest>d__18.MoveNext()\\r\\n--- End of stack trace from previous location where exception was thrown ---\\r\\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\\r\\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\\r\\n   at OmniSharp.Endpoint.EndpointHandler`2.<Process>d__16.MoveNext()\\r\\n--- End of stack trace from previous location where exception was thrown ---\\r\\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\\r\\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\\r\\n   at OmniSharp.Stdio.Host.<HandleRequest>d__15.MoveNext()\"",
  "Body": null,
  "Seq": 193,
  "Type": "response"
}

Stacktrace

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.CodeAnalysis.Rename.RenameLocations.ReferenceProcessing.<GetRenamableDefinitionLocationsAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.CodeAnalysis.Rename.RenameLocations.<AddLocationsReferenceSymbolsAsync>d__30.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.CodeAnalysis.Rename.RenameLocations.<FindAsync>d__26.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.CodeAnalysis.Rename.Renamer.<RenameSymbolAsync>d__4.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at OmniSharp.Roslyn.CSharp.Services.Refactoring.RenameService.<Handle>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at OmniSharp.Endpoint.EndpointHandler`2.<HandleSingleRequest>d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at OmniSharp.Endpoint.EndpointHandler`2.<Process>d__16.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at OmniSharp.Stdio.Host.<HandleRequest>d__15.MoveNext()\"

[filipw]

Looks like this should be open in the Roslyn repo instead though? I think we could put together a simple repro that is completely independent of anything OmniSharp specific. Seems like a variant of this https://github.com/dotnet/roslyn/issues/20040

[filipw]

Actually I am pretty sure this used to work when script references were added manually as project references in the project model. We went away from that as it had a bunch of other problems, but perhaps we can be a bit smarter about what is happening when the resolver is plugged in and how script references are incorporated into the workspace.

[bjorkstromm]

Tried reproducing using unit test by adding this test to RenameFacts.cs:

        [Fact]
        public async Task Rename_UpdatesMultipleCsxDocumentsIfNecessary()
        {
            const string code1 = "public class Foo {}";
            const string code2 = @"
#load ""test1.csx""
var foo = new F$$oo();";

            var testFiles = new[]
            {
                new TestFile("test1.csx", code1),
                new TestFile("test2.csx", code2)
            };

            using (var host = CreateOmniSharpHost(testFiles))
            {
                var result = await PerformRename(host, testFiles, "xxx", wantsTextChanges: true, applyTextChanges: false);
                var changes = result.Changes.ToArray();

                Assert.Equal(2, changes.Length);
            }
        }

However, I'm unable to reproduce this exact error. But what I'm seeing is that RenameService can't find the symbol here.

[bjorkstromm]

@filipw

when script references were added manually as project references in the project model.

Did /findusages also work then? In this case test1.csx would find than test2.csx uses Foo?

We went away from that as it had a bunch of other problems, but perhaps we can be a bit smarter about what is happening...

When/if .cake moves towards "roslyn-style-load" and skips current "flat-file" approach, I would be keen on having references between script files. Would be happy to help where needed.