Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong Tree returned by askTypeAt (wrong hyperlink) #7915

Closed
scabug opened this issue Oct 16, 2013 · 10 comments
Closed

Wrong Tree returned by askTypeAt (wrong hyperlink) #7915

scabug opened this issue Oct 16, 2013 · 10 comments

Comments

@scabug
Copy link

scabug commented Oct 16, 2013

// Foo.scala
package foo

class Foo {
  def foo() {
    new Bar().bar()  // <-- try hyperlinking on `bar` method, you'll jump to the enclosing `foo` method definition
  }
}

class Bar {
  def bar(b: Int = 2) {}
}

I've debugged the problem inside the Scala IDE, and the reason why this happen is that askTypeAt returns the enclosing foo DefDef method symbol, instead of the expected Select Bar.bar node. The enclosing method is returned becuase all Trees inside the method block have a TransparentPosition, all the way down. The impelmentation of askTypeAt relies on RangePositions.Locator.traverse to return the deepest, non-transparent, tree that includes the passed position. And, because all positions are transparent inside the foo method's body, then the foo symbol is returned.

An additional interesting fact is that it looks like one of the transformation
happening during typechecking is to blame for updating the Tree with the wrong kind of positions. In fact, if the source file isn't typechecked, the correct symbol is returned.

Attached, you can find a patch that demonstrates the issue by running the presentation compiler in isolation. To run the test, simply type: ./test/partest test/files/presentation/t0000

@scabug
Copy link
Author

scabug commented Oct 16, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7915?orig=1
Reporter: @dotta
Affected Versions: 2.10.3, 2.11.0-M6
Other Milestones: 2.11.0-M8
Attachments:

@scabug
Copy link
Author

scabug commented Nov 6, 2013

@dotta said:
Alright, let's see if I can fix it myself.

@scabug
Copy link
Author

scabug commented Nov 9, 2013

@retronym said:
Here are some of the tools you need:

scalac-hash origin/master -Xprint:typer -Yrangepos -Xprint-pos sandbox/test.scala
[info] downloading http://scala-webapps.epfl.ch/artifacts/2c1e9af40e252bd9ed5bfe4305991ee6f7d25636/pack.tgz ...done.
[info] scala revision from 2013-10-29 05:39:42 -0700 downloaded to /Users/jason/usr/scala-v2.11.0-M6-134-g2c1e9af
Password:
[info] origin/mas => /Users/jason/usr/scala-v2.11.0-M6-134-g2c1e9af
[[syntax trees at end of                     typer]] // test.scala
[0:201]package [8:11]foo {
  [14:160]class Foo extends [24:160][163]scala.AnyRef {
    [163]def <init>(): [24]foo.Foo = [163]{
      [163][163][163]Foo.super.<init>();
      [24]()
    };
    [28:158]def foo(): [32]Unit = <44:59>{
      [44:53]<artifact> val qual$1: [44]foo.Bar = [44:53][44:53][44:53]new [48:51]Bar();
      [54]<artifact> val x$1: [54]Int = [54]qual$1.bar$default$1;
      <44:59><44:57>qual$1.bar([54]x$1)
    }
  };
  [163:201]class Bar extends [173:201][201]scala.AnyRef {
    [201]def <init>(): [173]foo.Bar = [201]{
      [201][201][201]Bar.super.<init>();
      [173]()
    };
    [177:199]def bar([185:195]b: [188:191]<type: [188:191]scala.Int> = [194:195]2): [181]Unit = [197:199]();
    [185]<synthetic> def bar$default$1: [185]Int = [194]2
  }
}

% ack  makeTrans /Users/jason/code/scala3/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala
        val baseFunTransformed = atPos(baseFun.pos.makeTransparent) {
              res.setPos(res.pos.makeTransparent)
              val block = Block(stats ::: valDefs.flatten, res).setType(res.tpe).setPos(tree.pos.makeTransparent)

@scabug
Copy link
Author

scabug commented Nov 11, 2013

@dragos said:
scalac-hash looks like a very handy script...

@scabug
Copy link
Author

scabug commented Nov 11, 2013

@scabug
Copy link
Author

scabug commented Nov 19, 2013

Mirko Stocker (misto) said:
Great! This might also fix #4287.

@scabug
Copy link
Author

scabug commented Nov 19, 2013

@dotta said:
Fixed in scala/scala#3136

Backport to 2.10.x scala/scala#3157

@scabug
Copy link
Author

scabug commented Nov 19, 2013

@dotta said:
@MirkoStocker Unfortunately no, my fix doesn't touch default argument in constructors.

@scabug
Copy link
Author

scabug commented Nov 19, 2013

Mirko Stocker (misto) said:
Oh, too bad, but I have faith in you :-)

@scabug
Copy link
Author

scabug commented Nov 20, 2013

@JamesIry said:
scala/scala#3157

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants