Skip to content

Commit

Permalink
Scala 2.13 Scalafix: ExplicitNonNullaryApply - Auto-application to …
Browse files Browse the repository at this point in the history
…`()` 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:[email protected]: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
  • Loading branch information
rtyley committed Oct 11, 2024
1 parent 6206d62 commit f326380
Show file tree
Hide file tree
Showing 15 changed files with 90 additions and 90 deletions.
6 changes: 3 additions & 3 deletions app/controllers/Reindex.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class Reindex(
if (reindexing) {
Future.successful(Forbidden)
} else {
ReindexTagsCommand().process.map { result =>
ReindexTagsCommand().process().map { result =>
result.map { count => Ok } getOrElse InternalServerError
}
}
Expand All @@ -39,7 +39,7 @@ class Reindex(
if (reindexing) {
Future.successful(Forbidden)
} else {
ReindexSectionsCommand().process.map { result =>
ReindexSectionsCommand().process().map { result =>
result.map { count => Ok } getOrElse InternalServerError
}
}
Expand All @@ -54,7 +54,7 @@ class Reindex(
if (reindexing) {
Future.successful(Forbidden)
} else {
ReindexPillarsCommand().process.map { result =>
ReindexPillarsCommand().process().map { result =>
result.map { count => Ok } getOrElse InternalServerError
}
}
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/Support.scala
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class Support(
req.body.asJson.map { json =>
val sectionId = (json \ "sectionId").as[Long]

UnexpireSectionContentCommand(sectionId).process.map{ result =>
UnexpireSectionContentCommand(sectionId).process().map{ result =>
result.map(t => Ok("Unexpiry Completed Successfully")) getOrElse BadRequest("Failed to trigger unexpiry")

} recover {
Expand All @@ -166,7 +166,7 @@ class Support(
req.body.asJson.map { json =>
val sectionId = (json \ "sectionId").as[Long]

ExpireSectionContentCommand(sectionId).process.map { result =>
ExpireSectionContentCommand(sectionId).process().map { result =>
result.map(t => Ok("Expiry Completed Successfully")) getOrElse BadRequest("Failed to trigger expiry")
} recover {
commandErrorAsResult
Expand Down Expand Up @@ -211,7 +211,7 @@ class Support(

val updatedTag = tag.copy(parents = tag.parents.filterNot(_.equals(parentId)))

Some(UpdateTagCommand(DenormalisedTag(updatedTag)).process.map {result =>
Some(UpdateTagCommand(DenormalisedTag(updatedTag)).process().map {result =>
result getOrElse InternalServerError(s"Could not update tag: ${tag.id}")
danglingParentsCount += 1
})
Expand Down
32 changes: 16 additions & 16 deletions app/controllers/TagManagementApi.scala
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class TagManagementApi(
implicit val username = Option(req.user.email)

req.body.asJson.map { json =>
UpdateTagCommand(json.as[DenormalisedTag]).process.map { result =>
UpdateTagCommand(json.as[DenormalisedTag]).process().map { result =>
result.map{t => Ok(Json.toJson(DenormalisedTag(t))) } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -57,7 +57,7 @@ class TagManagementApi(
(APIAuthAction andThen CreateTagPermissionsCheck() andThen CreateTagSpecificPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
req.body.asJson.map { json =>
json.as[CreateTagCommand].process.map { result =>
json.as[CreateTagCommand].process().map { result =>
result.map { t => Ok(Json.toJson(DenormalisedTag(t))) } getOrElse NotFound
} recover {
commandErrorAsResult
Expand Down Expand Up @@ -129,7 +129,7 @@ class TagManagementApi(
def createSection() = (APIAuthAction andThen CreateSectionPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
req.body.asJson.map { json =>
json.as[CreateSectionCommand].process.map { result =>
json.as[CreateSectionCommand].process().map { result =>
result.map{t => Ok(Json.toJson(t)) } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -142,7 +142,7 @@ class TagManagementApi(
def updateSection(id: Long) = (APIAuthAction andThen UpdateSectionPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
req.body.asJson.map { json =>
UpdateSectionCommand(json.as[Section]).process.map { result =>
UpdateSectionCommand(json.as[Section]).process().map { result =>
result.map{ t => Ok(Json.toJson(t)) } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -157,7 +157,7 @@ class TagManagementApi(
req.body.asJson.map { json =>
val editionName = (json \ "editionName").as[String]

AddEditionToSectionCommand(id, editionName.toUpperCase).process.map { result =>
AddEditionToSectionCommand(id, editionName.toUpperCase).process().map { result =>
result.map{ t => Ok(Json.toJson(t)) } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -169,7 +169,7 @@ class TagManagementApi(

def removeEditionFromSection(id: Long, editionName: String) = (APIAuthAction andThen RemoveEditionFromSectionPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
RemoveEditionFromSectionCommand(id, editionName.toUpperCase).process.map {result =>
RemoveEditionFromSectionCommand(id, editionName.toUpperCase).process().map {result =>
result.map{ t => Ok(Json.toJson(t)) } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -193,7 +193,7 @@ class TagManagementApi(
def createPillar() = (APIAuthAction andThen PillarPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
req.body.asJson.map { json =>
json.as[CreatePillarCommand].process.map { result =>
json.as[CreatePillarCommand].process().map { result =>
result.map{t => Ok(Json.toJson(t)) } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -206,7 +206,7 @@ class TagManagementApi(
def updatePillar(id: Long) = (APIAuthAction andThen PillarPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
req.body.asJson.map { json =>
UpdatePillarCommand(json.as[Pillar]).process.map { result =>
UpdatePillarCommand(json.as[Pillar]).process().map { result =>
result.map { t => Ok(Json.toJson(t)) } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -218,7 +218,7 @@ class TagManagementApi(

def deletePillar(id: Long) = (APIAuthAction andThen PillarPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
DeletePillarCommand(id).process.map { result =>
DeletePillarCommand(id).process().map { result =>
result.fold[Result](NotFound)(_ => NoContent)
} recover {
commandErrorAsResult
Expand All @@ -231,7 +231,7 @@ class TagManagementApi(

def checkPathInUse(tagType: String, slug: String, section: Option[Long], tagSubType: Option[String]) = APIAuthAction.async { req =>
implicit val username = Option(req.user.email)
new PathUsageCheck(tagType, slug, section, tagSubType).process.map{ result =>
new PathUsageCheck(tagType, slug, section, tagSubType).process().map{ result =>
result.map{ t => Ok(Json.toJson(t)) } getOrElse BadRequest
} recover {
commandErrorAsResult
Expand All @@ -242,7 +242,7 @@ class TagManagementApi(

implicit val username = Option(req.user.email)
req.body.asJson.map { json =>
json.as[BatchTagCommand].process.map{ result =>
json.as[BatchTagCommand].process().map{ result =>
result.map{t => NoContent } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -255,7 +255,7 @@ class TagManagementApi(
def mergeTag = (APIAuthAction andThen MergeTagPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
req.body.asJson.map { json =>
json.as[MergeTagCommand].process.map { result =>
json.as[MergeTagCommand].process().map { result =>
result.map{t => NoContent } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -267,7 +267,7 @@ class TagManagementApi(

def deleteTag(id: Long) = (APIHMACAuthAction andThen DeleteTagPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
(DeleteTagCommand(id)).process.map { result =>
(DeleteTagCommand(id)).process().map { result =>
result.map { t => NoContent } getOrElse NotFound
} recover {
commandErrorAsResult
Expand Down Expand Up @@ -305,7 +305,7 @@ class TagManagementApi(
def createSponsorship = (APIAuthAction andThen ManageSponsorshipsPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
req.body.asJson.map { json =>
json.as[CreateSponsorshipCommand].process.map { result =>
json.as[CreateSponsorshipCommand].process().map { result =>
result.map{t => Ok(Json.toJson(t)) } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -318,7 +318,7 @@ class TagManagementApi(
def updateSponsorship(id: Long) = (APIAuthAction andThen ManageSponsorshipsPermissionsCheck()).async { req =>
implicit val username = Option(req.user.email)
req.body.asJson.map { json =>
json.as[UpdateSponsorshipCommand].process.map { result =>
json.as[UpdateSponsorshipCommand].process().map { result =>
result.map{s => Ok(Json.toJson(DenormalisedSponsorship(s))) } getOrElse NotFound
} recover {
commandErrorAsResult
Expand All @@ -335,7 +335,7 @@ class TagManagementApi(
val tagSearch: Option[List[Long]] = tagIds.map(_.split(",").toList.filter(_.length > 0).map(_.toLong))
val sectionSearch: Option[List[Long]] = sectionIds.map(_.split(",").toList.filter(_.length > 0).map(_.toLong))
new ClashingSponsorshipsFetch(id, tagSearch, sectionSearch, validFrom.map(new DateTime(_)), validTo.map(new DateTime(_)), editionSearch)
.process.map { result =>
.process().map { result =>
result.map{ ss => Ok(Json.toJson(ss.map(DenormalisedSponsorship(_)))) } getOrElse BadRequest
} recover {
commandErrorAsResult
Expand Down
2 changes: 1 addition & 1 deletion app/model/command/ReindexPillarsCommand.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ case class ReindexPillarsCommand() extends Command {
override type T = Unit

override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[T]] = Future{
JobHelper.beginPillarReindex
JobHelper.beginPillarReindex()
Some(())
}
}
2 changes: 1 addition & 1 deletion app/model/command/ReindexSectionsCommand.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ case class ReindexSectionsCommand() extends Command {
override type T = Unit

override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[T]] = Future{
JobHelper.beginSectionReindex
JobHelper.beginSectionReindex()
Some(())
}
}
2 changes: 1 addition & 1 deletion app/model/command/ReindexTagsCommand.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ case class ReindexTagsCommand() extends Command {
override type T = Unit

override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[T]] = Future{
JobHelper.beginTagReindex
JobHelper.beginTagReindex()
Some(())
}
}
2 changes: 1 addition & 1 deletion app/model/command/UpdateTagCommand.scala
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ case class UpdateTagCommand(denormalisedTag: DenormalisedTag) extends Command wi
existingTag foreach {(existing) =>
if (tag.externalReferences != existing.externalReferences) {
logger.info("Detected references change, triggering reindex")
FlexTagReindexCommand(tag).process
FlexTagReindexCommand(tag).process()
}
}

Expand Down
2 changes: 1 addition & 1 deletion app/model/jobs/Job.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ case class Job(
def rollback = {
if (rollbackEnabled) {
val revSteps = steps.reverse
revSteps.foreach(step => step.rollbackStep)
revSteps.foreach(step => step.rollbackStep())
jobStatus = JobStatus.rolledback
}
}
Expand Down
6 changes: 3 additions & 3 deletions app/model/jobs/JobHelper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ object JobHelper {
steps = List(ReindexTags())
)
)
AppAuditRepository.upsertAppAudit(AppAudit.reindexTags)
AppAuditRepository.upsertAppAudit(AppAudit.reindexTags())
}

def beginSectionReindex()(implicit username: Option[String], ec: ExecutionContext) = {
Expand All @@ -69,7 +69,7 @@ object JobHelper {
steps = List(ReindexSections())
)
)
AppAuditRepository.upsertAppAudit(AppAudit.reindexSections)
AppAuditRepository.upsertAppAudit(AppAudit.reindexSections())
}

def beginPillarReindex()(implicit username: Option[String], ec: ExecutionContext) = {
Expand All @@ -83,7 +83,7 @@ object JobHelper {
steps = List(ReindexPillars())
)
)
AppAuditRepository.upsertAppAudit(AppAudit.reindexPillars)
AppAuditRepository.upsertAppAudit(AppAudit.reindexPillars())
}

def beginMergeTag(from: Tag, to: Tag)(implicit username: Option[String], ec: ExecutionContext) = {
Expand Down
6 changes: 3 additions & 3 deletions app/model/jobs/JobRunner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class JobRunner @Inject() (lifecycle: ApplicationLifecycle)(implicit ec: Executi

def tryRun() = {
try {
run
run()
} catch {
case NonFatal(e) => {
logger.error(s"An unexpected exception occurred in the job runner: ${e.getStackTrace}")
Expand Down Expand Up @@ -64,7 +64,7 @@ class JobRunner @Inject() (lifecycle: ApplicationLifecycle)(implicit ec: Executi
logger.error(s"Background job failed on ${JobRunner.nodeId}. $e")
}
} finally {
job.checkIfComplete
job.checkIfComplete()
JobRepository.upsertJobIfOwned(job, JobRunner.nodeId)
JobRepository.unlock(job, JobRunner.nodeId)
}
Expand Down Expand Up @@ -97,6 +97,6 @@ object JobRunner {
}

class JobRunnerScheduler(runner: JobRunner) extends AbstractScheduledService {
override def runOneIteration(): Unit = runner.tryRun
override def runOneIteration(): Unit = runner.tryRun()
override def scheduler(): Scheduler = Scheduler.newFixedDelaySchedule(1, 10, TimeUnit.SECONDS)
}
12 changes: 6 additions & 6 deletions app/model/jobs/Step.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ trait Step extends Logging {
// Public methods that wrap the status updates
def processStep(implicit ec: ExecutionContext) = {
try {
beginProcessing
beginProcessing()
process
doneProcessing
doneProcessing()
} catch {
case NonFatal(e) => {
logger.error(s"Error thrown during step processing: ${e}")
processFailed
processFailed()
throw e // Need to rethrow the exception to inform the job to start a rollback
}
}
Expand All @@ -41,18 +41,18 @@ trait Step extends Logging {
def checkStep(implicit ec: ExecutionContext) = {
attempts += 1
if (attempts > Step.retryLimit) {
checkFailed
checkFailed()
throw TooManyAttempts(s"Too many attempts at checking ${this.`type`}")
}

try {
if (check) {
checkOk
checkOk()
}
} catch {
case NonFatal(e) => {
logger.error(s"Error thrown during step check: ${e}")
checkFailed
checkFailed()
throw e // Need to rethrow the exception to inform the job to start a rollback
}
}
Expand Down
2 changes: 1 addition & 1 deletion app/modules/clustersync/ClusterSynchronisation.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class ClusterSynchronisation @Inject() (lifecycle: ApplicationLifecycle) extends
AtomicReference[Option[KinesisConsumer]](None)


lifecycle.addStopHook{ () => Future.successful(stop) }
lifecycle.addStopHook{ () => Future.successful(stop()) }
serviceManager.startAsync()

initialise
Expand Down
Loading

0 comments on commit f326380

Please sign in to comment.