Fix rename in for comprehensions #156
Conversation
|
Refer to this link for build results (access rights to CI server needed): https://jenkins.scala-ide.org:8496/jenkins/job/ghprb-scala-refactoring-validator/296/ |
|
Nice! A very clever solution, and it's even shorter and easier to understand than the original. |
| private def termsWithSamePointButRange(s: Symbol): List[Symbol] = { | ||
| val lookAtSymbol = { | ||
| s != NoSymbol && !s.isSynthetic && s.isInstanceOf[TermSymbol] && | ||
| s.pos.isInstanceOf[OffsetPosition] && !s.pos.isInstanceOf[RangePosition] |
There was a problem hiding this comment.
just a question why s.pos.isRange and s.isTerm are too weak for this check. Or maybe it is not this position and symbol?
There was a problem hiding this comment.
Thanks for pointing that out! See #157.
There was a problem hiding this comment.
just asking, maybe 2.10 api is too weak so you decided to use
isInstanceOf?
2016-04-08 9:46 GMT+02:00 Matthias Langer notifications@github.com:
In src/main/scala/scala/tools/refactoring/analysis/SymbolExpanders.scala
#156 (comment)
:
- * The reason that we need this is that in some cases, the PC generates multiple
- * symbols for one and the same symbol in user source code, one of them with a
- * proper range position, and others just with offset positions. One place where
- * this happens is in desugared for comprehensions with filter clauses.
- * See Assembler Ticket #1002650.
- */
- trait TermsWithMissingRanges extends SymbolExpander { this: IndexLookup =>
- protected abstract override def doExpand(s: Symbol): List[Symbol] = {
termsWithSamePointButRange(s) ::: super.doExpand(s)- }
- private def termsWithSamePointButRange(s: Symbol): List[Symbol] = {
val lookAtSymbol = {s != NoSymbol && !s.isSynthetic && s.isInstanceOf[TermSymbol] &&s.pos.isInstanceOf[OffsetPosition] && !s.pos.isInstanceOf[RangePosition]Thanks for pointing that out! See #157
#157.—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
https://github.com/scala-ide/scala-refactoring/pull/156/files/44270070343f8811cc3e9524e9fe35921b3b1f1c#r58990634
There was a problem hiding this comment.
No, I was indeed thinking about 2.10 compatibility when writing these isInstanceOf tests, but I was wrong. The only thing missing in 2.10 is isOffset.
Note that this PR actually makes the implementation simpler, and removes more code than it adds.
Fix #1002650