-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Harden IDE, 2nd attempt #2692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Harden IDE, 2nd attempt #2692
Conversation
They used to crash when applied to NoPosition. We now handle this case gracefully.
This happens, e.g. in realApply in Applications. Change TreeAccumulator and TreeMap to survive on untyped tree nodes in error cases. Also, add a configurable check that typed nodes point to untyped ones only in error cases.
Seems to happen in several scenarios. For now, ignore, so that we can get real work done in the IDE. Ignoring is configurable, we can turn it off if we want to hunt down problems.
Avoids repeated crashes when hitting a tree that exists only in untyped form.
@@ -14,4 +15,7 @@ class InteractiveCompiler extends Compiler { | |||
override def phases: List[List[Phase]] = List( | |||
List(new FrontEnd) | |||
) | |||
|
|||
override def rootContext(implicit ctx: Context) = | |||
super.rootContext.fresh.addMode(Mode.Interactive) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's also Mode.ReadPositions that we always need in the IDE, it's currently set in InteractiveDriver but could be moved here.
/** Configurable: Accept stale symbol with warning if in IDE */ | ||
def acceptStale(denot: SingleDenotation): Boolean = | ||
(mode.is(Mode.Interactive) && Config.ignoreStaleInIDE) && { | ||
ctx.warning(denot.staleSymbolMsg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such a warning will popup in the IDE (not currently because we ignore warnings without positions, but that's just because of an issue in the LSP (microsoft/language-server-protocol#249)), maybe use ctx.debug instead ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM
On stale symbols: There are legitimate scenarios how they can arise in the IDE.
For instance, we define a class, refer to it, edit the class definition and introduce errors. The class symbol temporarily no longer exists until the errors are fixed. But typed trees elsewhere will refer to it. That's why StaleSymbol errors now give warnings by default. A config option can turn them back into exceptions, which is useful if we want to find the root cause of a particular warning.