Skip to content
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

Merged
merged 6 commits into from Jun 6, 2017
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jun 6, 2017

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.

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.
@odersky odersky mentioned this pull request Jun 6, 2017
@@ -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)
Copy link
Member

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)
Copy link
Member

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 ?

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

@smarter smarter merged commit 6c20773 into scala:master Jun 6, 2017
@allanrenucci allanrenucci deleted the change-harden-ide-2 branch December 14, 2017 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants