Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-45610][BUILD][CORE][SQL][SS][CONNECT][GRAPHX][DSTREAM][ML][MLLIB][K8S][YARN][SHELL][PYTHON][R][AVRO][UI][EXAMPLES] Fix the compilation warning "Auto-application to () is deprecated" and turn it into a compilation error #43472

Closed
wants to merge 2 commits into from

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Oct 21, 2023

What changes were proposed in this pull request?

This PR mainly does two things:

  1. Clean up all compilation warnings related to "Auto-application to () is deprecated".
  2. Change the compilation options to convert this compilation warning into a compilation error.

Additionally, due to an issue with scalatest(scalatest/scalatest#2297), there are some false positives. Therefore, this PR has added the corresponding rules to suppress them, and left the corresponding TODO(SPARK-45615). We can clean up these rules after scalatest fixes this issue(scalatest/scalatest#2298).

Why are the changes needed?

  1. Clean up the deprecated usage methods.
  2. As this compilation warning will become a compilation error in Scala 3, to ensure it does not occur again, this PR also converts it into a compilation error in Scala 2.13.

For example, for the following code:

class Foo {
  def isEmpty(): Boolean = true
}
val foo = new Foo
val ret = foo.isEmpty

In Scala 2.13:

Welcome to Scala 2.13.12 (OpenJDK 64-Bit Server VM, Java 17.0.8).
Type in expressions for evaluation. Or try :help.

scala> class Foo {
     |   def isEmpty(): Boolean = true
     | }
class Foo

scala> val foo = new Foo
     | 
val foo: Foo = Foo@7e15f4d4

scala> val ret = foo.isEmpty
                     ^
       warning: Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method isEmpty,
       or remove the empty argument list from its definition (Java-defined methods are exempt).
       In Scala 3, an unapplied method like this will be eta-expanded into a function. [quickfixable]
val ret: Boolean = true

In Scala 3:

Welcome to Scala 3.3.1 (17.0.8, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                                                                                                                                                  
scala> class Foo {
     |   def isEmpty(): Boolean = true
     | }
// defined class Foo
                                                                                                                                                                                                                                  
scala> val foo = new Foo
val foo: Foo = Foo@150d6eaf
                                                                                                                                                                                                                                  
scala> val ret = foo.isEmpty
-- [E100] Syntax Error: --------------------------------------------------------
1 |val ret = foo.isEmpty
  |          ^^^^^^^^^^^
  |          method isEmpty in class Foo must be called with () argument
  |-----------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  | Previously an empty argument list () was implicitly inserted when calling a nullary method without arguments. E.g.
  |
  | def next(): T = ...
  |         |next     // is expanded to next()
  |
  | In Dotty, this idiom is an error. The application syntax has to follow exactly the parameter syntax.
  | Excluded from this rule are methods that are defined in Java or that override methods defined in Java.
   -----------------------------------------------------------------------------
1 error found

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GitHub Actions

Was this patch authored or co-authored using generative AI tooling?

No

@LuciferYang LuciferYang changed the title [SPARK-45610] Fix the compilation warning "Auto-application to () is deprecated" and turn it into a compilation error. [SPARK-45610][BUILD][CORE][SQL][SS][CONNECT][GRAPHX][DSTREAM][ML][MLLIB][K8S][YARN][SHELL][PYTHON][R][AVRO][UI][EXAMPLES] Fix the compilation warning "Auto-application to () is deprecated" and turn it into a compilation error Oct 21, 2023
@@ -237,7 +237,14 @@ object SparkBuild extends PomBuild {
// fixed.
"-Wconf:msg=^(?=.*?method|value|type|object|trait|inheritance)(?=.*?deprecated)(?=.*?since 2.13).+$:s",
"-Wconf:msg=^(?=.*?Widening conversion from)(?=.*?is deprecated because it loses precision).+$:s",
"-Wconf:msg=Auto-application to \\`\\(\\)\\` is deprecated:s",
// SPARK-45610 Convert "Auto-application to `()` is deprecated" to compile error, as it will become a compile error in Scala 3.
"-Wconf:cat=deprecation&msg=Auto-application to \\`\\(\\)\\` is deprecated:e",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this compilation warning will become a compilation error in Scala 3, to ensure it does not occur again, this PR also converts it into a compilation error in Scala 2.13.

If we don't need to do this protection now, please let me know.

cc @srowen FYI

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems OK, but I can't access the test result for some reason to see why this failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recently, the Run / Build modules: streaming, sql-kafka-0-10, streaming-kafka-0-10, mllib-local, mllib, yarn, kubernetes, hadoop-cloud, spark-ganglia-lgpl, connect, protobuf task is not easily successful, the compilation container is always getting killed, I have triggered it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All test passed

@srowen
Copy link
Member

srowen commented Oct 22, 2023

Merged to master

@srowen srowen closed this in 48e207f Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment