Use the most up-to-date analysis for binary to source class name lookup

[Friendseeker]

This PR fixes #1268

Cause

During Dependency Extraction, internalBinaryToSourceClassName is used to map binary class name to source class name. It is constructed at the first cycle. Hence in subsequent cycles, it is outdated. This results in missing internal member reference dependency between a.Use pkgName.Test.

To fix it, we rebuild lookup using the most up-to-date analysis.

Other changes

This PR also removed internalSourceToClassNamesMap. It is currently not used, but it has the same staleness problem as internalBinaryToSourceClassName. So might as well remove it to prevent someone from accidentally using it in the future.

Why empty-package erroneously passed on zinc repo

The complier bridge did recognize a dependency between a.Use and pkgName.Test exists. However, since internalBinaryToSourceClassName was stale, zinc failed to map pkgName.Test back to its .scala source file, and instead misclassified the dependency as a external library dependency. This can be observed in the zinc test log:

[debug] previous = Stamps for: 4 products, 2 sources, 1 libraries
[debug] current source = Set(${BASE}/pro/Use.scala)
[debug] Invalidating '${BASE}/pro/target/classes/pkgName/Test.class' because pkgName.Test$ came from analysis Analysis: 2 Scala sources, 4 classes

Then in invalidateInitial, all sources that depends on pkgName.Test library is invalidated via val byLibraryDep = changes.libraryDeps.flatMap(previous.usesLibrary)

Validating the fix

empty-package test is modified to assert dependency between a.Use pkgName.Test must exist after the 2nd compilation run.

Running the modified empty-package on current latest develop branch commit now results in test failure.

[Friendseeker]

TODO:

• The edited empty-package test passes in current latest 'develop' commit.
  • Expected: edited empty-package test should fail.

[eed3si9n]

I feel slightly nervous about deleting things, but hopefully this is ok.

[Friendseeker]

I feel slightly nervous about deleting things, but hopefully this is ok.

Here's my chain of thought:

The only case for problem to emerge is for incHandlerOpt to be None while internalBinaryToSourceClassName lookup is still needed. For Scala compilation, incHandlerOpt is never None. For Java compilation, incHandlerOpt is None, but internalBinaryToSourceClassName is called in one place only:

// JavaAnalyze.scala
(getMappedSource(fromBinaryName), getMappedSource(onBinaryName)) match {
  case (Some(fromClassName), Some(onClassName)) =>
    trapAndLog(log) {
      analysis.classDependency(onClassName, fromClassName, context)
    }
  case (Some(fromClassName), None) =>
    trapAndLog(log) {
      val cachedOrigin = classfilesCache.get(onBinaryName)
      for (file <- cachedOrigin.orElse(loadFromClassloader())) {
        val binaryFile: Path = remapClassFile(file)
        analysis.binaryDependency(
          binaryFile,
          onBinaryName,
          fromClassName,
          source,
          context
        )
      }
    }
  case (None, _) => // It could be a stale class file, ignore

In the above code, analysis.binaryDependency calls internalBinaryToSourceClassName, however it only happens when getMappedSource(onBinaryName) is None, which means the source file does not exist anyways.

---

Therefore, if the above chain of thought is sound, this change should be okay.