Merge pull request #308 from apex-dev-tools/307-improve-type-finding-…

…performance

307 improve type finding performance

jvm/src/main/scala/com/nawforce/apexlink/org/ModuleFind.scala

@@ -19,7 +19,7 @@ import com.nawforce.apexlink.finding.TypeResolver.TypeResponse
 import com.nawforce.apexlink.names.TypeNames
 import com.nawforce.apexlink.types.core.TypeDeclaration
 import com.nawforce.pkgforce.modifiers.GLOBAL_MODIFIER
-import com.nawforce.pkgforce.names.{EncodedName, TypeName}
+import com.nawforce.pkgforce.names.{EncodedName, Name, TypeName}
 
 trait ModuleFind {
   self: OPM.Module =>
@@ -58,6 +58,17 @@ trait ModuleFind {
     typeName: TypeName,
     from: Option[TypeDeclaration],
     inPackage: Boolean = true
+  ): Option[TypeDeclaration] = {
+    // Use aliased type name here so we don't mishandle an ambiguous typename when searching
+    val targetType = TypeNames.aliasOrReturn(typeName)
+
+    findPackageTypeInternal(targetType, from, inPackage)
+  }
+
+  private def findPackageTypeInternal(
+    typeName: TypeName,
+    from: Option[TypeDeclaration],
+    inPackage: Boolean
   ): Option[TypeDeclaration] = {
     // Might be an outer in this module
     var declaration = findModuleType(typeName)
@@ -70,7 +81,7 @@ trait ModuleFind {
 
     // Or maybe an inner
     if (typeName.outer.nonEmpty) {
-      declaration = findPackageType(typeName.outer.get, from, inPackage = inPackage)
+      declaration = findPackageTypeInternal(typeName.outer.get, from, inPackage = inPackage)
         .flatMap(_.findNestedType(typeName.name).filter(td => td.isExternallyVisible || inPackage))
       if (declaration.nonEmpty)
         return declaration
@@ -82,28 +93,22 @@ trait ModuleFind {
 
   /** Find a type just in this module. */
   def findModuleType(typeName: TypeName): Option[TypeDeclaration] = {
-    // Use aliased type name here so we don't mishandle an ambiguous typename when searching
-    val targetType = TypeNames.aliasOrReturn(typeName)
-
     // Direct hit
-    var declaration = types.get(targetType)
+    val declaration = types.get(typeName)
     if (declaration.nonEmpty)
       return declaration
 
     // SObject and alike, we want module specific version of these
-    declaration = types.getWithSchema(targetType)
-    if (declaration.nonEmpty)
-      return declaration
-
     if (
-      targetType.params.isEmpty &&
-      (targetType.outer.isEmpty || targetType.outer.contains(TypeNames.Schema))
+      typeName.params.isEmpty &&
+      (typeName.outer.isEmpty || typeName.outer.contains(TypeNames.Schema))
     ) {
-      val encName = EncodedName(targetType.name, namespace)
-      if (encName.ext.nonEmpty) {
-        return types.getWithSchema(TypeName(encName.fullName, Nil, None))
-      }
+      if (EncodedName.encodedNeedsNamespace(typeName.name))
+        types.getWithSchema(TypeName(Name(namespacePrefix + typeName.name.value), Nil, None))
+      else
+        types.getWithSchema(typeName)
+    } else {
+      None
     }
-    None
   }
 }

shared/src/main/scala/com/nawforce/pkgforce/names/EncodedName.scala

@@ -13,18 +13,17 @@
  */
 package com.nawforce.pkgforce.names
 
-/** A name with optional namespace prefixing & optional type suffix such as foo__field__c. Only a
+/** A name with optional namespace prefixing & optional type suffix such as foo__field__c. Only a
   * small set of suffixes are supported. It's not clear what the full list used by Salesforce is.
-  * This class is safe to use on non-encoded names which don't contain \_\_, although you can not
+  * This class is safe to use on non-encoded names which don't contain __, although you can not
   * default the namespace on them.
   *
-  * Where namespace, name and suffix are provided the code will assert on a bad suffix. Where
-  * we only have two parts of the name then if the last parts is not a valid suffix the handling
-  * will assume the first part is a namespace and the second the name, as in Page.pkg1\_\_TestPage.
+  * Where we only have two parts of the name then if the last parts is not a valid suffix the handling
+  * will assume the first part is a namespace and the second the name, as in Page.pkg1__TestPage.
   *
-  * The code deals with a few exception cases where splitting on \_\_ would give either a wrong
+  * The code deals with a few exception cases where splitting on __ would give either a wrong
   * name or ext part. For subfields the subfield name is combined with the extension. For supporting
-  * SObjects such as MyObject\_\_Feed the 'Feed' is considered an extension in the same way that 'c'
+  * SObjects such as MyObject__Feed the 'Feed' is considered an extension in the same way that 'c'
   * would be.
   */
 final case class EncodedName(name: Name, ext: Option[Name], namespace: Option[Name]) {
@@ -49,6 +48,10 @@ final case class EncodedName(name: Name, ext: Option[Name], namespace: Option[Na
 }
 
 object EncodedName {
+
+  /** Standard set of extensions, note that the subfield extension "s" is missing as this
+    * often needs special handling because it does not fit the <ns__>name__ext pattern
+    */
   private final val extensions = Set("c", "r", "e", "b", "mdt", "share", "history", "feed")
 
   def apply(name: Name): EncodedName = {
@@ -108,4 +111,47 @@ object EncodedName {
         name.substring(tailSplit + 2)
       )
   }
+
+  /** Test if name contains an encoded name that may need a namespace adding for type
+    * resolving. This is broken out from the EncodedName handling to improve performance.
+    */
+  def encodedNeedsNamespace(name: Name): Boolean = {
+    val value  = name.value
+    val suffix = extractSuffixOrReturn(value)
+    if (suffix eq value)
+      return false
+
+    val firstSeparator = value.indexOf("__")
+    if (firstSeparator == value.length - suffix.length - 2) {
+      true
+    } else {
+      val secondSeparator = value.indexOf("__", firstSeparator + 1)
+      if (secondSeparator == value.length - suffix.length - 2) {
+        suffix == "s"
+      } else {
+        false
+      }
+    }
+  }
+
+  private def extractSuffixOrReturn(name: String): String = {
+    val len = name.length
+    if (len >= 4) {
+      name.charAt(len - 1) match {
+        case 'c' | 'C' if name.charAt(len - 2) == '_' && name.charAt(len - 3) == '_' => "c"
+        case 'r' | 'R' if name.charAt(len - 2) == '_' && name.charAt(len - 3) == '_' => "r"
+        case 'e' | 'E' if name.charAt(len - 2) == '_' && name.charAt(len - 3) == '_' => "e"
+        case 'b' | 'B' if name.charAt(len - 2) == '_' && name.charAt(len - 3) == '_' => "b"
+        case 's' | 'S' if name.charAt(len - 2) == '_' && name.charAt(len - 3) == '_' => "s"
+        case 't' | 'T' if len >= 6 && name.toLowerCase.endsWith("__mdt")             => "mdt"
+        case 'e' | 'E' if len >= 8 && name.toLowerCase.endsWith("__share")           => "share"
+        case 'y' | 'Y' if len >= 10 && name.toLowerCase.endsWith("__history")        => "history"
+        case 'd' | 'D' if len >= 7 && name.toLowerCase.endsWith("__feed")            => "feed"
+        case _                                                                       => name
+      }
+    } else {
+      name
+    }
+  }
+
 }

shared/src/main/scala/com/nawforce/pkgforce/pkgs/TriHierarchy.scala

@@ -55,13 +55,13 @@ abstract class TriHierarchy {
     /** Is this a ghost package, aka it has no modules. */
     lazy val isGhosted: Boolean = modules.isEmpty
 
-    /* Find first module in search order (may not be in this package) */
-    def firstModule: Option[TModule] = {
+    /** Find first module in search order (may not be in this package) */
+    lazy val firstModule: Option[TModule] = {
       orderedModules.headOption
         .orElse(basePackages.headOption.flatMap(_.firstModule))
     }
 
-    /* Check if a type is ghosted in this package */
+    /** Check if a type is ghosted in this package */
     def isGhostedType(typeName: TypeName): Boolean = {
       if (typeName.outer.contains(TypeName.Schema)) {
         val encName = EncodedName(typeName.name)
@@ -92,34 +92,30 @@ abstract class TriHierarchy {
     /** The modules that this module depends on deploy order */
     val dependents: ArraySeq[TModule]
 
-    /** The module (& owning package namespace) */
+    /** The module (& owning package) namespace */
     lazy val namespace: Option[Name] = pkg.namespace
 
+    /** The module (& owning package) namespace prefix */
+    lazy val namespacePrefix: String = namespace.map(ns => ns.toString + "__").getOrElse("")
+
     /** The package the parent package depends on in reverse deploy order */
     lazy val basePackages: ArraySeq[TPackage] = pkg.basePackages.reverse
 
     /** The modules that this module depends on in reverse deploy order */
     lazy val baseModules: ArraySeq[TModule] = dependents.reverse
 
-    /** Test if a file is visible to this module, i.e. in scope & not ignored */
-    def isVisibleFile(path: PathLike): Boolean
-
-    /* Transitive Modules (dependent modules for this modules & its dependents) */
-    def transitiveModules: Set[TModule] = {
-      namespace
-        .map(_ => dependents.toSet ++ dependents.flatMap(_.transitiveModules))
-        .getOrElse(baseModules.toSet)
-    }
-
-    /* Find next module in search order */
-    def nextModule: Option[TModule] = {
+    /** Find next module in search order */
+    lazy val nextModule: Option[TModule] = {
       baseModules.headOption.orElse(basePackages.headOption.flatMap(_.firstModule))
     }
 
-    /* Check if a type name is ghosted in this module */
+    /** Test if a file is visible to this module, i.e. in scope & not ignored */
+    def isVisibleFile(path: PathLike): Boolean
+
+    /** Check if a type name is ghosted in this module */
     def isGhostedType(typeName: TypeName): Boolean = pkg.isGhostedType(typeName)
 
-    /* Check if a field name is ghosted in this module */
+    /** Check if a field name is ghosted in this module */
     def isGhostedFieldName(name: Name): Boolean = pkg.isGhostedFieldName(name)
   }
 }

shared/src/test/scala/com/nawforce/pkgforce/names/EncodedNameTest.scala

@@ -27,6 +27,7 @@
  */
 package com.nawforce.pkgforce.names
 
+import org.scalatest.Inspectors.forAll
 import org.scalatest.funsuite.AnyFunSuite
 
 class EncodedNameTest extends AnyFunSuite {
@@ -138,4 +139,29 @@ class EncodedNameTest extends AnyFunSuite {
     assert(testName.fullName == Name("ns__Foo__x"))
     assert(testName.asTypeName == TypeName(Name("ns__Foo__x"), Nil, None))
   }
+
+  forAll(List("c", "r", "e", "b", "mdt", "share", "history", "feed")) { ext =>
+    test(s"needs namespace simple extensions $ext") {
+      assert(!EncodedName.encodedNeedsNamespace(Name("")))
+      assert(!EncodedName.encodedNeedsNamespace(Name(s"$ext")))
+      assert(!EncodedName.encodedNeedsNamespace(Name(s"__$ext")))
+      assert(EncodedName.encodedNeedsNamespace(Name(s"a__$ext")))
+      assert(EncodedName.encodedNeedsNamespace(Name(s"abc__$ext")))
+      assert(!EncodedName.encodedNeedsNamespace(Name(s"__abc__$ext")))
+      assert(!EncodedName.encodedNeedsNamespace(Name(s"n__abc__$ext")))
+    }
+  }
+
+  test("needs namespace subfield extension") {
+    assert(!EncodedName.encodedNeedsNamespace(Name("")))
+    assert(!EncodedName.encodedNeedsNamespace(Name("s")))
+    assert(!EncodedName.encodedNeedsNamespace(Name("__s")))
+    assert(EncodedName.encodedNeedsNamespace(Name("a__s")))
+    assert(EncodedName.encodedNeedsNamespace(Name("abc__s")))
+    assert(EncodedName.encodedNeedsNamespace(Name("__abc__s")))
+    assert(EncodedName.encodedNeedsNamespace(Name("a__abc__s")))
+    assert(EncodedName.encodedNeedsNamespace(Name("abc__abc__s")))
+    assert(!EncodedName.encodedNeedsNamespace(Name("n__abc__abc__s")))
+  }
+
 }