Skip to content

Commit

Permalink
Fix scala inference doesn't infer name after function call (#15565)
Browse files Browse the repository at this point in the history 
This fix solved most of our compilation errors without adding explicit dependencies.

The problem is if the `qual` part of the Select term is Apply term the ScalaParser doesn't build the full name, only the outer term name and the dependency cannot be inferred automatically. This PR continues and visits the `qual` node of the select term.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
@somdoron
somdoron authored May 23, 2022
1 parent 3ef6f12 commit f755c9b
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 9 deletions.
30 changes: 21 additions & 9 deletions src/python/pants/backend/scala/dependency_inference/ScalaParser.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,26 @@ class SourceAnalysisTraverser extends Traverser {

// Extract a qualified name from a tree.
def extractName(tree: Tree): String = {
tree match {
case Term.Select(qual, name) => s"${extractName(qual)}.${extractName(name)}"
case Type.Select(qual, name) => s"${extractName(qual)}.${extractName(name)}"
case Term.Name(name) => name
case Type.Name(name) => name
case Pat.Var(node) => extractName(node)
case Name.Indeterminate(name) => name
case _ => ""
}
def extractNameSelect(qual: Tree, name: Tree): Option[String] =
(maybeExtractName(qual), maybeExtractName(name)) match {
case (Some(qual), Some(name)) => Some(s"$qual.$name")
case (Some(qual), None) => Some(qual)
case (None, Some(name)) => Some(name)
case (None, None) => None
}

def maybeExtractName(tree: Tree): Option[String] =
tree match {
case Term.Select(qual, name) => extractNameSelect(qual, name)
case Type.Select(qual, name) => extractNameSelect(qual, name)
case Term.Name(name) => Some(name)
case Type.Name(name) => Some(name)
case Pat.Var(node) => maybeExtractName(node)
case Name.Indeterminate(name) => Some(name)
case _ => None
}

maybeExtractName(tree).getOrElse("")
}

def extractNamesFromTypeTree(tree: Tree): Vector[String] = {
Expand Down Expand Up @@ -313,6 +324,7 @@ class SourceAnalysisTraverser extends Traverser {
case node @ Term.Select(_, _) => {
val name = extractName(node)
recordConsumedSymbol(name)
super.apply(node.qual)
}

case node @ Term.Name(_) => {
Expand Down
26 changes: 26 additions & 0 deletions src/python/pants/backend/scala/dependency_inference/scala_parser_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ def test_parser_simple(rule_runner: RuleRunner) -> None:
import scala.collection.mutable.{ArrayBuffer, HashMap => RenamedHashMap}
import java.io._
import anotherPackage.calc
class OuterClass {
import foo.bar.SomeItem
Expand Down Expand Up @@ -133,13 +134,19 @@ def this(bar: SomeTypeInSecondaryConstructor) {
this(bar)
}
}
object ApplyQualifier {
def func4(a: Integer) = calc.calcFunc(a).toInt
}
"""
),
)

assert sorted(analysis.provided_symbols) == [
"org.pantsbuild.example.ASubClass",
"org.pantsbuild.example.ASubTrait",
"org.pantsbuild.example.ApplyQualifier",
"org.pantsbuild.example.ApplyQualifier.func4",
"org.pantsbuild.example.Functions",
"org.pantsbuild.example.Functions.func1",
"org.pantsbuild.example.Functions.func2",
Expand Down Expand Up @@ -172,6 +179,10 @@ def this(bar: SomeTypeInSecondaryConstructor) {
assert sorted(analysis.provided_symbols_encoded) == [
"org.pantsbuild.example.ASubClass",
"org.pantsbuild.example.ASubTrait",
"org.pantsbuild.example.ApplyQualifier",
"org.pantsbuild.example.ApplyQualifier$",
"org.pantsbuild.example.ApplyQualifier$.MODULE$",
"org.pantsbuild.example.ApplyQualifier.func4",
"org.pantsbuild.example.Functions",
"org.pantsbuild.example.Functions$",
"org.pantsbuild.example.Functions$.MODULE$",
Expand Down Expand Up @@ -226,6 +237,7 @@ def this(bar: SomeTypeInSecondaryConstructor) {
is_wildcard=False,
),
ScalaImport(name="java.io", alias=None, is_wildcard=True),
ScalaImport(name="anotherPackage.calc", alias=None, is_wildcard=False),
),
}
)
Expand All @@ -252,6 +264,9 @@ def this(bar: SomeTypeInSecondaryConstructor) {
"org.pantsbuild.example.HasPrimaryConstructor": FrozenOrderedSet(
["bar", "SomeTypeInSecondaryConstructor"]
),
"org.pantsbuild.example.ApplyQualifier": FrozenOrderedSet(
["Integer", "a", "toInt", "calc.calcFunc"]
),
"org.pantsbuild.example": FrozenOrderedSet(
["ABaseClass", "ATrait1", "ATrait2.Nested", "BaseWithConstructor"]
),
Expand All @@ -262,6 +277,8 @@ def this(bar: SomeTypeInSecondaryConstructor) {
# Because they contain dots, and thus might be fully qualified. See #13545.
"ATrait2.Nested",
"OuterObject.NestedVal",
"anotherPackage.calc.calcFunc",
"calc.calcFunc",
# Because of the wildcard import.
"java.io.+",
"java.io.ABaseClass",
Expand All @@ -272,13 +289,16 @@ def this(bar: SomeTypeInSecondaryConstructor) {
"java.io.OuterObject.NestedVal",
"java.io.String",
"java.io.Unit",
"java.io.a",
"java.io.Integer",
"java.io.LambdaReturnType",
"java.io.LambdaTypeArg1",
"java.io.LambdaTypeArg2",
"java.io.SomeTypeInSecondaryConstructor",
"java.io.bar",
"java.io.calc.calcFunc",
"java.io.foo",
"java.io.toInt",
"java.io.TupleTypeArg1",
"java.io.TupleTypeArg2",
# Because it's the top-most scope in the file.
Expand All @@ -293,8 +313,11 @@ def this(bar: SomeTypeInSecondaryConstructor) {
"org.pantsbuild.example.OuterObject.NestedVal",
"org.pantsbuild.example.String",
"org.pantsbuild.example.Unit",
"org.pantsbuild.example.a",
"org.pantsbuild.example.bar",
"org.pantsbuild.example.calc.calcFunc",
"org.pantsbuild.example.foo",
"org.pantsbuild.example.toInt",
"org.pantsbuild.example.LambdaReturnType",
"org.pantsbuild.example.LambdaTypeArg1",
"org.pantsbuild.example.LambdaTypeArg2",
Expand All @@ -316,8 +339,11 @@ def this(bar: SomeTypeInSecondaryConstructor) {
"org.pantsbuild.TupleTypeArg1",
"org.pantsbuild.TupleTypeArg2",
"org.pantsbuild.Unit",
"org.pantsbuild.a",
"org.pantsbuild.bar",
"org.pantsbuild.calc.calcFunc",
"org.pantsbuild.foo",
"org.pantsbuild.toInt",
}


Expand Down

0 comments on commit f755c9b

Please sign in to comment.