Implement ExplicitNonNullaryApply #14
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
olafurpg
reviewed
Apr 2, 2020
This comment has been minimized.
This comment has been minimized.
ohze
reviewed
Apr 15, 2020
There was a problem hiding this comment.
From https://gitter.im/scalameta/scalameta?at=5e86fbe5c79eff2492ff4b61, maybe go down the route of invoking the presentation compiler to discover override info?
Or take code from scalameta/metals#1487
rewrites/src/main/scala/fix/scala213/ExplicitNonNullaryApply.scala
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
Two problems:
* ScalaMeta's SymbolInformation seems to record asInstanceOf as nilary
instead of nullary, so the rewrite erroneously rewrites the
_definitions_ propNullAs and methNullAs, into invalid code... :( I
could move the definitions into another file, but I just decided to
drop it, also because...
* Infix invocation of nilary methods seems to be no longer valid in
2.13, for example:
[error] /d/scala-rewrites/output/src/main/scala/fix/scala213/ExplicitNonNullaryApply.scala:30:42: no arguments allowed for nullary method methNullAs: ()A
[error] def def_this_methNullAs_m_in = this methNullAs ()
[error] ^
Also, "nullAs" is the closest and only method I could think of that is
nilary (actually I normally define "nullAs" as nullary) and takes no
term arguments but makes legitimate use of the type parameter. So maybe
the whole edge-case is a pointless variation.
What we'd like is to leave any call to a method that was defined in Java (or any "universal" method, like ##, eq, isInstanceOf) to be exempt. It appears Scalameta/Semanticdb/Scalafix doesn't capture that information, and thus basically all of them get parens... :( Apparently in Metals they use the presentation compiler to recover all the missing info and I know in ScalaClean they have their own nsc Traverser, I'm guessing for the same reason.
Fortunately not necessary: I was able to use ScalafixGlobal to implement a "isJavaDefined" so any method defined in Java is invoked as it is (with or without Need to review the diff as a whole now before declaring it ready. |
dwijnand
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Heavily inspired by Olaf's scalacenter/scalafix#204.