Don't unnecessarily use fullyExploreTree (bazelbuild#1009)

SocksDevil · Jul 6, 2020 · c626f7c · c626f7c
1 parent 2c06950
commit c626f7c
Showing 1 changed file with 59 additions and 60 deletions.
diff --git a/...ependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/AstUsedJarFinder.scala b/...ependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/AstUsedJarFinder.scala
@@ -33,11 +33,6 @@ class AstUsedJarFinder(
       tpe.typeArgs.foreach(exploreType(_, pos))
     }
 
-    def fullyExploreTree(tree: Tree): Unit = {
-      exploreTree(tree)
-      tree.foreach(exploreTree)
-    }
-
     def exploreClassfileAnnotArg(arg: ClassfileAnnotArg, pos: Position): Unit = {
       arg match {
         case LiteralAnnotArg(value) =>
@@ -68,77 +63,81 @@ class AstUsedJarFinder(
       }
     }
 
-    def exploreTree(tree: Tree): Unit = {
-      tree match {
-        case node: TypeTree =>
-          if (node.original != null) {
-            node.original.foreach(fullyExploreTree)
-          }
-        case node: Literal =>
-          // We should examine OriginalTreeAttachment but that was only
-          // added in 2.12.4, so include a version check
-          ScalaVersion.conditional(
-            Some("2.12.4"),
-            None,
-            """
+    def fullyExploreTree(tree: Tree): Unit = {
+      def visitNode(tree: Tree): Unit = {
+        tree match {
+          case node: TypeTree =>
+            if (node.original != null) {
+              fullyExploreTree(node.original)
+            }
+          case node: Literal =>
+            // We should examine OriginalTreeAttachment but that was only
+            // added in 2.12.4, so include a version check
+            ScalaVersion.conditional(
+              Some("2.12.4"),
+              None,
+              """
               node.attachments
                 .get[global.treeChecker.OriginalTreeAttachment]
                 .foreach { attach =>
                   fullyExploreTree(attach.original)
                 }
             """
-          )
+            )
 
-          exploreConstant(node.value, tree.pos)
-        case _ =>
-      }
+            exploreConstant(node.value, tree.pos)
+          case _ =>
+        }
 
-      // If this expression is the result of a macro, then we
-      // should also examine the original macro expression
-      tree.attachments
-        .get[global.treeChecker.MacroExpansionAttachment]
-        .foreach { attach =>
-          // When we explore the original, the original also has
-          // this attachment. So we should not examine the original
-          // again if so.
-          if (attach.expandee != tree) {
-            fullyExploreTree(attach.expandee)
+        // If this expression is the result of a macro, then we
+        // should also examine the original macro expression
+        tree.attachments
+          .get[global.treeChecker.MacroExpansionAttachment]
+          .foreach { attach =>
+            // When we explore the original, the original also has
+            // this attachment. So we should not examine the original
+            // again if so.
+            if (attach.expandee != tree) {
+              fullyExploreTree(attach.expandee)
+            }
           }
-        }
 
-      val shouldExamine =
-        tree match {
-          case select: Select if select.symbol.isDefaultGetter =>
-            false
-          case _ =>
-            true
-        }
+        val shouldExamine =
+          tree match {
+            case select: Select if select.symbol.isDefaultGetter =>
+              false
+            case _ =>
+              true
+          }
 
-      if (shouldExamine) {
-        if (tree.hasSymbolField) {
-          tree.symbol.annotations
-            // We skip annotations without positions. The reason for
-            // this is the case of
-            //   @SomeAnnotation class A
-            //   class B extends A
-            // Now assuming A and B are in separate packages, while
-            // examining B we will examine A as well, and hence
-            // examine A's annotations. However we don't wish to examine
-            // A's annotations as we don't care about those details of A.
-            // Hence we only examine annotations with positions (hence,
-            // they were defined in the same compilation unit and thus
-            // matter).
-            .filter(_.pos.isDefined)
-            .foreach(exploreAnnotationInfo)
-        }
-        if (tree.tpe != null) {
-          exploreType(tree.tpe, tree.pos)
+        if (shouldExamine) {
+          if (tree.hasSymbolField) {
+            tree.symbol.annotations
+              // We skip annotations without positions. The reason for
+              // this is the case of
+              //   @SomeAnnotation class A
+              //   class B extends A
+              // Now assuming A and B are in separate packages, while
+              // examining B we will examine A as well, and hence
+              // examine A's annotations. However we don't wish to examine
+              // A's annotations as we don't care about those details of A.
+              // Hence we only examine annotations with positions (hence,
+              // they were defined in the same compilation unit and thus
+              // matter).
+              .filter(_.pos.isDefined)
+              .foreach(exploreAnnotationInfo)
+          }
+          if (tree.tpe != null) {
+            exploreType(tree.tpe, tree.pos)
+          }
         }
       }
+
+      tree.foreach(visitNode)
     }
 
     currentRun.units.foreach { unit =>
-      unit.body.foreach(fullyExploreTree)
+      fullyExploreTree(unit.body)
     }
     jars.toMap
   }