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

Scala 2 Macro Fix for isEmpty #2298

Merged
merged 1 commit into from Dec 12, 2023

Conversation

cheeseng
Copy link
Contributor

Fixed unexpected warning in Scala 2 macro for assert(foo.isEmpty()) as reported in #2297 .

@LuciferYang
Copy link

Thanks very much for your fix, I will build a snapshot to test it later

Copy link

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

+1, LGTM

The test results are as expected after local packaging, thanks @cheeseng

srowen pushed a commit to apache/spark that referenced this pull request Oct 22, 2023
…IB][K8S][YARN][SHELL][PYTHON][R][AVRO][UI][EXAMPLES] Fix the compilation warning "Auto-application to `()` is deprecated" and turn it into a compilation error

### 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:

```scala
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 = Foo7e15f4d4

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 = Foo150d6eaf

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

Closes #43472 from LuciferYang/SPARK-45610.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
@cheeseng cheeseng merged commit 5e172d4 into scalatest:3.2.x-new Dec 12, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants