Fix #9462: Identify inserted apply's more reliably in the Dynamic treatment. #9582
Conversation
There was a problem hiding this comment.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Commit Messages
We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).
Please stick to these guidelines for commit messages:
- Separate subject from body with a blank line
- When fixing an issue, start your commit message with
Fix #<ISSUE-NBR>:- Limit the subject line to 72 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line ("Add" instead of "Added")
- Wrap the body at 80 characters
- Use the body to explain what and why vs. how
adapted from https://chris.beams.io/posts/git-commit
Have an awesome day! ☀️
…c treatment. Previously, identifying when there was an inserted apply, which had to be carried through the Dynamic treatment, was done on a syntactical basis. This would mishandle cases written as someContainer.someDynamic() as it would think that this was an explicit dynamic call for `"someDynamic"` on `someContainer` (although `someContainer` is not even a `scala.Dynamic` itself, but `someContainer.someDynamic` is). We now reliably detect inserted applys by looking at the result of the pre-type-checking of the function part, and testing whether there is a synthetic `Select(_, nme.apply)` node.
sjrd
force-pushed
the
fix-dynamic-insertion-of-apply-insertion
branch
from
50f351f to
22010f6
Compare
sjrd
changed the title
| * my depth there. | ||
| * In the meantime, this makes tests pass. | ||
| */ | ||
| case TypeApply(fun, _) => !fun.isInstanceOf[Select] |
There was a problem hiding this comment.
Shouldn't we also check that the name of fun has a synthetic span?
There was a problem hiding this comment.
What kind of trees do we get here?
There was a problem hiding this comment.
Anything. For the test that was failing, it was TypeApply(Ident("qual"), List(oneTArg)).
There was a problem hiding this comment.
Where I was expecting typedFunPart to have returned TypeApply(Select(Ident("qual"), nme.apply), List(oneTarg)) instead, where the Select node would have had a synthetic span.
There was a problem hiding this comment.
I'm wondering if we can get a TypeApply(Block(..., Select(..., nme.apply)), ...). I guess those would never be inserted applies.
There was a problem hiding this comment.
No, in that case, for an inserted apply, the Select should be around the Block, not inside it.
Previously, identifying when there was an inserted apply, which had to be carried through the Dynamic treatment, was done on a syntactical basis. This would mishandle cases written as
as it would think that this was an explicit dynamic call for
"someDynamic"onsomeContainer(althoughsomeContaineris not even ascala.Dynamicitself, butsomeContainer.someDynamicis).We now reliably detect inserted applys by looking at the result of the pre-type-checking of the function part, and testing whether there is a synthetic
Select(_, nme.apply)node.