Track visited element set in ResolveState to prevent StackOverflowError

Fixes #1467 Fixes #1480 Fixes #1418 Numerous users have reported that annotation can encounter a `StackOverflowError`. [A reproduction case](#1475 (comment)) shows that they are called by Phoenix `Web` modules where one function containing `quote` block `use`s the same module again, such as an `admin_view` depending on the base `view` through `use App.Web, :view`. When the `use App.Web, :view` is resolving, the `defmacro __using__` is re-entered as is the `admin_view` because there was no tracking of already visited `PsiElement`s. The fix is to track the visited elements and not re-enter the visited elements so that `admin_view` is skipped and the other call definition clauses can be checked to find `view`.
KronicDeth · May 20, 2019 · 027ded2 · 027ded2
1 parent 69ffa6f
commit 027ded2
Show file tree

Hide file tree

Showing 12 changed files with 296 additions and 219 deletions.
diff --git a/src/org/elixir_lang/beam/psi/impl/ModuleImpl.java b/src/org/elixir_lang/beam/psi/impl/ModuleImpl.java
@@ -2,6 +2,7 @@
 
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.PsiElement;
+import com.intellij.psi.ResolveState;
 import com.intellij.psi.StubBasedPsiElement;
 import com.intellij.psi.impl.source.tree.TreeElement;
 import com.intellij.psi.stubs.IStubElementType;
@@ -11,6 +12,7 @@
 import org.elixir_lang.psi.Modular;
 import org.elixir_lang.psi.call.Call;
 import org.elixir_lang.psi.call.MaybeExported;
+import org.elixir_lang.psi.scope.CallDefinitionClauseKt;
 import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;
@@ -30,6 +32,35 @@ public ModuleImpl(T stub) {
         this.stub = stub;
     }
 
+    @Contract(pure = true)
+    @NotNull
+    private static MaybeExported[] callDefinitions(@NotNull TreeElement mirror) {
+        PsiElement mirrorPsi = mirror.getPsi();
+        MaybeExported[] callDefinitions;
+
+        if (mirrorPsi instanceof Call) {
+            Call mirrorCall = (Call) mirrorPsi;
+            final List<MaybeExported> callDefinitionList = new ArrayList<>();
+            final ResolveState initialResolveState = CallDefinitionClauseKt.putInitialVisitedElement(ResolveState.initial(), mirrorCall);
+
+            Modular.callDefinitionClauseCallWhile(mirrorCall, initialResolveState, (call, accResolvedState) -> {
+                if (call instanceof MaybeExported) {
+                    MaybeExported maybeExportedCall = (MaybeExported) call;
+
+                    callDefinitionList.add(maybeExportedCall);
+                }
+
+                return true;
+            });
+
+            callDefinitions = callDefinitionList.toArray(new MaybeExported[callDefinitionList.size()]);
+        } else {
+            callDefinitions = new MaybeExported[0];
+        }
+
+        return callDefinitions;
+    }
+
     /**
      * Returns the array of children for the PSI element.
      * Important: In some implementations children are only composite elements, i.e. not a leaf elements
@@ -74,34 +105,6 @@ public void setMirror(@NotNull TreeElement element) throws InvalidMirrorExceptio
         setMirrors(callDefinitions(), callDefinitions(element));
     }
 
-    @Contract(pure = true)
-    @NotNull
-    private static MaybeExported[] callDefinitions(@NotNull TreeElement mirror) {
-        PsiElement mirrorPsi = mirror.getPsi();
-        MaybeExported[] callDefinitions;
-
-        if (mirrorPsi instanceof Call) {
-            Call mirrorCall = (Call) mirrorPsi;
-            final List<MaybeExported> callDefinitionList = new ArrayList<>();
-
-            Modular.callDefinitionClauseCallWhile(mirrorCall, call -> {
-                if (call instanceof MaybeExported) {
-                    MaybeExported maybeExportedCall = (MaybeExported) call;
-
-                    callDefinitionList.add(maybeExportedCall);
-                }
-
-                return true;
-            });
-
-            callDefinitions = callDefinitionList.toArray(new MaybeExported[callDefinitionList.size()]);
-        } else {
-            callDefinitions = new MaybeExported[0];
-        }
-
-        return callDefinitions;
-    }
-
     private MaybeExported[] callDefinitions() {
         return (MaybeExported[]) getStub().getChildrenByType(CALL_DEFINITION, CallDefinitionImpl[]::new);
     }

diff --git a/src/org/elixir_lang/psi/Import.kt b/src/org/elixir_lang/psi/Import.kt
@@ -5,6 +5,7 @@ import com.ericsson.otp.erlang.OtpErlangLong
 import com.ericsson.otp.erlang.OtpErlangRangeException
 import com.intellij.psi.ElementDescriptionLocation
 import com.intellij.psi.PsiElement
+import com.intellij.psi.ResolveState
 import com.intellij.usageView.UsageViewNodeTextLocation
 import com.intellij.usageView.UsageViewTypeLocation
 import com.intellij.util.Function
@@ -35,7 +36,7 @@ object Import {
      * matching names in `:except` list.
      */
     @JvmStatic
