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
rtyley
added a commit
to guardian/tagmanager
that referenced
this pull request
Oct 22, 2024
…`()` is deprecated Scala 2.13 deprecates (with PR scala/scala#8833) the old behaviour of Scala that zero-parameter methods could be called with either one or zero pairs of parenthesis - ie if you have a method `def foo()` you could call it as `foo()` or just `foo`. With Scala 3, you have to match the number of brackets *used in the method declaration* when you call it - so you'd _have_ to use `foo()` or you'd get an error like this: ``` [error] ~/code/presence-indicator/app/actor/OpenSocketActor.scala:79:7: Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method sender, [error] or remove the empty argument list from its definition (Java-defined methods are exempt). [error] In Scala 3, an unapplied method like this will be eta-expanded into a function. [quickfixable] [error] sender ! Map(serverId -> (LiveActors(connectionPing, subscription))) [error] ^ ``` Java methods are exempt from this restriction - you can call either `hashCode()` (which, in Java, is how the method _has_ to be defined, with empty brackets) or just `hashCode` (which is how that method would have been declared if it was declared in Scala, in Scala methods with no side-effects should be declared without brackets: https://docs.scala-lang.org/style/method-invocation.html#arity-0). ## Automatically fixing this code issue There are two possible ways of automating this code fix - in this small project, they both produce the same code changes: ### Fixing if the project is already on Scala 2.13 - use `-quickfix` in `scalac` You can use the `-quickfix` support added to Scala 2.13.12 with scala/scala#10482: Add either of these to the `scalacOptions` in `build.sbt`: * `"-quickfix:any"` ...to apply *all* available quick fixes * `"-quickfix:msg=Auto-application"` ...to apply quick fixes where the message contains "Auto-application" Then run `compile` on the sbt console - the first compile will still fail, but it will subtly change the error message to say `[rewritten by -quickfix]` - your files have been edited to receive the fix: ``` [error] /Users/Roberto_Tyley/code/presence-indicator/app/actor/OpenSocketActor.scala:79:7: [rewritten by -quickfix] Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method sender, [error] or remove the empty argument list from its definition (Java-defined methods are exempt). [error] In Scala 3, an unapplied method like this will be eta-expanded into a function. [error] sender ! Map(serverId -> (LiveActors(connectionPing, subscription))) [error] ^ ``` ...run `compile` a second time, and compiler will be much happier. Examples of other PRs using `-quickfix` to fix this code issue: * guardian/ophan#5719 ### Fixing while still on Scala 2.12 - use Scalafix Fixing this everywhere in a project can be tedious, but thankfully there is a `ExplicitNonNullaryApply` Scalafix rule to fix this in the https://github.com/lightbend-labs/scala-rewrites project. The Scalafix rule needs to be run while the project is still on Scala 2.12, not Scala 2.13 (otherwise sbt will say: "Error downloading ch.epfl.scala:sbt-scalafix;sbtVersion=1.0;scalaVersion=2.13:0.13.0"). Once the Scalafix plugin is made available to sbt (by adding `addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.13.0")` to either `project/plugins.sbt` or `~/.sbt/1.0/plugins.sbt`), you can run these commands on the sbt prompt to automatically generate the changes in this PR: ``` scalafixEnable scalafixAll dependency:fix.scala213.ExplicitNonNullaryApply@org.scala-lang:scala-rewrites:0.1.5 ``` Examples of other PRs using Scalafix to fix this code issue: * guardian/mobile-apps-api#2728 * guardian/presence-indicator#196 See also: * scalacenter/scalafix#204 * lightbend-labs/scala-rewrites#14
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.