Skip to content

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented May 29, 2023

What changes were proposed in this pull request?

This pr aims to enable unused imports check for Scala 2.13.

There is an exception here for import scala.language.higherKinds(Scala 2.13 does not need to import them , while Scala 2.12 must import them), so this pr add two new suppression rules for it.

Why are the changes needed?

Enable unused imports check for Scala 2.13.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass Github Actions

import org.apache.spark.connect.proto.{ExecutePlanResponse, SqlCommand, StreamingQueryCommand, StreamingQueryCommandResult, StreamingQueryInstanceId, WriteStreamOperationStart, WriteStreamOperationStartResult}
import org.apache.spark.connect.proto.ExecutePlanResponse.SqlCommandResult
import org.apache.spark.connect.proto.Parse.ParseFormat
import org.apache.spark.connect.proto.StreamingQueryCommand
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Scala 2.13 checked them unused as they have already been imports in line 31


package org.apache.spark.sql.catalyst.expressions

import scala.collection.{mutable, IterableFactory, IterableOps}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IterableFactory and IterableOps are unused imports

@LuciferYang
Copy link
Contributor Author

LuciferYang commented May 29, 2023

cc @dongjoon-hyun @HyukjinKwon FYI

@LuciferYang LuciferYang marked this pull request as draft May 29, 2023 05:31
@LuciferYang LuciferYang marked this pull request as ready for review May 29, 2023 05:38
pom.xml Outdated
<arg>-Wconf:cat=unchecked&amp;msg=outer reference:s</arg>
<arg>-Wconf:cat=unchecked&amp;msg=eliminated by erasure:s</arg>
<arg>-Wconf:msg=^(?=.*?a value of type)(?=.*?cannot also be).+$:s</arg>

Copy link
Member

Choose a reason for hiding this comment

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

nit. Redundant empty line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @dongjoon-hyun , e80f584 removed it

Copy link
Member

@dongjoon-hyun dongjoon-hyun 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 (Pending CIs)

@dongjoon-hyun
Copy link
Member

Merged to master.

@LuciferYang
Copy link
Contributor Author

Thanks @dongjoon-hyun @MaxGekk

czxm pushed a commit to czxm/spark that referenced this pull request Jun 12, 2023
### What changes were proposed in this pull request?
This pr aims to enable `unused imports` check for Scala 2.13.

There is an exception here for `import scala.language.higherKinds`(Scala 2.13 does not need to import them , while Scala 2.12 must import them), so this pr add two new suppression rules for it.

### Why are the changes needed?
Enable `unused imports` check for Scala 2.13.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass Github Actions

Closes apache#41356 from LuciferYang/SPARK-43849.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
LuciferYang added a commit that referenced this pull request Sep 27, 2023
…gherKinds` and delete the corresponding suppression rule

### What changes were proposed in this pull request?
`scala.language.higherKinds` is deprecated and no longer needs to be imported explicitly in Scala 2.13, so this PR removes the imports for scala.language.higherKinds and the corresponding compiler suppression rules.

### Why are the changes needed?
In SPARK-43849(#41356), I added compiler suppression rules to allow `unused imports` checks to work with both Scala 2.12 and Scala 2.13. As there is no longer a need to support Scala 2.12, we can now clean them up.

### 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 #43128 from LuciferYang/SPARK-43850.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
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.

3 participants