-    fun callDefinitionClauseCallWhile(importCall: Call, function: (Call) -> Boolean): Boolean =
+    fun callDefinitionClauseCallWhile(importCall: Call, resolveState: ResolveState, function: (Call, ResolveState) -> Boolean): Boolean =
         try {
             modular(importCall)
         } catch (stackOverflowError: StackOverflowError) {
@@ -45,7 +46,7 @@ object Import {
                 ?.let { modularCall ->
                     val optionsFilter = callDefinitionClauseCallFilter(importCall)
 
-                    Modular.callDefinitionClauseCallWhile(modularCall) { call -> !optionsFilter(call) || function(call) }
+                    Modular.callDefinitionClauseCallWhile(modularCall, resolveState) { call, accResolveState -> !optionsFilter(call) || function(call, accResolveState) }
                 }
                 ?: true
 

diff --git a/src/org/elixir_lang/psi/Modular.kt b/src/org/elixir_lang/psi/Modular.kt
@@ -1,11 +1,14 @@
 package org.elixir_lang.psi
 
+import com.intellij.psi.ResolveState
 import org.elixir_lang.Arity
 import org.elixir_lang.ArityRange
 import org.elixir_lang.Name
 import org.elixir_lang.psi.call.Call
 import org.elixir_lang.psi.impl.call.macroChildCallSequence
 import org.elixir_lang.psi.impl.call.macroChildCalls
+import org.elixir_lang.psi.scope.hasBeenVisited
+import org.elixir_lang.psi.scope.putVisitedElement
 
 data class AccumulatorContinue<out R>(val accumulator: R, val `continue`: Boolean)
 
@@ -15,15 +18,19 @@ object Modular {
             modular.macroChildCallSequence().filter { CallDefinitionClause.`is`(it) }
 
     @JvmStatic
-    fun callDefinitionClauseCallWhile(modular: Call, function: (Call) -> Boolean): Boolean {
+    fun callDefinitionClauseCallWhile(modular: Call, resolveState: ResolveState, function: (Call, ResolveState) -> Boolean): Boolean {
         val childCalls = modular.macroChildCalls()
         var keepProcessing = true
 
         for (childCall in childCalls) {
-            if (CallDefinitionClause.`is`(childCall) && !function(childCall)) {
-                keepProcessing = false
+            if (!resolveState.hasBeenVisited(childCall) && CallDefinitionClause.`is`(childCall)) {
+                val childResolveState = resolveState.putVisitedElement(childCall)
 
-                break
+                if (!function(childCall, childResolveState)) {
+                    keepProcessing = false
+
+                    break
+                }
             }
         }
 

diff --git a/src/org/elixir_lang/psi/QuoteMacro.kt b/src/org/elixir_lang/psi/QuoteMacro.kt
@@ -1,24 +1,41 @@
 package org.elixir_lang.psi
 
+import com.intellij.psi.ResolveState
 import org.elixir_lang.psi.call.Call
 import org.elixir_lang.psi.call.name.Function.QUOTE
 import org.elixir_lang.psi.call.name.Module.KERNEL
 import org.elixir_lang.psi.impl.call.macroChildCallSequence
+import org.elixir_lang.psi.scope.hasBeenVisited
+import org.elixir_lang.psi.scope.putVisitedElement
 
 object QuoteMacro {
-    fun callDefinitionClauseCallWhile(quoteCall: Call, keepProcessing: (Call) -> Boolean): Boolean {
+    fun callDefinitionClauseCallWhile(quoteCall: Call, resolveState: ResolveState, keepProcessing: (Call, ResolveState) -> Boolean): Boolean {
         var accumulatorKeepProcessing = true
 
         for (childCall in quoteCall.macroChildCallSequence()) {
-            accumulatorKeepProcessing = when {
-                CallDefinitionClause.`is`(childCall) -> keepProcessing(childCall)
-                Import.`is`(childCall) -> Import.callDefinitionClauseCallWhile(childCall, keepProcessing)
-                Use.`is`(childCall) -> Use.callDefinitionClauseCallWhile(childCall, keepProcessing)
-                else -> true
-            }
+            if (!resolveState.hasBeenVisited(childCall)) {
+                accumulatorKeepProcessing = when {
+                    CallDefinitionClause.`is`(childCall) -> {
+                        val childResolveState = resolveState.putVisitedElement(childCall)
+
+                        keepProcessing(childCall, childResolveState)
+                    }
+                    Import.`is`(childCall) -> {
+                        val childResolveState = resolveState.putVisitedElement(childCall)
+
+                        Import.callDefinitionClauseCallWhile(childCall, childResolveState, keepProcessing)
+                    }
+                    Use.`is`(childCall) -> {
+                        val childResolveState = resolveState.putVisitedElement(childCall)
+
+                        Use.callDefinitionClauseCallWhile(childCall, childResolveState, keepProcessing)
+                    }
+                    else -> true
+                }
 
-            if (!accumulatorKeepProcessing) {
-                break
+                if (!accumulatorKeepProcessing) {
+                    break
+                }
             }
         }
 

diff --git a/src/org/elixir_lang/psi/Use.kt b/src/org/elixir_lang/psi/Use.kt
@@ -1,12 +1,14 @@
 package org.elixir_lang.psi
 
 import com.intellij.psi.ElementDescriptionLocation
+import com.intellij.psi.ResolveState
 import com.intellij.usageView.UsageViewTypeLocation
 import org.elixir_lang.psi.call.Call
 import org.elixir_lang.psi.call.name.Function.USE
 import org.elixir_lang.psi.call.name.Module.KERNEL
 import org.elixir_lang.psi.impl.call.finalArguments
 import org.elixir_lang.psi.impl.maybeModularNameToModular
+import org.elixir_lang.psi.scope.putVisitedElement
 
 /**
  * A `use` call
@@ -16,14 +18,17 @@ object Use {
      * Calls `function` on each call definition clause added to the scope from the `quote` block inside the `__using__`
      * macro called by `useCall` while `function` returns `true`.  Stops the first time `function` returns `false`.
      */
-    fun callDefinitionClauseCallWhile(useCall: Call, keepProcessing: (Call) -> Boolean): Boolean =
+    fun callDefinitionClauseCallWhile(useCall: Call, resolveState: ResolveState, keepProcessing: (Call, ResolveState) -> Boolean): Boolean =
         modular(useCall)?.let { modularCall ->
             var accumulatedKeepProcessing = true
 
             for (definer in Using.definers(modularCall)) {
+                val childResolveState = resolveState.putVisitedElement(definer)
+
                 accumulatedKeepProcessing = Using.callDefinitionClauseCallWhile(
                         usingCall = definer,
                         useCall = useCall,
+                        resolveState = childResolveState,
                         keepProcessing = keepProcessing
                 )
 
@@ -35,7 +40,7 @@ object Use {
             accumulatedKeepProcessing
         } ?: true
 
-    fun elementDescription(call: Call, location: ElementDescriptionLocation): String? {
+    fun elementDescription(@Suppress("UNUSED_PARAMETER") call: Call, location: ElementDescriptionLocation): String? {
         var elementDescription: String? = null
 
         if (location === UsageViewTypeLocation.INSTANCE) {

diff --git a/src/org/elixir_lang/psi/Using.kt b/src/org/elixir_lang/psi/Using.kt
@@ -2,35 +2,42 @@ package org.elixir_lang.psi
 
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiPolyVariantReference
+import com.intellij.psi.ResolveState
 import org.elixir_lang.psi.CallDefinitionClause.nameArityRange
 import org.elixir_lang.psi.call.Call
 import org.elixir_lang.psi.call.name.Function.*
 import org.elixir_lang.psi.call.name.Module.KERNEL
 import org.elixir_lang.psi.impl.call.finalArguments
 import org.elixir_lang.psi.impl.call.macroChildCallSequence
 import org.elixir_lang.psi.impl.maybeModularNameToModular
+import org.elixir_lang.psi.scope.putVisitedElement
 
 object Using {
-    fun callDefinitionClauseCallWhile(usingCall: Call, useCall: Call?, keepProcessing: (Call) -> Boolean): Boolean =
+    fun callDefinitionClauseCallWhile(usingCall: Call, useCall: Call?, resolveState: ResolveState, keepProcessing: (Call, ResolveState) -> Boolean): Boolean =
         usingCall.macroChildCallSequence().lastOrNull()?.let { lastChildCall ->
             val resolvedModuleName = lastChildCall.resolvedModuleName()
             val functionName = lastChildCall.functionName()
 
             if (resolvedModuleName != null && functionName != null) {
                 when {
-                    resolvedModuleName == KERNEL && functionName == QUOTE ->
-                        QuoteMacro.callDefinitionClauseCallWhile(lastChildCall, keepProcessing)
+                    resolvedModuleName == KERNEL && functionName == QUOTE -> {
+                        val lastChildCallResolveState = resolveState.putVisitedElement(lastChildCall)
+
+                        QuoteMacro.callDefinitionClauseCallWhile(lastChildCall, lastChildCallResolveState, keepProcessing)
+                    }
 
                     resolvedModuleName == KERNEL && functionName == APPLY -> {
                         lastChildCall.finalArguments()?.let { arguments ->
                             // TODO pipelines to apply/3
                             if (arguments.size == 3) {
                                 arguments[0].let { maybeModularName ->
                                     maybeModularName.maybeModularNameToModular(maxScope = maybeModularName.containingFile, useCall = useCall)?.let { modular ->
+                                        val modularResolveState = resolveState.putVisitedElement(modular)
+
                                         // TODO resolve argument[1] AND use its inferred value to select only one of the functions
-                                        Modular.callDefinitionClauseCallWhile(modular) { callDefinitionClauseCall ->
+                                        Modular.callDefinitionClauseCallWhile(modular, modularResolveState) { callDefinitionClauseCall, accResolveState ->
                                             if (CallDefinitionClause.isFunction(callDefinitionClauseCall)) {
-                                                Using.callDefinitionClauseCallWhile(callDefinitionClauseCall, useCall, keepProcessing)
+                                                Using.callDefinitionClauseCallWhile(callDefinitionClauseCall, useCall, accResolveState, keepProcessing)
                                             } else {
                                                 true
                                             }
@@ -56,12 +63,13 @@ object Using {
                             }
 
                             for (resolved in resolvedList) {
-                                val text = resolved.text
-
                                 accumulatedKeepProcessing = if (resolved is Call && CallDefinitionClause.`is`(resolved)) {
+                                    val resolvedResolveSet = resolveState.putVisitedElement(resolved)
+
                                     Using.callDefinitionClauseCallWhile(
                                             usingCall = resolved,
                                             useCall = useCall,
+                                            resolveState = resolvedResolveSet,
                                             keepProcessing = keepProcessing
                                     )
                                 } else {