Issue with collectLocations

[daddykotex]

So we've hit a bug in some part of the language-server. I found what's triggering it and I have a small reproduction. When the bug happens, the project is not loaded and the rest of the IDE experience is poorly degraded.

The cause is the following NPE:

[11:47:51] java.lang.NullPointerException: Cannot invoke "org.eclipse.lsp4j.Location.getRange()" because "<local6>" is null
	at software.amazon.smithy.lsp.ext.SmithyProject.collectMemberLocations(SmithyProject.java:274)
	at software.amazon.smithy.lsp.ext.SmithyProject.collectLocations(SmithyProject.java:239)
	at software.amazon.smithy.lsp.ext.SmithyProject.lambda$new$0(SmithyProject.java:70)
	at java.base/java.util.Optional.ifPresent(Optional.java:178)
	at software.amazon.smithy.lsp.ext.SmithyProject.<init>(SmithyProject.java:70)
	at software.amazon.smithy.lsp.ext.SmithyProject.load(SmithyProject.java:174)
	at software.amazon.smithy.lsp.ext.SmithyProject.load(SmithyProject.java:145)

This is rather explicit: we pull something out of a map, it can be null if the thing is not in the map, but we don't check for that, so it blows up.

The quick fix is to do a null check. The right fix is to find why this happens. So I went on a hunt.

Given the following:

.
├── core
│   └── common.smithy
└── test
    └── test.smithy

With the following content:

> cat core/common.smithy 
$version: "2"

namespace core

boolean IsTestInput

@mixin
structure HasIsTestParam {
  isTest: IsTestInput = false
}

> cat test/test.smithy 
$version: "2"

namespace test

use core#HasIsTestParam

service AService {
  version: "1",
  operations: [SomeOp]
}
operation SomeOp {
  input: SomeOpInput
}

structure SomeOpInput with [HasIsTestParam] {
  @required
  body: String
}

apply SomeOpInput$isTest @documentation("Some documentation")

This specific workspace will blow up the LSP w/ the error above.

Everything is fine if I comment the following line: apply SomeOpInput$isTest @documentation("Some doc")

Also, everything is fine if I pull the shapes from core into test (placing the shapes in the same file / same namespace).

[daddykotex]

While debugging, I tweaked the implementation of LspLog a little bit with two improvements:

• when we fill the buffer to eventually write to the file, we duplicate the timestamp
• when doing println(exception)

it would be great if the whole stack trace was flushed in the file, it's much easier to find the cause of an issue.

Do you want a PR for that?

[milesziemer]

That PR would be welcome.

I think there are a variety of things that could be contributing to the issue, but I suspect that the main cause is that collectLocations doesn't seem to be handling apply statements targeting members, at least not properly. Its a bit more complicated than I thought at first... When apply is used on a member that has been mixed in from a different namespace (it seems to be this very specific case from my testing), the namespace of the shapes are changed to the namespace from which the apply is used. Using the example: within SmithyProject::collectLocations, when collecting locations from core/common.smithy we find the member shape test#SomeOpInput$isTest with the container test#SomeOpInput. But the namespace is incorrect, so when we go into collectMemberLocations and call locations.get, there's no container with the id test#SomeOpInput. I'll have to do some more digging to figure out what is causing that.