Fix #2570: Part 3 - The great () insert #2716
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 ("Added" instead of "Add")
- 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! ☀️
|
Migration strategy: We don't complain if the function was compiled by Scala 2 (this is because the Scala libraries need to be cleaned up first, so that they have We issue a migration warning under So, to migrate code to Dotty, just compile it with It will insert the missing |
| def paramIndices(param: untpd.ValDef, args: List[untpd.Tree], start: Int): List[Int] = args match { | ||
| case arg :: args1 => | ||
| if (refersTo(arg, param)) | ||
| if (paramIndices(param, args1, start + 1).isEmpty) start :: Nil |
There was a problem hiding this comment.
This method returns a singleton list if param appears in args any odd number of times, not only "exactly one".
Please see this code snippet.
|
We seem to have an error early on in the CI. The building |
|
@odersky Noted. scalacenter/scalafix#204 |
|
This PR needs to be rebased because scala-backend was updated in #2719 |
|
We also need to tighten the rules about overriding. It should be no longer legal to override |
odersky
force-pushed
the
fix-#2570-2
branch
2 times, most recently
from
a78a559 to
eedb461
Compare
| * apply of a function class? (implicit functions are excluded) | ||
| */ | ||
| def isSyntheticApply(tree: Tree): Boolean = tree match { | ||
| case Select(qual, nme.apply) => tree.pos.end == qual.pos.end |
There was a problem hiding this comment.
It seems dangerous to me that the logic completely depends on positions -- if we have macros or synthetic trees from other sources, this assumption no longer holds.
There was a problem hiding this comment.
Yes, but even for macro-generated apply's I would assume that we should not eta-expand (since the eta-expansion would simply undo what was generated).
| matchLoosely && { | ||
| val this1 = widenNullary(this) | ||
| val that1 = widenNullary(that) | ||
| ((this1 `ne` this) || (that1 `ne` that)) && this1.overrides(this1, matchLoosely = false) |
There was a problem hiding this comment.
Why use backquotes around ne?
Follows the scheme outlined in scala#2570. The reason not to flag bad calls to methods compiled from Scala 2 is that some Scala 2 libraries are not yet in a form where the parentheses are as they should be. For instance def isWhole() in some of the numeric libraries should not have the ().
Fix errors everywhere where a nullary method is called without a () argument.
Fix situations where a ()T method overrides a => T one, and vice versa.
The reason to do this only under -strict mode for now is so that we can still compile the same code under both Scala 2 and Dotty. If we rejected trailing `_` outright, Scala 2 code would need to get either explicitly eta-expanded or get an expected type to still work. But in Dotty, we can simply drop `_` for functions with arity >= 1. So to keep code dual compilable, we'd need to add boilerplate which under Dotty would be no longer needed.
Add sections to reference that explain the new rules for eta expansion and auto-application.
|
|
||
| Existing Scala code with inconsistent parameters can still be compiled | ||
| in Dotty under `-language:Scala2`. When paired with the `-rewrite` | ||
| option, the code will be automatcally rewritten to conforrm to Dotty's |
There was a problem hiding this comment.
Typo: conforrm -> conform.
Require explicit () for nullary method applications. Fix a lot of files that were in violation.
This is the final part of addressing #2570.